psbt: AddInput and AddOutput should take only PSBTInput and PSBTOutput
What the developer wrote
Authored by Ava Chow
psbt: AddInput and AddOutput should take only PSBTInput and PSBTOutput
What changed, and why it matters
This commit refactors how inputs and outputs are added to Partially Signed Bitcoin Transactions (PSBTs). Previously, callers had to pass both a raw transaction input/output and a PSBT-specific input/output, which could become inconsistent. Now the functions accept only the PSBT-specific object and derive the raw transaction data from it. For PSBT version 2, the code currently just returns false and does not actually add anything, which appears to be intentional but incomplete support. The change reduces the risk of mismatched data and duplicate inputs, but it is primarily a code-quality and API-safety improvement rather than a fix for an active exploit.
Treat as a routine refactor with minor defensive-security benefits. Review whether the PSBT v2 return-false behavior is intentional and whether follow-up work is needed to enable v2 input/output addition. No urgent patch or incident response is indicated by this commit alone.
Security signals we found
API hardening: removes possibility of inconsistent CTxIn/PSBTInput or CTxOut/PSBTOutput pairs
Duplicate-input prevention added for PSBT v0 via tx->vin search
PSBT v2 AddInput/AddOutput currently return false, indicating incomplete support rather than a vulnerability fix
No explicit security bug or CVE referenced in commit message or diff
Evidence from the diff
The patch changes PartiallySignedTransaction::AddInput and AddOutput to take only PSBTInput/PSBTOutput instead of separate CTxIn/CTxOut and PSBTInput/PSBTOutput pairs. For PSBT v0, it constructs CTxIn/CTxOut from the PSBT object’s prev_txid/prev_out or amount/script. It also adds duplicate-input prevention for v0 by checking tx->vin. For PSBT v2, AddInput checks for duplicates among existing PSBTInput entries but then returns false without adding, and AddOutput also returns false. Call sites in rpc/rawtransaction.cpp and the fuzz target are updated accordingly. The change removes an API where caller-supplied CTxIn/CTxOut could disagree with the PSBT structure, but the v2 behavior is non-functional (returns false), suggesting this is a partial step toward v2 support.
Changed components
src/psbt.cppsrc/psbt.hsrc/rpc/rawtransaction.cppsrc/test/fuzz/psbt.cppInspect captured patch +44 / −22
diff --git a/src/psbt.cpp b/src/psbt.cpp
index 85c2f25a..0885519b 100644
--- a/src/psbt.cpp
+++ b/src/psbt.cpp
@@ -58,24 +58,46 @@ bool PartiallySignedTransaction::Merge(const PartiallySignedTransaction& psbt)
return true;
}
-bool PartiallySignedTransaction::AddInput(const CTxIn& txin, PSBTInput& psbtin)
+bool PartiallySignedTransaction::AddInput(const PSBTInput& psbtin)
{
- if (std::find(tx->vin.begin(), tx->vin.end(), txin) != tx->vin.end()) {
+ if (GetVersion() < 2) {
+ // This is a v0 psbt, so do the v0 AddInput
+ CTxIn txin(COutPoint(psbtin.prev_txid, psbtin.prev_out));
+ if (std::find(tx->vin.begin(), tx->vin.end(), txin) != tx->vin.end()) {
+ // Prevent duplicate inputs
+ return false;
+ }
+ tx->vin.push_back(std::move(txin));
+ inputs.push_back(psbtin);
+ inputs.back().partial_sigs.clear();
+ inputs.back().final_script_sig.clear();
+ inputs.back().final_script_witness.SetNull();
+ return true;
+ }
+
+ // Prevent duplicate inputs
+ if (std::find_if(inputs.begin(), inputs.end(),
+ [psbtin](const PSBTInput& psbt) {
+ return psbt.prev_txid == psbtin.prev_txid && psbt.prev_out == psbtin.prev_out;
+ }
+ ) != inputs.end()) {
return false;
}
- tx->vin.push_back(txin);
- psbtin.partial_sigs.clear();
- psbtin.final_script_sig.clear();
- psbtin.final_script_witness.SetNull();
- inputs.push_back(psbtin);
- return true;
+
+ return false;
}
-bool PartiallySignedTransaction::AddOutput(const CTxOut& txout, const PSBTOutput& psbtout)
+bool PartiallySignedTransaction::AddOutput(const PSBTOutput& psbtout)
{
- tx->vout.push_back(txout);
- outputs.push_back(psbtout);
- return true;
+ if (GetVersion() < 2) {
+ // This is a v0 psbt, do the v0 AddOutput
+ CTxOut txout(psbtout.amount, psbtout.script);
+ tx->vout.push_back(txout);
+ outputs.push_back(psbtout);
+ return true;
+ }
+
+ return false;
}
bool PartiallySignedTransaction::GetInputUTXO(CTxOut& utxo, int input_index) const
diff --git a/src/psbt.h b/src/psbt.h
index bb5ce491..70bdae96 100644
--- a/src/psbt.h
+++ b/src/psbt.h
@@ -1084,8 +1084,8 @@ public:
/** Merge psbt into this. The two psbts must have the same underlying CTransaction (i.e. the
* same actual Bitcoin transaction.) Returns true if the merge succeeded, false otherwise. */
[[nodiscard]] bool Merge(const PartiallySignedTransaction& psbt);
- bool AddInput(const CTxIn& txin, PSBTInput& psbtin);
- bool AddOutput(const CTxOut& txout, const PSBTOutput& psbtout);
+ bool AddInput(const PSBTInput& psbtin);
+ bool AddOutput(const PSBTOutput& psbtout);
explicit PartiallySignedTransaction(const CMutableTransaction& tx);
/**
* Finds the UTXO for a given input index
diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp
index b5a06385..d3076dbe 100644
--- a/src/rpc/rawtransaction.cpp
+++ b/src/rpc/rawtransaction.cpp
@@ -1822,12 +1822,12 @@ static RPCMethod joinpsbts()
// Merge
for (auto& psbt : psbtxs) {
for (unsigned int i = 0; i < psbt.tx->vin.size(); ++i) {
- if (!merged_psbt.AddInput(psbt.tx->vin[i], psbt.inputs[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 (unsigned int i = 0; i < psbt.tx->vout.size(); ++i) {
- merged_psbt.AddOutput(psbt.tx->vout[i], psbt.outputs[i]);
+ merged_psbt.AddOutput(psbt.outputs[i]);
}
for (auto& xpub_pair : psbt.m_xpubs) {
if (!merged_psbt.m_xpubs.contains(xpub_pair.first)) {
@@ -1851,10 +1851,10 @@ static RPCMethod joinpsbts()
PartiallySignedTransaction shuffled_psbt(tx);
for (int i : input_indices) {
- shuffled_psbt.AddInput(merged_psbt.tx->vin[i], merged_psbt.inputs[i]);
+ shuffled_psbt.AddInput(merged_psbt.inputs[i]);
}
for (int i : output_indices) {
- shuffled_psbt.AddOutput(merged_psbt.tx->vout[i], merged_psbt.outputs[i]);
+ shuffled_psbt.AddOutput(merged_psbt.outputs[i]);
}
shuffled_psbt.unknown.insert(merged_psbt.unknown.begin(), merged_psbt.unknown.end());
diff --git a/src/test/fuzz/psbt.cpp b/src/test/fuzz/psbt.cpp
index dd7c61eb..d0abfc19 100644
--- a/src/test/fuzz/psbt.cpp
+++ b/src/test/fuzz/psbt.cpp
@@ -98,11 +98,11 @@ FUZZ_TARGET(psbt)
if (comb_res) {
psbt_mut = *comb_res;
}
- for (unsigned int i = 0; i < psbt_merge.tx->vin.size(); ++i) {
- (void)psbt_mut.AddInput(psbt_merge.tx->vin[i], psbt_merge.inputs[i]);
+ for (const auto& psbt_in : psbt_merge.inputs) {
+ (void)psbt_mut.AddInput(psbt_in);
}
- for (unsigned int i = 0; i < psbt_merge.tx->vout.size(); ++i) {
- Assert(psbt_mut.AddOutput(psbt_merge.tx->vout[i], psbt_merge.outputs[i]));
+ for (const auto& psbt_out : psbt_merge.outputs) {
+ Assert(psbt_mut.AddOutput(psbt_out));
}
psbt_mut.unknown.insert(psbt_merge.unknown.begin(), psbt_merge.unknown.end());
}
Why this scored 27/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.