fix(osmosis): version-gate the denom restriction

osmosis_sign_tx rejected every non-uosmo denomination unconditionally, so
the IBC and factory denoms firmware actually supports were unreachable
through the public helper -- a caller had to drive OsmosisMsgAck by hand.

Firmware commits the host-supplied denom to the signed Amino document from
7.14.2 on (osmosis_signTxUpdateMsgSend escapes it verbatim). Before 7.14.2
the serializer hardcoded uosmo and would sign a uosmo transfer the caller
never asked for, so the fail-closed behaviour is correct there and only
there. Gated on that boundary, matching thorchain_sign_tx.

Adds TestOsmosisClientDenom: forwarding on 7.15.0 and on 7.14.2, rejection
on 7.14.1, and uosmo still signing on legacy firmware. Offline, so it runs
without an emulator.

Also makes zcash_display_address's address_n optional. messages-zcash.proto
marks address_n and account each required only if the other is omitted, but
the required positional made the documented account-only form fail in Python
before a request was built.

Author: pastaghost

keepkeylib/client.py

@@ -1054,27 +1054,33 @@ def osmosis_sign_tx(
                 # OsmosisMsgSend.amount, which is a string field and would have
                 # raised even for uatom.
                 #
-                # This restriction is a HOST policy, not a firmware invariant.
+                # Version-gated exactly like thorchain_sign_tx above, and for
+                # the same reason.
+                #
                 # Firmware does not reject a non-uosmo denom on the
-                # OsmosisMsgAck path: since 7.14.2 (firmware c9dccf68),
+                # OsmosisMsgAck path. Since 7.14.2 (firmware c9dccf68)
                 # osmosis_signTxUpdateMsgSend escapes the host-supplied denom
-                # straight into the signed Amino document, which is what
+                # straight into the signed Amino document -- which is what
                 # test_osmosis_send_denom_is_committed_to_the_signature proves
-                # over the raw wire, and the only strcmp against "uosmo" left
+                # over the raw wire -- and the only strcmp against "uosmo" left
                 # in firmware picks the display exponent.
                 #
-                # The check stays because this helper is not version-gated and
-                # firmware older than 7.14.2 hardcoded "uosmo" in the
-                # serializer: it would ignore the denom sent here and sign a
-                # uosmo transfer the caller never asked for. Fail closed rather
-                # than silently mis-sign. A caller that needs an IBC or factory
-                # denom on 7.15 can drive OsmosisMsgAck directly, or this
-                # helper can grow the same version gate thorchain_sign_tx uses.
+                # BEFORE 7.14.2 the serializer hardcoded "uosmo": it would
+                # ignore the denom sent here and sign a uosmo transfer the
+                # caller never asked for. So fail closed there, and expose the
+                # field on firmware that actually commits it. An unconditional
+                # rejection made the supported IBC and factory-denom cases
+                # unreachable through this helper.
                 coin = msg['value']['amount'][0]
-                if coin['denom'] != 'uosmo':
+                firmware_version = (
+                    self.features.major_version,
+                    self.features.minor_version,
+                    self.features.patch_version,
+                )
+                if coin['denom'] != 'uosmo' and firmware_version < (7, 14, 2):
                     raise CallException(
                         "Osmosis.MsgSend",
-                        "Only uosmo is signable by Osmosis MsgSend (got %s)" %
+                        "Unsupported denomination before firmware 7.14.2: %s" %
                         coin['denom'])
                 resp = self.call(osmosis_proto.OsmosisMsgAck(
                     send=osmosis_proto.OsmosisMsgSend(
@@ -1900,7 +1906,7 @@ def ton_sign_message(self, address_n, message, show_display=False):
 
     # ── Zcash Address Display ─────────────────────────────────
     @expect(zcash_proto.ZcashAddress)
-    def zcash_display_address(self, address_n, account=None,
+    def zcash_display_address(self, address_n=None, account=None,
                               expected_seed_fingerprint=None):
         """Display a Zcash unified address on the device for user confirmation.
 
@@ -1910,7 +1916,9 @@ def zcash_display_address(self, address_n, account=None,
         are reserved on ZcashDisplayAddress).
 
         Args:
-            address_n: ZIP-32 derivation path [32', 133', account']
+            address_n: ZIP-32 derivation path [32', 133', account'].
+                Optional -- messages-zcash.proto marks it "required if account
+                omitted", so either form is valid and exactly one is needed.
             account: account index (alternative to full path)
             expected_seed_fingerprint: optional 32-byte ZIP-32 §6.1 seed
                 fingerprint. If provided, device verifies the match before
@@ -1920,7 +1928,16 @@ def zcash_display_address(self, address_n, account=None,
             ZcashAddress with .address and .seed_fingerprint of the
             attesting device.
         """
-        kwargs = dict(address_n=address_n)
+        # The protocol accepts EITHER form. Sending address_n unconditionally
+        # made the documented account-only call impossible: it failed in Python
+        # before a request was built.
+        if address_n is None and account is None:
+            raise ValueError(
+                "zcash_display_address needs address_n or account "
+                "(messages-zcash.proto: each is required if the other is omitted)")
+        kwargs = {}
+        if address_n is not None:
+            kwargs['address_n'] = address_n
         if account is not None:
             kwargs['account'] = account
         if expected_seed_fingerprint is not None:

tests/test_msg_osmosis_signtx.py

@@ -31,8 +31,9 @@
 
 from binascii import hexlify
 
+from keepkeylib import messages_pb2 as base_proto
 from keepkeylib import messages_osmosis_pb2 as osmosis_proto
-from keepkeylib.client import CallException
+from keepkeylib.client import CallException, ProtocolMixin
 from keepkeylib.tools import parse_path
 
 # Osmosis uses the Cosmos coin type (118), not one of its own.
@@ -232,5 +233,111 @@ def test_osmosis_signing_is_deterministic(self):
         self.assertEqual(hexlify(first.signature), hexlify(second.signature))
 
 
+class _SessionTransport(object):
+    def session_begin(self):
+        pass
+
+    def session_end(self):
+        pass
+
+
+class _ScriptedOsmosisClient(object):
+    """Offline driver for the public osmosis_sign_tx helper.
+
+    Mirrors _ScriptedThorchainClient in test_msg_thorchain_signtx.py: no
+    device, so the version gate can be exercised at both firmware versions in
+    a run that does not need an emulator.
+    """
+
+    osmosis_sign_tx = ProtocolMixin.osmosis_sign_tx
+
+    def __init__(self, version):
+        self.features = base_proto.Features(
+            major_version=version[0],
+            minor_version=version[1],
+            patch_version=version[2],
+        )
+        self.transport = _SessionTransport()
+        self.responses = [
+            osmosis_proto.OsmosisMsgRequest(),
+            osmosis_proto.OsmosisSignedTx(
+                public_key=b'\x02' + b'\x11' * 32,
+                signature=b'\x22' * 64,
+            ),
+        ]
+        self.sent = []
+
+    def call(self, message):
+        self.sent.append(message)
+        if not self.responses:
+            raise AssertionError('unexpected device call: %s' % type(message))
+        return self.responses.pop(0)
+
+
+class TestOsmosisClientDenom(unittest.TestCase):
+    """The public helper must reach the denominations firmware supports.
+
+    Firmware commits the host-supplied denom to the signed Amino document from
+    7.14.2 on (osmosis_signTxUpdateMsgSend escapes it verbatim), so an
+    unconditional uosmo-only check in the helper made every supported IBC and
+    factory denom unreachable except by driving OsmosisMsgAck by hand.
+    Before 7.14.2 the serializer hardcoded uosmo, so a non-uosmo send there
+    would sign a uosmo transfer the caller never asked for -- fail closed.
+    """
+
+    ADDRESS_N = [0x8000002C, 0x80000076, 0x80000000, 0, 0]
+    ADDR = 'osmo1ls33ayg26kmltw7jjy55p32ghjna09zp6z69y8'
+    IBC_DENOM = 'ibc/' + ('A' * 64)
+
+    def _sign(self, client, denom):
+        return client.osmosis_sign_tx(
+            address_n=self.ADDRESS_N,
+            account_number=92,
+            chain_id='osmosis-1',
+            fee=3000,
+            gas=200000,
+            msgs=[{
+                'type': 'osmosis-sdk/MsgSend',
+                'value': {
+                    'amount': [{'denom': denom, 'amount': '1500000'}],
+                    'from_address': self.ADDR,
+                    'to_address': self.ADDR,
+                },
+            }],
+            memo='client denom test',
+            sequence=3,
+        )
+
+    def test_ibc_denom_is_forwarded_on_7_15(self):
+        client = _ScriptedOsmosisClient((7, 15, 0))
+        response = self._sign(client, self.IBC_DENOM)
+
+        self.assertIsInstance(response, osmosis_proto.OsmosisSignedTx)
+        self.assertEqual(client.sent[1].send.denom, self.IBC_DENOM)
+
+    def test_ibc_denom_is_forwarded_on_7_14_2(self):
+        """7.14.2 is the first release whose serializer commits the denom."""
+        client = _ScriptedOsmosisClient((7, 14, 2))
+        response = self._sign(client, self.IBC_DENOM)
+
+        self.assertIsInstance(response, osmosis_proto.OsmosisSignedTx)
+        self.assertEqual(client.sent[1].send.denom, self.IBC_DENOM)
+
+    def test_non_uosmo_denom_is_rejected_before_7_14_2(self):
+        client = _ScriptedOsmosisClient((7, 14, 1))
+
+        with self.assertRaises(CallException) as ctx:
+            self._sign(client, self.IBC_DENOM)
+        self.assertIn('Unsupported denomination before firmware 7.14.2',
+                      str(ctx.exception))
+
+    def test_uosmo_still_signs_on_legacy_firmware(self):
+        client = _ScriptedOsmosisClient((7, 14, 1))
+        response = self._sign(client, 'uosmo')
+
+        self.assertIsInstance(response, osmosis_proto.OsmosisSignedTx)
+        self.assertEqual(client.sent[1].send.denom, 'uosmo')
+
+
 if __name__ == '__main__':
     unittest.main()