Conversation
970f826 to
b5b6c3a
Compare
|
Curious if there is a libbitcoin implementation available of bip 54 @evoskuil, at least for a subset of the bip fixes. Even if libbitcoin never goes to support activation of them for different reasons, I think I can be willing to review the two in parallel, and see if there are things to make more consistent between the compatible implems. |
No, we have more immediate fish to fry. But PR's are certainly welcome! |
|
@ariard if you are interested in looking at an implementation in another client, i believe @kcalvinalvin is working on a BIP 54 implementation for btcd. EDIT: this is btcsuite/btcd#2537. |
037a019 to
c102172
Compare
|
Rebased on master following the merge of bitcoin#35333 and bitcoin#35335. |
3052881 to
3fdd16e
Compare
BIP54 counts sigops differently from existing sigops-based checks. Since we are overloading the sigops term, make clear the constant refers to BIP54-sigops, not other kinds of pre-existing sigops. Also update the functional test framework's constant that has "standardness" in its name, since we are about to make it consensus critical (func test scripted-diff courtesy of Anthony Towns). -BEGIN VERIFY SCRIPT- sed -i 's/MAX_TX_LEGACY_SIGOPS/MAX_TX_BIP54_SIGOPS/g' $(git grep -l MAX_TX_LEGACY_SIGOPS src/) sed -i 's/MAX_STD_LEGACY_SIGOPS/MAX_TX_BIP54_SIGOPS/g' $(git grep -l MAX_STD_LEGACY_SIGOPS) sed -i 's/signature operations in validating a transaction./signature operations in a single transaction, per BIP54./' test/functional/test_framework/script_util.py -END VERIFY SCRIPT- Co-Authored-by: Anthony Towns <aj@erisian.com.au>
Move the function that checks whether a transaction respects the BIP54 sigops rule to the consensus folder (along with the accompanying constant), as it will be made consensus-critical in the next commit. Can be reviewed with git's --color-moved option.
When BIP54 is active, enforce that block transactions do not violate the BIP54 limit on the number of legacy sigops present in Scripts that get executed during block validation.
In Taproot the signature commits to the list of spent outputs.
Test the newly introduced limit with various combinations of inputs and outputs types, historical transactions, and exercise some implementation-specific edge cases. Record each test case and optionally write them to disk as JSON to generate the BIP test vectors.
The fuzz target was specifically crafted to support seeding it with the BIP54 test vectors generated by the unit test in the previous commit.
We are going to introduce the timewarp fix for mainnet with a greater grace period. Rename the MAX_TIMEWARP value for testnet to differentiate them. -BEGIN VERIFY SCRIPT- for f in $(git grep -l MAX_TIMEWARP); do sed -i "s/MAX_TIMEWARP/MAX_TIMEWARP_TESTNET4/g" "$f"; done -END VERIFY SCRIPT-
Documentation about the test vectors, including about their structure and content, as well as reproduction instructions, is available here: https://github.com/bitcoin/bips/tree/master/bip-0054/test_vectors
…k height When BIP 54 is active, coinbase transactions must have their nLockTime field set to the block height minus 1 (since it encodes the last height at which the transaction is invalid), and their nSequence field may be anything but the maximum value (which indicates "final", bypassing timelock validation).
Documentation about the test vectors' structure and content, as well as instructions for generating them is available at https://github.com/bitcoin/bips/tree/master/bip-0054/test_vectors .
64-byte transactions are also now treated as a consensus failure in PreChecks, like BIP54-sigops check failures. Note this only changes the RPC error, and not the disconnection behaviour in P2P since 266dd0e.
… vectors) This adds tests exercising the bounds of the checks on the invalid transaction size, for various types of transactions (legacy, Segwit, bytes in input/output to get to 64 bytes) as well as sanity checking against some known historical violations. Thanks to Chris Stewart for digging up the historical violations to this rule.
The previously introduced unit tests extensively test the specific implementation of each mitigation. This functional test complements them by end-to-end testing all mitigations. For the added timestamp constraints, it mimicks how they would get exploited (by implementing pseudo timewarp and Murch-Zawy attacks) and demonstrates those exploits are not possible anymore after BIP54 activates.
|
Opened the PR upstream at bitcoin#35793, closing this. |
This implements the Consensus Cleanup validation rules proposed in BIP 54. These rules are only enabled on regtest. Mainnet activation, if any, is to be considered separately.
This patchset is based on the code that was previously reviewed (1, 2) and tested (for instance here) on Bitcoin Inquisition. The tests and documentation have since been improved, but the consensus-critical commits have been carried over with only minor differences (the only behavioural one being the addition of the stripped-size check to
PreChecks).Roughly 95% of the added lines are tests or test data. The format, contents, and reproduction procedure for the BIP 54 test vectors are documented here. The
bip54_testsunit test module exercises each mitigation extensively in isolation. Thefeature_bip54.pyfunctional test verifies all of them end-to-end after and prior to activation. For the timestamp rules, it simulates timewarp and Murch–Zawy attacks, demonstrating the new timestamp rules prevent these exploits. A fuzz harness for the sigop accounting logic is also included, which can be seeded from the BIP test vectors (implemented here).See commit messages for details.