core/bitcoin: Stream prev_tx after confirmation.

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

@@ -36,9 +36,9 @@ class Approver:
         self.total_out = 0  # sum of output amounts
         self.change_out = 0  # change output amount
 
-    async def add_internal_input(self, txi: TxInputType, amount: int) -> None:
+    async def add_internal_input(self, txi: TxInputType) -> None:
         self.weight.add_input(txi)
-        self.total_in += amount
+        self.total_in += txi.amount
         self.min_sequence = min(self.min_sequence, txi.sequence)
 
     def add_external_input(self, txi: TxInputType) -> None:
@@ -70,11 +70,11 @@ class BasicApprover(Approver):
         super().__init__(tx, coin)
         self.change_count = 0  # the number of change-outputs
 
-    async def add_internal_input(self, txi: TxInputType, amount: int) -> None:
+    async def add_internal_input(self, txi: TxInputType) -> None:
         if not addresses.validate_full_path(txi.address_n, self.coin, txi.script_type):
             await helpers.confirm_foreign_address(txi.address_n)
 
-        await super().add_internal_input(txi, amount)
+        await super().add_internal_input(txi)
 
     def add_change_output(self, txo: TxOutputType, script_pubkey: bytes) -> None:
         super().add_change_output(txo, script_pubkey)
@@ -143,12 +143,12 @@ class CoinJoinApprover(Approver):
         # flag indicating whether our outputs are gaining any anonymity
         self.anonymity = False
 
-    async def add_internal_input(self, txi: TxInputType, amount: int) -> None:
+    async def add_internal_input(self, txi: TxInputType) -> None:
         self.our_weight.add_input(txi)
         if not self.authorization.check_sign_tx_input(txi, self.coin):
             raise wire.ProcessError("Unauthorized path")
 
-        await super().add_internal_input(txi, amount)
+        await super().add_internal_input(txi)
 
     def add_change_output(self, txo: TxOutputType, script_pubkey: bytes) -> None:
         super().add_change_output(txo, script_pubkey)

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

@@ -40,8 +40,7 @@ _MAX_SERIALIZED_CHUNK_SIZE = const(2048)
 
 class Bitcoin:
     async def signer(self) -> None:
-        # Add inputs to hash143 and h_approved and compute the sum of input amounts
+        # Add inputs to hash143 and h_approved and compute the sum of input amounts.
-        # by requesting each previous transaction and checking its output amounts.
         await self.step1_process_inputs()
 
         # Add outputs to hash143 and h_approved, approve outputs and compute
@@ -51,9 +50,10 @@ class Bitcoin:
         # Check fee, approve lock_time and total.
         await self.approver.approve_tx()
 
-        # Verify external inputs which have already been signed or which come with
+        # Verify the transaction input amounts by requesting each previous transaction
-        # a proof of non-ownership.
+        # and checking its output amount. Verify external inputs which have already
-        await self.step3_verify_external_inputs()
+        # been signed or which come with a proof of non-ownership.
+        await self.step3_verify_inputs()
 
         # Check that inputs are unchanged. Serialize inputs and sign the non-segwit ones.
         await self.step4_serialize_inputs()
@@ -103,10 +103,10 @@ class Bitcoin:
         # legacy inputs in Step 4.
         self.h_approved = self.create_hash_writer()  # not a real tx hash
 
-        # h_external is used to make sure that the signed external inputs streamed for
+        # h_inputs is a digest of the inputs streamed for approval in Step 1, which
-        # approval in Step 1 are the same as the ones streamed for verification
+        # is used to ensure that the inputs streamed for verification in Step 3 are
-        # in Step 3.
+        # the same as those in Step 1.
-        self.h_external = self.create_hash_writer()
+        self.h_inputs = None  # type: Optional[bytes]
 
         # BIP-0143 transaction hashing
         self.init_hash143()
@@ -120,6 +120,10 @@ class Bitcoin:
         for i in range(self.tx.inputs_count):
             # STAGE_REQUEST_1_INPUT in legacy
             txi = await helpers.request_tx_input(self.tx_req, i, self.coin)
