BIP-119 (OP_CHECKTEMPLATEVERIFY) (regtest only)

jamesob commented at 7:08 pm on March 4, 2025: contributor

This implements BIP-119 (OP_CHECKTEMPLATEVERIFY), but only specifies a regtest deployment. There is no effective policy change, since the SCRIPT_VERIFY_* flags (as used) result in the same NOP-like behavior.

This change can be composed with other opcode specifications (e.g. CSFS, CAT) and bundled into the same deployment (yet to be specified).

I encourage more general, conceptual discussion to happen on Delving Bitcoin and not on this pull request.

Some related discussion on Delving Bitcoin here:

https://delvingbitcoin.org/t/ctv-csfs-can-we-reach-consensus-on-a-first-step-towards-covenants/

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31989.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept NACK	BitcoinErrorLog
Concept ACK	jaybny, moonsettler, jonatack

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#32080 (OP_CHECKCONTRACTVERIFY by bigspider)
#29843 (policy: Allow non-standard scripts with -acceptnonstdtxn=1 (test nets only) by ajtowns)
#29247 (CAT in Tapscript (BIP-347) by 0xBEEFCAF3)
#29039 (versionbits refactoring by ajtowns)
#26201 (Remove Taproot activation height by Sjors)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

glozow added the label Consensus on Mar 4, 2025

jamesob force-pushed on Mar 4, 2025

jamesob force-pushed on Mar 5, 2025

jamesob renamed this:
~~Add BIP-119 (OP_CHECKTEMPLATEVERIFY) (no activation)~~
BIP-119 (OP_CHECKTEMPLATEVERIFY) (no activation)
on Mar 5, 2025

BitcoinErrorLog commented at 7:47 am on March 5, 2025: none

NACK. As Towns just pointed out on the mailing list, this proposal represents an unsolicited modification of Bitcoin’s code and consensus rules. There is no clear user demand, no demonstrated market need, and no compelling application that justifies taking on this risk. The absence of significant business interest or ecosystem momentum further underscores the speculative nature of this change.

Bitcoin’s resilience depends on its conservatism. To safeguard against unnecessary complexity and centralized agenda-setting, we should reject rule changes that lack overwhelming, emergent consensus.

Until we establish clear standards for what constitutes legitimate consensus-driven changes and market-driven necessity, rather than academic or developer curiosity, proposals of this nature should be treated as research, not candidates for inclusion in Core.

melvincarvalho commented at 10:50 am on March 5, 2025: none

Echoing what @BitcoinErrorLog said—clear user demand must be shown to justify this. Any proposed change should be weighed against the worst-case scenario: civil war or a chain split, which we’ve seen before. The use case needs to be so compelling that it decisively outweighs these risks and gains overwhelming community support.

1440000bytes commented at 11:20 am on March 5, 2025: none

@BitcoinErrorLog @melvincarvalho

I encourage more general, conceptual discussion to happen on Delving Bitcoin and not on this pull request.

In case you want to read other developers’ evaluations of CHECKTEMPLATEVERIFY and its use cases, here are a couple of links:

https://en.bitcoin.it/wiki/Covenants_support
https://en.bitcoin.it/wiki/Covenants_Uses

willcl-ark commented at 2:28 pm on March 5, 2025: member

@jamesob I’d be happy to help you keep this thread focused by moderating off-topic comments and pointing them to the appropriate forums.

You’ve suggested and linked to Delving which seems appropriate for general/conceptual discussion, however there is no thread/post over there to actually point folks to. Would you mind starting a thread over there so that we have somewhere concrete to point these types of discussions to?

melvincarvalho commented at 3:32 pm on March 5, 2025: none

@1440000bytes wrote:

not on this pull request

Discussions related to this GitHub PR should remain on GitHub and the mailing list, where Bitcoin development has historically been debated and recorded. Moving discussions to multiple external platforms with different registration requirements, user bases, and change control mechanisms creates fragmentation and makes it harder to maintain a clear, consistent record of technical decisions.

For example, your tweet from Jan 26, which got 71 likes, demonstrates how statements made in one venue can later be deleted or altered, making it difficult to ensure accountability and track how discussions evolve over time.

Furthermore, the belligerent posture some advocates have taken toward the Bitcoin Core project could be perceived as undermining the process, further increasing the risk of a contentious chain split—a scenario that should be the primary concern of all Bitcoin developers.

