Skip to content

Commit 3820090

Browse files
sipasipa
committed
Make all SignatureChecker explicit about missing data
Remove the implicit MissingDataBehavior::ASSERT_FAIL in the *TransationSignatureChecker constructors, and instead specify it explicit in all call sites: * Test code uses ASSERT_FAIL * Validation uses ASSERT_FAIL (through CachingTransactionSignatureChecker) (including signet) * libconsensus uses FAIL, matching the existing behavior of the non-amount API (and the extended required data for taproot validation is not available yet) * Signing code uses FAIL
1 parent b77b0cc commit 3820090

13 files changed

+39
-39
lines changed

‎src/bench/verify_script.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ static void VerifyScriptBench(benchmark::Bench& bench)
5656
txCredit.vout[0].scriptPubKey,
5757
&txSpend.vin[0].scriptWitness,
5858
flags,
59-
MutableTransactionSignatureChecker(&txSpend, 0, txCredit.vout[0].nValue),
59+
MutableTransactionSignatureChecker(&txSpend, 0, txCredit.vout[0].nValue, MissingDataBehavior::ASSERT_FAIL),
6060
&err);
6161
assert(err == SCRIPT_ERR_OK);
6262
assert(success);

‎src/script/bitcoinconsensus.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ static int verify_script(const unsigned char *scriptPubKey, unsigned int scriptP
9292
set_error(err, bitcoinconsensus_ERR_OK);
9393

9494
PrecomputedTransactionData txdata(tx);
95-
return VerifyScript(tx.vin[nIn].scriptSig, CScript(scriptPubKey, scriptPubKey + scriptPubKeyLen), &tx.vin[nIn].scriptWitness, flags, TransactionSignatureChecker(&tx, nIn, amount, txdata), nullptr);
95+
return VerifyScript(tx.vin[nIn].scriptSig, CScript(scriptPubKey, scriptPubKey + scriptPubKeyLen), &tx.vin[nIn].scriptWitness, flags, TransactionSignatureChecker(&tx, nIn, amount, txdata, MissingDataBehavior::FAIL), nullptr);
9696
} catch (const std::exception&) {
9797
return set_error(err, bitcoinconsensus_ERR_TX_DESERIALIZE); // Error deserializing
9898
}

‎src/script/interpreter.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -271,8 +271,8 @@ class GenericTransactionSignatureChecker : public BaseSignatureChecker
271271
virtual bool VerifySchnorrSignature(Span<const unsigned char> sig, const XOnlyPubKey& pubkey, const uint256& sighash) const;
272272

273273
public:
274-
GenericTransactionSignatureChecker(const T* txToIn, unsigned int nInIn, const CAmount& amountIn, MissingDataBehavior mdb = MissingDataBehavior::ASSERT_FAIL) : txTo(txToIn), m_mdb(mdb), nIn(nInIn), amount(amountIn), txdata(nullptr) {}
275-
GenericTransactionSignatureChecker(const T* txToIn, unsigned int nInIn, const CAmount& amountIn, const PrecomputedTransactionData& txdataIn, MissingDataBehavior mdb = MissingDataBehavior::ASSERT_FAIL) : txTo(txToIn), m_mdb(mdb), nIn(nInIn), amount(amountIn), txdata(&txdataIn) {}
274+
GenericTransactionSignatureChecker(const T* txToIn, unsigned int nInIn, const CAmount& amountIn, MissingDataBehavior mdb) : txTo(txToIn), m_mdb(mdb), nIn(nInIn), amount(amountIn), txdata(nullptr) {}
275+
GenericTransactionSignatureChecker(const T* txToIn, unsigned int nInIn, const CAmount& amountIn, const PrecomputedTransactionData& txdataIn, MissingDataBehavior mdb) : txTo(txToIn), m_mdb(mdb), nIn(nInIn), amount(amountIn), txdata(&txdataIn) {}
276276
bool CheckECDSASignature(const std::vector<unsigned char>& scriptSig, const std::vector<unsigned char>& vchPubKey, const CScript& scriptCode, SigVersion sigversion) const override;
277277
bool CheckSchnorrSignature(Span<const unsigned char> sig, Span<const unsigned char> pubkey, SigVersion sigversion, const ScriptExecutionData& execdata, ScriptError* serror = nullptr) const override;
278278
bool CheckLockTime(const CScriptNum& nLockTime) const override;

‎src/script/sigcache.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ class CachingTransactionSignatureChecker : public TransactionSignatureChecker
2727
bool store;
2828

