Skip to content

Conversation

darosior
Copy link
Member

This introduces test vectors for BIP54. There is one set of vectors per each of the 4 mitigations.

The vectors were generated using the BIP54 implementation against Bitcoin Inquisition available here, as well as a custom miner as a Bitcoin Core unit test available here. Documentation is provided with more details about each set of test vectors and describing how to use them.

This introduces a set of test vectors for each of the 4 mitigations in the BIP. The sigops and
transaction size vectors were generated using the unit tests included with the Bitcoin Core
implementation of BIP54, available at https://github.com/darosior/bitcoin/tree/2509_inquisition_consensus_cleanup.
The timestamps and coinbases required mainnet blocks for maximum compatibility, and were generated
by two dedicated unit tests not included with the Bitcoin Core implementation above but available at
https://github.com/darosior/bitcoin/tree/bip54_miner.
Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK, successfully built the test branch at https://github.com/darosior/bitcoin/blob/2509_inquisition_consensus_cleanup and ran the unit and functional tests.

$ ./build/bin/test_bitcoin --run_test=bip54_tests
Running 4 test cases...

*** No errors detected
$ ./build/test/functional/feature_bip54.py     
2025-10-21T16:05:13.483000Z TestFramework (INFO): PRNG seed is: 80635940825195715
2025-10-21T16:05:13.483000Z TestFramework (INFO): Initializing test directory /var/folders/bz/mn3hr6td37bczwp7j89frs4w0000gn/T/bitcoin_func_test_dznr0x1z
2025-10-21T16:05:14.300000Z TestFramework (INFO): BIP54 tests before activation
2025-10-21T16:05:14.558000Z TestFramework (INFO): Activating BIP54
2025-10-21T16:05:14.745000Z TestFramework (INFO): BIP54 tests after activation
2025-10-21T16:05:15.296000Z TestFramework (INFO): Stopping nodes
2025-10-21T16:05:15.460000Z TestFramework (INFO): Cleaning up /var/folders/bz/mn3hr6td37bczwp7j89frs4w0000gn/T/bitcoin_func_test_dznr0x1z on exit
2025-10-21T16:05:15.460000Z TestFramework (INFO): Tests successful

Are the functional tests worth mentioning in the test_vectors README? Perhaps with some of the content in the commit message:

The previously introduced unit tests extensively test the specific implementation of each
mitigation. This functional test complements them by sanity checking all mitigations in a "black
box" manner. 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.

## BIP54 test vectors

This folder contains a set of test vectors for each mitigation introduced in the BIP. This document
presents them in more details.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
presents them in more details.
presents them in more detail.

Copy link
Contributor

@murchandamus murchandamus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, see nit

Comment on lines +86 to +88
genesis block), and whether this block chain is valid according to BIP54. All test cases are valid
according to current Bitcoin's consensus rules, except one that features a block containing a
coinbase transaction timelocked to a future block height.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there then perhaps also be a test case that has a coinbase transaction locked to a block height that is lower than required?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There definitely was one but looks like i messed up somewhere in re-generating the vectors. Will re-add it, thank you.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be clear, I was just looking at the description, I did not check the test vectors, so it might just be the description that is off.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, then i just checked and it still is in here! Link.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants