From 82c9fe31793baf5995f6e9adb3fd79154d9d6f6e Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 16 Mar 2026 15:44:43 -0700 Subject: [PATCH] psbt: Use PSBTInput and PSBTOutput fields instead of accessing global tx PSBTInput now has the previous txid and output index, and PSBTOutput has the amount and script. We no longer need to access the global unsigned tx for these fields. Additionally, we can change iterating tx.vin and tx.vout to psbtx.inputs and psbtx.outputs. This is in prepration for use with PSBTv2 where the global unsigned tx will not exist. --- src/node/psbt.cpp | 18 +++++++-------- src/psbt.cpp | 2 +- src/qt/psbtoperationsdialog.cpp | 16 ++++++------- src/rpc/rawtransaction.cpp | 33 ++++++++++++--------------- src/test/fuzz/psbt.cpp | 2 +- src/wallet/scriptpubkeyman.cpp | 11 ++++----- src/wallet/test/psbt_wallet_tests.cpp | 1 + src/wallet/wallet.cpp | 7 ++---- 8 files changed, 41 insertions(+), 49 deletions(-) diff --git a/src/node/psbt.cpp b/src/node/psbt.cpp index 749007d2440..cbcbfa0ef5d 100644 --- a/src/node/psbt.cpp +++ b/src/node/psbt.cpp @@ -22,11 +22,11 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx) CAmount in_amt = 0; - result.inputs.resize(psbtx.tx->vin.size()); + result.inputs.resize(psbtx.inputs.size()); const PrecomputedTransactionData txdata = PrecomputePSBTData(psbtx); - for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { + for (unsigned int i = 0; i < psbtx.inputs.size(); ++i) { PSBTInput& input = psbtx.inputs[i]; PSBTInputAnalysis& input_analysis = result.inputs[i]; @@ -43,7 +43,7 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx) in_amt += utxo.nValue; input_analysis.has_utxo = true; } else { - if (input.non_witness_utxo && psbtx.tx->vin[i].prevout.n >= input.non_witness_utxo->vout.size()) { + if (input.non_witness_utxo && input.prev_out >= input.non_witness_utxo->vout.size()) { result.SetInvalid(strprintf("PSBT is not valid. Input %u specifies invalid prevout", i)); return result; } @@ -89,7 +89,7 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx) // Calculate next role for PSBT by grabbing "minimum" PSBTInput next role result.next = PSBTRole::EXTRACTOR; - for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { + for (unsigned int i = 0; i < psbtx.inputs.size(); ++i) { PSBTInputAnalysis& input_analysis = result.inputs[i]; result.next = std::min(result.next, input_analysis.next); } @@ -97,12 +97,12 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx) if (calc_fee) { // Get the output amount - CAmount out_amt = std::accumulate(psbtx.tx->vout.begin(), psbtx.tx->vout.end(), CAmount(0), - [](CAmount a, const CTxOut& b) { - if (!MoneyRange(a) || !MoneyRange(b.nValue) || !MoneyRange(a + b.nValue)) { + CAmount out_amt = std::accumulate(psbtx.outputs.begin(), psbtx.outputs.end(), CAmount(0), + [](CAmount a, const PSBTOutput& b) { + if (!MoneyRange(a) || !MoneyRange(b.amount) || !MoneyRange(a + b.amount)) { return CAmount(-1); } - return a += b.nValue; + return a += b.amount; } ); if (!MoneyRange(out_amt)) { @@ -119,7 +119,7 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx) CCoinsViewCache view{&CoinsViewEmpty::Get()}; bool success = true; - for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { + for (unsigned int i = 0; i < psbtx.inputs.size(); ++i) { PSBTInput& input = psbtx.inputs[i]; Coin newcoin; diff --git a/src/psbt.cpp b/src/psbt.cpp index 0885519bc2c..cff76611cb8 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -605,7 +605,7 @@ bool FinalizePSBT(PartiallySignedTransaction& psbtx) // script. bool complete = true; const PrecomputedTransactionData txdata = PrecomputePSBTData(psbtx); - for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { + for (unsigned int i = 0; i < psbtx.inputs.size(); ++i) { PSBTInput& input = psbtx.inputs.at(i); complete &= (SignPSBTInput(DUMMY_SIGNING_PROVIDER, psbtx, i, &txdata, {.sighash_type = input.sighash_type, .finalize = true}, /*out_sigdata=*/nullptr) == PSBTError::OK); } diff --git a/src/qt/psbtoperationsdialog.cpp b/src/qt/psbtoperationsdialog.cpp index c77c827d930..0a466d5a2be 100644 --- a/src/qt/psbtoperationsdialog.cpp +++ b/src/qt/psbtoperationsdialog.cpp @@ -145,13 +145,13 @@ void PSBTOperationsDialog::saveTransaction() { QString selected_filter; QString filename_suggestion = ""; bool first = true; - for (const CTxOut& out : m_transaction_data->tx->vout) { + for (const PSBTOutput& out : m_transaction_data->outputs) { if (!first) { filename_suggestion.append("-"); } CTxDestination address; - ExtractDestination(out.scriptPubKey, address); - QString amount = BitcoinUnits::format(m_client_model->getOptionsModel()->getDisplayUnit(), out.nValue); + ExtractDestination(out.script, address); + QString amount = BitcoinUnits::format(m_client_model->getOptionsModel()->getDisplayUnit(), out.amount); QString address_str = QString::fromStdString(EncodeDestination(address)); filename_suggestion.append(address_str + "-" + amount); first = false; @@ -180,15 +180,15 @@ QString PSBTOperationsDialog::renderTransaction(const PartiallySignedTransaction QString tx_description; QLatin1String bullet_point(" * "); CAmount totalAmount = 0; - for (const CTxOut& out : psbtx.tx->vout) { + for (const PSBTOutput& out : psbtx.outputs) { CTxDestination address; - ExtractDestination(out.scriptPubKey, address); - totalAmount += out.nValue; + ExtractDestination(out.script, address); + totalAmount += out.amount; tx_description.append(bullet_point).append(tr("Sends %1 to %2") - .arg(BitcoinUnits::formatWithUnit(BitcoinUnit::BTC, out.nValue)) + .arg(BitcoinUnits::formatWithUnit(BitcoinUnit::BTC, out.amount)) .arg(QString::fromStdString(EncodeDestination(address)))); // Check if the address is one of ours - if (m_wallet_model != nullptr && m_wallet_model->wallet().txoutIsMine(out)) tx_description.append(" (" + tr("own address") + ")"); + if (m_wallet_model != nullptr && m_wallet_model->wallet().txoutIsMine(CTxOut(out.amount, out.script))) tx_description.append(" (" + tr("own address") + ")"); tx_description.append("
"); } diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index d3076dbeaa9..1ed94fac545 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -144,10 +144,7 @@ PartiallySignedTransaction ProcessPSBT(const std::string& psbt_string, const std // 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); - + for (PSBTInput& psbt_input : psbtx.inputs) { // The `non_witness_utxo` is the whole previous transaction if (psbt_input.non_witness_utxo) continue; @@ -156,11 +153,11 @@ PartiallySignedTransaction ProcessPSBT(const std::string& psbt_string, const std // Look in the txindex if (g_txindex) { uint256 block_hash; - g_txindex->FindTx(tx_in.prevout.hash, block_hash, tx); + g_txindex->FindTx(psbt_input.prev_txid, block_hash, tx); } // If we still don't have it look in the mempool if (!tx) { - tx = node.mempool->get(tx_in.prevout.hash); + tx = node.mempool->get(psbt_input.prev_txid); } if (tx) { psbt_input.non_witness_utxo = tx; @@ -172,9 +169,7 @@ PartiallySignedTransaction ProcessPSBT(const std::string& psbt_string, const std // 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); - + for (PSBTInput& input : psbtx.inputs) { // If there are still missing utxos, add them if they were found in the utxo set if (!input.non_witness_utxo) { const Coin& coin = coins.at(input.GetOutPoint()); @@ -187,7 +182,7 @@ PartiallySignedTransaction ProcessPSBT(const std::string& psbt_string, const std const PrecomputedTransactionData& txdata = PrecomputePSBTData(psbtx); - for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { + for (unsigned int i = 0; i < psbtx.inputs.size(); ++i) { if (PSBTInputSigned(psbtx.inputs.at(i))) { continue; } @@ -203,7 +198,7 @@ PartiallySignedTransaction ProcessPSBT(const std::string& psbt_string, const std } // Update script/keypath information using descriptor data. - for (unsigned int i = 0; i < psbtx.tx->vout.size(); ++i) { + for (unsigned int i = 0; i < psbtx.outputs.size(); ++i) { UpdatePSBTOutput(provider, psbtx, i); } @@ -1142,7 +1137,7 @@ static RPCMethod decodepsbt() have_a_utxo = true; } if (input.non_witness_utxo) { - txout = input.non_witness_utxo->vout[psbtx.tx->vin[i].prevout.n]; + txout = input.non_witness_utxo->vout[input.prev_out]; UniValue non_wit(UniValue::VOBJ); TxToUniv(*input.non_witness_utxo, /*block_hash=*/uint256(), /*entry=*/non_wit, /*include_hex=*/false); @@ -1501,8 +1496,8 @@ static RPCMethod decodepsbt() outputs.push_back(std::move(out)); // Fee calculation - if (MoneyRange(psbtx.tx->vout[i].nValue) && MoneyRange(output_value + psbtx.tx->vout[i].nValue)) { - output_value += psbtx.tx->vout[i].nValue; + if (MoneyRange(output.amount) && MoneyRange(output_value + output.amount)) { + output_value += output.amount; } else { // Hack to just not show fee later have_all_utxos = false; @@ -1821,13 +1816,13 @@ static RPCMethod joinpsbts() // Merge for (auto& psbt : psbtxs) { - for (unsigned int i = 0; i < psbt.tx->vin.size(); ++i) { - if (!merged_psbt.AddInput(psbt.inputs[i])) { - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Input %s:%d exists in multiple PSBTs", psbt.tx->vin[i].prevout.hash.ToString(), psbt.tx->vin[i].prevout.n)); + for (const PSBTInput& input : psbt.inputs) { + if (!merged_psbt.AddInput(input)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Input %s:%d exists in multiple PSBTs", input.prev_txid.ToString(), input.prev_out)); } } - for (unsigned int i = 0; i < psbt.tx->vout.size(); ++i) { - merged_psbt.AddOutput(psbt.outputs[i]); + for (const PSBTOutput& output : psbt.outputs) { + merged_psbt.AddOutput(output); } for (auto& xpub_pair : psbt.m_xpubs) { if (!merged_psbt.m_xpubs.contains(xpub_pair.first)) { diff --git a/src/test/fuzz/psbt.cpp b/src/test/fuzz/psbt.cpp index d0abfc19a87..a1b1a5e9289 100644 --- a/src/test/fuzz/psbt.cpp +++ b/src/test/fuzz/psbt.cpp @@ -68,7 +68,7 @@ FUZZ_TARGET(psbt) (void)output.IsNull(); } - for (size_t i = 0; i < psbt.tx->vin.size(); ++i) { + for (size_t i = 0; i < psbt.inputs.size(); ++i) { CTxOut tx_out; if (psbt.GetInputUTXO(tx_out, i)) { (void)tx_out.IsNull(); diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 911b247d4ef..0241fe481f6 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -1311,8 +1311,7 @@ std::optional DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTran if (n_signed) { *n_signed = 0; } - for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { - const CTxIn& txin = psbtx.tx->vin[i]; + for (unsigned int i = 0; i < psbtx.inputs.size(); ++i) { PSBTInput& input = psbtx.inputs.at(i); if (PSBTInputSigned(input)) { @@ -1324,10 +1323,10 @@ std::optional DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTran if (!input.witness_utxo.IsNull()) { script = input.witness_utxo.scriptPubKey; } else if (input.non_witness_utxo) { - if (txin.prevout.n >= input.non_witness_utxo->vout.size()) { + if (input.prev_out >= input.non_witness_utxo->vout.size()) { return PSBTError::MISSING_INPUTS; } - script = input.non_witness_utxo->vout[txin.prevout.n].scriptPubKey; + script = input.non_witness_utxo->vout[input.prev_out].scriptPubKey; } else { // There's no UTXO so we can just skip this now continue; @@ -1391,8 +1390,8 @@ std::optional DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTran } // Fill in the bip32 keypaths and redeemscripts for the outputs so that hardware wallets can identify change - for (unsigned int i = 0; i < psbtx.tx->vout.size(); ++i) { - std::unique_ptr keys = GetSolvingProvider(psbtx.tx->vout.at(i).scriptPubKey); + for (unsigned int i = 0; i < psbtx.outputs.size(); ++i) { + std::unique_ptr keys = GetSolvingProvider(psbtx.outputs.at(i).script); if (!keys) { continue; } diff --git a/src/wallet/test/psbt_wallet_tests.cpp b/src/wallet/test/psbt_wallet_tests.cpp index 639cb233759..68b5f36ee0b 100644 --- a/src/wallet/test/psbt_wallet_tests.cpp +++ b/src/wallet/test/psbt_wallet_tests.cpp @@ -73,6 +73,7 @@ BOOST_AUTO_TEST_CASE(psbt_updater_test) // Mutate the transaction so that one of the inputs is invalid psbtx.tx->vin[0].prevout.n = 2; + psbtx.inputs[0].prev_out = 2; // Try to sign the mutated input SignatureData sigdata; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 823e1b48211..7473f80f4d0 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2227,17 +2227,14 @@ std::optional CWallet::FillPSBT(PartiallySignedTransaction& psbtx, co } LOCK(cs_wallet); // Get all of the previous transactions - for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { - const CTxIn& txin = psbtx.tx->vin[i]; - PSBTInput& input = psbtx.inputs.at(i); - + for (PSBTInput& input : psbtx.inputs) { if (PSBTInputSigned(input)) { continue; } // If we have no utxo, grab it from the wallet. if (!input.non_witness_utxo) { - const Txid& txhash = txin.prevout.hash; + const Txid& txhash = input.prev_txid; const auto it = mapWallet.find(txhash); if (it != mapWallet.end()) { const CWalletTx& wtx = it->second;