psbt: set locktime on the transaction, not the PSBT
What changed, and why it matters
This commit fixes a bug where a transaction's locktime (a time-based or block-height-based restriction on when a Bitcoin transaction can be mined) was being set on the wrong object. Instead of assigning it to the actual transaction, the code accidentally assigned it to the PSBT wrapper object, so the transaction kept a locktime of 0. This could cause transactions with intended time locks to become immediately spendable, potentially breaking security assumptions in wallets or contracts that rely on locktime.
Review any transactions created through HWI's PSBT conversion path that were intended to use nLockTime, as they may have been produced with locktime 0 and could be mined earlier than intended. Update to the patched version and verify locktime values in signed transactions.
Security signals we found
Incorrect assignment of locktime to PSBT object instead of transaction
Locktime bypass: intended time/block-height restrictions silently ignored
PSBTv2 to PSBTv0 conversion path affected
Test vector updated to cover locktime fields
Evidence from the diff
In hwilib/psbt.py, get_unsigned_tx() constructs a CTransaction from a PSBT. The bug was self.nLockTime = self.compute_lock_time() where self is the PSBT object, not the CTransaction. The fix changes it to tx.nLockTime = self.compute_lock_time(). The test is updated to use a BIP 370 vector that includes locktime fields and asserts that after convert_to_v0(), psbt.tx.nLockTime equals 10000 (the winning height locktime).
Changed components
hwilib/psbt.pyPSBT.get_unsigned_tx()PSBT.convert_to_v0()test/test_psbt.pyInspect captured patch +5 / −3
### hwilib/psbt.py
@@ -1117,7 +1117,7 @@ def get_unsigned_tx(self) -> CTransaction:
tx = CTransaction()
tx.nVersion = self.tx_version
- self.nLockTime = self.compute_lock_time()
+ tx.nLockTime = self.compute_lock_time()
for psbt_in in self.inputs:
assert psbt_in.prev_txid is not None
### test/test_psbt.py
@@ -29,16 +29,18 @@ def test_valid_psbt(self):
self.assertEqual(valid, serd)
def test_convert_to_v0(self):
- # BIP 370 vector "1 input, 2 output updated PSBTv2, with PSBT_IN_SEQUENCE"
+ # BIP 370 vector "1 input, 2 output updated PSBTv2, with PSBT_IN_SEQUENCE,
+ # and all locktime fields"; the height locktime of 10000 wins
psbt = PSBT()
- psbt.deserialize("cHNidP8BAgQCAAAAAQQBAQEFAQIB+wQCAAAAAAEAUgIAAAABwaolbiFLlqGCL5PeQr/ztfP/jQUZMG41FddRWl6AWxIAAAAAAP////8BGMaaOwAAAAAWABSwo68UQghBJpPKfRZoUrUtsK7wbgAAAAABAR8Yxpo7AAAAABYAFLCjrxRCCEEmk8p9FmhStS2wrvBuAQ4gCwrZIUGcHIcZc11y3HOfnqngY40f5MHu8PmUQISBX8gBDwQAAAAAARAE/v///wAiAgLWAfhIRqZ1X3dr4A49nej7EKzJNfuDxF+wFi1MrVq3khj2nYc+VAAAgAEAAIAAAACAAAAAACoAAAABAwgACK8vAAAAAAEEFgAUxDD2TEdW2jENvRoIVXLvKZkmJywAIgIC42+/9T3VNAcM+P05ZhRoDzV6m4Xbc0C/HPp0XSrXs0AY9p2HPlQAAIABAACAAAAAgAEAAABkAAAAAQMIi73rCwAAAAABBBYAFE3Rk6yWSlasG54cyoRU/i9HT4UTAA==")
+ psbt.deserialize("cHNidP8BAgQCAAAAAQMEAAAAAAEEAQEBBQECAfsEAgAAAAABAFICAAAAAcGqJW4hS5ahgi+T3kK/87Xz/40FGTBuNRXXUVpegFsSAAAAAAD/////ARjGmjsAAAAAFgAUsKOvFEIIQSaTyn0WaFK1LbCu8G4AAAAAAQEfGMaaOwAAAAAWABSwo68UQghBJpPKfRZoUrUtsK7wbgEOIAsK2SFBnByHGXNdctxzn56p4GONH+TB7vD5lECEgV/IAQ8EAAAAAAEQBP7///8BEQSMjcRiARIEECcAAAAiAgLWAfhIRqZ1X3dr4A49nej7EKzJNfuDxF+wFi1MrVq3khj2nYc+VAAAgAEAAIAAAACAAAAAACoAAAABAwgACK8vAAAAAAEEFgAUxDD2TEdW2jENvRoIVXLvKZkmJywAIgIC42+/9T3VNAcM+P05ZhRoDzV6m4Xbc0C/HPp0XSrXs0AY9p2HPlQAAIABAACAAAAAgAEAAABkAAAAAQMIi73rCwAAAAABBBYAFE3Rk6yWSlasG54cyoRU/i9HT4UTAA==")
psbt.convert_to_v0()
self.assertEqual(psbt.version, 0)
self.assertEqual(psbt.tx.nVersion, 2)
self.assertEqual(len(psbt.tx.vin), 1)
self.assertEqual(psbt.tx.vin[0].nSequence, 0xfffffffe)
self.assertEqual(len(psbt.tx.vout), 2)
+ self.assertEqual(psbt.tx.nLockTime, 10000)
if __name__ == "__main__":
unittest.main()Why this scored 60/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.