Skip to content

Implement Templatehash #4

Open
instagibbs wants to merge 6 commits into
masterfrom
2026-05-op_templatehash
Open

Implement Templatehash #4
instagibbs wants to merge 6 commits into
masterfrom
2026-05-op_templatehash

Conversation

@instagibbs

Copy link
Copy Markdown
Owner

No description provided.

@darosior darosior left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice, thanks for going over all the annoying small conflicts (it's not .rehash() anymore, it's .txid_int, hah!). Just a few comments. Also CI linter is whining.

Comment thread src/test/script_tests.cpp Outdated
Comment on lines +115 to +118
static CScript ScriptFromHex(const std::string& str)
{
return ToScript(*Assert(TryParseHex(str)));
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe use the modern way instead of reintroducing the helper dropped in 8756ccd?

diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp
index d1c7419a73b..45b6cfa879b 100644
--- a/src/test/script_tests.cpp
+++ b/src/test/script_tests.cpp
@@ -112,11 +112,6 @@ static CScript ToScript(const T& byte_container)
     return {span.begin(), span.end()};
 }
 
-static CScript ScriptFromHex(const std::string& str)
-{
-    return ToScript(*Assert(TryParseHex(str)));
-}
-
 static std::string FormatScriptError(ScriptError_t err)
 {
     for (const auto& se : script_errors)
@@ -1827,7 +1822,7 @@ BOOST_AUTO_TEST_CASE(templatehash)
         CTxIn{*Txid::FromHex("0437cd7f8525ceed2324359c2d0ba26006d92d856a9c20fa0241106ee5a597c9"), 21},
         CTxIn{*Txid::FromHex("f4184fc596403b9d638783cf57adfe4c75c605f6356fbc91338530e9831e9e16"), 12}
     };
-    tx.vout.emplace_back(424242, ScriptFromHex("001482074bdf6ce32b071dd120a17cf99cbc01ad3080"));
+    tx.vout.emplace_back(424242, ToScript("001482074bdf6ce32b071dd120a17cf99cbc01ad3080"_hex));
 
     // Construct the script to be spent, checking the (valid) hash of this transaction.
     const uint256 vanilla_hash{GetTemplateHash(tx, 0)};
@@ -1838,7 +1833,7 @@ BOOST_AUTO_TEST_CASE(templatehash)
     const CScript spent_spk{GetScriptForDestination(builder.GetOutput())};
 
     // Outputs spent by the transaction whose template hash is to be checked.
-    CTxOut dummy_txo{1'085'986, ScriptFromHex("76a914079ded3e3befdab0757fe0e8842aeffc0ff2160288ac")};
+    CTxOut dummy_txo{1'085'986, ToScript("76a914079ded3e3befdab0757fe0e8842aeffc0ff2160288ac"_hex)};
     std::vector<CTxOut> spent_outputs{{CTxOut{424243, spent_spk}, dummy_txo}};
 
     // Construct the witness data for the transaction.
@@ -1886,7 +1881,7 @@ BOOST_AUTO_TEST_CASE(templatehash)
     // Output script:
     {
         CMutableTransaction tx2{tx};
-        tx2.vout[0].scriptPubKey = ScriptFromHex("001482074bdf6ce32b071dd120a17cf99cbc01ad3081");
+        tx2.vout[0].scriptPubKey = ToScript("001482074bdf6ce32b071dd120a17cf99cbc01ad3081"_hex);
         CheckTemplateMatch(tx2, Clone(spent_outputs), 0, /*is_valid=*/false, test_cases, std::string{"Template hash mismatches: incorrect output script."});
     }
     // This input's sequence:
@@ -1949,7 +1944,7 @@ BOOST_AUTO_TEST_CASE(templatehash)
     // Other spent output's scriptpubkey:
     {
         std::vector<CTxOut> spent_outputs2(spent_outputs);
-        spent_outputs2[1].scriptPubKey = ScriptFromHex("0014266a4832c001885db26e853ef1d1dde840f7dbaf");
+        spent_outputs2[1].scriptPubKey = ToScript("0014266a4832c001885db26e853ef1d1dde840f7dbaf"_hex);
         CheckTemplateMatch(tx, std::move(spent_outputs2), 0, /*is_valid=*/true, test_cases, std::string{"Template hash matches with malleated scriptpubkey for other spent output."});
     }

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

modernized

Comment thread src/test/script_tests.cpp Outdated
{
auto span{MakeUCharSpan(byte_container)};
return {span.begin(), span.end()};
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is re-defining the ToScript helper defined down below, maybe move the existing one up instead?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

done

Comment thread src/script/interpreter.h Outdated
// Making unknown public key versions (in BIP 342 scripts) non-standard
SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_PUBKEYTYPE,

// OP_TEMPLATEHASH validation (BIP xx)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

s/xx/446/

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

done

Comment thread src/validation.cpp Outdated
flags |= SCRIPT_VERIFY_NULLDUMMY;
}

