chore(core): Simplify path warning UI.

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

@@ -63,7 +63,7 @@ class Approver:
         if txi.orig_hash:
             self.orig_total_in += txi.amount
 
-    async def check_internal_input(self, txi: TxInput) -> None:
+    def check_internal_input(self, txi: TxInput) -> None:
         pass
 
     def add_external_input(self, txi: TxInput) -> None:
@@ -135,23 +135,25 @@ class BasicApprover(Approver):
     def __init__(self, tx: SignTx, coin: CoinInfo) -> None:
         super().__init__(tx, coin)
         self.change_count = 0  # the number of change-outputs
+        self.foreign_address_confirmed = False
 
     async def add_internal_input(self, txi: TxInput) -> None:
         if not validate_path_against_script_type(self.coin, txi):
             await helpers.confirm_foreign_address(txi.address_n)
+            self.foreign_address_confirmed = True
 
         await super().add_internal_input(txi)
 
-    async def check_internal_input(self, txi: TxInput) -> None:
-        if not validate_path_against_script_type(self.coin, txi):
-            # The following can be removed once we start validating script_pubkey in step3_verify_inputs().
-            if self.orig_total_in:
-                # Replacement transaction.
-                # This mitigates a cross-coin spending attack when safety checks are disabled.
-                raise wire.ProcessError(
-                    "Non-standard paths not allowed in replacement transactions."
-                )
-            await helpers.confirm_foreign_address(txi.address_n)
+    def check_internal_input(self, txi: TxInput) -> None:
+        # Sanity check not critical for security.
+        # The main reason for this is that we are not comfortable with using the same private key
+        # in multiple signatures schemes (ECDSA and Schnorr) and we want to be sure that the user
+        # went through a warning screen before we sign the input.
+        if (
+            not validate_path_against_script_type(self.coin, txi)
+            and not self.foreign_address_confirmed
+        ):
+            raise wire.ProcessError("Transaction has changed during signing")
 
     def add_change_output(self, txo: TxOutput, script_pubkey: bytes) -> None:
         super().add_change_output(txo, script_pubkey)
@@ -353,8 +355,11 @@ class CoinJoinApprover(Approver):
 
         await super().add_internal_input(txi)
 
-    async def check_internal_input(self, txi: TxInput) -> None:
-        # The following can be removed once we start validating script_pubkey in step3_verify_inputs().
+    def check_internal_input(self, txi: TxInput) -> None:
+        # Sanity check not critical for security.
+        # The main reason for this is that we are not comfortable with using the same private key
+        # in multiple signatures schemes (ECDSA and Schnorr) and we want to be sure that the user
+        # went through a warning screen before we sign the input.
         if not self.authorization.check_sign_tx_input(txi, self.coin):
             raise wire.ProcessError("Unauthorized path")
 

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

@@ -584,7 +584,7 @@ class Bitcoin:
         # STAGE_REQUEST_SEGWIT_WITNESS in legacy
         txi = await helpers.request_tx_input(self.tx_req, i, self.coin)
         self.tx_info.check_input(txi)
-        await self.approver.check_internal_input(txi)
+        self.approver.check_internal_input(txi)
         if txi.script_type not in common.SEGWIT_INPUT_SCRIPT_TYPES:
             raise wire.ProcessError("Transaction has changed during signing")
 

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

@@ -19,7 +19,7 @@ class Bitcoinlike(Bitcoin):
     async def sign_nonsegwit_bip143_input(self, i_sign: int) -> None:
         txi = await helpers.request_tx_input(self.tx_req, i_sign, self.coin)
         self.tx_info.check_input(txi)
-        await self.approver.check_internal_input(txi)
+        self.approver.check_internal_input(txi)
 
         if txi.script_type not in NONSEGWIT_INPUT_SCRIPT_TYPES:
             raise wire.ProcessError("Transaction has changed during signing")

core/tests/test_apps.bitcoin.segwit.signtx.native_p2wpkh.py

@@ -150,9 +150,6 @@ class TestSignSegwitTxNativeP2WPKH(unittest.TestCase):
             )),
             TxAckInput(tx=TxAckInputWrapper(input=inp1)),
 
-            helpers.UiConfirmForeignAddress(address_n=inp1.address_n),
-            True,
-
             TxRequest(request_type=TXFINISHED, details=TxRequestDetailsType(), serialized=TxRequestSerializedType(
                 serialized_tx=unhexlify('02483045022100a7ca8f097525f9044e64376dc0a0f5d4aeb8d15d66808ba97979a0475b06b66502200597c8ebcef63e047f9aeef1a8001d3560470cf896c12f6990eec4faec599b950121033add1f0e8e3c3136f7428dd4a4de1057380bd311f5b0856e2269170b4ffa65bf00000000'),
                 signature_index=0,
@@ -281,9 +278,6 @@ class TestSignSegwitTxNativeP2WPKH(unittest.TestCase):
             )),
             TxAckInput(tx=TxAckInputWrapper(input=inp1)),
 
-            helpers.UiConfirmForeignAddress(address_n=inp1.address_n),
-            True,
-
             TxRequest(request_type=TXFINISHED, details=TxRequestDetailsType(), serialized=TxRequestSerializedType(
                 serialized_tx=unhexlify('02483045022100a7ca8f097525f9044e64376dc0a0f5d4aeb8d15d66808ba97979a0475b06b66502200597c8ebcef63e047f9aeef1a8001d3560470cf896c12f6990eec4faec599b950121033add1f0e8e3c3136f7428dd4a4de1057380bd311f5b0856e2269170b4ffa65bf00000000'),
                 signature_index=0,