Skip to content
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

TxLock and finalize #485

Merged
merged 1 commit into from
Jul 31, 2020
Merged

Conversation

quentinlesceller
Copy link
Member

Past behavior of the init_send_tx method with InitSendTxArgs (only when the recipient wallet was reachable):

  1. Sender sends transaction to recipient
  2. Recipients receives it and send back
  3. Sender lock outputs
  4. Sender finalise iff InitSendTxArgs contains finalize=true and returns the final slate
  5. Sender post_tx iff InitSendTxArgs contains post_tx=true and returns the final slate

Current behavior with the same condition:

  1. Sender sends transaction to recipient
  2. Recipients receives it and send back
  3. A. If post_tx=true Sender lock outputs, finalize and post_tx iff post_tx=true and returns the final slate
  4. B. If post_tx=false Sender returns the first slate

This PR locks outputs, finalise and return the final transaction but do not post when post_tx=false.

Fixes #480.

Might not be the best solution though. Thoughts @yeastplume ?

Copy link
Member

@yeastplume yeastplume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All looks fine, thanks for looking into this. You think it's worth cherry-picking to 4.0.x as well?

@quentinlesceller
Copy link
Member Author

quentinlesceller commented Jul 22, 2020

Do you think that'll be more logical to do something else like:

  • init_send_tx when InitSendTxArgs is present should finalize by default and retrieve the S3 transaction. (this PR)
  • init_send_tx when InitSendTxArgs is present should returns the S2 transaction. The user will then "manually" call finalize on it and post.
  • init_sed_tx reintroduce the finalize arg in InitSendTxArgs and returns S2 if finalize=false and S3 if finalize=true

@yeastplume
Copy link
Member

The send args are in place to emulate the wallet send command as closely as possible, so the idea was that if they're present, the command attempt to send, retrieve the S3 result and finalize in one call. So current behavior in this PR is consistent with how it's been working to date

@yeastplume yeastplume merged commit 7a00337 into mimblewimble:master Jul 31, 2020
antiochp pushed a commit to antiochp/grin-wallet that referenced this pull request Aug 7, 2020
@quentinlesceller quentinlesceller deleted the lockandreturn branch August 10, 2020 14:55
@quentinlesceller
Copy link
Member Author

Also this is worth cherry picking into v4.0.x branch. Missed it somehow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Removal of "finalize" params in InitSendTxArgs
2 participants