+
+            self.hash143_add_input(txi)  # all inputs are included (non-segwit as well)
+            writers.write_tx_input_check(self.h_approved, txi)
+
             if input_is_segwit(txi):
                 self.segwit.add(i)
 
@@ -127,23 +131,25 @@ class Bitcoin:
                 self.external.add(i)
                 await self.process_external_input(txi)
             else:
-                progress.advance()
                 await self.process_internal_input(txi)
 
+        self.h_inputs = self.h_approved.get_digest()
+
     async def step2_approve_outputs(self) -> None:
         for i in range(self.tx.outputs_count):
-            # STAGE_REQUEST_3_OUTPUT in legacy
+            # STAGE_REQUEST_2_OUTPUT in legacy
             txo = await helpers.request_tx_output(self.tx_req, i, self.coin)
             script_pubkey = self.output_derive_script(txo)
             await self.approve_output(txo, script_pubkey)
 
-    async def step3_verify_external_inputs(self) -> None:
+    async def step3_verify_inputs(self) -> None:
-        # should come out the same as h_external, checked before continuing
+        # should come out the same as h_inputs, checked before continuing
         h_check = self.create_hash_writer()
 
-        for i in sorted(self.external):
+        for i in range(self.tx.inputs_count):
             progress.advance()
             txi = await helpers.request_tx_input(self.tx_req, i, self.coin)
+
             writers.write_tx_input_check(h_check, txi)
             prev_amount, script_pubkey = await self.get_prevtx_output(
                 txi.prev_hash, txi.prev_index
@@ -151,25 +157,11 @@ class Bitcoin:
             if prev_amount != txi.amount:
                 raise wire.DataError("Invalid amount specified")
 
-            if txi.ownership_proof:
+            if i in self.external:
-                if not verify_nonownership(
+                await self.verify_external_input(i, txi, script_pubkey)
-                    txi.ownership_proof, script_pubkey, b"", self.keychain, self.coin
-                ):
-                    raise wire.DataError("Invalid external input")
-            else:
-                verifier = SignatureVerifier(
-                    script_pubkey, txi.script_sig, txi.witness, self.coin
-                )
-
-                verifier.ensure_hash_type(self.get_hash_type(txi))
-
-                tx_digest = await self.get_tx_digest(
-                    i, txi, verifier.public_keys, verifier.threshold, script_pubkey
-                )
-                verifier.verify(tx_digest)
 
         # check that the inputs were the same as those streamed for approval
-        if self.h_external.get_digest() != h_check.get_digest():
+        if h_check.get_digest() != self.h_inputs:
             raise wire.ProcessError("Transaction has changed during signing")
 
     async def step4_serialize_inputs(self) -> None:
@@ -215,28 +207,13 @@ class Bitcoin:
     async def process_internal_input(self, txi: TxInputType) -> None:
         self.wallet_path.add_input(txi)
         self.multisig_fingerprint.add_input(txi)
-        writers.write_tx_input_check(self.h_approved, txi)
-        self.hash143_add_input(txi)  # all inputs are included (non-segwit as well)
 
         if txi.script_type not in common.INTERNAL_INPUT_SCRIPT_TYPES:
             raise wire.DataError("Wrong input script type")
 
-        prev_amount, script_pubkey = await self.get_prevtx_output(
+        await self.approver.add_internal_input(txi)
-            txi.prev_hash, txi.prev_index
-        )
-
-        if txi.amount is not None and prev_amount != txi.amount:
-            raise wire.DataError("Invalid amount specified")
-
-        await self.approver.add_internal_input(txi, prev_amount)
 
     async def process_external_input(self, txi: TxInputType) -> None:
-        if txi.amount is None:
-            raise wire.DataError("Expected input with amount")
-
-        writers.write_tx_input_check(self.h_external, txi)
-        writers.write_tx_input_check(self.h_approved, txi)
-        self.hash143_add_input(txi)  # all inputs are included (non-segwit as well)
         self.approver.add_external_input(txi)
 
     async def approve_output(self, txo: TxOutputType, script_pubkey: bytes) -> None:
@@ -263,6 +240,26 @@ class Bitcoin:
             digest, _, _ = await self.get_legacy_tx_digest(i, script_pubkey)
             return digest
 
+    async def verify_external_input(
+        self, i: int, txi: TxInputType, script_pubkey: bytes
+    ) -> None:
+        if txi.ownership_proof:
+            if not verify_nonownership(
+                txi.ownership_proof, script_pubkey, bytes(), self.keychain, self.coin
+            ):
+                raise wire.DataError("Invalid external input")
+        else:
+            verifier = SignatureVerifier(
+                script_pubkey, txi.script_sig, txi.witness, self.coin
+            )
+
+            verifier.ensure_hash_type(self.get_hash_type(txi))
+
+            tx_digest = await self.get_tx_digest(
+                i, txi, verifier.public_keys, verifier.threshold, script_pubkey
+            )
+            verifier.verify(tx_digest)
+
     async def serialize_external_input(self, i: int) -> None:
         txi = await helpers.request_tx_input(self.tx_req, i, self.coin)
         if not input_is_external(txi):
@@ -286,9 +283,6 @@ class Bitcoin:
         self.write_tx_input(self.serialized_tx, txi, script_sig)
 
     def sign_bip143_input(self, txi: TxInputType) -> Tuple[bytes, bytes]:
-        if txi.amount is None:
-            raise wire.DataError("Expected input with amount")
-
         self.wallet_path.check_input(txi)
         self.multisig_fingerprint.check_input(txi)
 
@@ -414,7 +408,7 @@ class Bitcoin:
     ) -> Tuple[int, bytes]:
         amount_out = 0  # output amount
 
