refactor(change): factor out ChangeDetector

core/src/all_modules.py

@@ -271,6 +271,8 @@ apps.bitcoin.sign_tx.bitcoin
 import apps.bitcoin.sign_tx.bitcoin
 apps.bitcoin.sign_tx.bitcoinlike
 import apps.bitcoin.sign_tx.bitcoinlike
+apps.bitcoin.sign_tx.change_detector
+import apps.bitcoin.sign_tx.change_detector
 apps.bitcoin.sign_tx.decred
 import apps.bitcoin.sign_tx.decred
 apps.bitcoin.sign_tx.helpers

core/src/apps/bitcoin/sign_tx/approvers.py

@@ -227,7 +227,9 @@ class BasicApprover(Approver):
                     "Adding new OP_RETURN outputs in replacement transactions is not supported."
                 )
         elif txo.payment_req_index is None or self.show_payment_req_details:
-            source_path = tx_info.wallet_path.get_path() if tx_info else None
+            source_path = (
+                tx_info.change_detector.wallet_path.get_path() if tx_info else None
+            )
             # Ask user to confirm output, unless it is part of a payment
             # request, which gets confirmed separately.
             await helpers.confirm_output(
@@ -294,7 +296,7 @@ class BasicApprover(Approver):
         if self.has_unverified_external_input:
             await helpers.confirm_unverified_external_input()
 
-        if tx_info.wallet_path.get_path() is None:
+        if tx_info.change_detector.wallet_path.get_path() is None:
             await helpers.confirm_multiple_accounts()
 
         fee = self.total_in - self.total_out
@@ -390,7 +392,7 @@ class BasicApprover(Approver):
                     fee_rate,
                     coin,
                     amount_unit,
-                    tx_info.wallet_path.get_path(),
+                    tx_info.change_detector.wallet_path.get_path(),
                 )
             else:
                 await helpers.confirm_joint_total(spending, total, coin, amount_unit)

core/src/apps/bitcoin/sign_tx/change_detector.py

@@ -0,0 +1,65 @@
+from micropython import const
+from typing import TYPE_CHECKING
+
+from .. import common
+
+if TYPE_CHECKING:
+    from trezor.messages import TxInput, TxOutput
+
+# The chain id used for change.
+_BIP32_CHANGE_CHAIN = const(1)
+
+# The maximum allowed change address. This should be large enough for normal
+# use and still allow to quickly brute-force the correct BIP32 path.
+_BIP32_MAX_LAST_ELEMENT = const(1_000_000)
+
+
+class ChangeDetector:
+    def __init__(self) -> None:
+        from .matchcheck import (
+            MultisigFingerprintChecker,
+            ScriptTypeChecker,
+            WalletPathChecker,
+        )
+
+        # Checksum of multisig inputs, used to validate change-output.
+        self.multisig_fingerprint = MultisigFingerprintChecker()
+
+        # Common prefix of input paths, used to validate change-output.
+        self.wallet_path = WalletPathChecker()
+
+        # Common script type, used to validate change-output.
+        self.script_type = ScriptTypeChecker()
+
+    def add_input(self, txi: TxInput) -> None:
+        if not common.input_is_external(txi):
+            self.wallet_path.add_input(txi)
+            self.script_type.add_input(txi)
+            self.multisig_fingerprint.add_input(txi)
+
+    def check_input(self, txi: TxInput) -> None:
+        self.wallet_path.check_input(txi)
+        self.script_type.check_input(txi)
+        self.multisig_fingerprint.check_input(txi)
+
+    def output_is_change(self, txo: TxOutput) -> bool:
+        if txo.script_type not in common.CHANGE_OUTPUT_SCRIPT_TYPES:
+            return False
+
+        # Check the multisig fingerprint only for multisig outputs. This means
+        # that a transfer from a multisig account to a singlesig account is
+        # treated as a change-output as long as all other change-output
+        # conditions are satisfied. This goes a bit against the concept of a
+        # multisig account but the other cosigners will notice that they are
+        # relinquishing control of the funds, so there is no security risk.
+        if txo.multisig and not self.multisig_fingerprint.output_matches(txo):
+            return False
+
+        return (
+            self.wallet_path.output_matches(txo)
+            and self.script_type.output_matches(txo)
+            and len(txo.address_n) >= common.BIP32_WALLET_DEPTH
+            and txo.address_n[-2] <= _BIP32_CHANGE_CHAIN
+            and txo.address_n[-1] <= _BIP32_MAX_LAST_ELEMENT
+            and txo.amount > 0
+        )

