Merge bitcoin/bitcoin#35477: test: exercise Schnorr signature cache in txvalidationcache_tests.cpp

3ba1bbfa3f test: exercise Schnorr signature cache in txvalidationcache_tests.cpp (Sebastian Falbesoner)
198b36bc85 test: respect "TAPROOT requires WITNESS" rule in `ValidateCheckInputsForAllFlags` (Sebastian Falbesoner)
e78a2a0d00 test: refactor: simplify tx vin/vout creation in txvalidationcache_tests.cpp (Sebastian Falbesoner)

Pull request description:

  The Schnorr verification path of the signature cache is currently never hit in the unit tests, i.e. with the following patch they still pass:
  ```diff
  diff --git a/src/script/sigcache.cpp b/src/script/sigcache.cpp
  index c6fcc8f8eb..87688c1049 100644
  --- a/src/script/sigcache.cpp
  +++ b/src/script/sigcache.cpp
  @@ -44,6 +44,7 @@ void SignatureCache::ComputeEntryECDSA(uint256& entry, const uint256& hash, cons

   void SignatureCache::ComputeEntrySchnorr(uint256& entry, const uint256& hash, std::span<const unsigned char> sig, const XOnlyPubKey& pubkey) const
   {
  +    assert(false);
       CSHA256 hasher = m_salted_hasher_schnorr;
       hasher.Write(hash.begin(), 32).Write(pubkey.data(), pubkey.size()).Write(sig.data(), sig.size()).Finalize(entry.begin());
   }
  ```
  This PR adds missing coverage for that by adding a Taproot key-path spend to `checkinputs_test` in `txvalidationcache_tests.cpp`. Same as for the already-existing ECDSA spends, the caching is tested across a large number of flag combinations (using `ValidateCheckInputsForAllFlags`), both with an invalid Schnorr signature (-> should only fail if `SCRIPT_VERIFY_TAPROOT` is set) and a valid one (-> should pass for all flag combinations).

ACKs for top commit:
  Bortlesboat:
    tACK 3ba1bbfa3f
  sedited:
    ACK 3ba1bbfa3f
  instagibbs:
    ACK 3ba1bbfa3f

Tree-SHA512: e43f7077d9e9ab6f8b5e9e70f0187767d65f686ce24350ce5d61cc4cdf07d5eebdf5e4327ce665c212b7cc02be1e8632a6a9fbcf6be2916f8e058993e5fb2650
This commit is contained in:
merge-script
2026-08-31 15:31:15 +02:00

View File

