From fd3f6890f3dfd683f6f13db912caf5c4288adf08 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 3 Mar 2021 16:47:44 -0800 Subject: [PATCH] Construct and use PrecomputedTransactionData in PSBT signing --- src/node/psbt.cpp | 6 +++-- src/psbt.cpp | 26 ++++++++++++++++--- src/psbt.h | 11 ++++++-- src/rpc/rawtransaction.cpp | 3 ++- .../external_signer_scriptpubkeyman.cpp | 4 +-- src/wallet/external_signer_scriptpubkeyman.h | 2 +- src/wallet/scriptpubkeyman.cpp | 8 +++--- src/wallet/scriptpubkeyman.h | 6 ++--- src/wallet/test/psbt_wallet_tests.cpp | 2 +- src/wallet/wallet.cpp | 3 ++- 10 files changed, 50 insertions(+), 21 deletions(-) diff --git a/src/node/psbt.cpp b/src/node/psbt.cpp index c1890182680..b013b6d5793 100644 --- a/src/node/psbt.cpp +++ b/src/node/psbt.cpp @@ -23,6 +23,8 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx) result.inputs.resize(psbtx.tx->vin.size()); + const PrecomputedTransactionData txdata = PrecomputePSBTData(psbtx); + for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { PSBTInput& input = psbtx.inputs[i]; PSBTInputAnalysis& input_analysis = result.inputs[i]; @@ -61,7 +63,7 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx) // Figure out what is missing SignatureData outdata; - bool complete = SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, 1, &outdata); + bool complete = SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, &txdata, 1, &outdata); // Things are missing if (!complete) { @@ -121,7 +123,7 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx) PSBTInput& input = psbtx.inputs[i]; Coin newcoin; - if (!SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, 1, nullptr, true) || !psbtx.GetInputUTXO(newcoin.out, i)) { + if (!SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, nullptr, 1) || !psbtx.GetInputUTXO(newcoin.out, i)) { success = false; break; } else { diff --git a/src/psbt.cpp b/src/psbt.cpp index a849b2ea535..9894749fab6 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -227,7 +227,24 @@ void UpdatePSBTOutput(const SigningProvider& provider, PartiallySignedTransactio psbt_out.FromSignatureData(sigdata); } -bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index, int sighash, SignatureData* out_sigdata, bool use_dummy) +PrecomputedTransactionData PrecomputePSBTData(const PartiallySignedTransaction& psbt) +{ + const CMutableTransaction& tx = *psbt.tx; + bool have_all_spent_outputs = true; + std::vector utxos(tx.vin.size()); + for (size_t idx = 0; idx < tx.vin.size(); ++idx) { + if (!psbt.GetInputUTXO(utxos[idx], idx)) have_all_spent_outputs = false; + } + PrecomputedTransactionData txdata; + if (have_all_spent_outputs) { + txdata.Init(tx, std::move(utxos), true); + } else { + txdata.Init(tx, {}, true); + } + return txdata; +} + +bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index, const PrecomputedTransactionData* txdata, int sighash, SignatureData* out_sigdata) { PSBTInput& input = psbt.inputs.at(index); const CMutableTransaction& tx = *psbt.tx; @@ -267,10 +284,10 @@ bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& sigdata.witness = false; bool sig_complete; - if (use_dummy) { + if (txdata == nullptr) { sig_complete = ProduceSignature(provider, DUMMY_SIGNATURE_CREATOR, utxo.scriptPubKey, sigdata); } else { - MutableTransactionSignatureCreator creator(&tx, index, utxo.nValue, sighash); + MutableTransactionSignatureCreator creator(&tx, index, utxo.nValue, txdata, sighash); sig_complete = ProduceSignature(provider, creator, utxo.scriptPubKey, sigdata); } // Verify that a witness signature was produced in case one was required. @@ -302,8 +319,9 @@ bool FinalizePSBT(PartiallySignedTransaction& psbtx) // PartiallySignedTransaction did not understand them), this will combine them into a final // script. bool complete = true; + const PrecomputedTransactionData txdata = PrecomputePSBTData(psbtx); for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { - complete &= SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, SIGHASH_ALL); + complete &= SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, &txdata, SIGHASH_ALL); } return complete; diff --git a/src/psbt.h b/src/psbt.h index 96ae39fdb81..f6b82b43de6 100644 --- a/src/psbt.h +++ b/src/psbt.h @@ -567,11 +567,18 @@ enum class PSBTRole { std::string PSBTRoleName(PSBTRole role); +/** Compute a PrecomputedTransactionData object from a psbt. */ +PrecomputedTransactionData PrecomputePSBTData(const PartiallySignedTransaction& psbt); + /** Checks whether a PSBTInput is already signed. */ bool PSBTInputSigned(const PSBTInput& input); -/** Signs a PSBTInput, verifying that all provided data matches what is being signed. */ -bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index, int sighash = SIGHASH_ALL, SignatureData* out_sigdata = nullptr, bool use_dummy = false); +/** Signs a PSBTInput, verifying that all provided data matches what is being signed. + * + * txdata should be the output of PrecomputePSBTData (which can be shared across + * multiple SignPSBTInput calls). If it is nullptr, a dummy signature will be created. + **/ +bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index, const PrecomputedTransactionData* txdata, int sighash = SIGHASH_ALL, SignatureData* out_sigdata = nullptr); /** Counts the unsigned inputs of a PSBT. */ size_t CountPSBTUnsignedInputs(const PartiallySignedTransaction& psbt); diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 414c6637a52..2fa033aee36 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1655,6 +1655,7 @@ static RPCHelpMan utxoupdatepsbt() } // Fill the inputs + const PrecomputedTransactionData txdata = PrecomputePSBTData(psbtx); for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { PSBTInput& input = psbtx.inputs.at(i); @@ -1671,7 +1672,7 @@ static RPCHelpMan utxoupdatepsbt() // Update script/keypath information using descriptor data. // Note that SignPSBTInput does a lot more than just constructing ECDSA signatures // we don't actually care about those here, in fact. - SignPSBTInput(public_provider, psbtx, i, /* sighash_type */ 1); + SignPSBTInput(public_provider, psbtx, i, &txdata, /* sighash_type */ 1); } // Update script/keypath information using descriptor data. diff --git a/src/wallet/external_signer_scriptpubkeyman.cpp b/src/wallet/external_signer_scriptpubkeyman.cpp index fe2c810afa7..03bb5d8b2c0 100644 --- a/src/wallet/external_signer_scriptpubkeyman.cpp +++ b/src/wallet/external_signer_scriptpubkeyman.cpp @@ -62,10 +62,10 @@ bool ExternalSignerScriptPubKeyMan::DisplayAddress(const CScript scriptPubKey, c } // If sign is true, transaction must previously have been filled -TransactionError ExternalSignerScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbt, int sighash_type, bool sign, bool bip32derivs, int* n_signed) const +TransactionError ExternalSignerScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type, bool sign, bool bip32derivs, int* n_signed) const { if (!sign) { - return DescriptorScriptPubKeyMan::FillPSBT(psbt, sighash_type, false, bip32derivs, n_signed); + return DescriptorScriptPubKeyMan::FillPSBT(psbt, txdata, sighash_type, false, bip32derivs, n_signed); } // Already complete if every input is now signed diff --git a/src/wallet/external_signer_scriptpubkeyman.h b/src/wallet/external_signer_scriptpubkeyman.h index 1786958912c..166b81d886d 100644 --- a/src/wallet/external_signer_scriptpubkeyman.h +++ b/src/wallet/external_signer_scriptpubkeyman.h @@ -29,7 +29,7 @@ class ExternalSignerScriptPubKeyMan : public DescriptorScriptPubKeyMan bool DisplayAddress(const CScript scriptPubKey, const ExternalSigner &signer) const; - TransactionError FillPSBT(PartiallySignedTransaction& psbt, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr) const override; + TransactionError FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr) const override; }; #endif diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 2eb9ca5c6df..c8669f4b03a 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -597,7 +597,7 @@ SigningResult LegacyScriptPubKeyMan::SignMessage(const std::string& message, con return SigningResult::SIGNING_FAILED; } -TransactionError LegacyScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, int sighash_type, bool sign, bool bip32derivs, int* n_signed) const +TransactionError LegacyScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, const PrecomputedTransactionData& txdata, int sighash_type, bool sign, bool bip32derivs, int* n_signed) const { if (n_signed) { *n_signed = 0; @@ -626,7 +626,7 @@ TransactionError LegacyScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psb } SignatureData sigdata; input.FillSignatureData(sigdata); - SignPSBTInput(HidingSigningProvider(this, !sign, !bip32derivs), psbtx, i, sighash_type); + SignPSBTInput(HidingSigningProvider(this, !sign, !bip32derivs), psbtx, i, &txdata, sighash_type); bool signed_one = PSBTInputSigned(input); if (n_signed && (signed_one || !sign)) { @@ -2083,7 +2083,7 @@ SigningResult DescriptorScriptPubKeyMan::SignMessage(const std::string& message, return SigningResult::OK; } -TransactionError DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, int sighash_type, bool sign, bool bip32derivs, int* n_signed) const +TransactionError DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psbtx, const PrecomputedTransactionData& txdata, int sighash_type, bool sign, bool bip32derivs, int* n_signed) const { if (n_signed) { *n_signed = 0; @@ -2133,7 +2133,7 @@ TransactionError DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& } } - SignPSBTInput(HidingSigningProvider(keys.get(), !sign, !bip32derivs), psbtx, i, sighash_type); + SignPSBTInput(HidingSigningProvider(keys.get(), !sign, !bip32derivs), psbtx, i, &txdata, sighash_type); bool signed_one = PSBTInputSigned(input); if (n_signed && (signed_one || !sign)) { diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index b8e34fbac35..3c4603608c1 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -235,7 +235,7 @@ public: /** Sign a message with the given script */ virtual SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const { return SigningResult::SIGNING_FAILED; }; /** Adds script and derivation path information to a PSBT, and optionally signs it. */ - virtual TransactionError FillPSBT(PartiallySignedTransaction& psbt, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr) const { return TransactionError::INVALID_PSBT; } + virtual TransactionError FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr) const { return TransactionError::INVALID_PSBT; } virtual uint256 GetID() const { return uint256(); } @@ -394,7 +394,7 @@ public: bool SignTransaction(CMutableTransaction& tx, const std::map& coins, int sighash, std::map& input_errors) const override; SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const override; - TransactionError FillPSBT(PartiallySignedTransaction& psbt, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr) const override; + TransactionError FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr) const override; uint256 GetID() const override; @@ -605,7 +605,7 @@ public: bool SignTransaction(CMutableTransaction& tx, const std::map& coins, int sighash, std::map& input_errors) const override; SigningResult SignMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) const override; - TransactionError FillPSBT(PartiallySignedTransaction& psbt, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr) const override; + TransactionError FillPSBT(PartiallySignedTransaction& psbt, const PrecomputedTransactionData& txdata, int sighash_type = 1 /* SIGHASH_ALL */, bool sign = true, bool bip32derivs = false, int* n_signed = nullptr) const override; uint256 GetID() const override; diff --git a/src/wallet/test/psbt_wallet_tests.cpp b/src/wallet/test/psbt_wallet_tests.cpp index ce7e661b677..1cefa386b7f 100644 --- a/src/wallet/test/psbt_wallet_tests.cpp +++ b/src/wallet/test/psbt_wallet_tests.cpp @@ -71,7 +71,7 @@ BOOST_AUTO_TEST_CASE(psbt_updater_test) // Try to sign the mutated input SignatureData sigdata; - BOOST_CHECK(spk_man->FillPSBT(psbtx, SIGHASH_ALL, true, true) != TransactionError::OK); + BOOST_CHECK(spk_man->FillPSBT(psbtx, PrecomputePSBTData(psbtx), SIGHASH_ALL, true, true) != TransactionError::OK); } BOOST_AUTO_TEST_CASE(parse_hd_keypath) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 4b6630de3c7..ac96e8fcaf4 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1830,6 +1830,7 @@ TransactionError CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bool& comp if (n_signed) { *n_signed = 0; } + const PrecomputedTransactionData txdata = PrecomputePSBTData(psbtx); LOCK(cs_wallet); // Get all of the previous transactions for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { @@ -1856,7 +1857,7 @@ TransactionError CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bool& comp // Fill in information from ScriptPubKeyMans for (ScriptPubKeyMan* spk_man : GetAllScriptPubKeyMans()) { int n_signed_this_spkm = 0; - TransactionError res = spk_man->FillPSBT(psbtx, sighash_type, sign, bip32derivs, &n_signed_this_spkm); + TransactionError res = spk_man->FillPSBT(psbtx, txdata, sighash_type, sign, bip32derivs, &n_signed_this_spkm); if (res != TransactionError::OK) { return res; }