diff --git a/src/psbt.cpp b/src/psbt.cpp index 50ccd9e2c03..16f4fa857c1 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -430,6 +430,38 @@ bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& return sig_complete; } +void RemoveUnnecessaryTransactions(PartiallySignedTransaction& psbtx, const int& sighash_type) +{ + // Only drop non_witness_utxos if sighash_type != SIGHASH_ANYONECANPAY + if ((sighash_type & 0x80) != SIGHASH_ANYONECANPAY) { + // Figure out if any non_witness_utxos should be dropped + std::vector to_drop; + for (unsigned int i = 0; i < psbtx.inputs.size(); ++i) { + const auto& input = psbtx.inputs.at(i); + int wit_ver; + std::vector wit_prog; + if (input.witness_utxo.IsNull() || !input.witness_utxo.scriptPubKey.IsWitnessProgram(wit_ver, wit_prog)) { + // There's a non-segwit input or Segwit v0, so we cannot drop any witness_utxos + to_drop.clear(); + break; + } + if (wit_ver == 0) { + // Segwit v0, so we cannot drop any non_witness_utxos + to_drop.clear(); + break; + } + if (input.non_witness_utxo) { + to_drop.push_back(i); + } + } + + // Drop the non_witness_utxos that we can drop + for (unsigned int i : to_drop) { + psbtx.inputs.at(i).non_witness_utxo = nullptr; + } + } +} + bool FinalizePSBT(PartiallySignedTransaction& psbtx) { // Finalize input signatures -- in case we have partial signatures that add up to a complete diff --git a/src/psbt.h b/src/psbt.h index 40d69cd4545..0a581933f0e 100644 --- a/src/psbt.h +++ b/src/psbt.h @@ -1231,6 +1231,9 @@ bool PSBTInputSignedAndVerified(const PartiallySignedTransaction psbt, unsigned **/ bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index, const PrecomputedTransactionData* txdata, int sighash = SIGHASH_ALL, SignatureData* out_sigdata = nullptr, bool finalize = true); +/** Reduces the size of the PSBT by dropping unnecessary `non_witness_utxos` (i.e. complete previous transactions) from a psbt when all inputs are segwit v1. */ +void RemoveUnnecessaryTransactions(PartiallySignedTransaction& psbtx, const int& sighash_type); + /** 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 011983a158c..879e847acb3 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -169,7 +169,7 @@ static std::vector CreateTxDoc() }; } -// Update PSBT with information from the mempool, the UTXO set, and the provided descriptors +// Update PSBT with information from the mempool, the UTXO set, the txindex, and the provided descriptors PartiallySignedTransaction ProcessPSBT(const std::string& psbt_string, const std::any& context, const HidingSigningProvider& provider) { // Unserialize the transactions @@ -179,50 +179,78 @@ PartiallySignedTransaction ProcessPSBT(const std::string& psbt_string, const std throw JSONRPCError(RPC_DESERIALIZATION_ERROR, strprintf("TX decode failed %s", error)); } - // Fetch previous transactions (inputs): - CCoinsView viewDummy; - CCoinsViewCache view(&viewDummy); - { - NodeContext& node = EnsureAnyNodeContext(context); - const CTxMemPool& mempool = EnsureMemPool(node); - ChainstateManager& chainman = EnsureChainman(node); - LOCK2(cs_main, mempool.cs); - CCoinsViewCache &viewChain = chainman.ActiveChainstate().CoinsTip(); - CCoinsViewMemPool viewMempool(&viewChain, mempool); - view.SetBackend(viewMempool); // temporarily switch cache backend to db+mempool view + if (g_txindex) g_txindex->BlockUntilSyncedToCurrentChain(); + const NodeContext& node = EnsureAnyNodeContext(context); - for (const CTxIn& txin : psbtx.tx->vin) { - view.AccessCoin(txin.prevout); // Load entries from viewChain into view; can fail. + // If we can't find the corresponding full transaction for all of our inputs, + // this will be used to find just the utxos for the segwit inputs for which + // the full transaction isn't found + std::map coins; + + // Fetch previous transactions: + // First, look in the txindex and the mempool + for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { + PSBTInput& psbt_input = psbtx.inputs.at(i); + const CTxIn& tx_in = psbtx.tx->vin.at(i); + + // The `non_witness_utxo` is the whole previous transaction + if (psbt_input.non_witness_utxo) continue; + + CTransactionRef tx; + + // Look in the txindex + if (g_txindex) { + uint256 block_hash; + g_txindex->FindTx(tx_in.prevout.hash, block_hash, tx); + } + // If we still don't have it look in the mempool + if (!tx) { + tx = node.mempool->get(tx_in.prevout.hash); + } + if (tx) { + psbt_input.non_witness_utxo = tx; + } else { + coins[tx_in.prevout]; // Create empty map entry keyed by prevout } - - view.SetBackend(viewDummy); // switch back to avoid locking mempool for too long } - // 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); + // If we still haven't found all of the inputs, look for the missing ones in the utxo set + if (!coins.empty()) { + FindCoins(node, coins); + for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { + PSBTInput& input = psbtx.inputs.at(i); - if (input.non_witness_utxo || !input.witness_utxo.IsNull()) { - continue; + // If there are still missing utxos, add them if they were found in the utxo set + if (!input.non_witness_utxo) { + const CTxIn& tx_in = psbtx.tx->vin.at(i); + const Coin& coin = coins.at(tx_in.prevout); + if (!coin.out.IsNull() && IsSegWitOutput(provider, coin.out.scriptPubKey)) { + input.witness_utxo = coin.out; + } + } } + } - const Coin& coin = view.AccessCoin(psbtx.tx->vin[i].prevout); + const PrecomputedTransactionData& txdata = PrecomputePSBTData(psbtx); - if (IsSegWitOutput(provider, coin.out.scriptPubKey)) { - input.witness_utxo = coin.out; + for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { + if (PSBTInputSigned(psbtx.inputs.at(i))) { + continue; } // 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(provider, psbtx, i, &txdata, /*sighash=*/1); + SignPSBTInput(provider, psbtx, /*index=*/i, &txdata, /*sighash=*/1); } // Update script/keypath information using descriptor data. for (unsigned int i = 0; i < psbtx.tx->vout.size(); ++i) { UpdatePSBTOutput(provider, psbtx, i); } + + RemoveUnnecessaryTransactions(psbtx, /*sighash_type=*/1); + return psbtx; } @@ -1632,7 +1660,7 @@ static RPCHelpMan converttopsbt() static RPCHelpMan utxoupdatepsbt() { return RPCHelpMan{"utxoupdatepsbt", - "\nUpdates all segwit inputs and outputs in a PSBT with data from output descriptors, the UTXO set or the mempool.\n", + "\nUpdates all segwit inputs and outputs in a PSBT with data from output descriptors, the UTXO set, txindex, or the mempool.\n", { {"psbt", RPCArg::Type::STR, RPCArg::Optional::NO, "A base64 string of a PSBT"}, {"descriptors", RPCArg::Type::ARR, RPCArg::Optional::OMITTED_NAMED_ARG, "An array of either strings or objects", { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 6158ff033ca..3ead8bee349 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2156,34 +2156,7 @@ TransactionError CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bool& comp } } - // Only drop non_witness_utxos if sighash_type != SIGHASH_ANYONECANPAY - if ((sighash_type & 0x80) != SIGHASH_ANYONECANPAY) { - // Figure out if any non_witness_utxos should be dropped - std::vector to_drop; - for (unsigned int i = 0; i < psbtx.inputs.size(); ++i) { - const auto& input = psbtx.inputs.at(i); - int wit_ver; - std::vector wit_prog; - if (input.witness_utxo.IsNull() || !input.witness_utxo.scriptPubKey.IsWitnessProgram(wit_ver, wit_prog)) { - // There's a non-segwit input or Segwit v0, so we cannot drop any witness_utxos - to_drop.clear(); - break; - } - if (wit_ver == 0) { - // Segwit v0, so we cannot drop any non_witness_utxos - to_drop.clear(); - break; - } - if (input.non_witness_utxo) { - to_drop.push_back(i); - } - } - - // Drop the non_witness_utxos that we can drop - for (unsigned int i : to_drop) { - psbtx.inputs.at(i).non_witness_utxo = nullptr; - } - } + RemoveUnnecessaryTransactions(psbtx, sighash_type); // Complete if every input is now signed complete = true; diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 58a80e37a20..44113296882 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -611,17 +611,17 @@ class PSBTTest(BitcoinTestFramework): # Bech32 inputs should be filled with witness UTXO. Other inputs should not be filled because they are non-witness updated = self.nodes[1].utxoupdatepsbt(psbt) decoded = self.nodes[1].decodepsbt(updated) - test_psbt_input_keys(decoded['inputs'][0], ['witness_utxo']) - test_psbt_input_keys(decoded['inputs'][1], []) - test_psbt_input_keys(decoded['inputs'][2], []) + test_psbt_input_keys(decoded['inputs'][0], ['witness_utxo', 'non_witness_utxo']) + test_psbt_input_keys(decoded['inputs'][1], ['non_witness_utxo']) + test_psbt_input_keys(decoded['inputs'][2], ['non_witness_utxo']) # Try again, now while providing descriptors, making P2SH-segwit work, and causing bip32_derivs and redeem_script to be filled in descs = [self.nodes[1].getaddressinfo(addr)['desc'] for addr in [addr1,addr2,addr3]] updated = self.nodes[1].utxoupdatepsbt(psbt=psbt, descriptors=descs) decoded = self.nodes[1].decodepsbt(updated) - test_psbt_input_keys(decoded['inputs'][0], ['witness_utxo', 'bip32_derivs']) - test_psbt_input_keys(decoded['inputs'][1], []) - test_psbt_input_keys(decoded['inputs'][2], ['witness_utxo', 'bip32_derivs', 'redeem_script']) + test_psbt_input_keys(decoded['inputs'][0], ['witness_utxo', 'non_witness_utxo', 'bip32_derivs']) + test_psbt_input_keys(decoded['inputs'][1], ['non_witness_utxo', 'bip32_derivs']) + test_psbt_input_keys(decoded['inputs'][2], ['non_witness_utxo','witness_utxo', 'bip32_derivs', 'redeem_script']) # Two PSBTs with a common input should not be joinable psbt1 = self.nodes[1].createpsbt([{"txid":txid1, "vout":vout1}], {self.nodes[0].getnewaddress():Decimal('10.999')})