← Watch feed
Replace PSBT.tx with PSBT::GetUnsignedTx and PSBT::GetUniqueID
Public commit record
85/100 · Strong
What the developer wrote
Authored by Ava Chow
Replace PSBT.tx with PSBT::GetUnsignedTx and PSBT::GetUniqueID
The global unsigned tx is decomposed into separate fields inside of
PSBT, which mirrors what PSBTv2 will do. However, we still need to get
the global unsigned tx so PSBT::GetUnsignedTx is introduced to do that.
In order to also have a stable unique ID, we also introduce
PSBT::GetUniqueID to replace uses of PSBT.tx.GetHash().
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Explains rationale or failure mode
What changed, and why it matters
This commit refactors how Bitcoin Core handles Partially Signed Bitcoin Transactions (PSBTs). Previously, a PSBT stored a complete copy of the unsigned transaction. Now, the transaction is broken into separate fields and reconstructed on demand. The change also introduces a stable unique ID for each PSBT. This is primarily a code-cleanup and preparation step for a future PSBT version 2, not a direct fix for an active security bug.
Recommended action
Treat as a routine refactor with defensive hardening. Review that all former direct uses of `psbt.tx` are now safely handled via `GetUnsignedTx()` and that `GetUniqueID()` correctly identifies PSBTs for merge operations. No urgent action required unless downstream code still references the removed `tx` member.
Security signals we found
01
Refactor of PSBT internal representation to remove direct unsigned transaction member
02
Introduction of GetUnsignedTx() and GetUniqueID() accessors
03
Addition of optional/null checks when reconstructing the unsigned transaction
04
Alignment with PSBTv2 field decomposition
05
No explicit security bug or vulnerability described in commit message
Evidence from the diff
The commit removes the public PSBT.tx member and replaces direct access with PSBT::GetUnsignedTx() and PSBT::GetUniqueID(). It decomposes the global unsigned transaction into tx_version, fallback_locktime, and per-input/per-output fields, aligning the internal representation with PSBTv2. Call sites in node, RPC, wallet, and tests are updated to use the new accessors. The change adds null/optional checks where the reconstructed transaction may be invalid.
Changed components
src/psbt.hsrc/psbt.cppsrc/node/psbt.cppsrc/rpc/rawtransaction.cppsrc/test/fuzz/psbt.cppsrc/wallet/test/psbt_wallet_tests.cppInspect captured patch +97 / −41
diff --git a/src/node/psbt.cpp b/src/node/psbt.cpp
index 5b78001c..699d1d5f 100644
--- a/src/node/psbt.cpp
+++ b/src/node/psbt.cpp
@@ -18,6 +18,13 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx)
// Go through each input and build status
PSBTAnalysis result;
+ std::optional<CMutableTransaction> unsigned_tx = psbtx.GetUnsignedTx();
+ if (!unsigned_tx) {
+ result.SetInvalid("PSBT cannot be made into a valid transaction");
+ return result;
+ }
+ CMutableTransaction& mtx = *unsigned_tx;
+
bool calc_fee = true;
CAmount in_amt = 0;
@@ -116,7 +123,6 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx)
result.fee = fee;
// Estimate the size
- CMutableTransaction mtx(*psbtx.tx);
CCoinsViewCache view{&CoinsViewEmpty::Get()};
bool success = true;
diff --git a/src/psbt.cpp b/src/psbt.cpp
index d7320682..a8707961 100644
--- a/src/psbt.cpp
+++ b/src/psbt.cpp
@@ -7,6 +7,7 @@
#include <common/types.h>
#include <node/types.h>
#include <policy/policy.h>
+#include <primitives/transaction.h>
#include <script/signingprovider.h>
#include <util/check.h>
#include <util/result.h>
@@ -14,7 +15,7 @@
using common::PSBTError;
-PartiallySignedTransaction::PartiallySignedTransaction(const CMutableTransaction& tx) : tx(tx)
+PartiallySignedTransaction::PartiallySignedTransaction(const CMutableTransaction& tx)
{
tx_version = tx.version;
fallback_locktime = tx.nLockTime;
@@ -30,13 +31,15 @@ PartiallySignedTransaction::PartiallySignedTransaction(const CMutableTransaction
bool PartiallySignedTransaction::IsNull() const
{
- return !tx && inputs.empty() && outputs.empty() && unknown.empty();
+ return inputs.empty() && outputs.empty() && unknown.empty();
}
bool PartiallySignedTransaction::Merge(const PartiallySignedTransaction& psbt)
{
// Prohibited to merge two PSBTs over different transactions
- if (tx->GetHash() != psbt.tx->GetHash()) {
+ std::optional<Txid> this_id = GetUniqueID();
+ std::optional<Txid> psbt_id = psbt.GetUniqueID();
+ if (!this_id || !psbt_id || this_id != psbt_id) {
return false;
}
@@ -58,23 +61,45 @@ bool PartiallySignedTransaction::Merge(const PartiallySignedTransaction& psbt)
return true;
}
-bool PartiallySignedTransaction::AddInput(const PSBTInput& psbtin)
+std::optional<CMutableTransaction> PartiallySignedTransaction::GetUnsignedTx() const
{
- 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;
+ CMutableTransaction mtx;
+ mtx.version = tx_version;
+ mtx.nLockTime = fallback_locktime.value_or(0);
+ uint32_t max_sequence = CTxIn::SEQUENCE_FINAL;
+ for (const PSBTInput& input : inputs) {
+ CTxIn txin;
+ txin.prevout.hash = input.prev_txid;
+ txin.prevout.n = input.prev_out;
+ txin.nSequence = input.sequence.value_or(max_sequence);
+ mtx.vin.push_back(txin);
+ }
+ for (const PSBTOutput& output : outputs) {
+ CTxOut txout;
+ txout.nValue = output.amount;
+ txout.scriptPubKey = output.script;
+ mtx.vout.push_back(txout);
+ }
+ return mtx;
+}
+
+std::optional<Txid> PartiallySignedTransaction::GetUniqueID() const
+{
+ // Get the unsigned transaction
+ std::optional<CMutableTransaction> mtx = GetUnsignedTx();
+ if (!mtx) {
+ return std::nullopt;
+ }
+ if (GetVersion() >= 2) {
+ for (CTxIn& txin : mtx->vin) {
+ txin.nSequence = 0;
}
- 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;
}
+ return mtx->GetHash();
+}
+bool PartiallySignedTransaction::AddInput(const PSBTInput& psbtin)
+{
// Prevent duplicate inputs
if (std::find_if(inputs.begin(), inputs.end(),
[psbtin](const PSBTInput& psbt) {
@@ -84,6 +109,15 @@ bool PartiallySignedTransaction::AddInput(const PSBTInput& psbtin)
return false;
}
+ if (GetVersion() < 2) {
+ // This is a v0 psbt, so do the v0 AddInput
+ inputs.push_back(psbtin);
+ inputs.back().partial_sigs.clear();
+ inputs.back().final_script_sig.clear();
+ inputs.back().final_script_witness.SetNull();
+ return true;
+ }
+
return false;
}
@@ -91,8 +125,6 @@ bool PartiallySignedTransaction::AddOutput(const PSBTOutput& psbtout)
{
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;
}
@@ -391,10 +423,15 @@ bool PSBTInputSignedAndVerified(const PartiallySignedTransaction& psbt, unsigned
return false;
}
+ std::optional<CMutableTransaction> unsigned_tx = psbt.GetUnsignedTx();
+ if (!unsigned_tx) {
+ return false;
+ }
+ const CMutableTransaction& tx = *unsigned_tx;
if (txdata) {
- return VerifyScript(input.final_script_sig, utxo.scriptPubKey, &input.final_script_witness, STANDARD_SCRIPT_VERIFY_FLAGS, MutableTransactionSignatureChecker{&(*psbt.tx), input_index, utxo.nValue, *txdata, MissingDataBehavior::FAIL});
+ return VerifyScript(input.final_script_sig, utxo.scriptPubKey, &input.final_script_witness, STANDARD_SCRIPT_VERIFY_FLAGS, MutableTransactionSignatureChecker{&tx, input_index, utxo.nValue, *txdata, MissingDataBehavior::FAIL});
} else {
- return VerifyScript(input.final_script_sig, utxo.scriptPubKey, &input.final_script_witness, STANDARD_SCRIPT_VERIFY_FLAGS, MutableTransactionSignatureChecker{&(*psbt.tx), input_index, utxo.nValue, MissingDataBehavior::FAIL});
+ return VerifyScript(input.final_script_sig, utxo.scriptPubKey, &input.final_script_witness, STANDARD_SCRIPT_VERIFY_FLAGS, MutableTransactionSignatureChecker{&tx, input_index, utxo.nValue, MissingDataBehavior::FAIL});
}
}
@@ -411,7 +448,11 @@ size_t CountPSBTUnsignedInputs(const PartiallySignedTransaction& psbt) {
void UpdatePSBTOutput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index)
{
- CMutableTransaction& tx = *Assert(psbt.tx);
+ std::optional<CMutableTransaction> unsigned_tx = psbt.GetUnsignedTx();
+ if (!unsigned_tx) {
+ return;
+ }
+ CMutableTransaction& tx = *unsigned_tx;
const CTxOut& out = tx.vout.at(index);
PSBTOutput& psbt_out = psbt.outputs.at(index);
@@ -431,7 +472,11 @@ void UpdatePSBTOutput(const SigningProvider& provider, PartiallySignedTransactio
std::optional<PrecomputedTransactionData> PrecomputePSBTData(const PartiallySignedTransaction& psbt)
{
- const CMutableTransaction& tx = *psbt.tx;
+ std::optional<CMutableTransaction> unsigned_tx = psbt.GetUnsignedTx();
+ if (!unsigned_tx) {
+ return std::nullopt;
+ }
+ const CMutableTransaction& tx = *unsigned_tx;
bool have_all_spent_outputs = true;
std::vector<CTxOut> utxos;
for (const PSBTInput& input : psbt.inputs) {
@@ -449,7 +494,11 @@ std::optional<PrecomputedTransactionData> PrecomputePSBTData(const PartiallySign
PSBTError SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction& psbt, int index, const PrecomputedTransactionData* txdata, const common::PSBTFillOptions& options, SignatureData* out_sigdata)
{
PSBTInput& input = psbt.inputs.at(index);
- const CMutableTransaction& tx = *psbt.tx;
+ std::optional<CMutableTransaction> unsigned_tx = psbt.GetUnsignedTx();
+ if (!unsigned_tx) {
+ return PSBTError::INVALID_TX;
+ }
+ const CMutableTransaction& tx = *unsigned_tx;
if (PSBTInputSignedAndVerified(psbt, index, txdata)) {
return PSBTError::OK;
@@ -623,7 +672,11 @@ bool FinalizeAndExtractPSBT(PartiallySignedTransaction& psbtx, CMutableTransacti
return false;
}
- result = *psbtx.tx;
+ std::optional<CMutableTransaction> unsigned_tx = psbtx.GetUnsignedTx();
+ if (!unsigned_tx) {
+ return false;
+ }
+ result = *unsigned_tx;
for (unsigned int i = 0; i < result.vin.size(); ++i) {
result.vin[i].scriptSig = psbtx.inputs[i].final_script_sig;
result.vin[i].scriptWitness = psbtx.inputs[i].final_script_witness;
diff --git a/src/psbt.h b/src/psbt.h
index 6fe7a8b2..3c58f3c1 100644
--- a/src/psbt.h
+++ b/src/psbt.h
@@ -1073,7 +1073,6 @@ private:
std::optional<uint32_t> m_version;
public:
- std::optional<CMutableTransaction> tx;
// We use a vector of CExtPubKey in the event that there happens to be the same KeyOriginInfos for different CExtPubKeys
// Note that this map swaps the key and values from the serialization
std::map<KeyOriginInfo, std::set<CExtPubKey>> m_xpubs;
@@ -1093,6 +1092,8 @@ public:
[[nodiscard]] bool Merge(const PartiallySignedTransaction& psbt);
bool AddInput(const PSBTInput& psbtin);
bool AddOutput(const PSBTOutput& psbtout);
+ std::optional<CMutableTransaction> GetUnsignedTx() const;
+ std::optional<Txid> GetUniqueID() const;
explicit PartiallySignedTransaction(const CMutableTransaction& tx);
template <typename Stream>
@@ -1105,7 +1106,7 @@ public:
SerializeToVector(s, CompactSizeWriter(PSBT_GLOBAL_UNSIGNED_TX));
// Write serialized tx to a stream
- SerializeToVector(s, TX_NO_WITNESS(*tx));
+ SerializeToVector(s, TX_NO_WITNESS(*GetUnsignedTx()));
// Write xpubs
for (const auto& xpub_pair : m_xpubs) {
@@ -1168,6 +1169,7 @@ public:
// Read global data
bool found_sep = false;
+ std::optional<CMutableTransaction> tx;
while(!s.empty()) {
// Read the key of format "<keylen><keytype><keydata>" after which
// "key" will contain "<keytype><keydata>"
@@ -1197,10 +1199,9 @@ public:
case PSBT_GLOBAL_UNSIGNED_TX:
{
ExpectedKeySize("Global Unsigned TX", key, 1);
- CMutableTransaction mtx;
// Set the stream to serialize with non-witness since this should always be non-witness
- UnserializeFromVector(s, TX_NO_WITNESS(mtx));
- tx = std::move(mtx);
+ tx.emplace();
+ UnserializeFromVector(s, TX_NO_WITNESS(*tx));
// Make sure that all scriptSigs and scriptWitnesses are empty
for (const CTxIn& txin : tx->vin) {
if (!txin.scriptSig.empty() || !txin.scriptWitness.IsNull()) {
diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp
index f6d323fc..202a8660 100644
--- a/src/rpc/rawtransaction.cpp
+++ b/src/rpc/rawtransaction.cpp
@@ -1074,7 +1074,7 @@ static RPCMethod decodepsbt()
// Add the decoded tx
UniValue tx_univ(UniValue::VOBJ);
- TxToUniv(CTransaction(*psbtx.tx), /*block_hash=*/uint256(), /*entry=*/tx_univ, /*include_hex=*/false);
+ TxToUniv(CTransaction(*CHECK_NONFATAL(psbtx.GetUnsignedTx())), /*block_hash=*/uint256(), /*entry=*/tx_univ, /*include_hex=*/false);
result.pushKV("tx", std::move(tx_univ));
// Add the global xpubs
@@ -1803,12 +1803,13 @@ static RPCMethod joinpsbts()
const PartiallySignedTransaction& psbtx = *psbt_res;
psbtxs.push_back(psbtx);
// Choose the highest version number
- if (psbtx.tx->version > best_version) {
- best_version = psbtx.tx->version;
+ if (psbtx.tx_version > best_version) {
+ best_version = psbtx.tx_version;
}
// Choose the lowest lock time
- if (psbtx.tx->nLockTime < best_locktime) {
- best_locktime = psbtx.tx->nLockTime;
+ uint32_t psbt_locktime = psbtx.fallback_locktime.value_or(0);
+ if (psbt_locktime < best_locktime) {
+ best_locktime = psbt_locktime;
}
}
diff --git a/src/test/fuzz/psbt.cpp b/src/test/fuzz/psbt.cpp
index 7481d02e..7d0f775b 100644
--- a/src/test/fuzz/psbt.cpp
+++ b/src/test/fuzz/psbt.cpp
@@ -52,11 +52,7 @@ FUZZ_TARGET(psbt)
(void)psbt.IsNull();
- std::optional<CMutableTransaction> tx = psbt.tx;
- if (tx) {
- const CMutableTransaction& mtx = *tx;
- const PartiallySignedTransaction psbt_from_tx{mtx};
- }
+ (void)psbt.GetUnsignedTx();
for (const PSBTInput& input : psbt.inputs) {
(void)PSBTInputSigned(input);
diff --git a/src/wallet/test/psbt_wallet_tests.cpp b/src/wallet/test/psbt_wallet_tests.cpp
index 68b5f36e..8d8dc2ce 100644
--- a/src/wallet/test/psbt_wallet_tests.cpp
+++ b/src/wallet/test/psbt_wallet_tests.cpp
@@ -72,7 +72,6 @@ BOOST_AUTO_TEST_CASE(psbt_updater_test)
BOOST_CHECK_EQUAL(final_hex, "70736274ff01009a020000000258e87a21b56daf0c23be8e7070456c336f7cbaa5c8757924f545887bb2abdd750000000000ffffffff838d0427d0ec650a68aa46bb0b098aea4422c071b2ca78352a077959d07cea1d0100000000ffffffff0270aaf00800000000160014d85c2b71d0060b09c9886aeb815e50991dda124d00e1f5050000000016001400aea9a2e5f0f876a588df5546e8742d1d87008f00000000000100bb0200000001aad73931018bd25f84ae400b68848be09db706eac2ac18298babee71ab656f8b0000000048473044022058f6fc7c6a33e1b31548d481c826c015bd30135aad42cd67790dab66d2ad243b02204a1ced2604c6735b6393e5b41691dd78b00f0c5942fb9f751856faa938157dba01feffffff0280f0fa020000000017a9140fb9463421696b82c833af241c78c17ddbde493487d0f20a270100000017a91429ca74f8a08f81999428185c97b5d852e4063f6187650000000104475221029583bf39ae0a609747ad199addd634fa6108559d6c5cd39b4c2183f1ab96e07f2102dab61ff49a14db6a7d02b0cd1fbb78fc4b18312b5b4e54dae4dba2fbfef536d752ae2206029583bf39ae0a609747ad199addd634fa6108559d6c5cd39b4c2183f1ab96e07f10d90c6a4f000000800000008000000080220602dab61ff49a14db6a7d02b0cd1fbb78fc4b18312b5b4e54dae4dba2fbfef536d710d90c6a4f0000008000000080010000800001008a020000000158e87a21b56daf0c23be8e7070456c336f7cbaa5c8757924f545887bb2abdd7501000000171600145f275f436b09a8cc9a2eb2a2f528485c68a56323feffffff02d8231f1b0100000017a914aed962d6654f9a2b36608eb9d64d2b260db4f1118700c2eb0b0000000017a914b7f5faf40e3d40a5a459b1db3535f2b72fa921e8876500000001012000c2eb0b0000000017a914b7f5faf40e3d40a5a459b1db3535f2b72fa921e88701042200208c2353173743b595dfb4a07b72ba8e42e3797da74e87fe7d9d7497e3b2028903010547522103089dc10c7ac6db54f91329af617333db388cead0c231f723379d1b99030b02dc21023add904f3d6dcf59ddb906b0dee23529b7ffb9ed50e5e86151926860221f0e7352ae2206023add904f3d6dcf59ddb906b0dee23529b7ffb9ed50e5e86151926860221f0e7310d90c6a4f000000800000008003000080220603089dc10c7ac6db54f91329af617333db388cead0c231f723379d1b99030b02dc10d90c6a4f00000080000000800200008000220203a9a4c37f5996d3aa25dbac6b570af0650394492942460b354753ed9eeca5877110d90c6a4f000000800000008004000080002202027f6399757d2eff55a136ad02c684b1838b6556e5f1b6b34282a94b6b5005109610d90c6a4f00000080000000800500008000");
// 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
Risk score
Our methodology →Why this scored 33/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.