← Watch feed
psbt: Use PSBTInput and PSBTOutput fields instead of accessing global tx
Public commit record
73/100 · Adequate
What the developer wrote
Authored by Ava Chow
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.
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context
What changed, and why it matters
This commit is a code cleanup in Bitcoin Core's Partially Signed Bitcoin Transaction (PSBT) handling. It changes internal code to read input and output details from the PSBT data structures themselves rather than from the global unsigned transaction. The stated goal is to prepare for a future PSBT version (PSBTv2) where the global unsigned transaction may not exist. There is no direct security fix here, but any inconsistency between the PSBT fields and the global transaction could, in principle, lead to wrong analysis or signing behavior. The patch appears to keep behavior equivalent by using the same data through a different path.
Recommended action
Treat as a routine refactor. Reviewers should verify that PSBTInput.prev_txid/prev_out and PSBTOutput.amount/script are always kept in sync with the global unsigned transaction during serialization/deserialization, especially for PSBTv1, so that behavior does not change. No immediate security patch or user action is required.
Security signals we found
01
Refactor only: switches data source from global unsigned tx to PSBT input/output fields
02
No new input validation or bounds checks introduced
03
No explicit security bug fixed in commit message or diff
04
Potential consistency risk if PSBTInput/PSBTOutput fields diverge from global tx, but no evidence of exploitable divergence in this commit
05
Preparation for PSBTv2, a future protocol change
Evidence from the diff
The commit refactors PSBT handling across node, RPC, wallet, GUI, and test code to use PSBTInput fields (prev_txid, prev_out) and PSBTOutput fields (amount, script) instead of psbtx.tx->vin/vout. It also replaces index-based loops over tx->vin/vout with iteration over psbtx.inputs/outputs where appropriate. The change is preparatory for PSBTv2, which may not include a global unsigned transaction. The diff shows no new validation, no bounds-check additions, and no behavior changes beyond data-source substitution. One test is updated to mutate both the global tx and the PSBTInput field, confirming the code now reads from the PSBTInput. No vulnerability, CVE, or security disclosure is mentioned in the commit or supplied references.
Changed components
src/node/psbt.cppsrc/psbt.cppsrc/qt/psbtoperationsdialog.cppsrc/rpc/rawtransaction.cppsrc/test/fuzz/psbt.cppsrc/wallet/scriptpubkeyman.cppsrc/wallet/test/psbt_wallet_tests.cppsrc/wallet/wallet.cppInspect captured patch +41 / −49
diff --git a/src/node/psbt.cpp b/src/node/psbt.cpp
index 749007d2..cbcbfa0e 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 0885519b..cff76611 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 c77c827d..0a466d5a 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("<br>");
}
diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp
index d3076dbe..1ed94fac 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 d0abfc19..a1b1a5e9 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 911b247d..0241fe48 100644
--- a/src/wallet/scriptpubkeyman.cpp
+++ b/src/wallet/scriptpubkeyman.cpp
@@ -1311,8 +1311,7 @@ std::optional<PSBTError> 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<PSBTError> 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<PSBTError> 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<SigningProvider> keys = GetSolvingProvider(psbtx.tx->vout.at(i).scriptPubKey);
+ for (unsigned int i = 0; i < psbtx.outputs.size(); ++i) {
+ std::unique_ptr<SigningProvider> 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 639cb233..68b5f36e 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 823e1b48..7473f80f 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2227,17 +2227,14 @@ std::optional<PSBTError> 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;
Risk score
Our methodology →Why this scored 19/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.