refactor(core/bitcoin): Consolidate external input classification.
What the developer wrote
Authored by Andrew Kozlik
refactor(core/bitcoin): Consolidate external input classification.
(cherry picked from commit 2042aec0ba7316c268548b1297691e750ed2112c)
What changed, and why it matters
This commit is a code cleanup (refactor) in the Bitcoin signing code of the Trezor firmware. It introduces a helper that classifies external transaction inputs into three categories (presigned, has ownership proof, or unverified) and uses that helper in two places instead of duplicating the same checks. The behavior appears unchanged; there is no indication this fixes or introduces a security bug.
No security action required. Treat as normal code-quality refactor. If reviewing for release, verify via existing Bitcoin signing tests that external/presigned/ownership-proof/unverified inputs still behave identically.
Security signals we found
No security-relevant behavior change: same input classification conditions are preserved
Refactor only: logic moved into a new helper enum/classmethod
No new trust boundaries, no new parsing, no new memory handling
No vendor disclosure or advisory references present
Evidence from the diff
The change consolidates external-input classification logic into a new ExternalInputType.from_input() method in core/src/apps/bitcoin/common.py and replaces inline checks in input_is_external_unverified() and process_external_input(). The previous logic checked txi.witness/script_sig to identify presigned inputs and txi.ownership_proof for ownership proofs; the new code uses the same checks via the enum. The ordering of operations in process_external_input() is slightly restructured but functionally equivalent: presigned inputs are still added to self.presigned and written to h_presigned_inputs_check, ownership proofs are still verified, and unverified inputs still pass through to the approver. No security-relevant behavior change is visible in the diff.
Changed components
core/src/apps/bitcoin/common.pycore/src/apps/bitcoin/sign_tx/bitcoin.pyInspect captured patch +38 / −12
diff --git a/core/src/apps/bitcoin/common.py b/core/src/apps/bitcoin/common.py
index bd1aebc9..874b96aa 100644
--- a/core/src/apps/bitcoin/common.py
+++ b/core/src/apps/bitcoin/common.py
@@ -56,6 +56,24 @@ class SigHashType(IntEnum):
raise ValueError("Unsupported sighash type.")
+class ExternalInputType(IntEnum):
+ PRESIGNED = 0
+ HAS_OWNERSHIP_PROOF = 1
+ UNVERIFIED = 2
+
+ @classmethod
+ def from_input(cls, txi: TxInput) -> "ExternalInputType":
+ if txi.script_type != InputScriptType.EXTERNAL:
+ raise RuntimeError(
+ "ExternalInputType.from_input called on non-external input"
+ )
+ if txi.witness or txi.script_sig:
+ return cls.PRESIGNED
+ if txi.ownership_proof:
+ return cls.HAS_OWNERSHIP_PROOF
+ return cls.UNVERIFIED
+
+
# The number of bip32 levels used in a wallet (chain and address)
BIP32_WALLET_DEPTH = const(2)
@@ -183,12 +201,9 @@ def input_is_external(txi: TxInput) -> bool:
def input_is_external_unverified(txi: TxInput) -> bool:
- # Evaluate fields as bool, same as in `process_external_input()` for consistency in case of empty bytes.
return (
- txi.script_type == InputScriptType.EXTERNAL
- and not txi.ownership_proof
- and not txi.witness
- and not txi.script_sig
+ input_is_external(txi)
+ and ExternalInputType.from_input(txi) == ExternalInputType.UNVERIFIED
)
diff --git a/core/src/apps/bitcoin/sign_tx/bitcoin.py b/core/src/apps/bitcoin/sign_tx/bitcoin.py
index 8ee52c13..d8fea0a7 100644
--- a/core/src/apps/bitcoin/sign_tx/bitcoin.py
+++ b/core/src/apps/bitcoin/sign_tx/bitcoin.py
@@ -10,7 +10,7 @@ from trezor.wire import DataError, ProcessError
from apps.common.writers import write_compact_size
from .. import addresses, common, multisig, scripts, writers
-from ..common import SigHashType, ecdsa_sign, input_is_external
+from ..common import ExternalInputType, SigHashType, ecdsa_sign, input_is_external
from ..ownership import verify_nonownership
from ..verification import SignatureVerifier
from . import helpers
@@ -193,10 +193,7 @@ class Bitcoin:
if input_is_external(txi):
node = None
self.external.add(i)
- if txi.witness or txi.script_sig:
- self.presigned.add(i)
- writers.write_tx_input_check(h_presigned_inputs_check, txi)
- await self.process_external_input(txi)
+ await self.process_external_input(txi, i, h_presigned_inputs_check)
else:
node = self.keychain.derive(txi.address_n)
await self.process_internal_input(txi, node)
@@ -352,12 +349,22 @@ class Bitcoin:
await self.approver.add_internal_input(txi, node)
- async def process_external_input(self, txi: TxInput) -> None:
+ async def process_external_input(
+ self,
+ txi: TxInput,
+ i: int,
+ h_presigned_inputs_check: HashWriter,
+ ) -> None:
assert txi.script_pubkey is not None # checked in sanitize_tx_input
self.approver.add_external_input(txi)
- if txi.ownership_proof:
+ input_type = ExternalInputType.from_input(txi)
+ if input_type == ExternalInputType.PRESIGNED:
+ self.presigned.add(i)
+ writers.write_tx_input_check(h_presigned_inputs_check, txi)
+ elif input_type == ExternalInputType.HAS_OWNERSHIP_PROOF:
+ assert txi.ownership_proof is not None # implied by input_type
if not verify_nonownership(
txi.ownership_proof,
txi.script_pubkey,
@@ -366,6 +373,10 @@ class Bitcoin:
self.coin,
):
raise DataError("Invalid external input")
+ elif input_type == ExternalInputType.UNVERIFIED:
+ pass # admission policy enforced by self.approver
+ else:
+ raise RuntimeError("Unknown external input type")
async def process_original_input(
self, txi: TxInput, script_pubkey: AnyBytes
Why this scored 12/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.