feat(legacy): support sortedmulti

legacy/firmware/crypto.c

@@ -18,6 +18,7 @@
  */
 
 #include "crypto.h"
+#include <stdlib.h>
 #include <string.h>
 #include "address.h"
 #include "aes/aes.h"
@@ -368,6 +369,11 @@ uint32_t cryptoMultisigPubkeyCount(const MultisigRedeemScriptType *multisig) {
                                : multisig->pubkeys_count;
 }
 
+static int comparePubkeysLexicographically(const void *first,
+                                           const void *second) {
+  return memcmp(first, second, 33);
+}
+
 uint32_t cryptoMultisigPubkeys(const CoinInfo *coin,
                                const MultisigRedeemScriptType *multisig,
                                uint8_t *pubkeys) {
@@ -384,6 +390,11 @@ uint32_t cryptoMultisigPubkeys(const CoinInfo *coin,
     memcpy(pubkeys + i * 33, pubnode->public_key, 33);
   }
 
+  if (multisig->has_pubkeys_order &&
+      multisig->pubkeys_order == MultisigPubkeysOrder_LEXICOGRAPHIC) {
+    qsort(pubkeys, n, 33, comparePubkeysLexicographically);
+  }
+
   return n;
 }
 
@@ -399,9 +410,15 @@ int cryptoMultisigPubkeyIndex(const CoinInfo *coin,
   return -1;
 }
 
