feat(core):: support sortedmulti

core/.changelog.d/4351.added

@@ -0,0 +1 @@
+Added support for lexicographic sorting of pubkeys in multisig.

core/src/apps/bitcoin/multisig.py

@@ -1,5 +1,6 @@
 from typing import TYPE_CHECKING
 
+from trezor.enums import MultisigPubkeysOrder
 from trezor.wire import DataError
 
 if TYPE_CHECKING:
@@ -28,11 +29,14 @@ def multisig_fingerprint(multisig: MultisigRedeemScriptType) -> bytes:
         if len(d.public_key) != 33 or len(d.chain_code) != 32:
             raise DataError("Invalid multisig parameters")
 
-    pubnodes = sorted(pubnodes, key=lambda n: n.public_key + n.chain_code)
+    if multisig.pubkeys_order == MultisigPubkeysOrder.LEXICOGRAPHIC:
+        # If the order of pubkeys is lexicographic, we don't want the fingerprint to depend on the order of the pubnodes, so we sort the pubnodes before hashing.
+        pubnodes.sort(key=lambda n: n.public_key + n.chain_code)
 
     h = HashWriter(sha256())
     write_uint32(h, m)
     write_uint32(h, n)
+    write_uint32(h, multisig.pubkeys_order)
     for d in pubnodes:
         write_uint32(h, d.depth)
         write_uint32(h, d.fingerprint)
@@ -84,9 +88,14 @@ def multisig_get_pubkey(n: HDNodeType, p: paths.Bip32Path) -> bytes:
 def multisig_get_pubkeys(multisig: MultisigRedeemScriptType) -> list[bytes]:
     validate_multisig(multisig)
     if multisig.nodes:
-        return [multisig_get_pubkey(hd, multisig.address_n) for hd in multisig.nodes]
+        pubkeys = [multisig_get_pubkey(hd, multisig.address_n) for hd in multisig.nodes]
     else:
-        return [multisig_get_pubkey(hd.node, hd.address_n) for hd in multisig.pubkeys]
+        pubkeys = [
+            multisig_get_pubkey(hd.node, hd.address_n) for hd in multisig.pubkeys
+        ]
+    if multisig.pubkeys_order == MultisigPubkeysOrder.LEXICOGRAPHIC:
+        pubkeys.sort()
+    return pubkeys
 
 
 def multisig_get_pubkey_count(multisig: MultisigRedeemScriptType) -> int:

core/tests/test_apps.bitcoin.change_detector.py

@@ -1,7 +1,8 @@
 from common import H_, await_result, unittest  # isort:skip
 from ubinascii import hexlify, unhexlify
 
-from trezor.enums import InputScriptType, OutputScriptType
+from trezor.enums import InputScriptType, OutputScriptType, MultisigPubkeysOrder
+from trezor.enums.MultisigPubkeysOrder import LEXICOGRAPHIC, PRESERVED
 from trezor.messages import (
     TxInput,
     TxOutput,
@@ -39,12 +40,13 @@ xpub3 = HDNodeType(
 )
 
 
-def get_multisig(path: list[int], xpubs: list[HDNodeType]) -> MultisigRedeemScriptType:
+def get_multisig(path: list[int], xpubs: list[HDNodeType], pubkeys_order: MultisigPubkeysOrder) -> MultisigRedeemScriptType:
     return MultisigRedeemScriptType(
         nodes=xpubs,
         signatures=b"" * len(xpubs),
         address_n=path[-2:],
         m=2,
+        pubkeys_order=pubkeys_order,
     )
 
 
@@ -58,23 +60,23 @@ def get_singlesig_input(path: list[int]) -> TxInput:
     )
 
 
-def get_multisig_input(path: list[int], xpubs: list[HDNodeType]) -> TxInput:
+def get_multisig_input(path: list[int], xpubs: list[HDNodeType], pubkeys_order: MultisigPubkeysOrder) -> TxInput:
     return TxInput(
         address_n=path,
         amount=1_000_000,
         prev_hash=bytes(32),
         prev_index=0,
         script_type=InputScriptType.SPENDMULTISIG,
-        multisig=get_multisig(path, xpubs),
+        multisig=get_multisig(path, xpubs, pubkeys_order),
     )
 
 
