Add blockchain.NewUtxoEntry() #1588
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
Rjected
approved these changes
May 29, 2020
jcvernaleo
requested changes
Jun 8, 2020
There was a problem hiding this comment.
Looks good to me. Happy to merge it in. One minor request. Could you change the commit message to follow these guidelines a bit better:
https://github.com/btcsuite/btcd/blob/master/docs/code_contribution_guidelines.md#ModelGitCommitMessages
Specifically, the length of the summary and also you give a great description in the PR comment, but not in the commit message itself.
…point The current methods to add to a UtxoViewpoint don't allow for a situation where we have only UTXO data but not a whole transaction. This commit allows contstruction of a UtxoEntry without requiring a full MsgTx. AddTxOut() and AddTxOuts() both require a whole transaction, including the inputs, which are only used in order to calculate the txid. In some situations, such as with use of the utreexo accumulator, we only have the utxo data but not the transaction which created it. For reference, utreexo's initial usage of the blockchain.NewUtxoEntry() function is at https://github.com/mit-dci/utreexo/pull/135/files#diff-3f7b8f9991ea957f1f4ad9f5a95415f0R96
|
OK, I ammended the git commit message to have more of the info from the PR text (minus alternatives since that might be confusing in a commit message) |
Closed
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.
In developing utreexo we're using btcd & libraries quite a bit. The current exported methods for adding entries to a UtxoViewpoint don't work for utreexo though.
AddTxOut()andAddTxOuts()both require a whole transaction, including the inputs, which are only there to calculate the txid. In the utreexo setup, we only have the outpoint and txout data, but not the rest of the transaction we'd like to add into the UtxoViewpoint. Directly creating one withNewUtxoEntry()seems like the smallest change that doesn't affect anything else in btcd.Another way to do it would be exporting the current
addTxOut()method, but that would conflict with the current exportedAddTxOut()method. (which, granted, is only used in 1 place in mempool.go, and that usage could be adapted to give the TxOut rather than whole transaction) Adding a NewUtxoEntry() constructor seemed simplest but changingAddTxOut()would work just as well.For reference, our usage of UtxoViewpoint is here: https://github.com/mit-dci/utreexo/pull/135/files#diff-3f7b8f9991ea957f1f4ad9f5a95415f0R96
If there are other options we'd also look at those. Hope we can get something like this in and be able to use the master branch of btcd for uteexo software. Thanks!