2929
public:
30-
CachingTransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, bool storeIn, PrecomputedTransactionData& txdataIn) : TransactionSignatureChecker(txToIn, nInIn, amountIn, txdataIn), store(storeIn) {}
30+
CachingTransactionSignatureChecker(const CTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, bool storeIn, PrecomputedTransactionData& txdataIn) : TransactionSignatureChecker(txToIn, nInIn, amountIn, txdataIn, MissingDataBehavior::ASSERT_FAIL), store(storeIn) {}
3131

3232
bool VerifyECDSASignature(const std::vector<unsigned char>& vchSig, const CPubKey& vchPubKey, const uint256& sighash) const override;
3333
bool VerifySchnorrSignature(Span<const unsigned char> sig, const XOnlyPubKey& pubkey, const uint256& sighash) const override;

‎src/script/sign.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
typedef std::vector<unsigned char> valtype;
1616

17-
MutableTransactionSignatureCreator::MutableTransactionSignatureCreator(const CMutableTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, int nHashTypeIn) : txTo(txToIn), nIn(nInIn), nHashType(nHashTypeIn), amount(amountIn), checker(txTo, nIn, amountIn) {}
17+
MutableTransactionSignatureCreator::MutableTransactionSignatureCreator(const CMutableTransaction* txToIn, unsigned int nInIn, const CAmount& amountIn, int nHashTypeIn) : txTo(txToIn), nIn(nInIn), nHashType(nHashTypeIn), amount(amountIn), checker(txTo, nIn, amountIn, MissingDataBehavior::FAIL) {}
1818

