Merge pull request #4148 from SomberNight/hw_wallet_output_ismine_der

trezor/keepkey/dbb: provide derivation info for all is_mine txn outputs

lib/wallet.py

@@ -1444,7 +1444,7 @@ class Abstract_Wallet(PrintError):
         xpubs = self.get_master_public_keys()
         for txout in tx.outputs():
             _type, addr, amount = txout
-            if self.is_change(addr):
+            if self.is_mine(addr):
                 index = self.get_address_index(addr)
                 pubkeys = self.get_public_keys(addr)
                 # sort xpubs using the order of pubkeys

plugins/keepkey/plugin.py

@@ -303,38 +303,34 @@ class KeepKeyCompatiblePlugin(HW_PluginBase):
         return inputs
 
     def tx_outputs(self, derivation, tx, segwit=False):
-        outputs = []
-        has_change = False
 
-        for _type, address, amount in tx.outputs():
-            info = tx.output_info.get(address)
-            if info is not None and not has_change:
-                has_change = True # no more than one change address
-                addrtype, hash_160 = b58_address_to_hash160(address)
+        def create_output_by_derivation(info):
             index, xpubs, m = info
             if len(xpubs) == 1:
                 script_type = self.types.PAYTOP2SHWITNESS if segwit else self.types.PAYTOADDRESS
-                    address_n = self.client_class.expand_path(derivation + "/%d/%d"%index)
+                address_n = self.client_class.expand_path(derivation + "/%d/%d" % index)
                 txoutputtype = self.types.TxOutputType(
-                        amount = amount,
-                        script_type = script_type,
-                        address_n = address_n,
+                    amount=amount,
+                    script_type=script_type,
+                    address_n=address_n,
                 )
             else:
                 script_type = self.types.PAYTOP2SHWITNESS if segwit else self.types.PAYTOMULTISIG
-                    address_n = self.client_class.expand_path("/%d/%d"%index)
+                address_n = self.client_class.expand_path("/%d/%d" % index)
                 nodes = map(self.ckd_public.deserialize, xpubs)
-                    pubkeys = [ self.types.HDNodePathType(node=node, address_n=address_n) for node in nodes]
+                pubkeys = [self.types.HDNodePathType(node=node, address_n=address_n) for node in nodes]
                 multisig = self.types.MultisigRedeemScriptType(
-                        pubkeys = pubkeys,
-                        signatures = [b''] * len(pubkeys),
-                        m = m)
+                    pubkeys=pubkeys,
+                    signatures=[b''] * len(pubkeys),
+                    m=m)
                 txoutputtype = self.types.TxOutputType(
-                        multisig = multisig,
-                        amount = amount,
-                        address_n = self.client_class.expand_path(derivation + "/%d/%d"%index),
-                        script_type = script_type)
-            else:
+                    multisig=multisig,
+                    amount=amount,
+                    address_n=self.client_class.expand_path(derivation + "/%d/%d" % index),
+                    script_type=script_type)
+            return txoutputtype
+
+        def create_output_by_address():
             txoutputtype = self.types.TxOutputType()
             txoutputtype.amount = amount
             if _type == TYPE_SCRIPT:
@@ -352,7 +348,38 @@ class KeepKeyCompatiblePlugin(HW_PluginBase):
                     else:
                         raise BaseException('addrtype: ' + str(addrtype))
                 txoutputtype.address = address
+            return txoutputtype
 
+        def is_any_output_on_change_branch():
+            for _type, address, amount in tx.outputs():
+                info = tx.output_info.get(address)
+                if info is not None:
+                    index, xpubs, m = info
+                    if index[0] == 1:
+                        return True
+            return False
+
+        outputs = []
+        has_change = False
+        any_output_on_change_branch = is_any_output_on_change_branch()
+
+        for _type, address, amount in tx.outputs():
+            use_create_by_derivation = False
+
+            info = tx.output_info.get(address)
+            if info is not None and not has_change:
+                index, xpubs, m = info
+                on_change_branch = index[0] == 1
+                # prioritise hiding outputs on the 'change' branch from user
+                # because no more than one change address allowed
+                if on_change_branch == any_output_on_change_branch:
+                    use_create_by_derivation = True
+                    has_change = True
+
+            if use_create_by_derivation:
+                txoutputtype = create_output_by_derivation(info)
+            else:
+                txoutputtype = create_output_by_address()
             outputs.append(txoutputtype)
 
         return outputs

plugins/ledger/ledger.py

@@ -377,7 +377,8 @@ class Ledger_KeyStore(Hardware_KeyStore):
             for _type, address, amount in tx.outputs():
                 assert _type == TYPE_ADDRESS
                 info = tx.output_info.get(address)
