BIP 54 test vectors improvements following review in Inquisition

darosior commented at 9:35 pm on March 13, 2026: member

This addresses the feedback received on the test vectors following the merge of #2015, the review of the Bitcoin Inquisition implementation (https://github.com/bitcoin-inquisition/bitcoin/pull/99), and Chris Stewart’s implementation for Bitcoin-s (https://github.com/bitcoin-s/bitcoin-s/pull/6170).

There are 4 categories of changes here:

Rewording and clarifications in the BIP text.
Some typo fixes and field reordering in the test vectors.
A change in the structure of the timestamps test vectors, to reduce the size of the file by ~80%.
Test case addition(s).

See commit messages for details.

bip54: reword "potentially executed" language in sigops limit specification

The paragraph in its entirety is already unambiguous that all signature-checking operations
*present* in the Script (as opposed to *executed*) are counted. However i received feedback that the
"potentially executed" language in the first sentence of this paragraph may be confusing. This is
because it is in theory possible to have a more accurate upper bound by analyzing the possible
spending paths and use the maximum number of signature-checking operations in either to check
against the limit.

This commit rewords the first sentence to use the word "present" to be extra-clear before even
describing how the accounting is performed in later sentences.

aef4d9e084

bip54: expand on rationale for non-final coinbase nSequence

Making sure that the coinbase is timelocked is a neat simplification in reasoning about duplicates,
but it has caused some confusions. For instance here:
https://gnusha.org/pi/bitcoindev/e758af9b-72fc-4fdd-8e07-e1126635780an@googlegroups.com/

Fix this by explicitly stating the implications.

1f21139957

bip54: update sigops test vectors following Inquisition review

This is a batch update to the tests vectors for the limit on legacy signature-checking operations
introduced in BIP 54, following feedack received on the Bitcoin Inquisition PR and from Chris
Stewart's implementation in Bitcoin-S.

ff39733e70

bip54: move comment as first element in txsize test vectors

This is part of feedback received during the Bitcoin Inquisition PR review.

78126a5f0c

bip54: make test vectors POSIX-compliant (newline at EOF)

This was pointed out during the Bitcoin Inquisition PR review

4f94c5f01f

murchandamus added the label BIP update by author on Mar 13, 2026

in bip-0054.md:68 in aef4d9e084 outdated

63@@ -64,7 +64,7 @@ Given a block at height `N`:
64   or equal to the value of the timestamp of the block at height `N-2015` (T<sub>N</sub> &ge;
65   T<sub>N−2015</sub>).
66 
67-A limit is set on the number of potentially executed signature operations in validating a
68+A limit is set on the number of signature operations present in the scripts used to validate a
69 transaction. It applies to all transactions in the block except the coinbase transaction[^1]. For

ariard commented at 3:28 am on March 16, 2026:

minor: find the commit message being clear enough that it would be actually valuable to mention that the sum (or union ?) of all the non-executed spending paths is the value on which the limit is applied.

in bip-0054.md:224 in 1f21139957 outdated

220@@ -218,6 +221,10 @@ bip-0034 height commitment and the corresponding future block height.
221 coinbase transactions as not having duplicate past Consensus Cleanup activation would be consistent
222 for any implementation which enforces `nLockTime` from the genesis block, which is the behaviour
223 notably of Bitcoin Core but also of all other implementations the authors are aware of.
224+[^12]: For instance Bitcoin Core only disables [bip-0030][BIP30] validation for a specific chain

ariard commented at 3:53 am on March 16, 2026:

i think the footnote can be improved here. something something like, “as a matter of performance optimizations, Bitcoin Core since $HASH_COMMIT_OR_RELEASE has only been enforcing BIP34, which for guaranteeing uniqueness of txid is a superset of BIP30 due to the mandatory inclusion of the height in the coinbase transaction’s scriptSig.. Without enforcing the setting of the nLocktime field to be equal to the block height for the coinbase transaction, this would have to be perpetuated after the Consensus Cleanup activation”.

darosior commented at 3:16 pm on March 16, 2026:

Without enforcing the setting of the nLocktime field to be equal to the block height for the coinbase transaction, this would have to be perpetuated after the Consensus Cleanup activation”.

