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.
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
feat: add function for splitting UTXOs and docs on
maxOutputs
#3435base: master
Are you sure you want to change the base?
feat: add function for splitting UTXOs and docs on
maxOutputs
#3435Changes from all commits
c105246
35ac54c
db1d8cd
799cedb
69f721c
bf6a2da
08a08ff
81588ab
1340ca5
34bfb91
922796e
5e1049c
8d616c7
a04142b
f1b8088
5aa78b8
1d4e5fb
04fc429
2f01c06
5d71683
4600252
ebf5d27
0568b11
ecefb57
b7e6fb4
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand this part of the documentation. Why is the wallet being funded a second time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the wallet has a balance of 1000 comprised of 5 UTXOs each worth 200. Therefore there is an insufficient amount to cover the fee.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we better clarify this in the comment where we fund the wallet the second time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe more clear?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This indicates that we should define this helper somewhere else. We can put it within the
account
package. The result will be the same since users will import it fromfuels
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I disagree, this is a utility specific to UTXOs and had I chosen to accept the
destination
param as a string this wouldn't be necessary.I could move it to the
account
package to avoid this cast but I don't think that this functionality is structurally apart of what theaccount
package should offerThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The naming
splitUTXOs
implies that this function would be accepting actual UTXOs and creating transaction outputs that would be added to a transaction's outputs in a fashion like this:With the current implementation of this function, a more appropriate name would be
generateTransactionOutputs
. One could still use it in a similar fashion as in the code block above by summing theamount
s of the coins, but because it's not taking in any UTXO its name is misaligned.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe
destination
could just take a string as it is returning a string? And then we don't need the unknown cast in the above test?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion the benefits of enforcing the address type outweigh the cast in the test. The primary goal here is to validate user input not to have "clean" tests per say.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.