-def get_internal_multisig_output(path: list[int], xpubs: list[HDNodeType]) -> TxOutput:
+def get_internal_multisig_output(path: list[int], xpubs: list[HDNodeType], pubkeys_order: MultisigPubkeysOrder) -> TxOutput:
     return TxOutput(
         address_n=path,
         amount=1_000_000,
         script_type=OutputScriptType.PAYTOMULTISIG,
-        multisig=get_multisig(path, xpubs),
+        multisig=get_multisig(path, xpubs, pubkeys_order),
     )
 
 
@@ -115,7 +117,8 @@ class TestChangeDetector(unittest.TestCase):
         assert self.d.output_is_change(get_internal_singlesig_output([H_(45), 1, 0, 0])) == False
 
         # Multisig instead of singlesig
-        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1])) == False
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1], PRESERVED)) == False
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1], LEXICOGRAPHIC)) == False
 
         # External output
         assert self.d.output_is_change(get_external_singlesig_output()) == False
@@ -130,23 +133,24 @@ class TestChangeDetector(unittest.TestCase):
         assert self.d.output_is_change(get_internal_singlesig_output([H_(45), 1, 0, 0])) == False
 
         # Multisig instead of singlesig
-        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1])) == False
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 1, 0, 0], [xpub1], PRESERVED)) == False
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 1, 0, 0], [xpub1], LEXICOGRAPHIC)) == False
 
         # External output
         assert self.d.output_is_change(get_external_singlesig_output()) == False
 
-    def test_multisig(self):
+    def test_unsorted_multisig(self):
         # Different change and account index
-        self.d.add_input(get_multisig_input([H_(45), 0, 0, 0], [xpub1, xpub2]))
-        self.d.add_input(get_multisig_input([H_(45), 0, 0, 1], [xpub1, xpub2]))
-        self.d.add_input(get_multisig_input([H_(45), 0, 1, 0], [xpub1, xpub2]))
-        self.d.add_input(get_multisig_input([H_(45), 0, 1, 1], [xpub1, xpub2]))
+        self.d.add_input(get_multisig_input([H_(45), 0, 0, 0], [xpub1, xpub2], PRESERVED))
+        self.d.add_input(get_multisig_input([H_(45), 0, 0, 1], [xpub1, xpub2], PRESERVED))
+        self.d.add_input(get_multisig_input([H_(45), 0, 1, 0], [xpub1, xpub2], PRESERVED))
+        self.d.add_input(get_multisig_input([H_(45), 0, 1, 1], [xpub1, xpub2], PRESERVED))
 
         # Same outputs as inputs
-        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1, xpub2])) == True
-        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 1], [xpub1, xpub2])) == True
-        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 1, 0], [xpub1, xpub2])) == True
-        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 1, 1], [xpub1, xpub2])) == True
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1, xpub2], PRESERVED)) == True
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 1], [xpub1, xpub2], PRESERVED)) == True
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 1, 0], [xpub1, xpub2], PRESERVED)) == True
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 1, 1], [xpub1, xpub2], PRESERVED)) == True
 
         # Singlesig instead of multisig
         assert self.d.output_is_change(get_internal_singlesig_output([H_(45), 0, 0, 0])) == False
@@ -155,22 +159,60 @@ class TestChangeDetector(unittest.TestCase):
         assert self.d.output_is_change(get_internal_singlesig_output([H_(45), 1, 0, 0])) == False
 
         # Different order of xpubs
-        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub2, xpub1])) == True
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub2, xpub1], PRESERVED)) == False
+
+        # Sorted instead of unsorted
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1, xpub2], LEXICOGRAPHIC)) == False
 
         # Different xpubs
-        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1, xpub3])) == False
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1, xpub3], PRESERVED)) == False
 
         # External output
         assert self.d.output_is_change(get_external_singlesig_output()) == False
 