While external forums like Delving Bitcoin may be useful for general discussions, they are not officially part of the Bitcoin project and do not provide the same permanence or transparency as GitHub and the mailing list. To maintain clarity and continuity, technical discussions should remain in publicly archived, historically relevant venues with established change control.

pinheadmz commented at 3:34 pm on March 5, 2025: member

@melvincarvalho please review the moderation policy: https://github.com/bitcoin-core/meta/blob/main/MODERATION-GUIDELINES.md

Github pull request review comments are restricted exclusively for PULL REQUEST REVIEW COMMENTS

average-gary commented at 5:22 pm on March 5, 2025: none

@jamesob I’d be happy to help you keep this thread focused by moderating off-topic comments and pointing them to the appropriate forums.

You’ve suggested and linked to Delving which seems appropriate for general/conceptual discussion, however there is no thread/post over there to actually point folks to. Would you mind starting a thread over there so that we have somewhere concrete to point these types of discussions to?

https://delvingbitcoin.org/t/bip-119-op-checktemplateverify-no-activation/1494

in src/script/interpreter.cpp:1503 in 112b02e31b outdated

1499@@ -1500,6 +1500,8 @@ void PrecomputedTransactionData::Init(const T& txTo, std::vector<CTxOut>&& spent
1500         m_spent_outputs_ready = true;
1501     }
1502 
1503+    // TODO: Improve this heuristic

TheCharlatan commented at 8:24 pm on March 5, 2025:

Are the TODOs going to be addressed here?

jamesob commented at 10:18 pm on March 5, 2025:

In this case, likely not - @JeremyRubin was initially thinking that if there was some way we could rule out use of OP_CTV in the script about to be evaluated, we could forgo warming the caches below. But (aside from that being probably impossible) this TODO was made prior to benchmarking that shows there isn’t any detectable slowdown to initializing those caches.

I’ll remove the comment.

moonsettler commented at 10:38 pm on March 5, 2025:

Shouldn’t it be set to force instead of true?

Makes this if completely superfluous:

 0      if (uses_bip143_segwit || uses_bip341_taproot || uses_bip119_ctv) {
 1        // Computations shared between both sighash schemes.
 2        m_prevouts_single_hash = GetPrevoutsSHA256(txTo);
 3        m_sequences_single_hash = GetSequencesSHA256(txTo);
 4        m_outputs_single_hash = GetOutputsSHA256(txTo);
 5
 6        // 0 hash used to signal if we should skip scriptSigs
 7        // when re-computing for different indexes.
 8        m_scriptSigs_single_hash = NoScriptSigs(txTo) ? uint256{} : GetScriptSigsSHA256(txTo);
 9        m_bip119_ctv_ready = true;
10    }

jamesob commented at 10:47 pm on March 5, 2025:

@moonsettler no - force is simply the initial value for the segwit/taproot detection variables. Since there isn’t a similar detection process for CTV, we must assume its cache prep is always required. But that’s a good point that it’s probably worthwhile to strip out the if.

Likely the only way we could make use_bip119_ctv actually variable is on the basis of activation height. Hence the original TODO.

moonsettler commented at 11:03 pm on March 5, 2025:

Likely the only way we could make use_bip119_ctv actually variable is on the basis of activation height. Hence the original TODO.

Sounds better than nothing.

jamesob force-pushed on Mar 5, 2025

ariard commented at 10:53 pm on March 5, 2025: contributor

