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

pull jamesob wants to merge 9 commits into bitcoin:master from jamesob:2025-03-ctv changing 34 files +3685 −23
  1. 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:

    See also:

  2. DrahtBot commented at 7:08 pm on March 4, 2025: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    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, stevenroose, jonatack
    Stale ACK JeremyRubin

    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)
    • #28710 (Remove the legacy wallet and BDB dependency by achow101)
    • #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.

  3. glozow added the label Consensus on Mar 4, 2025
  4. jamesob force-pushed on Mar 4, 2025
  5. jamesob force-pushed on Mar 5, 2025
  6. jamesob renamed this:
    Add BIP-119 (OP_CHECKTEMPLATEVERIFY) (no activation)
    BIP-119 (OP_CHECKTEMPLATEVERIFY) (no activation)
    on Mar 5, 2025
  7. 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.

  8. 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.
  9. 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

  10. 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?

  11. 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.

    image

    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.

  12. 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

  13. 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

  14. 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.

  15. jamesob force-pushed on Mar 5, 2025
  16. ariard commented at 10:53 pm on March 5, 2025: none

    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.

  17. jamesob force-pushed on Mar 5, 2025
  18. 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.
  19. ariard commented at 7:47 pm on March 6, 2025: none

    @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.

  20. 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.

  21. 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.

  22. 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?
  23. 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.

  24. 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.

  25. 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.

  26. 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!

  27. 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
  28. 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.
  29. in src/script/interpreter.cpp:1533 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?)
  30. 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
  31. 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.

  32. jamesob renamed this:
    BIP-119 (OP_CHECKTEMPLATEVERIFY) (no activation)
    BIP-119 (OP_CHECKTEMPLATEVERIFY) (regtest only)
    on Mar 19, 2025
  33. jamesob force-pushed on Mar 20, 2025
  34. DrahtBot added the label CI failed on Mar 25, 2025
  35. 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.

  36. jamesob force-pushed on Mar 26, 2025
  37. DrahtBot removed the label CI failed on Mar 26, 2025
  38. stevenroose commented at 2:10 am on April 9, 2025: contributor
    Concept ACK. There are a few things that look like rebase leftovers that are added and removed in the different commits.
  39. jonatack commented at 3:41 pm on April 11, 2025: member
    Concept ACK
  40. in src/script/interpreter.h:212 in 693bb527ec outdated
    208@@ -187,6 +209,11 @@ struct PrecomputedTransactionData
    209     explicit PrecomputedTransactionData(const T& tx);
    210 };
    211 
    212+/* Standard Template Hash Declarations */
    


    ariard commented at 2:15 am on April 12, 2025:
    minor: I think all mention to “Standard” template can be removed and just s/standard/default/g or s/standard/primary/g. We have already IsStandard as a checker utility for policy sanitization, though here IIUC it’s meant to be “standard” as the default template hash 32-byte as told by BIP 119, which does not preclude future template that wouldn’t be necessarily the default 32-byte one.
  41. in src/script/interpreter.cpp:1906 in 693bb527ec outdated
    1899+        assert(txTo != nullptr);
    1900+        uint256 hash_tmpl = txdata->m_scriptSigs_single_hash.IsNull() ?
    1901+            GetDefaultCheckTemplateVerifyHashEmptyScript(*txTo, txdata->m_outputs_single_hash, txdata->m_sequences_single_hash, nIn) :
    1902+            GetDefaultCheckTemplateVerifyHashWithScript(*txTo, txdata->m_outputs_single_hash, txdata->m_sequences_single_hash,
    1903+                    txdata->m_scriptSigs_single_hash, nIn);
    1904+        return std::equal(hash_tmpl.begin(), hash_tmpl.end(), hash.data());
    


    ariard commented at 2:42 am on April 12, 2025:

    If I’m understanding the biggest transaction size that can be verified by an OP_CTV is equal to ~MAX_BLOCK_WEIGHT.`

    While there is a limit on the max transaction size (MAX_STANDARD_TX_WEIGHT), this a transaction-relay only limit and if a miner is accepting non-standard tx, and there are of course some, a ~MAX_BLOCK_WEIGHT size transaction can be given as spending a CTV-locked input.

    Moreover, currently there are only 2 caches in bitcoin core to speed up the validation of a transaction (SignatureCache) and (m_script_execution_cache). IIRC, they both commit to the transaction hash spends with by a uint256 entry. So if a ~MAX_BLOCK_WEIGHT changes for 1-bit, I think there will be a re-computation of the hash for the whole ~MAX_BLOCK_WEIGHT.

    What prevents a peer to repeat broadcast of such ~MAX_BLOCK_WEIGHT tx CTV-spend to a target node ? Peer cost is just to tweaking some random 1-bit in the whole ~MAX_BLOCK_WEIGHT surface. Note, I believe the max size hashing you can do (i.e OP_HASH256) is limited by MAX_STACK_SIZE. I’m not sure this is considered in the BIP.


    jamesob commented at 6:04 pm on April 15, 2025:

    What prevents a peer to repeat broadcast of such ~MAX_BLOCK_WEIGHT tx CTV-spend to a target node ?

    The processing of such a CTV-spend is either limited by standardness or PoW; standardness prohibits the relay of a large, non-mined CTV transaction, and in the other case network-difficulty PoW will materially limit the number of times a node has to hash 4MB worth of transaction data.

    And anyway, hashing 4MB of transaction data is substantially faster than checking signatures for a correspondingly sized block full of sigops; see this rough benchmark:

     0 % g++ -o bench bench.cpp -lsecp256k1 -lcrypto -std=c++17 && ./bench
     1
     2Generating 4MB of random data for hashing...
     3Preparing 32768 signatures for validation benchmark...
     4Running SHA-256 benchmark...
     5Running signature validation benchmark...
     6
     7=== BENCHMARK RESULTS ===
     8Data size: 4 MB
     9Number of signatures: 32768 (1MB)
    10
    11SHA-256 hashing time: 2.15 ms
    12Signature validation time: 1084.43 ms
    13
    14Ratio (validation/hashing): 504.51x
    

    ariard commented at 0:03 am on April 18, 2025:

    Thanks for the bench.cpp, I had a look on benchmark_sha256 and benchmark_signature_validation.

    Though here few more thoughts on the 1-bit cost from the view of a DoSing peer.

    Currently, we have 2 caches the signature cache and the m_script_execution_cache.

    If a script has already an entry at CheckInputScripts(), bitcoin core returns true: https://github.com/bitcoin/bitcoin/blob/master/src/validation.cpp#L2185

    If there is already a cache entry, and the script includes signatures (and I believe all scripts currently used for spends are using an asymmetric keypair as a lock), all the signatures are going to be pre-processed by the SignatureChecker: https://github.com/bitcoin/bitcoin/blob/master/src/script/sigcache.cpp#L63 (ECDSA) or https://github.com/bitcoin/bitcoin/blob/master/src/script/sigcache.cpp#L76 (Schnorr).

    From the view of the DoSing peer, currently if you’re doing DoS with a script-based signature, for each signature associated with a pubkey committed in a redeemscript, there is a computing cost to generate fresh signature, as one has to re-generate a DER-encoded signature of the same length (IsValidSignatureEncoding() i.e > 9 bytes or < 73 bytes or CheckSchnorrSignature() i.e 64 or 65 byte), even if the signature verification check yields it’s consensus invalid. Relayed and announced invalid transactions are cached in the m_inventory_known_filter.

    With a world with a hash-only lock, this is not the same computing cost anymore at all for a DoSing peer, as there is just one 1-bit to tweak in the whole CTV MAX_BLOCK_WEIGHT block. On the other hand, a signature-based DoS, you’re very quickly limited by the size and malleability space of the signature, which it’s itself constrained by the pubkey type committed in an already confirmed coin (when it’s not a chain of tx validated in the same block).

    If this reasoning is correct, and please point out if there are not flaws, I could be worthy to add some policy-only rule for CTV spend, e.g MAX_CTV_INPUT_SPEND_WEIGHT or something similar, that can be accordingly adjusted in the idea.

    I have no idea in terms of inputs what is the biggest transactions considered for a CTV-spend for hypothetical CTV use-cases so far.

  42. ariard commented at 2:43 am on April 12, 2025: none

    I think it would be valuable to have CTV rather as a taproot op_success.

    There has been a proof-of-concept branch done here: https://delvingbitcoin.org/t/ctv-csfs-can-we-reach-consensus-on-a-first-step-towards-covenants/1509/58

  43. in src/test/ctvhash_tests.cpp:44 in 693bb527ec outdated
    39+
    40+            try {
    41+                auto& hash_arr = test["result"].get_array();
    42+                for (size_t i = 0; i < hash_arr.size(); ++i) {
    43+                    hash.emplace_back();
    44+                    hash.back().SetHexDeprecated(hash_arr[i].get_str());
    


    average-gary commented at 3:01 pm on April 15, 2025:

    SetHexDeprecated() was recently removed in #32237

    Possible resolution here:

    0                   auto opt_hash = uint256::FromHex(hash_arr[i].get_str());
    1                     if (!opt_hash) {
    2
    3
    4                         BOOST_ERROR("Bad test: Invalid hex string" << strTest);
    5                         continue;
    6                     }
    7                     hash.push_back(*opt_hash);
    
  44. DrahtBot added the label CI failed on Apr 15, 2025
  45. DrahtBot commented at 4:27 pm on April 15, 2025: contributor

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

    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.

  46. jamesob force-pushed on Apr 15, 2025
  47. jamesob force-pushed on Apr 15, 2025
  48. jamesob commented at 6:38 pm on April 15, 2025: contributor

    I’ve pushed a rebase that

    • simplifies the commit structure,
    • removes some of the intra-commit whitespace/indent changes,
    • adds @average-gary’s uint256::FromHex() test change (and author credits).
  49. DrahtBot removed the label CI failed on Apr 15, 2025
  50. in src/script/interpreter.h:279 in 3e090231b8 outdated
    275@@ -265,6 +276,11 @@ class BaseSignatureChecker
    276          return false;
    277     }
    278 
    279+    virtual bool CheckDefaultCheckTemplateVerifyHash(const std::vector<unsigned char>& hash) const
    


    JeremyRubin commented at 1:21 am on April 16, 2025:
    nit: can these be converted to span to minimize the diff on the next commit?
  51. in src/script/interpreter.cpp:1490 in a11706203d outdated
    1488 uint256 GetDefaultCheckTemplateVerifyHash(
    1489         const TxType& tx, const uint256& outputs_hash, const uint256& sequences_hash, const uint32_t input_index) {
    1490-    bool skip_scriptSigs = std::find_if(tx.vin.begin(), tx.vin.end(),
    1491-            [](const CTxIn& c) { return c.scriptSig != CScript(); }) == tx.vin.end();
    1492-    return skip_scriptSigs ? GetDefaultCheckTemplateVerifyHashEmptyScript(tx, outputs_hash, sequences_hash, input_index) :
    1493+    return NoScriptSigs(tx) ? GetDefaultCheckTemplateVerifyHashEmptyScript(tx, outputs_hash, sequences_hash, input_index) :
    


    JeremyRubin commented at 1:22 am on April 16, 2025:
    nit: can this be moved to the last patch? edit: by last, i mean prior. Can NoScriptSigs be introduced in the first commit
  52. JeremyRubin commented at 1:31 am on April 16, 2025: contributor

    Reviewed, code seems correct and to match the old prs closely.

    cr ACK 2b74adf

    will re-review as necessary

  53. DrahtBot requested review from moonsettler on Apr 16, 2025
  54. DrahtBot requested review from jonatack on Apr 16, 2025
  55. ariard commented at 0:17 am on April 18, 2025: none

    I think it would be valuable to have CTV rather as a taproot op_success.

    So I’ve re-read the current version of BIP119 as available in the BIP repository, there is a section about the usage of OP_NOP (NOP-Default and Recommended Standardness Rules), however it doesn’t say among the NOP upgradable path and the current upgrade path available with Taproot OP_SUCCESS or an upgrade of the tapscript version.

    I’m aware that CTV was designed and drafted for standard, before Taproot got activated. However, it would benefit for composability with future opcodes or if a use-case wish to compose CTV semantics + schnorr verification (e.g <OP_CHECKTEMPLATEVERIFY> <OP_CHECKSIGADD>.

    Non-malleable Schnorr signature verification combined with a CTV check is something clearly any use-case could be interested to build on, or at least to have that level of expressivity. Not sure, if it has been considered somewhere else, e.g in the BIP 119 footnote or else.

  56. DrahtBot added the label Needs rebase on Apr 29, 2025
  57. Add StandardTemplateHash definition
    and associated SignatureChecker method.
    81172de4a9
  58. Add OP_CHECKTEMPLATEVERIFY Opcode as OP_NOP4
    Also modifies script_tests.json to enable OP_NOP4 as OP_CHECKTEMPLATEVERIFY.
    fdf01f698b
  59. script: precompute the DefaultCheckTemplateVerifyHash
    Co-authored-by: James O'Beirne <github@au92.org>
    d08b65da55
  60. policy: make bare OP_CHECKTEMPLATEVERIFY standard
    Co-authored-by: James O'Beirne <github@au92.org>
    604d61ee53
  61. test: add tx_valid.json tests for BIP-119 CheckTemplateVerify e6f3ad95a8
  62. test: add tx_invalid.json examples for CTV 83a353699c
  63. test: add CTV hash computation unit test & mutation tester
    Co-authored-by: James O'Beirne <github@au92.org>
    Co-authored-by: Gary Krause <gary.krause@mara.com>
    4143256ac6
  64. consensus: add a CTV deployment for regtest only 8577fedd11
  65. test: add OP_CHECKTEMPLATEVERIFY functional tests
    Co-authored-by: James O'Beirne <github@au92.org>
    915e0f32c1
  66. jamesob force-pushed on Apr 30, 2025
  67. DrahtBot removed the label Needs rebase on May 1, 2025
  68. DrahtBot added the label Needs rebase on May 7, 2025
  69. DrahtBot commented at 2:23 pm on May 7, 2025: contributor

    🐙 This pull request conflicts with the target branch and needs rebase.


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-05-08 12:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me