From eab72d14d79a0ec3f66f247c083d6acfda2ea5b0 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Fri, 4 Jul 2025 17:07:45 +0200 Subject: [PATCH] refactor: use SignOptions for MutableTransactionSignatureCreator --- src/bitcoin-tx.cpp | 2 +- src/psbt.cpp | 6 +++--- src/rpc/rawtransaction.cpp | 2 +- src/script/sign.cpp | 18 +++++++++--------- src/script/sign.h | 6 +++--- src/test/descriptor_tests.cpp | 2 +- src/test/fuzz/script_sign.cpp | 2 +- src/test/script_tests.cpp | 4 ++-- src/test/transaction_tests.cpp | 2 +- src/test/txvalidationcache_tests.cpp | 4 ++-- src/test/util/transaction_utils.cpp | 2 +- 11 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index 4a891ad8f18..f21db40f3c3 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -681,7 +681,7 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr) SignatureData sigdata = DataFromTransaction(mergedTx, i, coin.out); // Only sign SIGHASH_SINGLE if there's a corresponding output: if (!fHashSingle || (i < mergedTx.vout.size())) - ProduceSignature(keystore, MutableTransactionSignatureCreator(mergedTx, i, amount, nHashType), prevPubKey, sigdata); + ProduceSignature(keystore, MutableTransactionSignatureCreator(mergedTx, i, amount, {.sighash_type = nHashType}), prevPubKey, sigdata); if (amount == MAX_MONEY && !sigdata.scriptWitness.IsNull()) { throw std::runtime_error(strprintf("Missing amount for CTxOut with scriptPubKey=%s", HexStr(prevPubKey))); diff --git a/src/psbt.cpp b/src/psbt.cpp index 8c39a6ce975..cfc720b06d9 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -375,7 +375,7 @@ void UpdatePSBTOutput(const SigningProvider& provider, PartiallySignedTransactio // Construct a would-be spend of this output, to update sigdata with. // Note that ProduceSignature is used to fill in metadata (not actual signatures), // so provider does not need to provide any private keys (it can be a HidingSigningProvider). - MutableTransactionSignatureCreator creator(tx, /*input_idx=*/0, out.nValue, SIGHASH_ALL); + MutableTransactionSignatureCreator creator(tx, /*input_idx=*/0, out.nValue, {.sighash_type = SIGHASH_ALL}); ProduceSignature(provider, creator, out.scriptPubKey, sigdata); // Put redeem_script, witness_script, key paths, into PSBTOutput. @@ -481,7 +481,7 @@ PSBTError SignPSBTInput(const SigningProvider& provider, PartiallySignedTransact if (txdata == nullptr) { sig_complete = ProduceSignature(provider, DUMMY_SIGNATURE_CREATOR, utxo.scriptPubKey, sigdata); } else { - MutableTransactionSignatureCreator creator(tx, index, utxo.nValue, txdata, sighash); + MutableTransactionSignatureCreator creator(tx, index, utxo.nValue, txdata, {.sighash_type = sighash}); sig_complete = ProduceSignature(provider, creator, utxo.scriptPubKey, sigdata); } // Verify that a witness signature was produced in case one was required. @@ -558,7 +558,7 @@ bool FinalizePSBT(PartiallySignedTransaction& psbtx) const PrecomputedTransactionData txdata = PrecomputePSBTData(psbtx); for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { PSBTInput& input = psbtx.inputs.at(i); - complete &= (SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, &txdata, {.sighash_type = input.sighash_type, .finalize = true}, nullptr) == PSBTError::OK); + complete &= (SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, &txdata, {.sighash_type = input.sighash_type, .finalize = true}, /*out_sigdata=*/nullptr) == PSBTError::OK); } return complete; diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 19305ba46a8..0f3cebb5dcf 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -658,7 +658,7 @@ static RPCMethod combinerawtransaction() sigdata.MergeSignatureData(DataFromTransaction(txv, i, coin.out)); } } - ProduceSignature(DUMMY_SIGNING_PROVIDER, MutableTransactionSignatureCreator(mergedTx, i, coin.out.nValue, 1), coin.out.scriptPubKey, sigdata); + ProduceSignature(DUMMY_SIGNING_PROVIDER, MutableTransactionSignatureCreator(mergedTx, i, coin.out.nValue, {.sighash_type = SIGHASH_ALL}), coin.out.scriptPubKey, sigdata); UpdateInput(txin, sigdata); } diff --git a/src/script/sign.cpp b/src/script/sign.cpp index 6184cc54151..6cbcf07e5e2 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -22,14 +22,14 @@ typedef std::vector valtype; -MutableTransactionSignatureCreator::MutableTransactionSignatureCreator(const CMutableTransaction& tx, unsigned int input_idx, const CAmount& amount, int hash_type) - : m_txto{tx}, nIn{input_idx}, nHashType{hash_type}, amount{amount}, checker{&m_txto, nIn, amount, MissingDataBehavior::FAIL}, +MutableTransactionSignatureCreator::MutableTransactionSignatureCreator(const CMutableTransaction& tx, unsigned int input_idx, const CAmount& amount, const SignOptions& options) + : m_txto{tx}, nIn{input_idx}, m_options{options}, amount{amount}, checker{&m_txto, nIn, amount, MissingDataBehavior::FAIL}, m_txdata(nullptr) { } -MutableTransactionSignatureCreator::MutableTransactionSignatureCreator(const CMutableTransaction& tx, unsigned int input_idx, const CAmount& amount, const PrecomputedTransactionData* txdata, int hash_type) - : m_txto{tx}, nIn{input_idx}, nHashType{hash_type}, amount{amount}, +MutableTransactionSignatureCreator::MutableTransactionSignatureCreator(const CMutableTransaction& tx, unsigned int input_idx, const CAmount& amount, const PrecomputedTransactionData* txdata, const SignOptions& options) + : m_txto{tx}, nIn{input_idx}, m_options{options}, amount{amount}, checker{txdata ? MutableTransactionSignatureChecker{&m_txto, nIn, amount, *txdata, MissingDataBehavior::FAIL} : MutableTransactionSignatureChecker{&m_txto, nIn, amount, MissingDataBehavior::FAIL}}, m_txdata(txdata) @@ -52,7 +52,7 @@ bool MutableTransactionSignatureCreator::CreateSig(const SigningProvider& provid if (sigversion == SigVersion::WITNESS_V0 && !MoneyRange(amount)) return false; // BASE/WITNESS_V0 signatures don't support explicit SIGHASH_DEFAULT, use SIGHASH_ALL instead. - const int hashtype = nHashType == SIGHASH_DEFAULT ? SIGHASH_ALL : nHashType; + const int hashtype = m_options.sighash_type == SIGHASH_DEFAULT ? SIGHASH_ALL : m_options.sighash_type; uint256 hash = SignatureHash(scriptCode, m_txto, nIn, hashtype, amount, sigversion, m_txdata); if (!key.Sign(hash, vchSig)) @@ -81,7 +81,7 @@ std::optional MutableTransactionSignatureCreator::ComputeSchnorrSignatu execdata.m_tapleaf_hash = *leaf_hash; } uint256 hash; - if (!SignatureHashSchnorr(hash, execdata, m_txto, nIn, nHashType, sigversion, *m_txdata, MissingDataBehavior::FAIL)) return std::nullopt; + if (!SignatureHashSchnorr(hash, execdata, m_txto, nIn, m_options.sighash_type, sigversion, *m_txdata, MissingDataBehavior::FAIL)) return std::nullopt; return hash; } @@ -96,7 +96,7 @@ bool MutableTransactionSignatureCreator::CreateSchnorrSig(const SigningProvider& sig.resize(64); // Use uint256{} as aux_rnd for now. if (!key.SignSchnorr(*hash, sig, merkle_root, {})) return false; - if (nHashType) sig.push_back(nHashType); + if (m_options.sighash_type) sig.push_back(m_options.sighash_type); return true; } @@ -197,7 +197,7 @@ bool MutableTransactionSignatureCreator::CreateMuSig2AggregateSig(const std::vec std::optional> res = ::CreateMuSig2AggregateSig(participants, aggregate_pubkey, tweaks, *sighash, pubnonces, partial_sigs); if (!res) return false; sig = res.value(); - if (nHashType) sig.push_back(nHashType); + if (m_options.sighash_type) sig.push_back(m_options.sighash_type); return true; } @@ -1044,7 +1044,7 @@ bool SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, SignatureData sigdata = DataFromTransaction(mtx, i, coin->second.out); // Only sign SIGHASH_SINGLE if there's a corresponding output: if (!fHashSingle || (i < mtx.vout.size())) { - ProduceSignature(*keystore, MutableTransactionSignatureCreator(mtx, i, amount, &txdata, options.sighash_type), prevPubKey, sigdata); + ProduceSignature(*keystore, MutableTransactionSignatureCreator(mtx, i, amount, &txdata, options), prevPubKey, sigdata); } UpdateInput(txin, sigdata); diff --git a/src/script/sign.h b/src/script/sign.h index c0b1b7683ef..44bc5060fe2 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -48,7 +48,7 @@ class MutableTransactionSignatureCreator : public BaseSignatureCreator { const CMutableTransaction& m_txto; unsigned int nIn; - int nHashType; + SignOptions m_options; CAmount amount; const MutableTransactionSignatureChecker checker; const PrecomputedTransactionData* m_txdata; @@ -56,8 +56,8 @@ class MutableTransactionSignatureCreator : public BaseSignatureCreator std::optional ComputeSchnorrSignatureHash(const uint256* leaf_hash, SigVersion sigversion) const; public: - MutableTransactionSignatureCreator(const CMutableTransaction& tx LIFETIMEBOUND, unsigned int input_idx, const CAmount& amount, int hash_type); - MutableTransactionSignatureCreator(const CMutableTransaction& tx LIFETIMEBOUND, unsigned int input_idx, const CAmount& amount, const PrecomputedTransactionData* txdata, int hash_type); + MutableTransactionSignatureCreator(const CMutableTransaction& tx LIFETIMEBOUND, unsigned int input_idx, const CAmount& amount, const SignOptions& options); + MutableTransactionSignatureCreator(const CMutableTransaction& tx LIFETIMEBOUND, unsigned int input_idx, const CAmount& amount, const PrecomputedTransactionData* txdata, const SignOptions& options); const BaseSignatureChecker& Checker() const override { return checker; } bool CreateSig(const SigningProvider& provider, std::vector& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const override; bool CreateSchnorrSig(const SigningProvider& provider, std::vector& sig, const XOnlyPubKey& pubkey, const uint256* leaf_hash, const uint256* merkle_root, SigVersion sigversion) const override; diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp index a646fc83476..0ede4dc9dfe 100644 --- a/src/test/descriptor_tests.cpp +++ b/src/test/descriptor_tests.cpp @@ -447,7 +447,7 @@ void DoCheck(std::string prv, std::string pub, const std::string& norm_pub, int std::vector utxos(1); PrecomputedTransactionData txdata; txdata.Init(spend, std::move(utxos), /*force=*/true); - MutableTransactionSignatureCreator creator{spend, 0, CAmount{0}, &txdata, SIGHASH_DEFAULT}; + MutableTransactionSignatureCreator creator{spend, 0, CAmount{0}, &txdata, {.sighash_type = SIGHASH_DEFAULT}}; SignatureData sigdata; // We assume there is no collision between the hashes (eg h1=SHA256(SHA256(x)) and h2=SHA256(x)) sigdata.sha256_preimages = preimages; diff --git a/src/test/fuzz/script_sign.cpp b/src/test/fuzz/script_sign.cpp index c709a902bbe..036b9ff3e49 100644 --- a/src/test/fuzz/script_sign.cpp +++ b/src/test/fuzz/script_sign.cpp @@ -116,7 +116,7 @@ FUZZ_TARGET(script_sign, .init = initialize_script_sign) auto amount = ConsumeMoney(fuzzed_data_provider); auto n_hash_type = fuzzed_data_provider.ConsumeIntegral(); (void)SignSignature(provider, from_pub_key, script_tx_to, n_in, amount, n_hash_type, empty); - MutableTransactionSignatureCreator signature_creator{tx_to, n_in, ConsumeMoney(fuzzed_data_provider), fuzzed_data_provider.ConsumeIntegral()}; + MutableTransactionSignatureCreator signature_creator{tx_to, n_in, ConsumeMoney(fuzzed_data_provider), {.sighash_type = fuzzed_data_provider.ConsumeIntegral()}}; std::vector vch_sig; CKeyID address; if (fuzzed_data_provider.ConsumeBool()) { diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index b07870207cb..ffb7acd2df9 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -1265,7 +1265,7 @@ SignatureData CombineSignatures(const CTxOut& txout, const CMutableTransaction& SignatureData data; data.MergeSignatureData(scriptSig1); data.MergeSignatureData(scriptSig2); - ProduceSignature(DUMMY_SIGNING_PROVIDER, MutableTransactionSignatureCreator(tx, 0, txout.nValue, SIGHASH_DEFAULT), txout.scriptPubKey, data); + ProduceSignature(DUMMY_SIGNING_PROVIDER, MutableTransactionSignatureCreator(tx, 0, txout.nValue, {.sighash_type = SIGHASH_DEFAULT}), txout.scriptPubKey, data); return data; } @@ -1681,7 +1681,7 @@ BOOST_AUTO_TEST_CASE(bip341_keypath_test_vectors) // Sign and verify signature. FlatSigningProvider provider; provider.keys[key.GetPubKey().GetID()] = key; - MutableTransactionSignatureCreator creator(tx, txinpos, utxos[txinpos].nValue, &txdata, hashtype); + MutableTransactionSignatureCreator creator(tx, txinpos, utxos[txinpos].nValue, &txdata, {.sighash_type = hashtype}); std::vector signature; BOOST_CHECK(creator.CreateSchnorrSig(provider, signature, pubkey, nullptr, &merkle_root, SigVersion::TAPROOT)); BOOST_CHECK_EQUAL(HexStr(signature), input["expected"]["witness"][0].get_str()); diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 19c416c225f..a8a596f5ad2 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -563,7 +563,7 @@ SignatureData CombineSignatures(const CMutableTransaction& input1, const CMutabl SignatureData sigdata; sigdata = DataFromTransaction(input1, 0, tx->vout[0]); sigdata.MergeSignatureData(DataFromTransaction(input2, 0, tx->vout[0])); - ProduceSignature(DUMMY_SIGNING_PROVIDER, MutableTransactionSignatureCreator(input1, 0, tx->vout[0].nValue, SIGHASH_ALL), tx->vout[0].scriptPubKey, sigdata); + ProduceSignature(DUMMY_SIGNING_PROVIDER, MutableTransactionSignatureCreator(input1, 0, tx->vout[0].nValue, {.sighash_type = SIGHASH_DEFAULT}), tx->vout[0].scriptPubKey, sigdata); return sigdata; } diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp index 9ea39c9150a..d695e08f5f5 100644 --- a/src/test/txvalidationcache_tests.cpp +++ b/src/test/txvalidationcache_tests.cpp @@ -331,7 +331,7 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup) // Sign SignatureData sigdata; - BOOST_CHECK(ProduceSignature(keystore, MutableTransactionSignatureCreator(valid_with_witness_tx, 0, 11 * CENT, SIGHASH_ALL), spend_tx.vout[1].scriptPubKey, sigdata)); + BOOST_CHECK(ProduceSignature(keystore, MutableTransactionSignatureCreator(valid_with_witness_tx, 0, 11 * CENT, {.sighash_type = SIGHASH_DEFAULT}), spend_tx.vout[1].scriptPubKey, sigdata)); UpdateInput(valid_with_witness_tx.vin[0], sigdata); // This should be valid under all script flags. @@ -359,7 +359,7 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, Dersig100Setup) // Sign for (int i = 0; i < 2; ++i) { SignatureData sigdata; - BOOST_CHECK(ProduceSignature(keystore, MutableTransactionSignatureCreator(tx, i, 11 * CENT, SIGHASH_ALL), spend_tx.vout[i].scriptPubKey, sigdata)); + BOOST_CHECK(ProduceSignature(keystore, MutableTransactionSignatureCreator(tx, i, 11 * CENT, {.sighash_type = SIGHASH_DEFAULT}), spend_tx.vout[i].scriptPubKey, sigdata)); UpdateInput(tx.vin[i], sigdata); } diff --git a/src/test/util/transaction_utils.cpp b/src/test/util/transaction_utils.cpp index 7b1e2eb3edd..b65a9568c65 100644 --- a/src/test/util/transaction_utils.cpp +++ b/src/test/util/transaction_utils.cpp @@ -95,7 +95,7 @@ bool SignSignature(const SigningProvider &provider, const CScript& fromPubKey, C { assert(nIn < txTo.vin.size()); - MutableTransactionSignatureCreator creator(txTo, nIn, amount, nHashType); + MutableTransactionSignatureCreator creator(txTo, nIn, amount, {.sighash_type = nHashType}); bool ret = ProduceSignature(provider, creator, fromPubKey, sig_data); UpdateInput(txTo.vin.at(nIn), sig_data);