pre-bip54#168
Draft
l0rinc wants to merge 29 commits into
Draft
Conversation
The test utility SignSignature helpers can sign legacy and Segwit v0 inputs, but Taproot signatures need the spent outputs committed by the signature hash. Add an overload that accepts the spent outputs and cover it with a Taproot key-path signing test.
ValidateInputsStandardness has a policy check for potentially executed legacy sigops across a transaction input set. Add a corpus-friendly fuzz target that deserializes scriptSigs and spent scripts, populates a coins cache, and runs the current policy entry point without exposing the helper outside policy validation.
The regtest-only -testactivationheight and -vbparams options both override deployment state, but their parsing and application are mixed into the broader regtest option handling and constructor body. Group those options in DeploymentOptions and apply them through a helper so the deployment override logic has one place to live.
The constant is only enforced on chains where consensus.enforce_BIP94 is set, which today is only testnet4. Rename the constant to make this scope clear at the call sites. -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-
Prior commits are preparatory work. Following commits is the implementation of BIP54.
This encapsulates the soft fork configuration logic as set by the `-testactivationheight` (for buried deployments) and `-vbparams` (for version bits deployments) options which for the moment are regtest-only, in order to make them available on other networks as well in the next commit. Can be reviewed using git's --color-moved option.
…n unit tests This allows unit tests to set `-testactivationheight` and `-vbparams` on all networks instead of exclusively on regtest. Those are kept test-network-only when used as startup parameters.
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.
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.
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. -BEGIN VERIFY SCRIPT- sed -i 's/MAX_TX_LEGACY_SIGOPS/MAX_TX_BIP54_SIGOPS/g' $(git grep -l MAX_TX_LEGACY_SIGOPS src/) -END VERIFY SCRIPT-
When BIP54 is active, make sure transaction in blocks do not violate the BIP54 limit on the number of potentially-executed legacy sigops.
The fuzz target was specifically crafted to support seeding it with the BIP54 test vectors generated by the unit test in the previous commit.
The test vectors were generated using https://github.com/darosior/bitcoin/tree/bip54_miner
That is, enforce nLockTime be set to height-1 and nSequence not be set to final.
The test vectors were generated using https://github.com/darosior/bitcoin/tree/bip54_miner
… 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.
It's not a standardness limit anymore, it was made consensus. Thanks to Anthony Towns for the scripted diff script. -BEGIN VERIFY SCRIPT- 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>
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.
It was requested by ariard during review.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
No description provided.