-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New trade protocol #3311
Closed
Closed
New trade protocol #3311
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add handler for DepositTxAndDelayedPayoutTxMessage - Change handler for DepositTxPublishedMessage - Add MakerSetsLockTime - Rename MakerProcessPayDepositRequest to MakerProcessPayDepositRequest - Rename MakerSendPublishDepositTxRequest to MakerSendsProvideInputsForDepositTxMessage - Rename DepositTxPublishedMessage to DelayedPayoutTxSignatureRequest - Rename MakerProcessDepositTxPublishedMessage to MakerAsBuyerProcessSignDelayedPayoutTxMessage
- Support daoFacade param
We change behaviour that the maker as seller does not send the pre signed deposit tx to the taker as the seller has more to lose and he wants to control the creation process of the delayed payout tx.
Delayed payout tx are now working for all scenarios but we use a small hack to get around an issue with not receiving confirmations and the peers tx. We add a tiny output to both peers, so we see the tx and confirmation. Without that only the publisher sees the tx and confirmations are not displayed. Need further work to get that working without that extra outputs.
We need add the delayed payout tx to the wallet once the peer publishes it. We will not see the confidence as we do not receive or sent funds from our address. Same is with dispute payouts where one peer does not receive anything. Then the confidence is not set. It seems that is a restriction in BitcoinJ or it requires some extra handling. We set the confidence indicator invisible in the dispute case and that might be an acceptable option here as well.
We dont need in the offer anymore the decision if reimbursement or arbitration is chosen.
e133fdd
to
038347a
Compare
Replaced by #3333 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implements bisq-network/proposals#52
This is work in progress but as the protocol is already implemented I thought it might be useful to share progress on it via making a draft PR.
It has already lots of code change and more will come. It is not expected that other devs review every commit here but to review the full diff once it is completed (similar to mediation support PR).
The new trade protocol will have a new TRADE_PROTOCOL_VERSION to separate it from not updated users, but it will not require a hard fork. Users need to complete their pending trades and distputes before updating. Updates while having pending trades and distputes would cause failures of the trade.