1919
bool MutableTransactionSignatureCreator::CreateSig(const SigningProvider& provider, std::vector<unsigned char>& vchSig, const CKeyID& address, const CScript& scriptCode, SigVersion sigversion) const
2020
{
@@ -292,7 +292,7 @@ SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nI
292292
Stacks stack(data);
293293

294294
// Get signatures
295-
MutableTransactionSignatureChecker tx_checker(&tx, nIn, txout.nValue);
295+
MutableTransactionSignatureChecker tx_checker(&tx, nIn, txout.nValue, MissingDataBehavior::FAIL);
296296
SignatureExtractorChecker extractor_checker(data, tx_checker);
297297
if (VerifyScript(data.scriptSig, txout.scriptPubKey, &data.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, extractor_checker)) {
298298
data.complete = true;
@@ -499,7 +499,7 @@ bool SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore,
499499
}
500500

501501
ScriptError serror = SCRIPT_ERR_OK;
502-
if (!VerifyScript(txin.scriptSig, prevPubKey, &txin.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i, amount), &serror)) {
502+
if (!VerifyScript(txin.scriptSig, prevPubKey, &txin.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i, amount, MissingDataBehavior::FAIL), &serror)) {
503503
if (serror == SCRIPT_ERR_INVALID_STACK_OPERATION) {
504504
// Unable to sign input and verification failed (possible attempt to partially sign).
505505
input_errors[i] = "Unable to sign input, invalid stack size (possibly missing key)";

‎src/signet.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ bool CheckSignetBlockSolution(const CBlock& block, const Consensus::Params& cons
139139
const CScript& scriptSig = signet_txs->m_to_sign.vin[0].scriptSig;
140140
const CScriptWitness& witness = signet_txs->m_to_sign.vin[0].scriptWitness;
141141

142-
TransactionSignatureChecker sigcheck(&signet_txs->m_to_sign, /*nIn=*/ 0, /*amount=*/ signet_txs->m_to_spend.vout[0].nValue);
142+
TransactionSignatureChecker sigcheck(&signet_txs->m_to_sign, /*nIn=*/ 0, /*amount=*/ signet_txs->m_to_spend.vout[0].nValue, MissingDataBehavior::ASSERT_FAIL);
143143

144144
if (!VerifyScript(scriptSig, signet_txs->m_to_spend.vout[0].scriptPubKey, &witness, BLOCK_SCRIPT_VERIFY_FLAGS, sigcheck)) {
145145
LogPrint(BCLog::VALIDATION, "CheckSignetBlockSolution: Errors in block (block solution invalid)\n");

‎src/test/fuzz/script_assets_test_minimizer.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ void Test(const std::string& str)
161161
tx.vin[idx].scriptWitness = ScriptWitnessFromJSON(test["success"]["witness"]);
162162
PrecomputedTransactionData txdata;
163163
txdata.Init(tx, std::vector<CTxOut>(prevouts));
164-
MutableTransactionSignatureChecker txcheck(&tx, idx, prevouts[idx].nValue, txdata);
164+
MutableTransactionSignatureChecker txcheck(&tx, idx, prevouts[idx].nValue, txdata, MissingDataBehavior::ASSERT_FAIL);
165165
for (const auto flags : ALL_FLAGS) {
166166
// "final": true tests are valid for all flags. Others are only valid with flags that are
167167
// a subset of test_flags.
@@ -176,7 +176,7 @@ void Test(const std::string& str)
176176
tx.vin[idx].scriptWitness = ScriptWitnessFromJSON(test["failure"]["witness"]);
177177
PrecomputedTransactionData txdata;
178178
txdata.Init(tx, std::vector<CTxOut>(prevouts));
179-
MutableTransactionSignatureChecker txcheck(&tx, idx, prevouts[idx].nValue, txdata);
179+
MutableTransactionSignatureChecker txcheck(&tx, idx, prevouts[idx].nValue, txdata, MissingDataBehavior::ASSERT_FAIL);
180180
for (const auto flags : ALL_FLAGS) {
181181
// If a test is supposed to fail with test_flags, it should also fail with any superset thereof.
182182
if ((flags & test_flags) == test_flags) {

‎src/test/fuzz/script_flags.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ FUZZ_TARGET_INIT(script_flags, initialize_script_flags)
5050

5151
for (unsigned i = 0; i < tx.vin.size(); ++i) {
5252
const CTxOut& prevout = txdata.m_spent_outputs.at(i);
53-
const TransactionSignatureChecker checker{&tx, i, prevout.nValue, txdata};
53+
const TransactionSignatureChecker checker{&tx, i, prevout.nValue, txdata, MissingDataBehavior::ASSERT_FAIL};
5454

5555
ScriptError serror;
5656
const bool ret = VerifyScript(tx.vin.at(i).scriptSig, prevout.scriptPubKey, &tx.vin.at(i).scriptWitness, verify_flags, checker, &serror);

‎src/test/multisig_tests.cpp

+8-8
Original file line numberDiff line numberDiff line change
@@ -77,20 +77,20 @@ BOOST_AUTO_TEST_CASE(multisig_verify)
7777
keys.assign(1,key[0]);
7878
keys.push_back(key[1]);
7979
s = sign_multisig(a_and_b, keys, CTransaction(txTo[0]), 0);
80-
BOOST_CHECK(VerifyScript(s, a_and_b, nullptr, flags, MutableTransactionSignatureChecker(&txTo[0], 0, amount), &err));
80+
BOOST_CHECK(VerifyScript(s, a_and_b, nullptr, flags, MutableTransactionSignatureChecker(&txTo[0], 0, amount, MissingDataBehavior::ASSERT_FAIL), &err));
8181
BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err));
8282

8383
for (int i = 0; i < 4; i++)
8484
{
8585
keys.assign(1,key[i]);
8686
s = sign_multisig(a_and_b, keys, CTransaction(txTo[0]), 0);
87-
BOOST_CHECK_MESSAGE(!VerifyScript(s, a_and_b, nullptr, flags, MutableTransactionSignatureChecker(&txTo[0], 0, amount), &err), strprintf("a&b 1: %d", i));
87+
BOOST_CHECK_MESSAGE(!VerifyScript(s, a_and_b, nullptr, flags, MutableTransactionSignatureChecker(&txTo[0], 0, amount, MissingDataBehavior::ASSERT_FAIL), &err), strprintf("a&b 1: %d", i));
8888
BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_INVALID_STACK_OPERATION, ScriptErrorString(err));
8989

9090
keys.assign(1,key[1]);
9191
keys.push_back(key[i]);
9292
s = sign_multisig(a_and_b, keys, CTransaction(txTo[0]), 0);
93-
BOOST_CHECK_MESSAGE(!VerifyScript(s, a_and_b, nullptr, flags, MutableTransactionSignatureChecker(&txTo[0], 0, amount), &err), strprintf("a&b 2: %d", i));
93+
BOOST_CHECK_MESSAGE(!VerifyScript(s, a_and_b, nullptr, flags, MutableTransactionSignatureChecker(&txTo[0], 0, amount, MissingDataBehavior::ASSERT_FAIL), &err), strprintf("a&b 2: %d", i));
9494
BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_EVAL_FALSE, ScriptErrorString(err));
9595
}
9696

@@ -101,18 +101,18 @@ BOOST_AUTO_TEST_CASE(multisig_verify)
101101
s = sign_multisig(a_or_b, keys, CTransaction(txTo[1]), 0);
102102
if (i == 0 || i == 1)
103103
{
104-
BOOST_CHECK_MESSAGE(VerifyScript(s, a_or_b, nullptr, flags, MutableTransactionSignatureChecker(&txTo[1], 0, amount), &err), strprintf("a|b: %d", i));
104+
BOOST_CHECK_MESSAGE(VerifyScript(s, a_or_b, nullptr, flags, MutableTransactionSignatureChecker(&txTo[1], 0, amount, MissingDataBehavior::ASSERT_FAIL), &err), strprintf("a|b: %d", i));
105105
BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err));
106106
}
107107
else
108108
{
109-
BOOST_CHECK_MESSAGE(!VerifyScript(s, a_or_b, nullptr, flags, MutableTransactionSignatureChecker(&txTo[1], 0, amount), &err), strprintf("a|b: %d", i));
109+
BOOST_CHECK_MESSAGE(!VerifyScript(s, a_or_b, nullptr, flags, MutableTransactionSignatureChecker(&txTo[1], 0, amount, MissingDataBehavior::ASSERT_FAIL), &err), strprintf("a|b: %d", i));
110110
BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_EVAL_FALSE, ScriptErrorString(err));
111111
}
112112
}
113113
s.clear();
114114
s << OP_0 << OP_1;
115-
BOOST_CHECK(!VerifyScript(s, a_or_b, nullptr, flags, MutableTransactionSignatureChecker(&txTo[1], 0, amount), &err));
115+
BOOST_CHECK(!VerifyScript(s, a_or_b, nullptr, flags, MutableTransactionSignatureChecker(&txTo[1], 0, amount, MissingDataBehavior::ASSERT_FAIL), &err));
116116
BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_SIG_DER, ScriptErrorString(err));
117117