+static int comparePubnodesLexicographically(const void *first,
+                                            const void *second) {
+  return memcmp(*(const HDNodeType **)first, *(const HDNodeType **)second,
+                sizeof(HDNodeType));
+}
+
 int cryptoMultisigFingerprint(const MultisigRedeemScriptType *multisig,
                               uint8_t *hash) {
-  static const HDNodeType *pubnodes[15], *swap;
+  static const HDNodeType *pubnodes[15];
   const uint32_t n = cryptoMultisigPubkeyCount(multisig);
   if (n < 1 || n > 15) {
     return 0;
@@ -422,21 +439,20 @@ int cryptoMultisigFingerprint(const MultisigRedeemScriptType *multisig,
     if (pubnodes[i]->public_key.size != 33) return 0;
     if (pubnodes[i]->chain_code.size != 32) return 0;
   }
-  // minsort according to pubkey
-  for (uint32_t i = 0; i < n - 1; i++) {
-    for (uint32_t j = n - 1; j > i; j--) {
-      if (memcmp(pubnodes[i]->public_key.bytes, pubnodes[j]->public_key.bytes,
-                 33) > 0) {
-        swap = pubnodes[i];
-        pubnodes[i] = pubnodes[j];
-        pubnodes[j] = swap;
-      }
-    }
+
+  if (multisig->has_pubkeys_order &&
+      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.
+    qsort(pubnodes, n, sizeof(HDNodeType *), comparePubnodesLexicographically);
   }
-  // hash sorted nodes
+
   SHA256_CTX ctx = {0};
   sha256_Init(&ctx);
   sha256_Update(&ctx, (const uint8_t *)&(multisig->m), sizeof(uint32_t));
+  sha256_Update(&ctx, (const uint8_t *)&(multisig->pubkeys_order),
+                sizeof(uint32_t));
   for (uint32_t i = 0; i < n; i++) {
     sha256_Update(&ctx, (const uint8_t *)&(pubnodes[i]->depth),
                   sizeof(uint32_t));

legacy/firmware/fsm_msg_coin.h

@@ -296,10 +296,15 @@ void fsm_msgGetAddress(const GetAddress *msg) {
   }
 
   if (msg->has_show_display && msg->show_display) {
-    char desc[20] = {0};
+    char desc[29] = {0};
     int multisig_index = 0;
     if (msg->has_multisig) {
-      strlcpy(desc, "Multisig __ of __:", sizeof(desc));
+      if (msg->multisig.has_pubkeys_order &&
+          msg->multisig.pubkeys_order == MultisigPubkeysOrder_LEXICOGRAPHIC) {
+        strlcpy(desc, "Multisig __ of __ (sorted):", sizeof(desc));
+      } else {
+        strlcpy(desc, "Multisig __ of __:", sizeof(desc));
+      }
       const uint32_t m = msg->multisig.m;
       const uint32_t n = cryptoMultisigPubkeyCount(&(msg->multisig));
       desc[9] = (m < 10) ? ' ' : ('0' + (m / 10));

legacy/firmware/transaction.c

@@ -364,11 +364,6 @@ uint32_t compile_script_sig(uint32_t address_type, const uint8_t *pubkeyhash,
 uint32_t compile_script_multisig(const CoinInfo *coin,
                                  const MultisigRedeemScriptType *multisig,
                                  uint8_t *out) {
-  if (multisig->pubkeys_order != MultisigPubkeysOrder_PRESERVED) {
-    fsm_sendFailure(FailureType_Failure_DataError,
-                    _("Sortedmulti is not supported"));
-    return 0;
-  }
   const uint32_t m = multisig->m;
   const uint32_t n = cryptoMultisigPubkeyCount(multisig);
   if (m < 1 || m > 15) return 0;
@@ -402,12 +397,6 @@ uint32_t compile_script_multisig(const CoinInfo *coin,
 uint32_t compile_script_multisig_hash(const CoinInfo *coin,
                                       const MultisigRedeemScriptType *multisig,
                                       uint8_t *hash) {
-  if (multisig->pubkeys_order != MultisigPubkeysOrder_PRESERVED) {
-    fsm_sendFailure(FailureType_Failure_DataError,
-                    _("Sortedmulti is not supported"));
-    return 0;
-  }
-
   const uint32_t m = multisig->m;
   const uint32_t n = cryptoMultisigPubkeyCount(multisig);
   if (m < 1 || m > 15) return 0;
@@ -457,11 +446,6 @@ uint32_t serialize_script_sig(const uint8_t *signature, uint32_t signature_len,
 uint32_t serialize_script_multisig(const CoinInfo *coin,
                                    const MultisigRedeemScriptType *multisig,
                                    uint8_t sighash, uint8_t *out) {
-  if (multisig->pubkeys_order != MultisigPubkeysOrder_PRESERVED) {
-    fsm_sendFailure(FailureType_Failure_DataError,
-                    _("Sortedmulti is not supported"));
-    return 0;
-  }
   uint32_t r = 0;
 #if !BITCOIN_ONLY
   if (!coin->decred) {

tests/device_tests/bitcoin/test_getaddress.py

@@ -197,7 +197,6 @@ def test_altcoin_address_mac(client: Client):
 
 
 @pytest.mark.multisig
-@pytest.mark.models(skip="legacy", reason="Sortedmulti is not supported")
 def test_multisig_pubkeys_order(client: Client):
     xpub_internal = btc.get_public_node(client, parse_path("m/45h/0")).xpub
     xpub_external = btc.get_public_node(client, parse_path("m/44h/1")).xpub

tests/device_tests/bitcoin/test_multisig.py

@@ -162,7 +162,6 @@ def test_2_of_3(client: Client, chunkify: bool):
 
 
 @pytest.mark.multisig
-@pytest.mark.models(skip="legacy", reason="Sortedmulti is not supported")
 def test_pubkeys_order(client: Client):
     node_internal = btc.get_public_node(
         client, parse_path("m/45h/0"), coin_name="Bitcoin"

tests/device_tests/bitcoin/test_multisig_change.py

@@ -83,6 +83,30 @@ multisig_in3 = messages.MultisigRedeemScriptType(
     m=2,
 )
 
+multisig_in4 = messages.MultisigRedeemScriptType(
+    nodes=[NODE_EXT1, NODE_EXT2, NODE_INT],
+    address_n=[0, 0],
+    signatures=[b"", b"", b""],
+    m=2,
+    pubkeys_order=messages.MultisigPubkeysOrder.LEXICOGRAPHIC,
+)
+
+multisig_in5 = messages.MultisigRedeemScriptType(
+    nodes=[NODE_EXT2, NODE_EXT1, NODE_INT],
+    address_n=[0, 1],
+    signatures=[b"", b"", b""],
+    m=2,
+    pubkeys_order=messages.MultisigPubkeysOrder.LEXICOGRAPHIC,
+)
+
+multisig_in6 = messages.MultisigRedeemScriptType(
+    nodes=[NODE_EXT1, NODE_EXT3, NODE_INT],
+    address_n=[0, 1],
+    signatures=[b"", b"", b""],
+    m=2,
+    pubkeys_order=messages.MultisigPubkeysOrder.LEXICOGRAPHIC,
+)
+
 prev_hash_1, prev_tx_1 = forge_prevtx(
     [("3Ltgk5WPUMLcT2QvwRXKj9CWsYuAKqeHJ8", 50_000_000)]
 )
@@ -119,7 +143,51 @@ INP3 = messages.TxInputType(
     multisig=multisig_in3,
 )
 
-TX_API = {prev_hash_1: prev_tx_1, prev_hash_2: prev_tx_2, prev_hash_3: prev_tx_3}
+prev_hash_4, prev_tx_4 = forge_prevtx(
+    [("3HwrvQEfYw4wUvGHpGmixWB15HPgqrvTh1", 50_000_000)]
+)
+INP4 = messages.TxInputType(
+    address_n=[H_(45), 0, 0, 0],
+    amount=50_000_000,
+    prev_hash=prev_hash_4,
+    prev_index=0,
+    script_type=messages.InputScriptType.SPENDMULTISIG,
+    multisig=multisig_in4,
+)
+
+prev_hash_5, prev_tx_5 = forge_prevtx(
+    [("3Md42fbNjSH3qwnj5jDvT6CSzJKVXHiXSc", 34_500_000)]
+)
+INP5 = messages.TxInputType(
+    address_n=[H_(45), 0, 0, 1],
+    amount=34_500_000,
+    prev_hash=prev_hash_5,
+    prev_index=0,
+    script_type=messages.InputScriptType.SPENDMULTISIG,
+    multisig=multisig_in5,
+)
+
+prev_hash_6, prev_tx_6 = forge_prevtx(
+    [("35PBSvszuvxhEDypGYcUhEQDigvKY8C5Rc", 55_500_000)]
+)
+INP6 = messages.TxInputType(
+    address_n=[H_(45), 0, 0, 1],
+    amount=55_500_000,
+    prev_hash=prev_hash_6,
+    prev_index=0,
+    script_type=messages.InputScriptType.SPENDMULTISIG,
+    multisig=multisig_in6,
+)
+
+
+TX_API = {
+    prev_hash_1: prev_tx_1,
+    prev_hash_2: prev_tx_2,
+    prev_hash_3: prev_tx_3,
+    prev_hash_4: prev_tx_4,
+    prev_hash_5: prev_tx_5,
+    prev_hash_6: prev_tx_6,
+}
 
 
 def _responses(
@@ -373,10 +441,46 @@ def test_multisig_change_match_second(client: Client):
         )
 
 
-# inputs match, change mismatches (second tries to be change but isn't)
+# inputs match, change matches (first is change)
+def test_sorted_multisig_change_match_first(client: Client):
+    multisig_out1 = messages.MultisigRedeemScriptType(
+        nodes=[NODE_EXT1, NODE_INT, NODE_EXT2],
+        address_n=[1, 0],
+        signatures=[b"", b"", b""],
+        m=2,
+        pubkeys_order=messages.MultisigPubkeysOrder.LEXICOGRAPHIC,
+    )
+
+    out1 = messages.TxOutputType(
+        address_n=[H_(45), 0, 1, 0],
+        multisig=multisig_out1,
+        amount=40_000_000,
+        script_type=messages.OutputScriptType.PAYTOMULTISIG,
+    )
+
+    out2 = messages.TxOutputType(
+        address="3PkXLsY7AUZCrCKGvX8FfP2EawowUBMbcg",
+        amount=44_000_000,
+        script_type=messages.OutputScriptType.PAYTOADDRESS,
+    )
+
+    with client:
+        client.set_expected_responses(
+            _responses(client, INP4, INP5, change_indices=[1])
+        )
+        btc.sign_tx(
+            client,
+            "Bitcoin",
+            [INP4, INP5],
+            [out1, out2],
+            prev_txes=TX_API,
+        )
+
+
+# inputs match, change mismatches (second tries to be change but isn't because the pubkeys are in different order)
 def test_multisig_mismatch_multisig_change(client: Client):
     multisig_out2 = messages.MultisigRedeemScriptType(
-        nodes=[NODE_EXT1, NODE_INT, NODE_EXT3],
+        nodes=[NODE_EXT1, NODE_INT, NODE_EXT2],
         address_n=[1, 0],
         signatures=[b"", b"", b""],
         m=2,
@@ -406,6 +510,39 @@ def test_multisig_mismatch_multisig_change(client: Client):
         )
 
 
+# inputs match, change mismatches (second tries to be change but isn't because the pubkeys are different)
+def test_sorted_multisig_mismatch_multisig_change(client: Client):
+    multisig_out2 = messages.MultisigRedeemScriptType(
+        nodes=[NODE_EXT1, NODE_INT, NODE_EXT3],
+        address_n=[1, 0],
+        signatures=[b"", b"", b""],
+        m=2,
+    )
+
+    out1 = messages.TxOutputType(
+        address="3B23k4kFBRtu49zvpG3Z9xuFzfpHvxBcwt",
+        amount=40_000_000,
+        script_type=messages.OutputScriptType.PAYTOADDRESS,
+    )
+
+    out2 = messages.TxOutputType(
+        address_n=[H_(45), 0, 1, 0],
+        multisig=multisig_out2,
+        amount=44_000_000,
+        script_type=messages.OutputScriptType.PAYTOMULTISIG,
+    )
+
+    with client:
+        client.set_expected_responses(_responses(client, INP4, INP5))
+        btc.sign_tx(
+            client,
+            "Bitcoin",
+            [INP4, INP5],
+            [out1, out2],
+            prev_txes=TX_API,
+        )
+
+
 # inputs match, change mismatches (second tries to be change but isn't)
 @pytest.mark.models(skip="legacy", reason="Not fixed")
 def test_multisig_mismatch_multisig_change_different_paths(client: Client):
@@ -443,10 +580,10 @@ def test_multisig_mismatch_multisig_change_different_paths(client: Client):
         )
 
 
-# inputs mismatch, change matches with first input
+# inputs mismatch because the pubkeys are in different order, change matches with first input
 def test_multisig_mismatch_inputs(client: Client):
     multisig_out1 = messages.MultisigRedeemScriptType(
-        nodes=[NODE_EXT2, NODE_EXT1, NODE_INT],
+        nodes=[NODE_EXT1, NODE_EXT2, NODE_INT],
         address_n=[1, 0],
         signatures=[b"", b"", b""],
         m=2,
@@ -474,3 +611,37 @@ def test_multisig_mismatch_inputs(client: Client):
             [out1, out2],
             prev_txes=TX_API,
         )
+
+
+# inputs mismatch because the pubkeys are different, change matches with first input
+def test_sorted_multisig_mismatch_inputs(client: Client):
+    multisig_out1 = messages.MultisigRedeemScriptType(
+        nodes=[NODE_EXT1, NODE_EXT2, NODE_INT],
+        address_n=[1, 0],
+        signatures=[b"", b"", b""],
+        m=2,
+        pubkeys_order=messages.MultisigPubkeysOrder.LEXICOGRAPHIC,
+    )
+
+    out1 = messages.TxOutputType(
+        address_n=[H_(45), 0, 1, 0],
+        multisig=multisig_out1,
+        amount=40_000_000,
+        script_type=messages.OutputScriptType.PAYTOMULTISIG,
+    )
+
+    out2 = messages.TxOutputType(
+        address="3PkXLsY7AUZCrCKGvX8FfP2EawowUBMbcg",
+        amount=65_000_000,
+        script_type=messages.OutputScriptType.PAYTOADDRESS,
+    )
+
+    with client:
+        client.set_expected_responses(_responses(client, INP4, INP6))
+        btc.sign_tx(
+            client,
+            "Bitcoin",
+            [INP4, INP6],
+            [out1, out2],
+            prev_txes=TX_API,
+        )