mempool/blockchain: Preserve seqlock behavior. #1570
Conversation
|
If current consensus rules have anything like version, it can be used in names e.g. |
davecgh
force-pushed
the
multi_preserve_pre_14_seqlock_behavior
branch
from
8c303d3
to
c070d14
Compare
|
I've updated this to include a full set of tests to ensure the proper legacy semantics are preserved as well as updated the implementation accordingly. |
|
@xaur That's a fair point, however, in this particular case, the code is only temporary and can entirely be removed after a vote which corrects the behavior is performed. It isn't always possible to remove legacy code, but it is in this case since the intended rules are more permissive. |
There was a problem hiding this comment.
My only question is whether it would be useful to add explicit tests for block disapproval cases.
They end up resolving into either the b4..b6 or b6..b9 case, depending on whether or not the disapproved block has a tx spending from b0.Transactions[0], so not entirely sure if it would require new test cases for that.
davecgh
force-pushed
the
multi_preserve_pre_14_seqlock_behavior
branch
from
c070d14
to
69e02d0
Compare
davecgh
force-pushed
the
multi_preserve_pre_14_seqlock_behavior
branch
from
69e02d0
to
9624137
Compare
|
Updated to include an explicit test for block disapproval case. |
davecgh
force-pushed
the
multi_preserve_pre_14_seqlock_behavior
branch
from
9624137
to
0bba84a
Compare
There was a problem hiding this comment.
Couldn't think of any more tests to add now.
Tested on mainnet by stopping before block 310728 and trying to submit block 00000000000000001905d9167008829dc055b7320808a788549ea6cc3f8092df and tx
495559c93da67b88b3ad92d8ecc4ec54df47fa441fdc56dc4f21728ee2782fe7 which end up blocked due to triggering the error.
The changes to reverse the utxoset semantics also had the side effect of correcting the behavior for sequence locks when inside of a block to the expected behavior instead of the actual current consensus behavior. In order to avoid that, this modifies the blockchain consensus rules to properly preserve the old incorrect behavior of sequence locks within blocks and also updates the mempool to to reject transactions accordingly thus providing parity between the two. The behavior needs to be corrected, but since it constitutes a change to the consensus rules, a vote will be necessary.
This adds tests to ensure the incorrect sequence lock behavior of the current consensus rules is preserved. In addition, in order to allow similar style tests in the future, this adds a function named quickVoteActivationParams which provides a unique set of chain parameters which allow the tests to more quickly activate an agenda while still creating a fully valid test chain that consists of full blocks.
davecgh
force-pushed
the
multi_preserve_pre_14_seqlock_behavior
branch
from
0bba84a
to
6088b18
Compare
The changes to reverse the utxoset semantics also had the side effect of correcting the behavior for sequence locks when inside of a block to the expected behavior instead of the actual current consensus behavior.
In order to avoid that, this modifies the
blockchainconsensus rules to properly preserve the old incorrect behavior of sequence locks within blocks and also updates themempoolto to reject transactions accordingly thus providing parity between the two.The behavior needs to be corrected, but since it constitutes a change to the consensus rules, a vote will be necessary.
A second commit adds tests to ensure the incorrect sequence lock behavior of the current consensus rules is preserved.
In addition, in order to allow similar style tests in the future, this adds a function named
quickVoteActivationParamswhich provides a unique set of chain parameters which allow the tests to more quickly activate an agenda while still creating a fully valid test chain that consists of full blocks.