PSBTv2 per input required lock time calculation
What changed, and why it matters
This commit adds support in the COLDCARD firmware for a newer Bitcoin transaction format feature (PSBTv2) that lets each input specify its own required lock time. Before this change, the device did not correctly compute the final transaction lock time from these per-input requirements, which could cause it to sign transactions with the wrong lock time or reject valid ones. The patch implements the BIP-370 rule: if any input requires a lock time, the transaction's final lock time must be the most restrictive (maximum) of those requirements, overriding any fallback. This is a correctness/feature fix rather than a remote exploit, but incorrect lock-time handling could in theory let a malicious co-signer or wallet software trick a user into signing a transaction that becomes spendable earlier or later than expected.
Review the locktime computation logic for off-by-one and boundary conditions, ensure user-facing warnings accurately reflect the computed lock time, and confirm that the assertion-based rejection surfaces a clear error message rather than a crash. Continue monitoring for related PSBTv2 fields that may need similar handling.
Security signals we found
PSBTv2 per-input locktime parsing and validation added
BIP-370 required locktime computation implemented
Incompatible height/time locktime requirements rejected with assertion
Required locktimes override global fallback locktime
New test coverage for locktime edge cases
Evidence from the diff
The change renames internal fields from req_time_locktime/req_height_locktime to req_time/req_height and adds validation logic in shared/psbt.py’s validate() to compute self._lock_time from PSBTv2 per-input required locktime fields per BIP-370. It asserts that height locktimes are in (0, 500000000) and time locktimes are >= 500000000, rejects incompatible mixes (height-only vs time-only across inputs), and chooses max height if height is possible, otherwise max time. Tests verify that required locktimes override fallback, max is chosen across inputs, height is preferred when both are present, incompatible combinations are rejected, and fallback is used when no input requires a locktime. The change is defensive: it prevents the device from ignoring or misinterpreting PSBTv2 locktime requirements.
Changed components
shared/psbt.pytesting/psbt.pytesting/test_sign.pytesting/test_multisig.pytesting/txn.pyInspect captured patch +191 / −33
### releases/Next-ChangeLog.md
@@ -10,7 +10,7 @@ your addition and anything else already in this file.**
- Bugfix: Fix device crash when message-signing input is valid JSON but not an
object (NFC / QR / SD `.json` file). Thanks to [@Amiga500](https://github.com/Amiga500).
-- tbd
+- Enhancement: Support per-input required height and time locktimes in PSBTv2 transactions.
# Mk Specific Changes
@@ -26,4 +26,3 @@ your addition and anything else already in this file.**
- tbd
-
### shared/psbt.py
@@ -599,7 +599,7 @@ class psbtInputProxy(psbtProxy):
'unknown', 'utxo', 'witness_utxo', 'sighash', 'redeem_script', 'witness_script',
'fully_signed', 'is_segwit', 'is_multisig', 'is_p2sh', 'num_our_keys',
'required_key', 'scriptSig', 'amount', 'scriptCode', 'previous_txid',
- 'prevout_idx', 'sequence', 'req_time_locktime', 'req_height_locktime', 'addr_fmt',
+ 'prevout_idx', 'sequence', 'req_time', 'req_height', 'addr_fmt',
'wif_redeem_script',
)
@@ -636,8 +636,8 @@ def __init__(self, fd, idx):
#self.previous_txid = None
#self.prevout_idx = None
#self.sequence = None
- #self.req_time_locktime = None
- #self.req_height_locktime = None
+ #self.req_time = None
+ #self.req_height = None
#self.addr_fmt = None address format as decided by determine_my signing key
@@ -1007,9 +1007,9 @@ def store(self, kt, key, val):
elif kt == PSBT_IN_SEQUENCE:
self.sequence = unpack("<I", self.get(val))[0]
elif kt == PSBT_IN_REQUIRED_TIME_LOCKTIME:
- self.req_time_locktime = unpack("<I", self.get(val))[0]
+ self.req_time = unpack("<I", self.get(val))[0]
elif kt == PSBT_IN_REQUIRED_HEIGHT_LOCKTIME:
- self.req_height_locktime = unpack("<I", self.get(val))[0]
+ self.req_height = unpack("<I", self.get(val))[0]
else:
# including: PSBT_IN_FINAL_SCRIPTSIG, PSBT_IN_FINAL_SCRIPTWITNESS
self.unknown = self.unknown or {}
@@ -1056,11 +1056,11 @@ def serialize(self, out_fd, is_v2):
if self.sequence is not None:
wr(PSBT_IN_SEQUENCE, pack("<I", self.sequence))
- if self.req_time_locktime is not None:
- wr(PSBT_IN_REQUIRED_TIME_LOCKTIME, pack("<I", self.req_time_locktime))
+ if self.req_time is not None:
+ wr(PSBT_IN_REQUIRED_TIME_LOCKTIME, pack("<I", self.req_time))
- if self.req_height_locktime is not None:
- wr(PSBT_IN_REQUIRED_HEIGHT_LOCKTIME, pack("<I", self.req_height_locktime))
+ if self.req_height is not None:
+ wr(PSBT_IN_REQUIRED_HEIGHT_LOCKTIME, pack("<I", self.req_height))
if self.unknown:
for k, v in self.unknown.items():
@@ -1571,24 +1571,42 @@ async def validate(self):
# block height based relative locks
bb_rel_locks = []
smallest_nsequence = 0xffffffff
+ # BIP-370 tx-level locktime, derived from per-input required locktimes
+ lt_required = False
+ height_possible = True
+ time_possible = True
+ max_height = 0
+ max_time = 0
# this parses the input TXN in-place
for idx, txin in self.input_iter():
inp = self.inputs[idx]
if self.is_v2:
# v2 requires inclusion
assert inp.prevout_idx is not None
assert inp.previous_txid
- if inp.req_time_locktime is not None:
- assert inp.req_time_locktime >= NLOCK_IS_TIME
- if inp.req_height_locktime is not None:
- assert 0 < inp.req_height_locktime < NLOCK_IS_TIME
+
+ if inp.req_time is not None:
+ assert inp.req_time >= NLOCK_IS_TIME
+ if inp.req_height is not None:
+ assert 0 < inp.req_height < NLOCK_IS_TIME
+
+ if inp.req_time is not None or inp.req_height is not None:
+ lt_required = True
+ if inp.req_height is None:
+ height_possible = False
+ elif inp.req_height > max_height:
+ max_height = inp.req_height
+ if inp.req_time is None:
+ time_possible = False
+ elif inp.req_time > max_time:
+ max_time = inp.req_time
else:
# v0 requires exclusion
assert inp.prevout_idx is None
assert inp.previous_txid is None
assert inp.sequence is None
- assert inp.req_time_locktime is None
- assert inp.req_height_locktime is None
+ assert inp.req_time is None
+ assert inp.req_height is None
self.inputs[idx].validate(idx, txin, self.my_xfp, self)
if self.txn_version >= 2:
@@ -1602,6 +1620,13 @@ async def validate(self):
if txin.nSequence < smallest_nsequence:
smallest_nsequence = txin.nSequence
+ if lt_required:
+ assert height_possible or time_possible, "incompatible locktime requirements"
+ # v2 only: _lock_time normally comes from the unsigned tx (v0);
+ # here it is computed from per-input required locktimes and so
+ # always wins over the global fallback locktime (BIP-370)
+ self._lock_time = max_height if height_possible else max_time
+
if isinstance(self.lock_time, int) and self.lock_time > 0:
if smallest_nsequence == 0xffffffff:
self.warnings.append((
### testing/psbt.py
@@ -129,8 +129,8 @@ def defaults(self):
self.previous_txid = None # v2
self.prevout_idx = None # v2
self.sequence = None # v2
- self.req_time_locktime = None # v2
- self.req_height_locktime = None # v2
+ self.req_time = None # v2
+ self.req_height = None # v2
self.others = {}
self.unknown = {}
@@ -153,8 +153,8 @@ def __eq__(a, b):
a.previous_txid == b.previous_txid and \
a.prevout_idx == b.prevout_idx and \
a.sequence == b.sequence and \
- a.req_time_locktime == b.req_time_locktime and \
- a.req_height_locktime == b.req_height_locktime and \
+ a.req_time == b.req_time and \
+ a.req_height == b.req_height and \
a.unknown == b.unknown
if rv:
# NOTE: equality test on signatures requires parsing DER stupidness
@@ -202,9 +202,9 @@ def parse_kv(self, kt, key, val):
elif kt == PSBT_IN_SEQUENCE:
self.sequence = struct.unpack("<I", val)[0]
elif kt == PSBT_IN_REQUIRED_TIME_LOCKTIME:
- self.req_time_locktime = struct.unpack("<I", val)[0]
+ self.req_time = struct.unpack("<I", val)[0]
elif kt == PSBT_IN_REQUIRED_HEIGHT_LOCKTIME:
- self.req_height_locktime = struct.unpack("<I", val)[0]
+ self.req_height = struct.unpack("<I", val)[0]
else:
self.unknown[bytes([kt]) + key] = val
@@ -245,10 +245,10 @@ def serialize_kvs(self, wr, v2):
wr(PSBT_IN_OUTPUT_INDEX, struct.pack("<I", self.prevout_idx))
if self.sequence is not None:
wr(PSBT_IN_SEQUENCE, struct.pack("<I", self.sequence))
- if self.req_time_locktime is not None:
- wr(PSBT_IN_REQUIRED_TIME_LOCKTIME, struct.pack("<I", self.req_time_locktime))
- if self.req_height_locktime is not None:
- wr(PSBT_IN_REQUIRED_HEIGHT_LOCKTIME, struct.pack("<I", self.req_height_locktime))
+ if self.req_time is not None:
+ wr(PSBT_IN_REQUIRED_TIME_LOCKTIME, struct.pack("<I", self.req_time))
+ if self.req_height is not None:
+ wr(PSBT_IN_REQUIRED_HEIGHT_LOCKTIME, struct.pack("<I", self.req_height))
for k in self.others:
wr(k, self.others[k])
@@ -595,8 +595,8 @@ def to_v0(self):
inp.prevout_idx = None
inp.previous_txid = None
inp.sequence = None
- inp.req_time_locktime = None
- inp.req_height_locktime = None
+ inp.req_time = None
+ inp.req_height = None
tx_outs = []
for out in self.outputs:
### testing/test_multisig.py
@@ -1430,8 +1430,8 @@ def doit(num_ins, num_outs, M, keys, fee=10000, outvals=None,
psbt.inputs[i].previous_txid = supply.hash
psbt.inputs[i].prevout_idx = 0
psbt.inputs[i].sequence = seq
- # psbt.inputs[i].req_time_locktime = None
- # psbt.inputs[i].req_height_locktime = None
+ # psbt.inputs[i].req_time = None
+ # psbt.inputs[i].req_height = None
spendable = CTxIn(COutPoint(supply.sha256, 0), nSequence=seq)
txn.vin.append(spendable)
### testing/test_sign.py
@@ -2946,6 +2946,140 @@ def test_far_future_locktime_warning(
end_sign(accept=False)
+@pytest.mark.parametrize("is_multi", [False, True])
+@pytest.mark.parametrize("lock_field, lock_value", [
+ ("req_height", 800000),
+ ("req_time", 1513209600),
+])
+def test_psbt_v2_required_locktime_fields_sign(is_multi, lock_field, lock_value,
+ fake_txn, fake_ms_txn, import_ms_wallet,
+ clear_ms, start_sign, end_sign, cap_story):
+ def add_required_locktime(psbt):
+ # BIP-370 required locktime fields are per-input requirements.
+ # The input sequence must be non-final for tx-level nLockTime to matter.
+ psbt.inputs[0].sequence = 0xfffffffd
+ setattr(psbt.inputs[0], lock_field, lock_value)
+
+ if not is_multi:
+ psbt = fake_txn(1, 1, segwit_in=True, psbt_v2=True,
+ psbt_hacker=add_required_locktime)
+ else:
+ clear_ms()
+ M = 2
+ N = 3
+ keys = import_ms_wallet(M, N, accept=True)
+ psbt = fake_ms_txn(1, 1, M, keys, psbt_v2=True,
+ hack_psbt=add_required_locktime)
+
+ start_sign(psbt, finalize=not is_multi)
+ title, story = cap_story()
+ assert title == "OK TO SEND?"
+ assert "TX LOCKTIMES" in story
+ assert "Abs Locktime" in story
+
+ signed = end_sign(accept=True, finalize=not is_multi)
+ tx = CTransaction()
+ if not is_multi:
+ tx.deserialize(BytesIO(signed))
+ assert tx.nLockTime == lock_value
+ assert tx.vin[0].nSequence == 0xfffffffd
+
+
+@pytest.mark.parametrize("lock_field, values", [
+ ("req_height", [700000, 800000, 750000]),
+ ("req_time", [1600000000, 1700000000, 1650000000]),
+])
+def test_psbt_v2_required_locktime_max(lock_field, values, fake_txn, start_sign,
+ end_sign, cap_story):
+ # Several inputs require the same type of locktime -> tx nLockTime is the
+ # maximum of them. An extra input with no requirement must not constrain it.
+ def hack(psbt):
+ for i, v in enumerate(values):
+ psbt.inputs[i].sequence = 0xfffffffd
+ setattr(psbt.inputs[i], lock_field, v)
+ # last input deliberately left without any required locktime
+
+ psbt = fake_txn(len(values) + 1, 1, segwit_in=True, psbt_v2=True, psbt_hacker=hack)
+ start_sign(psbt, finalize=True)
+ signed = end_sign(accept=True, finalize=True)
+ tx = CTransaction()
+ tx.deserialize(BytesIO(signed))
+ assert tx.nLockTime == max(values)
+
+
+def test_psbt_v2_required_locktime_both_prefers_height(fake_txn, start_sign,
+ end_sign, cap_story):
+ # Inputs specifying BOTH height and time allow either type, so BIP-370
+ # mandates the height-based locktime; the max height across inputs is used.
+ heights = [810000, 830000]
+ times = [1700000000, 1690000000]
+ def hack(psbt):
+ for i in range(len(heights)):
+ psbt.inputs[i].sequence = 0xfffffffd
+ psbt.inputs[i].req_height = heights[i]
+ psbt.inputs[i].req_time = times[i]
+
+ psbt = fake_txn(len(heights), 1, segwit_in=True, psbt_v2=True, psbt_hacker=hack)
+ start_sign(psbt, finalize=True)
+ signed = end_sign(accept=True, finalize=True)
+ tx = CTransaction()
+ tx.deserialize(BytesIO(signed))
+ assert tx.nLockTime == max(heights)
+
+
+def test_psbt_v2_required_locktime_incompatible(fake_txn, start_sign, cap_story,
+ press_cancel):
+ # One input requires height-only, another requires time-only -> no locktime
+ # type is acceptable to all inputs, so the PSBT must be rejected.
+ def hack(psbt):
+ psbt.inputs[0].sequence = 0xfffffffd
+ psbt.inputs[0].req_height = 800000
+ psbt.inputs[1].sequence = 0xfffffffd
+ psbt.inputs[1].req_time = 1700000000
+
+ psbt = fake_txn(2, 1, segwit_in=True, psbt_v2=True, psbt_hacker=hack)
+ start_sign(psbt, finalize=True)
+ time.sleep(.1)
+ title, story = cap_story()
+ assert title == "Failure"
+ assert "incompatible locktime requirements" in story
+ press_cancel()
+
+
+def test_psbt_v2_fallback_locktime(fake_txn, start_sign, end_sign, cap_story):
+ # No input specifies a required locktime -> the global fallback locktime is used.
+ fallback = 750000
+ def hack(psbt):
+ psbt.inputs[0].sequence = 0xfffffffd
+ psbt.fallback_locktime = fallback
+
+ psbt = fake_txn(1, 1, segwit_in=True, psbt_v2=True, psbt_hacker=hack)
+ start_sign(psbt, finalize=True)
+ signed = end_sign(accept=True, finalize=True)
+ tx = CTransaction()
+ tx.deserialize(BytesIO(signed))
+ assert tx.nLockTime == fallback
+
+
+def test_psbt_v2_required_locktime_overrides_fallback(fake_txn, start_sign,
+ end_sign, cap_story):
+ # BIP-370: fallback locktime is only used when NO input specifies a
+ # required locktime; a required locktime on any input takes precedence.
+ fallback = 750000
+ required = 800000
+ def hack(psbt):
+ psbt.inputs[0].sequence = 0xfffffffd
+ psbt.inputs[0].req_height = required
+ psbt.fallback_locktime = fallback
+
+ psbt = fake_txn(1, 1, segwit_in=True, psbt_v2=True, psbt_hacker=hack)
+ start_sign(psbt, finalize=True)
+ signed = end_sign(accept=True, finalize=True)
+ tx = CTransaction()
+ tx.deserialize(BytesIO(signed))
+ assert tx.nLockTime == required
+
+
@pytest.mark.bitcoind
@pytest.mark.parametrize("num_ins", [1, 4, 11])
@pytest.mark.parametrize("differ", [True, False])
### testing/txn.py
@@ -147,8 +147,8 @@ def doit(num_ins, num_outs, master_xpub=None, subpath="0/%d", fee=10000,
psbt.inputs[i].previous_txid = supply.hash
psbt.inputs[i].prevout_idx = 0
psbt.inputs[i].sequence = seq
- # psbt.inputs[i].req_time_locktime = None
- # psbt.inputs[i].req_height_locktime = None
+ # psbt.inputs[i].req_time = None
+ # psbt.inputs[i].req_height = None
else:
assert i != 0, 'cant dup first input'
txn.vin.append(txn.vin[-1])Why this scored 34/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.