-                if (info is not None) and (len(tx.outputs()) != 1):
+                if (info is not None) and len(tx.outputs()) > 1 \
+                        and info[0][0] == 1:  # "is on 'change' branch"
                     index, xpubs, m = info
                     changePath = self.get_derivation()[2:] + "/%d/%d"%index
                     changeAmount = amount

plugins/trezor/trezor.py

@@ -380,13 +380,8 @@ class TrezorPlugin(HW_PluginBase):
         return inputs
 
     def tx_outputs(self, derivation, tx, script_gen=SCRIPT_GEN_LEGACY):
-        outputs = []
-        has_change = False
 
-        for _type, address, amount in tx.outputs():
-            info = tx.output_info.get(address)
-            if info is not None and not has_change:
-                has_change = True # no more than one change address
+        def create_output_by_derivation(info):
             index, xpubs, m = info
             if len(xpubs) == 1:
                 if script_gen == SCRIPT_GEN_NATIVE_SEGWIT:
@@ -395,11 +390,11 @@ class TrezorPlugin(HW_PluginBase):
                     script_type = self.types.OutputScriptType.PAYTOP2SHWITNESS
                 else:
                     script_type = self.types.OutputScriptType.PAYTOADDRESS
-                    address_n = self.client_class.expand_path(derivation + "/%d/%d"%index)
+                address_n = self.client_class.expand_path(derivation + "/%d/%d" % index)
                 txoutputtype = self.types.TxOutputType(
-                        amount = amount,
-                        script_type = script_type,
-                        address_n = address_n,
+                    amount=amount,
+                    script_type=script_type,
+                    address_n=address_n,
                 )
             else:
                 if script_gen == SCRIPT_GEN_NATIVE_SEGWIT:
@@ -408,19 +403,21 @@ class TrezorPlugin(HW_PluginBase):
                     script_type = self.types.OutputScriptType.PAYTOP2SHWITNESS
                 else:
                     script_type = self.types.OutputScriptType.PAYTOMULTISIG
-                    address_n = self.client_class.expand_path("/%d/%d"%index)
+                address_n = self.client_class.expand_path("/%d/%d" % index)
                 nodes = map(self.ckd_public.deserialize, xpubs)
-                    pubkeys = [ self.types.HDNodePathType(node=node, address_n=address_n) for node in nodes]
+                pubkeys = [self.types.HDNodePathType(node=node, address_n=address_n) for node in nodes]
                 multisig = self.types.MultisigRedeemScriptType(
-                        pubkeys = pubkeys,
-                        signatures = [b''] * len(pubkeys),
-                        m = m)
+                    pubkeys=pubkeys,
+                    signatures=[b''] * len(pubkeys),
+                    m=m)
                 txoutputtype = self.types.TxOutputType(
-                        multisig = multisig,
-                        amount = amount,
-                        address_n = self.client_class.expand_path(derivation + "/%d/%d"%index),
-                        script_type = script_type)
-            else:
+                    multisig=multisig,
+                    amount=amount,
+                    address_n=self.client_class.expand_path(derivation + "/%d/%d" % index),
+                    script_type=script_type)
+            return txoutputtype
+
+        def create_output_by_address():
             txoutputtype = self.types.TxOutputType()
             txoutputtype.amount = amount
             if _type == TYPE_SCRIPT:
@@ -429,7 +426,40 @@ class TrezorPlugin(HW_PluginBase):
             elif _type == TYPE_ADDRESS:
                 txoutputtype.script_type = self.types.OutputScriptType.PAYTOADDRESS
                 txoutputtype.address = address
+            return txoutputtype
+
+        def is_any_output_on_change_branch():
+            for _type, address, amount in tx.outputs():
+                info = tx.output_info.get(address)
+                if info is not None:
+                    index, xpubs, m = info
+                    if index[0] == 1:
+                        return True
+            return False
+
+        outputs = []
+        has_change = False
+        any_output_on_change_branch = is_any_output_on_change_branch()
 
+        for _type, address, amount in tx.outputs():
+            use_create_by_derivation = False
+
+            info = tx.output_info.get(address)
+            if info is not None and not has_change:
+                index, xpubs, m = info
+                on_change_branch = index[0] == 1
+                # prioritise hiding outputs on the 'change' branch from user
+                # because no more than one change address allowed
+                # note: ^ restriction can be removed once we require fw
+                # that has https://github.com/trezor/trezor-mcu/pull/306
+                if on_change_branch == any_output_on_change_branch:
+                    use_create_by_derivation = True
+                    has_change = True
+
+            if use_create_by_derivation:
+                txoutputtype = create_output_by_derivation(info)
+            else:
+                txoutputtype = create_output_by_address()
             outputs.append(txoutputtype)
 
         return outputs