-        # STAGE_REQUEST_2_PREV_META in legacy
+        # STAGE_REQUEST_3_PREV_META in legacy
         tx = await helpers.request_tx_meta(self.tx_req, self.coin, prev_hash)
 
         if tx.outputs_cnt <= prev_index:
@@ -427,14 +421,14 @@ class Bitcoin:
         write_bitcoin_varint(txh, tx.inputs_cnt)
 
         for i in range(tx.inputs_cnt):
-            # STAGE_REQUEST_2_PREV_INPUT in legacy
+            # STAGE_REQUEST_3_PREV_INPUT in legacy
             txi = await helpers.request_tx_input(self.tx_req, i, self.coin, prev_hash)
             self.write_tx_input(txh, txi, txi.script_sig)
 
         write_bitcoin_varint(txh, tx.outputs_cnt)
 
         for i in range(tx.outputs_cnt):
-            # STAGE_REQUEST_2_PREV_OUTPUT in legacy
+            # STAGE_REQUEST_3_PREV_OUTPUT in legacy
             txo_bin = await helpers.request_tx_output(
                 self.tx_req, i, self.coin, prev_hash
             )

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

@@ -194,7 +194,7 @@ class Decred(Bitcoin):
     def write_tx_input_witness(
         self, w: writers.Writer, i: TxInputType, script_sig: bytes
     ) -> None:
-        writers.write_uint64(w, i.amount or 0)
+        writers.write_uint64(w, i.amount)
         writers.write_uint32(w, 0)  # block height fraud proof
         writers.write_uint32(w, 0xFFFFFFFF)  # block index fraud proof
         writers.write_bytes_prefixed(w, script_sig)

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

@@ -257,6 +257,8 @@ def sanitize_tx_meta(tx: TransactionType, coin: CoinInfo) -> TransactionType:
 
 def sanitize_tx_input(tx: TransactionType, coin: CoinInfo) -> TxInputType:
     txi = tx.inputs[0]
+    if txi.amount is None:
+        txi.amount = 0
     if txi.script_type is None:
         txi.script_type = InputScriptType.SPENDADDRESS
     if txi.sequence is None: