Tx combinator refactor #249
Merged
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
and update the kernel on the tx based on features from the slate
|
Thanks for doing the wallet-side changes as well.. will just wait for the node-side merge and retrigger. |
|
Just merged the related grin PR and kicked off a build here. |
|
Bumped grin dependency in Cargo.lock to reflect the changes on master. |
yeastplume
approved these changes
Nov 1, 2019
|
We should keep an eye on tx building using master for a bit - should be pretty obvious if this broke something unexpected. |
yyangli
pushed a commit
to mwcproject/mwc-wallet
that referenced
this pull request
May 13, 2020
* update tx building to use simpler kernel feature handling * pass in tx to build::partial_transaction and update the kernel on the tx based on features from the slate * bump SHA for grin dependencies
antiochp
added a commit
to antiochp/grin-wallet
that referenced
this pull request
Aug 7, 2020
* update tx building to use simpler kernel feature handling * pass in tx to build::partial_transaction and update the kernel on the tx based on features from the slate * bump SHA for grin dependencies
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.
Resolves #248
Related mimblewimble/grin#3057 (must be merged before this PR)
We no longer use tx building combinators for fee and lock_height.
We now pass an existing tx into
build::partial_transaction.We are also responsible for updating the tx kernel to reflect the features based on the current slate (fee changes based on the tx being built).
The motivation for these changes is to increase the flexibility to support future kernel feature variants, specifically "relative height locked" kernels.
The tx building combinators
with_feeandwith_lock_heightdid not fit cleanly with adding new feature variants as the different combinations of optional fields (fee, lock_height etc.) produce different kernel features.