UTXO hasher abstraction #80
Open
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
domob1812
force-pushed
the
utxo-hasher
branch
2 times, most recently
from
4032437 to
e28546a
Compare
domob1812
force-pushed
the
utxo-hasher
branch
2 times, most recently
from
c9535a5 to
131d377
Compare
domob1812
force-pushed
the
utxo-hasher
branch
3 times, most recently
from
d6ab5fd to
243dbff
Compare
|
Converting this to draft for now. I'll include also the wallet parts of it here, as well as look into making the whole "output hash" concept type-safe and compiler-enforced. |
domob1812
force-pushed
the
utxo-hasher
branch
11 times, most recently
from
2601ed3 to
ce3fe42
Compare
domob1812
force-pushed
the
utxo-hasher
branch
6 times, most recently
from
df9ac4a to
263c919
Compare
domob1812
force-pushed
the
utxo-hasher
branch
2 times, most recently
from
d8e7623 to
56fdd43
Compare
domob1812
force-pushed
the
utxo-hasher
branch
2 times, most recently
from
10726c8 to
a417b1b
Compare
domob1812
force-pushed
the
utxo-hasher
branch
3 times, most recently
from
fb96429 to
178d55a
Compare
domob1812
force-pushed
the
utxo-hasher
branch
4 times, most recently
from
187ef8f to
e33bfe6
Compare
domob1812
force-pushed
the
utxo-hasher
branch
4 times, most recently
from
31af915 to
28a63bc
Compare
Some places in the code need to determine the hash by which the UTXOs of a given transaction will be referred to; in particular, we need that when processing UTXO set updates for block connects and disconnects, in the mempool and for assembling transactions into a new block. This commit introduces a class TransactionUtxoHasher, which abstracts this step and is used in all those places in the code instead of just getting the txid directly. For now, this has no effects on behaviour; but it makes it more clear in the code where we need this particular logical feature; it will allow us to add some more unit tests for those parts with explicit mocks of the hasher class; and it will make it easier to implement segwit-light in the future (where we basically just need to flip the hasher implementation but no other parts in the code).
Add a unit test (together with the necessary framework around) for UTXO creation from UpdateCoins, based on the UTXO hasher (rather than the txid directly).
This extends the mempool unit tests to explicitly verify that adding transactions, removing transactions, checking the pool and looking up coins / transactions still works even if we use the bare txid for some transactions as UTXO hash (as will be the case with segwit-light in the future).
Use the UTXO hasher abstraction in the wallet and for staking (i.e. for places where coins are spent). The wallet gets its own instance, which will allow for dependency injection in tests. For now, the hasher used in the wallet is just the normal hasher, i.e. there are no actual changes in behaviour. In the future, the wallet hasher can be changed accordingly for the activation of segwit light.
Extend the unit tests for PoSTransactionCreator to verify that the UTXO hasher is used to build up the PoS transaction when staking.
Extend the unit tests in wallet_coinmanagement_tests.cpp to include also explicit checks for situations in which the wallet is supposed to the UTXO hasher rather than e.g. a plain transaction hash.
The VaultManager class needs a TransactionUtxoHasher instance so that it can properly track spent outputs with the outputTracker_. This adds a reference to the UTXO hasher to the instance and uses it in the proper place.
This updates the regtests to use "outputhash" for listunspent results in some places, to make sure the code will also work after activating segwit-light. Some other places remain where outputs are not from listunspent and that still needs to be updated when segwit-light gets activated generally, but this is a first step to reduce the amount of required changes then.
This introduces a new, different type OutputHash for hashes of outputs, e.g. in COutPoint and all related places in the code. This type is functionally equivalent to uint256, but it enables the compiler to ensure that all places that should be using an UTXO hasher are actually using it.
Update also the unit tests for the OutputHash type. This is a separate commit to make the review easier (as reviewing the main part of the change in production code is separate).
domob1812
force-pushed
the
utxo-hasher
branch
from
28a63bc to
18d2b08
Compare
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.
This creates a new
TransactionUtxoHasherabstraction: The class is responsible for mapping a givenCTransactionto theuint256that is used to refer to the UTXOs created by the transaction. This is currently just the normal txid (tx.GetHash()), but in the future with segwit-light (#61) will be changed.For now, this PR just refactors the code to make it explicit where in the consensus, mempool and wallet we actually use this concept of "UTXOs of a given transaction". Those places use now a new, distinct (but functionally equivalent) type
OutputHashinstead ofuint256, so that strong typing ensures that all relevant places are updated accordingly, starting from places likeCOutPoint.Apart from unit tests, the hasher always returns the txid that was used inline before, so this does not affect consensus or introduces a fork yet. But it will make it easier to do so in the future.
This is #65 reopened with correct branches.