-    def test_multisig_different_xpubs_order(self):
+    def test_sorted_multisig(self):
+        # Different change and account index
+        self.d.add_input(get_multisig_input([H_(45), 0, 0, 0], [xpub1, xpub2], LEXICOGRAPHIC))
+        self.d.add_input(get_multisig_input([H_(45), 0, 0, 1], [xpub1, xpub2], LEXICOGRAPHIC))
+        self.d.add_input(get_multisig_input([H_(45), 0, 1, 0], [xpub1, xpub2], LEXICOGRAPHIC))
+        self.d.add_input(get_multisig_input([H_(45), 0, 1, 1], [xpub1, xpub2], LEXICOGRAPHIC))
+
+        # Same outputs as inputs
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1, xpub2], LEXICOGRAPHIC)) == True
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 1], [xpub1, xpub2], LEXICOGRAPHIC)) == True
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 1, 0], [xpub1, xpub2], LEXICOGRAPHIC)) == True
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 1, 1], [xpub1, xpub2], LEXICOGRAPHIC)) == True
+
+        # Singlesig instead of multisig
+        assert self.d.output_is_change(get_internal_singlesig_output([H_(45), 0, 0, 0])) == False
+
+        # Different account index
+        assert self.d.output_is_change(get_internal_singlesig_output([H_(45), 1, 0, 0])) == False
+
         # Different order of xpubs
-        self.d.add_input(get_multisig_input([H_(45), 0, 0, 0], [xpub1, xpub2]))
-        self.d.add_input(get_multisig_input([H_(45), 0, 0, 0], [xpub2, xpub1]))
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub2, xpub1], LEXICOGRAPHIC)) == True
+
+        # Unsorted instead of sorted
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1, xpub2], PRESERVED)) == False
+
+        # Different xpubs
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1, xpub3], LEXICOGRAPHIC)) == False
+
+        # External output
+        assert self.d.output_is_change(get_external_singlesig_output()) == False
+
+    def test_unsorted_multisig_different_xpubs_order(self):
+        # Different order of xpubs
+        self.d.add_input(get_multisig_input([H_(45), 0, 0, 0], [xpub1, xpub2], PRESERVED))
+        self.d.add_input(get_multisig_input([H_(45), 0, 0, 0], [xpub2, xpub1], PRESERVED))
 
         # Same ouputs as inputs
-        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1, xpub2])) == True
-        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub2, xpub1])) == True
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1, xpub2], PRESERVED)) == False
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub2, xpub1], PRESERVED)) == False
+
+        # Sorted instead of unsorted
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1, xpub2], LEXICOGRAPHIC)) == False
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub2, xpub1], LEXICOGRAPHIC)) == False
 
         # Singlesig instead of multisig
         assert self.d.output_is_change(get_internal_singlesig_output([H_(45), 0, 0, 0])) == False
@@ -178,14 +220,70 @@ class TestChangeDetector(unittest.TestCase):
         # External output
         assert self.d.output_is_change(get_external_singlesig_output()) == False
 
-    def test_multisig_different_xpubs(self):
-        # Different xpubs
-        self.d.add_input(get_multisig_input([H_(45), 0, 0, 0], [xpub1, xpub2]))
-        self.d.add_input(get_multisig_input([H_(45), 0, 0, 0], [xpub1, xpub3]))
+    def test_sorted_multisig_different_xpubs_order(self):
+        # Different order of xpubs
+        self.d.add_input(get_multisig_input([H_(45), 0, 0, 0], [xpub1, xpub2], LEXICOGRAPHIC))
+        self.d.add_input(get_multisig_input([H_(45), 0, 0, 0], [xpub2, xpub1], LEXICOGRAPHIC))
 
         # Same ouputs as inputs
-        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1, xpub2])) == False
-        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1, xpub3])) == False
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1, xpub2], LEXICOGRAPHIC)) == True
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub2, xpub1], LEXICOGRAPHIC)) == True
+
+        # Sorted instead of unsorted
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1, xpub2], PRESERVED)) == False
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub2, xpub1], PRESERVED)) == False
+
+        # Singlesig instead of multisig
+        assert self.d.output_is_change(get_internal_singlesig_output([H_(45), 0, 0, 0])) == False
+
+        # External output
+        assert self.d.output_is_change(get_external_singlesig_output()) == False
+
+    def test_unsorted_multisig_different_xpubs(self):
+        # Different xpubs
+        self.d.add_input(get_multisig_input([H_(45), 0, 0, 0], [xpub1, xpub2], PRESERVED))
+        self.d.add_input(get_multisig_input([H_(45), 0, 0, 0], [xpub1, xpub3], PRESERVED))
+
+        # Same ouputs as inputs
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1, xpub2], PRESERVED)) == False
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1, xpub3], PRESERVED)) == False
+
+        # Sorted instead of unsorted
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1, xpub2], LEXICOGRAPHIC)) == False
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1, xpub3], LEXICOGRAPHIC)) == False
+
+        # Singlesig instead of multisig
+        assert self.d.output_is_change(get_internal_singlesig_output([H_(45), 0, 0, 0])) == False
+
+        # External output
+        assert self.d.output_is_change(get_external_singlesig_output()) == False
+
+    def test_sorted_multisig_different_xpubs(self):
+        # Different xpubs
+        self.d.add_input(get_multisig_input([H_(45), 0, 0, 0], [xpub1, xpub2], LEXICOGRAPHIC))
+        self.d.add_input(get_multisig_input([H_(45), 0, 0, 0], [xpub1, xpub3], LEXICOGRAPHIC))
+
+        # Same ouputs as inputs
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1, xpub2], LEXICOGRAPHIC)) == False
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1, xpub3], LEXICOGRAPHIC)) == False
+
+        # Sorted instead of unsorted
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1, xpub2], PRESERVED)) == False
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1, xpub3], PRESERVED)) == False
+
+        # Singlesig instead of multisig
+        assert self.d.output_is_change(get_internal_singlesig_output([H_(45), 0, 0, 0])) == False
+
+        # External output
+        assert self.d.output_is_change(get_external_singlesig_output()) == False
+
+    def test_mixed_sorted_and_unsorted_multisig_1(self):
+        self.d.add_input(get_multisig_input([H_(45), 0, 0, 0], [xpub1], PRESERVED))
+        self.d.add_input(get_multisig_input([H_(45), 0, 0, 0], [xpub1], LEXICOGRAPHIC))
+
+        # Same ouputs as inputs
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1], PRESERVED)) == False
+        assert self.d.output_is_change(get_internal_multisig_output([H_(45), 0, 0, 0], [xpub1], LEXICOGRAPHIC)) == False
 
         # Singlesig instead of multisig
         assert self.d.output_is_change(get_internal_singlesig_output([H_(45), 0, 0, 0])) == False

tests/device_tests/bitcoin/test_multisig_change.py

@@ -63,7 +63,7 @@ NODE_INT = bip32.deserialize(
 # m/2 => 038caebd6f753bbbd2bb1f3346a43cd32140648583673a31d62f2dfb56ad0ab9e3
 
 multisig_in1 = messages.MultisigRedeemScriptType(
-    nodes=[NODE_EXT2, NODE_EXT1, NODE_INT],
+    nodes=[NODE_EXT1, NODE_EXT2, NODE_INT],
     address_n=[0, 0],
     signatures=[b"", b"", b""],
     m=2,
@@ -84,7 +84,7 @@ multisig_in3 = messages.MultisigRedeemScriptType(
 )
 
 prev_hash_1, prev_tx_1 = forge_prevtx(
-    [("3HwrvQEfYw4wUvGHpGmixWB15HPgqrvTh1", 50_000_000)]
+    [("3Ltgk5WPUMLcT2QvwRXKj9CWsYuAKqeHJ8", 50_000_000)]
 )
 INP1 = messages.TxInputType(
     address_n=[H_(45), 0, 0, 0],