That would be inaccurate. You need the full timelocking of the coinbase transaction (nLockTime + nSequence), or you would need to do the same thing for Consensus Cleanup as was done for BIP 34.

ariard commented at 4:16 am on March 23, 2026:

Yeah I mean I’m still trying to understand the logic of the nLocktime + nSequence what problem it solves, given that putting a uniqueness marker in the form of the block height is enough to avoid future coinbase txid collision. Like I said, I’ll re-check IsFinalTx for my own sake.

I think I’m getting the rational of the purpose of this pair of nLocktime + nSequence by requesting at least one input to be non-final, you ensure that the nLocktime must have been enforced, therefore that a coinbase tx in the far past with a nLocktime=block_height_ABC couldn’t collide with coinbase tx at block_height_ABC due to the nSequence diff. To this, one could check that the historical blockchain does not contain coinbase transaction with nLocktime == height_in_the_future, but even it would be the case today (and I think there are some comments on this on the Delving Bitcoin thread), there would be not guarantee that a (miner) script kiddie falsify this assumption until the Consensus Cleanup got activated.

Design alternative would be to add a unique structure in the coinbase transaction (e.g with the commitment structure allowed by BIP141), but effectively it would be come with an artificial inflation size of the coinbase transaction (and I’m not even talking of the p2p changes that would comes as a necessity with that). Musing, if there is a more cleaner solution.

Generally, Lamport’s “State the Problem Before Describing the Solution” (https://lamport.azurewebsites.net/pubs/state-the-problem.pdf) is a good essay on how to explain clearly a problem, before coming with the solution.

in bip-0054.md:227 in 1f21139957 outdated

220@@ -218,6 +221,10 @@ bip-0034 height commitment and the corresponding future block height.
221 coinbase transactions as not having duplicate past Consensus Cleanup activation would be consistent
222 for any implementation which enforces `nLockTime` from the genesis block, which is the behaviour
223 notably of Bitcoin Core but also of all other implementations the authors are aware of.
224+[^12]: For instance Bitcoin Core only disables [bip-0030][BIP30] validation for a specific chain
225+where [bip-0034][BIP34] violations have been manually inspected (see [here][Core validation.cpp
226+BIP34]). Without the guarantee given by enforcing the timelock on coinbase transactions, this would
227+have to be perpetuated for the Consensus Cleanup.

ariard commented at 3:54 am on March 16, 2026:

see comment above, but i think it should say “after the Consensus Cleanup activation”, the usage of the for do not denotates any temporal referential.

darosior commented at 3:17 pm on March 16, 2026:

As per my response above, the same thing would have to be done for the Consensus Cleanup #2122 (review). Using for in this sentence is therefore appropriate, as far as i can tell.

ariard commented at 3:58 am on March 16, 2026: none

Ongoing review, up to 1f2113995766

Seen the commit message, I’m still thinking the change can be simplified by not enforcing non-finality of the nSequence field due to the order of checks in IsFinalTx(), while still ensuring uniqueness. Fair game, on me to go to write test vectors pre / post bip54 to verify the consensus logic and make the demonstration.

bip54: restructure timestamp test vectors into a tree

The test vectors were initially designed to maximally simple, which led to much redundancy. That was
probably too close to one extreme on the spectrum between simplicity and efficiency.

Here we shave off 20k lines by simply representing the header chains as a tree instead of list of
lists by duplicating all common headers.

977300dad6

bip54: add another transaction size test case

Ariard mentioned he would like to see a test case for a 64-byte transaction spending a Taproot
output with an annex. I took the opportunity to also make the output be an OP_RETURN with a 2-byte
push, as another semi-realistic transaction.

fa608987ea

darosior force-pushed on Mar 16, 2026

murchandamus approved

murchandamus commented at 5:24 pm on March 16, 2026: member

LGTM

ariard commented at 4:00 am on March 23, 2026: none

Light Light Review up to 977300dad631

For ff39733e70 and 977300dad631 this has been mostly grepp’ing the interesting parts, though I’ve not checked that the test coverage stay equivalent.

BIP 54 test vectors improvements following review in Inquisition #2122