118118

@@ -124,12 +124,12 @@ BOOST_AUTO_TEST_CASE(multisig_verify)
124124
s = sign_multisig(escrow, keys, CTransaction(txTo[2]), 0);
125125
if (i < j && i < 3 && j < 3)
126126
{
127-
BOOST_CHECK_MESSAGE(VerifyScript(s, escrow, nullptr, flags, MutableTransactionSignatureChecker(&txTo[2], 0, amount), &err), strprintf("escrow 1: %d %d", i, j));
127+
BOOST_CHECK_MESSAGE(VerifyScript(s, escrow, nullptr, flags, MutableTransactionSignatureChecker(&txTo[2], 0, amount, MissingDataBehavior::ASSERT_FAIL), &err), strprintf("escrow 1: %d %d", i, j));
128128
BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_OK, ScriptErrorString(err));
129129
}
130130
else
131131
{
132-
BOOST_CHECK_MESSAGE(!VerifyScript(s, escrow, nullptr, flags, MutableTransactionSignatureChecker(&txTo[2], 0, amount), &err), strprintf("escrow 2: %d %d", i, j));
132+
BOOST_CHECK_MESSAGE(!VerifyScript(s, escrow, nullptr, flags, MutableTransactionSignatureChecker(&txTo[2], 0, amount, MissingDataBehavior::ASSERT_FAIL), &err), strprintf("escrow 2: %d %d", i, j));
133133
BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_EVAL_FALSE, ScriptErrorString(err));
134134
}
135135
}

‎src/test/script_p2sh_tests.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ Verify(const CScript& scriptSig, const CScript& scriptPubKey, bool fStrict, Scri
4141
txTo.vin[0].scriptSig = scriptSig;
4242
txTo.vout[0].nValue = 1;
4343

44-
return VerifyScript(scriptSig, scriptPubKey, nullptr, fStrict ? SCRIPT_VERIFY_P2SH : SCRIPT_VERIFY_NONE, MutableTransactionSignatureChecker(&txTo, 0, txFrom.vout[0].nValue), &err);
44+
return VerifyScript(scriptSig, scriptPubKey, nullptr, fStrict ? SCRIPT_VERIFY_P2SH : SCRIPT_VERIFY_NONE, MutableTransactionSignatureChecker(&txTo, 0, txFrom.vout[0].nValue, MissingDataBehavior::ASSERT_FAIL), &err);
4545
}
4646

4747

0 commit comments

Comments
 (0)