Simplify txbuilder output addition #304
Merged
Conversation
This file contains hidden or 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
Contributor
Author
|
Note: this will soft-conflict with the following PR because it hardcodes |
rooooooooob
approved these changes
Dec 22, 2021
Contributor
rooooooooob
left a comment
rooooooooob
left a comment
There was a problem hiding this comment.
LGTM although @vsubhuman should check it over too since he was the one who added these convenience functions to begin with and his PR #273 touches some of this code (although from what I can see it should be fine since that PR doesn't change how the output is added for the mint+output convenience setters.)
…outputs # Conflicts: # rust/src/tx_builder.rs
vsubhuman
approved these changes
Feb 5, 2022
Member
vsubhuman
left a comment
vsubhuman
left a comment
There was a problem hiding this comment.
Great change, thank you, Seba!
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.
Currently a few places in cardano-serialization-lib hardcode that no data hash is present in outputs which can lead to the incorrect minimum ADA amount being calculated.
This PR fixes these by properly tracking data hash in the tx builder.
However, I ran into a problem: the tx builder currently has a combinatorial explosion of different functions for adding outputs. Notably,
We can't just add a new optional parameter to all these functions because WASM doesn't like
Option, so the naive solution would be to double the number of functions which isn't acceptable either.To tackle this, I instead created an
TransactionOutputBuilderso that it's easier to create plainTransactionOutputobjects based on certain requirements. This simplifies the txbuilder code and also makes it easier to deal withTransactionOutputcreation on the JS-side as well