This change can be composed with other opcode specifications (e.g. CSFS, @jamesob see my pending email in answer to AJ for the issues with CSFS building on @naumenkogs’s TxWithhold:

Especially if it’s possible to do a TxWithhold on the spend of any coinbase output beyond COINBASE_MATURITY. At least having an impossibility result that you cannot do a TxWithhold by grinding the miner’s scriptpubkey would be welcome from a safe engineering perspective. I’m not able to come with a practical offensive TxWithhold with OP_CTV alone, though as soon as you combine it with another primitive this is plausible.

jamesob force-pushed on Mar 5, 2025

pinheadmz commented at 11:44 pm on March 5, 2025: member

@ariard I think your comment is more appropriate on the mailing list or delving. You are even mentioning a specific post from the mailing list. I expect a lot of off-topic comments on this PR and would appreciate your help staying focused. PR comments MUST be entirely focused on THIS PR code.

ariard commented at 7:47 pm on March 6, 2025: contributor

@pinheadmz Sure, the conceptual aspects of this PR are better discussed on the mailing list or delving or even on nostr with no moderators. Generally, this is up to the PR author to indicate on which communication channel, they have posted the conceptual motivation of a code change for discussion. Here we have a code change motivated by other arguments (bundle OP_CSFS, OP_CAT) which are not mentioned on the Delving Bitcoin thread itself, and as such making the conversation on the technical rational of the change incomplete in itself.

I’ll silence that any conceptual review of a change has to be founded on the code itself, and doing back and forth between the code and the technical concepts is most of the time very insightful. I’m sure that something that you Pineheadmz you’re aware of as an experienced reviewer on Bitcoin Core code itself, well-known in this space for your technical contributions.

Finally, if this PR got reviewed elsewhere on another repository than the Bitcoin Core one, this is my pleasure to have the technical discussion on CTV and any corresponding technical review there.

darosior commented at 8:35 pm on March 6, 2025: member

The code in this pull request is of good quality and is well tested, but it’s not clear to me what’s the goal with this pull request nor how it fits in the big picture. As this introduces a new consensus feature, it seems premature to merge this into Bitcoin Core until widespread consensus is reached among Bitcoin users to eventually activate this new feature in a soft fork.

To be clear, i am not saying the details of the activation need to be figured out before the implementation of the feature goes in, but there needs to at least be rough consensus among Bitcoin users to activate this at all.

michaelfolkson commented at 9:55 am on March 7, 2025: none

I encourage more general, conceptual discussion to happen on Delving Bitcoin and not on this pull request.

@jamesob I’d be happy to help you keep this thread focused by moderating off-topic comments and pointing them to the appropriate forums.

To state the obvious if you push “conceptual discussion” (a critical part of the Bitcoin Core review process for a PR) to external forums (why not an accompanying issue in this repo?) the consideration of whether to merge this pull request would require monitoring these external forums. Merging this pull request would be a step towards a second declared activation attempt for CTV and a potential chain split if there isn’t community consensus. Pushing that conceptual discussion outside of this repo doesn’t change that fact.

moonsettler commented at 11:08 am on March 7, 2025: none

We are going to activate CTV I don’t think that’s a question. The question is will core merge it in before or after lock-in?

michaelfolkson commented at 11:15 am on March 7, 2025: none

We are going to activate CTV I don’t think that’s a question. The question is will core merge it in before or after lock-in?

If you don’t care about the Bitcoin Core review process, community consensus and causing chain splits probably better you close this PR and attempt to cause chaos in a different repo.

1440000bytes commented at 0:15 am on March 8, 2025: none

This pull request is last attempt IMO to get some “technical reviews” and find bugs.

It will obviously get merged when community wants to use it and looking for activation.

jaybny commented at 2:00 am on March 8, 2025: none

Before proceeding with a full technical review, I’d like to clarify whether this pull request represents a complete rewrite or simply a refactor of the original implementation from three years ago.

Off-topic

Concept ACK

CTV is an objectively well-reviewed, fundamental upgrade to Bitcoin, and it has gained broad consensus among subject matter experts over the past three years. There will only ever be a handful of legitimate, code-complete upgrades to Bitcoin that are truly ready for merge—and this is one of them, an ingenious, well-thought-out enhancement to Bitcoin with no downside.

I disagree with the notion that consensus-breaking code should receive special treatment compared to other changes. Every upgrade to Bitcoin carries some risk of unknowns (after all, the bug that caused the chain split in 2010 wasn’t consensus code). Moreover, not every low-level piece of C++ code needs to be judged solely on market demand. In Bitcoin, the only reason some people focus their complaints on consensus-breaking code is because such changes require a broad social activation method—to ensure miners are informed, upgrade their nodes, and keep the network in sync to avoid a chain split. This process isn’t a social vote; it’s simply a way of signaling to the hardware operators that Bitcoin software must be upgraded and indicating when it is safe to activate.

Frankly, the only reason this ends up on the non-technical timeline is because social consensus is needed to get the word out to miners. Otherwise, like the hundreds of other low-level Bitcoin Core C++ code merges, this wouldn’t attract such uninformed commentary.

While there’s naturally a significant overlap between consensus code and Bitcoin’s core fundamentals, BIP119 provides additional scaling techniques and supports second-layer mechanisms without fundamentally altering Bitcoin itself.

Ultimately, mergeable C++ code that has undergone thorough review and reached technical consensus should be merged in a timely manner to avoid staleness—even if activation debates continue elsewhere.

Given the limited number of humans capable of writing and reviewing low-level C++ Bitcoin upgrades, I hope this is the last time we code and review the same feature in the same codebase.

Unless there are substantive technical issues, I recommend we focus on the code review here and move broader conceptual discussions to the appropriate forums.

rot13maxi commented at 2:27 am on March 8, 2025: none

simple implementation, helpful comments, good test coverage, no backwards compatibility issues, passes all CI.

LGTM!

moonsettler commented at 6:04 pm on March 8, 2025: none

I don’t know how many times we have played this game, but on a positive note ofc I Concept ACK

jamesob commented at 8:08 pm on March 12, 2025: contributor

I’ve added a link to the active Delving Bitcoin discussion most pertaining to CTV, and will keep the PR description above updated with any other relevant discussions.

jonatack commented at 0:45 am on March 13, 2025: member

Concept ACK, will begin nuts and bolts review.

in src/script/interpreter.cpp:1531 in 4c5790105e outdated

1528@@ -1427,12 +1529,19 @@ void PrecomputedTransactionData::Init(const T& txTo, std::vector<CTxOut>&& spent
1529         if (uses_bip341_taproot && uses_bip143_segwit) break; // No need to scan further if we already need all.
1530     }
1531

moonsettler commented at 12:02 pm on March 13, 2025:

Computation required by CTV ideally would not be carried out when CTV could not have been active. In this context the tx.version field is available. If CTV (or at least bare CTV) required version 3 or higher (not sure this is a good idea or not?), then it would be trivial to augment the old logic that was more conservative with resource use. (again not sure how much that matters in practice?)

instagibbs commented at 7:12 pm on March 18, 2025: member

I suggest a regtest-only deployment is defined so functional testing can be re-instated. Currently there is zero consensus coverage at the functional level from what I can tell: https://github.com/bitcoin/bitcoin/pull/31989/commits/4c5790105e7fb3a2cd207403af769fbbccd7296d

jamesob commented at 8:25 pm on March 19, 2025: contributor

I suggest a regtest-only deployment is defined so functional testing can be re-instated. Currently there is zero consensus coverage at the functional level from what I can tell: 4c57901

Done in https://github.com/bitcoin/bitcoin/pull/31989/commits/5bb734b9d3628395c031fdf0eb0fbb249fe578e3.

jamesob renamed this:
~~BIP-119 (OP_CHECKTEMPLATEVERIFY) (no activation)~~
BIP-119 (OP_CHECKTEMPLATEVERIFY) (regtest only)
on Mar 19, 2025

jamesob force-pushed on Mar 20, 2025

DrahtBot added the label CI failed on Mar 25, 2025

DrahtBot commented at 2:08 am on March 25, 2025: contributor

🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/39083890759

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

Add StandardTemplateHash definition 52831116d5

Add SignatureChecker method for checking DefaultCheckTemplateVerifyHash. 47f9e116a4

Add OP_CHECKTEMPLATEVERIFY Opcode as OP_NOP4

[TESTS] modify script_tests.json to enable OP_NOP4 as OP_CHECKTEMPLATEVERIFY

7b73bcb6f1

Change printing of NOP4 to CheckTemplateVerify

Separate commit for ease of bisecting should downstream software expecting to parse NOP4
now fails.

3a72874599

script: precompute the DefaultCheckTemplateVerifyHash

Co-authored-by: James O'Beirne <github@au92.org>

488ae6c553

policy: make bare OP_CHECKTEMPLATEVERIFY standard

Co-authored-by: James O'Beirne <github@au92.org>

f85f809e58

test: add tx_valid.json tests for BIP-119 CheckTemplateVerify 3fe62c8972

test: add tx_invalid.json examples for CTV 08cad0c55b

test: add CTV hash computation unit test & mutation tester

Co-authored-by: James O'Beirne <github@au92.org>

c983506a8c

consensus: add a CTV deployment for regtest only aa546bee00

test: add OP_CHECKTEMPLATEVERIFY functional tests

Co-authored-by: James O'Beirne <github@au92.org>

693bb527ec

jamesob force-pushed on Mar 26, 2025

DrahtBot removed the label CI failed on Mar 26, 2025

BIP-119 (OP_CHECKTEMPLATEVERIFY) (regtest only) #31989

Code Coverage & Benchmarks

Reviews

Conflicts