Segwit light #82
Draft
Segwit light #82
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
Closed
domob1812
force-pushed
the
segwit-light
branch
3 times, most recently
from
7b25105 to
1a5ad73
Compare
domob1812
force-pushed
the
segwit-light
branch
2 times, most recently
from
1b1a6d1 to
439be50
Compare
domob1812
force-pushed
the
segwit-light
branch
4 times, most recently
from
da65c20 to
08cadf2
Compare
domob1812
force-pushed
the
segwit-light
branch
3 times, most recently
from
6aaff2f to
4194f08
Compare
domob1812
force-pushed
the
segwit-light
branch
2 times, most recently
from
4660b56 to
ed406c9
Compare
domob1812
force-pushed
the
segwit-light
branch
2 times, most recently
from
620f96b to
421b0d9
Compare
domob1812
force-pushed
the
segwit-light
branch
3 times, most recently
from
ae5ad86 to
662c39a
Compare
domob1812
force-pushed
the
segwit-light
branch
6 times, most recently
from
0709b93 to
a852eb4
Compare
domob1812
force-pushed
the
segwit-light
branch
5 times, most recently
from
790d19e to
5f04295
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).
This implements "segwit-light" in the node (consensus and mempool): After activation of a new fork, outputs created by new transactions are referred to by the creating transaction's bare txid rather than the normal hash (txid). This makes chains of constructed transactions immune to transaction malleability. In the mempool, we use the current chain tip to determine the activation state of the fork, as we can't know for sure when a transaction will be confirmed (and thus what activation state will be in effect then). This will lead to issues right around the fork activation time, but we will simply disallow spending of unconfirmed outputs in the mempool and wallet "around" the fork time temporarily to resolve this. The wallet is also not yet updated to take the change into account when selecting coins.
Handle segwit-light properly when spending coins, either for staking or normal transactions in the wallet. Depending on when a transaction was confirmed, we need to make sure to include the right outpoint (with txid or bare txid) in the transaction spending an output. This is done through a custom UTXO hasher used in the wallet, which specifically works for CWalletTx.
Around the segwit-light fork (some hours before and after), we should avoid spending unconfirmed outputs: If we do that, it is unclear whether or not the fork will be active in the block that confirms those spends, and thus we can't reliably construct a valid chain. With this change, we introduce a (temporary) measure to disallow those spends in the wallet and mempool policy in a window of time around the planned fork activation.
This adds the new regtest segwit_light.py, which verifies that sending of transactions and staking still works after the fork, and that the new rules work as expected. It also explicitly checks that transaction malleability is not an issue anymore for chains of pre-signed transactions.
domob1812
force-pushed
the
segwit-light
branch
from
5f04295 to
e1cd852
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 implements "segwit light" as described in the design doc. We create a new time-based fork (for now scheduled at block time 2'000'000'000, to be changed to a real timestamp later); all transactions made after the fork create UTXOs not indexed by their normal hash (txid), but instead by a hash not including any signature data (bare txid). This means that all transactions made after the fork can be safely used in a chain of unconfirmed, pre-signed transactions and won't be affected by malleability.
This PR is #61 reopened against the correct updated git branches.