// Enforce OP_TEMPLATEHASH (BIPxx)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

xx 446

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

done

Comment thread src/policy/policy.h
Comment on lines +110 to +111
SCRIPT_VERIFY_TAPROOT |
SCRIPT_VERIFY_TEMPLATEHASH};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice. Much cleaner to be able to add it to mandatory flags from the get go.

Prior to activation, we always run into the discouragement and therefore never incorrectly return a consensus error for it. Post activation, we correctly return a consensus error. (And the stakes for getting this wrong are much lower now anyways.)

Comment thread src/test/script_tests.cpp Outdated
bool is_valid, std::vector<TemplateHashTestCase>& cases, std::string comment)
{
Assert(tx.vin.size() == spent_outputs.size() && in_index < tx.vin.size());
constexpr script_verify_flags FLAGS{MANDATORY_SCRIPT_VERIFY_FLAGS | SCRIPT_VERIFY_TEMPLATEHASH};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

SCRIPT_VERIFY_TEMPLATEHASH is now part of MANDATORY_SCRIPT_VERIFY_FLAGS. I guess you can just get rid of the FLAGS constant entirely now.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

done, asserted it's in the set for locality of reasoning

@instagibbs instagibbs force-pushed the 2026-05-op_templatehash branch from b7bef84 to 3c22f0b Compare June 18, 2026 15:56
@instagibbs

instagibbs commented Jun 18, 2026

Copy link
Copy Markdown
Owner Author

fixed CI complaints, addressed all points, fixed some of the git commit messages

One note: MAX_TX_IN_OUT in the fuzz target is 10k, and causes the exec/s to be quite low. @darosior I see no value in it being that high, but I'll let you weigh in, as we could make it over 18x+ faster while having real coverage up to 100 inputs

@instagibbs instagibbs force-pushed the 2026-05-op_templatehash branch from 3c22f0b to 0aeb7df Compare June 18, 2026 16:36
@instagibbs

Copy link
Copy Markdown
Owner Author

dropping MAX_TX_IN_OUT to 1k, for a 10x speedup

darosior and others added 2 commits June 18, 2026 12:56
The deployment is never active on mainnet or testnet. Activation parameters are intentionally left
to be defined eventually and out of scope for this patchset.

The deployment is always active on regtest, allowing us to extensively test expected behaviour of
the proposed operation using the functional test suite.

Co-Authored-By: Greg Sanders <gsanders87@gmail.com>
Script validation of OP_TEMPLATEHASH is enabled past
its activation height, which means it is currently never
enabled on mainnet/testnet and always is on regtest.

Tapscript spends making use of OP_TEMPLATEHASH do not get
relayed nor included in blocks until the soft fork is
active. We no longer disconnect or ban for loose mempool
transaction failures, so we can add the new flag directly to
mandatory flag set, which makes disconnect and bans possible
in blocks only post-activation.

Co-Authored-by: Greg Sanders <gsanders87@gmail.com>
@instagibbs instagibbs force-pushed the 2026-05-op_templatehash branch from 0aeb7df to 53ff7dc Compare June 18, 2026 16:57
This introduces a new Script operation exclusively available in Tapscript context: OP_TEMPLATEHASH.
This operation pushes the hash of the spending transaction on the stack. See BIP446 for details.

This operation is introduced as replacing OP_SUCCESS206 (0xce).

Co-Authored-By: Greg Sanders <gsanders87@gmail.com>
@instagibbs instagibbs force-pushed the 2026-05-op_templatehash branch from 53ff7dc to 762eb05 Compare June 18, 2026 20:13
darosior and others added 3 commits June 24, 2026 11:47
Sanity check the template hash by using it to commit to the transaction that must spend an output.
Malleating committed fields must lead to a consensus failure, and changing non-committed fields is
fine.

We also add the option to generate test vectors from this unit test.
We introduce one specialized target focused on exercising the new `GetTemplateHash()` logic
introduced for OP_TEMPLATEHASH, and one broader fuzz target which exercises using OP_TEMPLATEHASH
on a variety of transactions while asserting invariants.
This leverages the extensive feature_taproot.py test framework to generate coverage for the numerous
scenarii and mutations exercised there.

Additionally, a separate feature_templatehash.py functional test is introduced for end-to-end
testing of the commit-to-next-transaction use case, which does not fit nicely into the
feature_taproot.py framework (assumes input independence).
@instagibbs instagibbs force-pushed the 2026-05-op_templatehash branch from 762eb05 to 8bea35b Compare June 24, 2026 15:47
@instagibbs

Copy link
Copy Markdown
Owner Author

Fixed an error in the taproot test, removed a fuzz TODO, and added a couple unit test gaps.

@darosior

Copy link
Copy Markdown

CI failure seems unrelated ("importdescriptors unexpectedly succeeded").

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants