Merge bitcoin-core/HWI#853: psbt: enable type checking
What the developer wrote
Authored by Ava Chow
Merge bitcoin-core/HWI#853: psbt: enable type checking
37643bd8e4f86324b75723525149eb945275395c psbt: enable type checking (Sjors Provoost)
Pull request description:
ACKs for top commit:
achow101:
ACK 37643bd8e4f86324b75723525149eb945275395c
Tree-SHA512: aa1722f2e3f65c387a3bb5d6631cb666c6aa83f8ccc3b46c6a8f4585eb6ba02462dd78ec8027fc278fca1b2b0fc7d1bb9f6729781c2cb9b82321017919a6aeba
What changed, and why it matters
This is a code-quality and type-safety patch. It adds the psbt.py file to the project's automated type-checking workflow and fixes two small logic issues where a value was assigned to an object field before being validated. The changes make the code safer and cleaner, but they do not appear to fix an active security vulnerability that could be exploited.
No urgent action required. Treat as routine maintenance. Reviewers may want to confirm that the type-checker passes and that no other PSBT deserialization paths have similar assignment-before-validation patterns.
Security signals we found
Validation moved before state mutation (defensive coding)
Type annotations added to public methods
File added to CI type-check coverage
Evidence from the diff
The commit enables mypy type checking on hwilib/psbt.py and makes three categories of changes: (1) adds type annotations to setup_from_tx and _convert_version, (2) reorders validation for PSBT_IN_REQUIRED_TIME_LOCKTIME and PSBT_IN_REQUIRED_HEIGHT_LOCKTIME so the parsed integer is checked before being stored, and (3) adds the file to the CI type-check list. The locktime reordering is a minor correctness improvement: previously an out-of-range value could be written to self.time_locktime/self.height_locktime before the exception was raised. Because deserialization aborts on the exception, the stored value is never used, so there is no practical security impact.
Changed components
hwilib/psbt.py.github/workflows/type-check.ymlInspect captured patch +9 / −6
### .github/workflows/type-check.yml
@@ -40,4 +40,5 @@ jobs:
hwilib/hwwclient.py
hwilib/__init__.py
hwilib/key.py
+ hwilib/psbt.py
hwilib/udevinstaller.py
### hwilib/psbt.py
@@ -289,9 +289,10 @@ def deserialize(self, f: Readable) -> None:
v = deser_string(f)
if len(v) != 4:
raise PSBTSerializationError("Input time based locktime is not 4 bytes")
- self.time_locktime = struct.unpack("<I", v)[0]
- if self.time_locktime < 500000000:
+ time_locktime = struct.unpack("<I", v)[0]
+ if time_locktime < 500000000:
raise PSBTSerializationError("Input time based locktime is less than 500000000")
+ self.time_locktime = time_locktime
elif key_type == PartiallySignedInput.PSBT_IN_REQUIRED_HEIGHT_LOCKTIME:
if self.version == 0:
raise PSBTSerializationError("PSBT_IN_REQUIRED_HEIGHT_LOCKTIME is not allowed in PSBTv0")
@@ -302,9 +303,10 @@ def deserialize(self, f: Readable) -> None:
v = deser_string(f)
if len(v) != 4:
raise PSBTSerializationError("Input height based locktime is not 4 bytes")
- self.height_locktime = struct.unpack("<I", v)[0]
- if self.height_locktime == 0 or self.height_locktime >= 500000000:
+ height_locktime = struct.unpack("<I", v)[0]
+ if height_locktime == 0 or height_locktime >= 500000000:
raise PSBTSerializationError("Input height based locktime is not greater than 0 and less than 500000000")
+ self.height_locktime = height_locktime
elif key_type == PartiallySignedInput.PSBT_IN_TAP_KEY_SIG:
if key in key_lookup:
raise PSBTSerializationError("Duplicate key, input Taproot key signature already provided")
@@ -1072,7 +1074,7 @@ def cache_unsigned_tx_pieces(self) -> None:
if self.version == 0:
self.setup_from_tx(self.tx)
- def setup_from_tx(self, tx: CTransaction):
+ def setup_from_tx(self, tx: CTransaction) -> None:
"""
Fills in the PSBTv2 fields for this PSBT given a transaction
@@ -1159,7 +1161,7 @@ def get_unsigned_tx(self) -> CTransaction:
tx.rehash()
return tx
- def _convert_version(self, version) -> None:
+ def _convert_version(self, version: int) -> None:
self.version = version
for psbt_in in self.inputs:
psbt_in.version = versionWhy this scored 18/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.