Implement PSBTv2 AddInput and AddOutput
What changed, and why it matters
This commit finishes implementing the 'AddInput' and 'AddOutput' methods for PSBT version 2 in Bitcoin Core. PSBTs are files used to collaboratively build or sign Bitcoin transactions offline. Previously, adding inputs/outputs to a PSBTv2 would silently fail (return false). The change now properly checks version compatibility, respects the 'modifiable' flags that say whether inputs/outputs may be added, and handles time-lock consistency when new inputs are appended. It is a feature-completion patch with defensive checks, not a fix for an active vulnerability.
Review as normal feature completion. Verify that the locktime logic matches BIP 370 exactly, especially the interaction with SIGHASH_SINGLE (bit 2 of m_tx_modifiable), which the commit message notes is intentionally skipped because inputs are only appended. Consider adding unit/functional tests for the new failure paths (version mismatch, modifiable-flag denial, locktime conflict with existing signatures).
Security signals we found
Defensive version-mismatch guard added to AddInput/AddOutput
Respects PSBTv2 modifiable flags before mutating inputs/outputs
Locktime consistency check prevents adding inputs that would invalidate existing signatures
New HasSignatures helper used to guard locktime mutation
No memory-unsafe operations, no deserialization changes, no network exposure visible
Evidence from the diff
The diff implements PartiallySignedTransaction::AddInput and AddOutput for PSBTv2 (BIP 370). AddInput now: (1) rejects inputs whose PSBT version mismatches the global PSBT version; (2) prevents duplicate inputs; (3) falls back to v0 behavior for version 0; (4) requires the global m_tx_modifiable bit 0 (inputs modifiable); (5) recomputes the transaction locktime when the new input carries a time or height locktime, enforcing BIP 370 rules that mixed time/height locks invalidate height locks and that the maximum locktime wins; (6) refuses to add the input if existing signatures would be invalidated by a changed locktime. AddOutput now: (1) rejects mismatched versions; (2) uses v0 append for PSBTv0; (3) requires m_tx_modifiable bit 1 (outputs modifiable) for PSBTv2. A new helper PSBTInput::HasSignatures() is added to detect any signature presence. The patch is purely internal logic; no RPC/CLI/P2P behavior changes are visible in the diff.
Changed components
src/psbt.cppsrc/psbt.hPartiallySignedTransaction::AddInputPartiallySignedTransaction::AddOutputPSBTInput::HasSignaturesInspect captured patch +85 / −2
diff --git a/src/psbt.cpp b/src/psbt.cpp
index 9967a816..d03ee404 100644
--- a/src/psbt.cpp
+++ b/src/psbt.cpp
@@ -140,6 +140,11 @@ std::optional<Txid> PartiallySignedTransaction::GetUniqueID() const
bool PartiallySignedTransaction::AddInput(const PSBTInput& psbtin)
{
+ // The input being added must be for this PSBT's version
+ if (psbtin.GetVersion() != GetVersion()) {
+ return false;
+ }
+
// Prevent duplicate inputs
if (std::find_if(inputs.begin(), inputs.end(),
[psbtin](const PSBTInput& psbt) {
@@ -158,18 +163,85 @@ bool PartiallySignedTransaction::AddInput(const PSBTInput& psbtin)
return true;
}
- return false;
+ // Check inputs modifiable flag
+ if (!m_tx_modifiable.has_value() || !m_tx_modifiable->test(0)) {
+ return false;
+ }
+
+ // Determine if we need to iterate the inputs.
+ // For now, we only do this if the new input has a required time lock.
+ // BIP 370 states that we should also do this if m_tx_modifiable's bit 2 is set
+ // (Has SIGHASH_SINGLE flag) but since we are only adding inputs at the end of the vector,
+ // we don't care about that.
+ bool iterate_inputs = psbtin.time_locktime != std::nullopt || psbtin.height_locktime != std::nullopt;
+ if (iterate_inputs) {
+ std::optional<uint32_t> old_timelock = ComputeTimeLock();
+ if (!old_timelock) {
+ return false;
+ }
+
+ std::optional<uint32_t> time_lock = psbtin.time_locktime;
+ std::optional<uint32_t> height_lock = psbtin.height_locktime;
+ bool has_sigs = false;
+ for (const PSBTInput& input : inputs) {
+ if (input.time_locktime.has_value() && !input.height_locktime.has_value()) {
+ height_lock.reset(); // Transaction can no longer have a height locktime
+ if (time_lock == std::nullopt) {
+ return false;
+ }
+ } else if (!input.time_locktime.has_value() && input.height_locktime.has_value()) {
+ time_lock.reset(); // Transaction can no longer have a time locktime
+ if (height_lock == std::nullopt) {
+ return false;
+ }
+ }
+ if (input.time_locktime && time_lock.has_value()) {
+ time_lock = std::max(time_lock, input.time_locktime);
+ }
+ if (input.height_locktime && height_lock.has_value()) {
+ height_lock = std::max(height_lock, input.height_locktime);
+ }
+ if (input.HasSignatures()) {
+ has_sigs = true;
+ }
+ }
+ uint32_t new_timelock = fallback_locktime.value_or(0);
+ if (height_lock.has_value() && *height_lock > 0) {
+ new_timelock = *height_lock;
+ } else if (time_lock.has_value() && *time_lock > 0) {
+ new_timelock = *time_lock;
+ }
+ if (has_sigs && *old_timelock != new_timelock) {
+ return false;
+ }
+ }
+
+ // Add the input to the end
+ inputs.push_back(psbtin);
+ return true;
}
bool PartiallySignedTransaction::AddOutput(const PSBTOutput& psbtout)
{
+ // The output being added must be for this PSBT's version
+ if (psbtout.GetVersion() != GetVersion()) {
+ return false;
+ }
+
if (GetVersion() < 2) {
// This is a v0 psbt, do the v0 AddOutput
outputs.push_back(psbtout);
return true;
}
- return false;
+ // No global tx, must be PSBTv2
+ // Check outputs are modifiable
+ if (!m_tx_modifiable.has_value() || !m_tx_modifiable->test(1)) {
+ return false;
+ }
+ outputs.push_back(psbtout);
+
+ return true;
}
bool PSBTInput::GetUTXO(CTxOut& utxo) const
@@ -364,6 +436,16 @@ void PSBTInput::Merge(const PSBTInput& input)
}
}
+bool PSBTInput::HasSignatures() const
+{
+ return !final_script_sig.empty()
+ || !final_script_witness.IsNull()
+ || !partial_sigs.empty()
+ || !m_tap_key_sig.empty()
+ || !m_tap_script_sigs.empty()
+ || !m_musig2_partial_sigs.empty();
+}
+
void PSBTOutput::FillSignatureData(SignatureData& sigdata) const
{
if (!redeem_script.empty()) {
diff --git a/src/psbt.h b/src/psbt.h
index fce90a83..083cfa53 100644
--- a/src/psbt.h
+++ b/src/psbt.h
@@ -333,6 +333,7 @@ public:
* @return Whether the UTXO could be retrieved
*/
bool GetUTXO(CTxOut& utxo) const;
+ bool HasSignatures() const;
explicit PSBTInput(uint32_t psbt_version, const Txid& prev_txid, uint32_t prev_out, std::optional<uint32_t> sequence = std::nullopt)
: m_psbt_version(psbt_version),
Why this scored 20/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.