core/src/apps/bitcoin/sign_tx/tx_info.py

@@ -1,7 +1,7 @@
 from micropython import const
 from typing import TYPE_CHECKING
 
-from .. import common, writers
+from .. import writers
 
 if TYPE_CHECKING:
     from typing import Protocol
@@ -32,13 +32,6 @@ if TYPE_CHECKING:
         ) -> None: ...
 
 
-# The chain id used for change.
-_BIP32_CHANGE_CHAIN = const(1)
-
-# The maximum allowed change address. This should be large enough for normal
-# use and still allow to quickly brute-force the correct BIP32 path.
-_BIP32_MAX_LAST_ELEMENT = const(1_000_000)
-
 # Setting nSequence to this value for every input in a transaction disables nLockTime.
 _SEQUENCE_FINAL = const(0xFFFF_FFFF)
 
@@ -52,20 +45,9 @@ class TxInfoBase:
         from trezor.crypto.hashlib import sha256
         from trezor.utils import HashWriter
 
-        from .matchcheck import (
+        from .change_detector import ChangeDetector
-            MultisigFingerprintChecker,
-            ScriptTypeChecker,
-            WalletPathChecker,
-        )
 
-        # Checksum of multisig inputs, used to validate change-output.
+        self.change_detector = ChangeDetector()
-        self.multisig_fingerprint = MultisigFingerprintChecker()
-
-        # Common prefix of input paths, used to validate change-output.
-        self.wallet_path = WalletPathChecker()
-
-        # Common script type, used to validate change-output.
-        self.script_type = ScriptTypeChecker()
 
         # h_tx_check is used to make sure that the inputs and outputs streamed in
         # different steps are the same every time, e.g. the ones streamed for approval
@@ -88,42 +70,17 @@ class TxInfoBase:
         self.sig_hasher.add_input(txi, script_pubkey)
         writers.write_tx_input_check(self.h_tx_check, txi)
         self.min_sequence = min(self.min_sequence, txi.sequence)
-
+        self.change_detector.add_input(txi)
-        if not common.input_is_external(txi):
-            self.wallet_path.add_input(txi)
-            self.script_type.add_input(txi)
-            self.multisig_fingerprint.add_input(txi)
 
     def add_output(self, txo: TxOutput, script_pubkey: bytes) -> None:
         self.sig_hasher.add_output(txo, script_pubkey)
         writers.write_tx_output(self.h_tx_check, txo, script_pubkey)
 
     def check_input(self, txi: TxInput) -> None:
-        self.wallet_path.check_input(txi)
+        self.change_detector.check_input(txi)
-        self.script_type.check_input(txi)
-        self.multisig_fingerprint.check_input(txi)
 
     def output_is_change(self, txo: TxOutput) -> bool:
-        if txo.script_type not in common.CHANGE_OUTPUT_SCRIPT_TYPES:
+        return self.change_detector.output_is_change(txo)
-            return False
-
-        # Check the multisig fingerprint only for multisig outputs. This means
-        # that a transfer from a multisig account to a singlesig account is
-        # treated as a change-output as long as all other change-output
-        # conditions are satisfied. This goes a bit against the concept of a
-        # multisig account but the other cosigners will notice that they are
-        # relinquishing control of the funds, so there is no security risk.
-        if txo.multisig and not self.multisig_fingerprint.output_matches(txo):
-            return False
-
-        return (
-            self.wallet_path.output_matches(txo)
-            and self.script_type.output_matches(txo)
-            and len(txo.address_n) >= common.BIP32_WALLET_DEPTH
-            and txo.address_n[-2] <= _BIP32_CHANGE_CHAIN
-            and txo.address_n[-1] <= _BIP32_MAX_LAST_ELEMENT
-            and txo.amount > 0
-        )
 
     def lock_time_disabled(self) -> bool:
         return self.min_sequence == _SEQUENCE_FINAL