@@ -49,12 +49,8 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_block_doublespend, Dersig100Setup)
for (int i = 0; i < 2; i++)
{
spends[i].version = 1;
spends[i].vin.resize(1);
spends[i].vin[0].prevout.hash = m_coinbase_txns[0]->GetHash();
spends[i].vin[0].prevout.n = 0;
spends[i].vout.resize(1);
spends[i].vout[0].nValue = 11*CENT;
spends[i].vout[0].scriptPubKey = scriptPubKey;
spends[i].vin = {CTxIn{m_coinbase_txns[0]->GetHash(), 0}};
spends[i].vout = {CTxOut{11*CENT, scriptPubKey}};
// Sign:
std::vector<unsigned char> vchSig;
@@ -118,8 +114,6 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_block_doublespend, Dersig100Setup)
// CHECKLOCKTIMEVERIFY (or CHECKSEQUENCEVERIFY), but the script does contain
// OP_CHECKLOCKTIMEVERIFY (or OP_CHECKSEQUENCEVERIFY), then script execution
// should fail.
// Capture this interaction with the upgraded_nop argument: set it when evaluating
// any script flag that is implemented as an upgraded NOP code.
static void ValidateCheckInputsForAllFlags(const CTransaction &tx, script_verify_flags failing_flags, bool add_to_cache, CCoinsViewCache& active_coins_tip, ValidationCache& validation_cache) EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
{
PrecomputedTransactionData txdata;
@@ -138,6 +132,10 @@ static void ValidateCheckInputsForAllFlags(const CTransaction &tx, script_verify
// script/interpreter.cpp
test_flags |= SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS;
}
if ((test_flags & SCRIPT_VERIFY_TAPROOT)) {
// TAPROOT requires WITNESS
test_flags |= SCRIPT_VERIFY_WITNESS;
}
if ((test_flags & SCRIPT_VERIFY_WITNESS)) {
// WITNESS requires P2SH
test_flags |= SCRIPT_VERIFY_P2SH;
@@ -172,6 +170,7 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup)
CScript p2sh_scriptPubKey = GetScriptForDestination(ScriptHash(p2pk_scriptPubKey));
CScript p2pkh_scriptPubKey = GetScriptForDestination(PKHash(coinbaseKey.GetPubKey()));
CScript p2wpkh_scriptPubKey = GetScriptForDestination(WitnessV0KeyHash(coinbaseKey.GetPubKey()));
CScript p2tr_scriptPubKey = GetScriptForDestination(WitnessV1Taproot(XOnlyPubKey(coinbaseKey.GetPubKey())));
FillableSigningProvider keystore;
BOOST_CHECK(keystore.AddKey(coinbaseKey));
@@ -184,18 +183,14 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup)
CMutableTransaction spend_tx;
spend_tx.version = 1;
spend_tx.vin.resize(1);
spend_tx.vin[0].prevout.hash = m_coinbase_txns[0]->GetHash();
spend_tx.vin[0].prevout.n = 0;
spend_tx.vout.resize(4);
spend_tx.vout[0].nValue = 11*CENT;
spend_tx.vout[0].scriptPubKey = p2sh_scriptPubKey;
spend_tx.vout[1].nValue = 11*CENT;
spend_tx.vout[1].scriptPubKey = p2wpkh_scriptPubKey;
spend_tx.vout[2].nValue = 11*CENT;
spend_tx.vout[2].scriptPubKey = CScript() << OP_CHECKLOCKTIMEVERIFY << OP_DROP << ToByteVector(coinbaseKey.GetPubKey()) << OP_CHECKSIG;
spend_tx.vout[3].nValue = 11*CENT;
spend_tx.vout[3].scriptPubKey = CScript() << OP_CHECKSEQUENCEVERIFY << OP_DROP << ToByteVector(coinbaseKey.GetPubKey()) << OP_CHECKSIG;
spend_tx.vin = {CTxIn{m_coinbase_txns[0]->GetHash(), 0}};
spend_tx.vout = {
CTxOut{11*CENT, p2sh_scriptPubKey},
CTxOut{11*CENT, p2wpkh_scriptPubKey},
CTxOut{11*CENT, CScript() << OP_CHECKLOCKTIMEVERIFY << OP_DROP << ToByteVector(coinbaseKey.GetPubKey()) << OP_CHECKSIG},
CTxOut{11*CENT, CScript() << OP_CHECKSEQUENCEVERIFY << OP_DROP << ToByteVector(coinbaseKey.GetPubKey()) << OP_CHECKSIG},
CTxOut{11*CENT, p2tr_scriptPubKey},
};
// Sign, with a non-DER signature
{
@@ -246,12 +241,8 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup)
{
CMutableTransaction invalid_under_p2sh_tx;
invalid_under_p2sh_tx.version = 1;
invalid_under_p2sh_tx.vin.resize(1);
invalid_under_p2sh_tx.vin[0].prevout.hash = spend_tx.GetHash();
invalid_under_p2sh_tx.vin[0].prevout.n = 0;
invalid_under_p2sh_tx.vout.resize(1);
invalid_under_p2sh_tx.vout[0].nValue = 11*CENT;
invalid_under_p2sh_tx.vout[0].scriptPubKey = p2pk_scriptPubKey;
invalid_under_p2sh_tx.vin = {CTxIn{spend_tx.GetHash(), 0}};
invalid_under_p2sh_tx.vout = {CTxOut{11*CENT, p2pk_scriptPubKey}};
std::vector<unsigned char> vchSig2(p2pk_scriptPubKey.begin(), p2pk_scriptPubKey.end());
invalid_under_p2sh_tx.vin[0].scriptSig << vchSig2;
@@ -263,13 +254,8 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup)
CMutableTransaction invalid_with_cltv_tx;
invalid_with_cltv_tx.version = 1;
invalid_with_cltv_tx.nLockTime = 100;
invalid_with_cltv_tx.vin.resize(1);
invalid_with_cltv_tx.vin[0].prevout.hash = spend_tx.GetHash();
invalid_with_cltv_tx.vin[0].prevout.n = 2;
invalid_with_cltv_tx.vin[0].nSequence = 0;
invalid_with_cltv_tx.vout.resize(1);
invalid_with_cltv_tx.vout[0].nValue = 11*CENT;
invalid_with_cltv_tx.vout[0].scriptPubKey = p2pk_scriptPubKey;
invalid_with_cltv_tx.vin = {CTxIn{spend_tx.GetHash(), 2, {}, /*nSequenceIn=*/0}};
invalid_with_cltv_tx.vout = {CTxOut{11*CENT, p2pk_scriptPubKey}};
// Sign
std::vector<unsigned char> vchSig;
@@ -291,13 +277,8 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup)
{
CMutableTransaction invalid_with_csv_tx;
invalid_with_csv_tx.version = 2;
invalid_with_csv_tx.vin.resize(1);
invalid_with_csv_tx.vin[0].prevout.hash = spend_tx.GetHash();
invalid_with_csv_tx.vin[0].prevout.n = 3;
invalid_with_csv_tx.vin[0].nSequence = 100;
invalid_with_csv_tx.vout.resize(1);
invalid_with_csv_tx.vout[0].nValue = 11*CENT;
invalid_with_csv_tx.vout[0].scriptPubKey = p2pk_scriptPubKey;
invalid_with_csv_tx.vin = {CTxIn{spend_tx.GetHash(), 3, {}, /*nSequenceIn=*/100}};
invalid_with_csv_tx.vout = {CTxOut{11*CENT, p2pk_scriptPubKey}};
// Sign
std::vector<unsigned char> vchSig;
@@ -322,12 +303,8 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup)
{
CMutableTransaction valid_with_witness_tx;
valid_with_witness_tx.version = 1;
valid_with_witness_tx.vin.resize(1);
valid_with_witness_tx.vin[0].prevout.hash = spend_tx.GetHash();
valid_with_witness_tx.vin[0].prevout.n = 1;
valid_with_witness_tx.vout.resize(1);
valid_with_witness_tx.vout[0].nValue = 11*CENT;
valid_with_witness_tx.vout[0].scriptPubKey = p2pk_scriptPubKey;
valid_with_witness_tx.vin = {CTxIn{spend_tx.GetHash(), 1}};
valid_with_witness_tx.vout = {CTxOut{11*CENT, p2pk_scriptPubKey}};
// Sign
SignatureData sigdata;
@@ -342,19 +319,42 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup)
ValidateCheckInputsForAllFlags(CTransaction(valid_with_witness_tx), SCRIPT_VERIFY_WITNESS, true, m_node.chainman->ActiveChainstate().CoinsTip(), m_node.chainman->m_validation_cache);
}
// Test a Taproot (witness v1) key-path spend, to exercise the Schnorr branch of the signature cache.
{
CMutableTransaction tr_tx;
tr_tx.vin = {CTxIn{spend_tx.GetHash(), 4}};
tr_tx.vout = {CTxOut{11*CENT, p2pk_scriptPubKey}};
// Sign P2TR output for key-path spending (i.e. add Schnorr signature to witness stack)
FlatSigningProvider tr_keystore;
tr_keystore.keys.emplace(coinbaseKey.GetPubKey().GetID(), coinbaseKey);
const std::map<COutPoint, Coin> coins{
{tr_tx.vin[0].prevout, Coin(spend_tx.vout[4], /*nHeightIn=*/0, /*fCoinBaseIn=*/false)}
};
std::map<int, bilingual_str> input_errors;
BOOST_REQUIRE(SignTransaction(tr_tx, &tr_keystore, coins, {.sighash_type = SIGHASH_DEFAULT}, input_errors));
auto& witness_stack = tr_tx.vin[0].scriptWitness.stack;
BOOST_REQUIRE(witness_stack.size() == 1 && witness_stack[0].size() == 64);
// Invalidate signature; an invalid Taproot key-path spend is only invalid if SCRIPT_VERIFY_TAPROOT is set
witness_stack[0][63] ^= 0x01; // damage signature
ValidateCheckInputsForAllFlags(CTransaction(tr_tx), SCRIPT_VERIFY_TAPROOT, true, m_node.chainman->ActiveChainstate().CoinsTip(), m_node.chainman->m_validation_cache);
witness_stack[0][63] ^= 0x01; // repair signature
// A valid Taproot key-path spend is valid under all flags
ValidateCheckInputsForAllFlags(CTransaction(tr_tx), 0, true, m_node.chainman->ActiveChainstate().CoinsTip(), m_node.chainman->m_validation_cache);
}
{
// Test a transaction with multiple inputs.
CMutableTransaction tx;
tx.version = 1;
tx.vin.resize(2);
tx.vin[0].prevout.hash = spend_tx.GetHash();
tx.vin[0].prevout.n = 0;
tx.vin[1].prevout.hash = spend_tx.GetHash();
tx.vin[1].prevout.n = 1;
tx.vout.resize(1);
tx.vout[0].nValue = 22*CENT;
tx.vout[0].scriptPubKey = p2pk_scriptPubKey;
tx.vin = {
CTxIn{spend_tx.GetHash(), 0},
CTxIn{spend_tx.GetHash(), 1},
};
tx.vout = {CTxOut{22*CENT, p2pk_scriptPubKey}};
// Sign
for (int i = 0; i < 2; ++i) {