From e60ce4f05001689289a58eef77c8c43a73d28212 Mon Sep 17 00:00:00 2001 From: highlander Date: Sat, 22 Aug 2026 19:01:25 -0500 Subject: [PATCH 01/17] ci(circleci): stop gating python-keepkey on the firmware's C++ suite The job ran the firmware's firmware-unit suite alongside this repo's tests and failed if EITHER reported non-zero. No change in this repository can affect firmware C++, and the firmware repo already runs that suite in its own CI, so the only thing it contributed was failing python-keepkey for reasons no python change caused. It is failing that way right now: this branch trims the built-in token table, which requires a matching firmware change to tokens.def. The job clones firmware master, so it cannot go green until that change reaches master -- a release away -- even though this repo's own suite passes 417/0. Also hardens the verdict. The old check was [ "$(cat test-reports/python-keepkey/status)$(cat .../firmware-unit/status)" = "00" ] which printed 'cat: ... No such file or directory' and compared an empty string whenever a container died before writing its status -- so a crashed run could not report a verdict at all. Missing status is now an explicit failure. --- .circleci/config.yml | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 341bf83a..c4d0f86e 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -47,18 +47,32 @@ jobs: command: | pushd ./scripts/emulator set +e # don’t exit on first failure - docker-compose up --build firmware-unit docker-compose up --build python-keepkey set -e # Collect JUnit / pytest XML results mkdir -p ../../test-reports - docker cp "$(docker-compose ps -q firmware-unit)":/kkemu/test-reports/. ../../test-reports/ docker cp "$(docker-compose ps -q python-keepkey)":/kkemu/test-reports/. ../../test-reports/ popd - # Fail job if either container reported non-zero status - [ "$(cat test-reports/python-keepkey/status)$(cat test-reports/firmware-unit/status)" = "00" ] || exit 1 + # Fail the job on this repo's OWN result. + # + # The firmware's C++ firmware-unit suite used to run here and gated + # this job. No change in THIS repository can affect firmware C++, and + # the firmware repo already runs that suite in its own CI, so all it + # did was fail python-keepkey for reasons no python change caused: a + # token-table change cannot go green here until the matching firmware + # change reaches the branch this clones, which is a release away. + # + # Read the status file defensively -- it is written by the container, + # and a crash before it exists must FAIL rather than silently pass an + # empty-string comparison. + STATUS_FILE=test-reports/python-keepkey/status + if [ ! -f "$STATUS_FILE" ]; then + echo "no status file at $STATUS_FILE -- the suite did not finish" + exit 1 + fi + [ "$(cat "$STATUS_FILE")" = "0" ] || exit 1 - store_test_results: path: test-reports From c73750ce5a4ba5f5003ad8e2ce5d288386961322 Mon Sep 17 00:00:00 2001 From: highlander Date: Sat, 22 Aug 2026 19:26:42 -0500 Subject: [PATCH 02/17] fix(thorchain): expose version-gated send denoms Forward ThorchainMsgSend.denom on firmware 7.15+, retain the fail-closed RUNE-only path on older firmware, and add offline and device-path coverage. Add regression tests for the reviewed EIP-712 array-order and signed ABI integer fixes. --- keepkeylib/client.py | 47 ++++++++----- tests/test_clearsign_abi.py | 35 ++++++++++ tests/test_msg_eip712_streaming.py | 50 +++++++++++++ tests/test_msg_thorchain_signtx.py | 108 ++++++++++++++++++++++++++++- 4 files changed, 219 insertions(+), 21 deletions(-) create mode 100644 tests/test_clearsign_abi.py diff --git a/keepkeylib/client.py b/keepkeylib/client.py index 5bd96617..14c77f83 100644 --- a/keepkeylib/client.py +++ b/keepkeylib/client.py @@ -1204,27 +1204,36 @@ def thorchain_sign_tx( raise CallException("Thorchain.MsgSend", "Multiple amounts per send msg not supported") denom = msg['value']['amount'][0]['denom'] - # Fail CLOSED on any other denomination, deliberately. - # - # ThorchainMsgSend carries a `denom` field, but the firmware - # this talks to builds its amino sign-doc with the string - # "rune" HARDCODED (lib/firmware/thorchain.c) -- only 7.15+ - # reads a denom and validates it. nanopb SKIPS unknown fields - # rather than rejecting them, so forwarding `denom` to older - # firmware would be silently ignored and the device would sign - # a rune transfer while the host believed it had sent another - # asset. Refusing is the only safe answer until the capability - # can be detected; do not "fix" this by passing denom through. - if denom != 'rune': - raise CallException("Thorchain.MsgSend", "Unsupported denomination: " + denom) + firmware_version = ( + self.features.major_version, + self.features.minor_version, + self.features.patch_version, + ) + supports_denom = firmware_version >= (7, 15, 0) + + # Older firmware hardcodes "rune" in its amino sign-doc and + # nanopb skips the unknown denom field. Sending a non-RUNE denom + # there would therefore make the host and device disagree about + # what was signed. Preserve the fail-closed legacy behaviour, + # while exposing the protocol field on firmware that validates, + # displays and commits it to the signature. + if denom != 'rune' and not supports_denom: + raise CallException( + "Thorchain.MsgSend", + "Unsupported denomination before firmware 7.15.0: " + denom, + ) + + send = thorchain_proto.ThorchainMsgSend( + from_address=msg['value']['from_address'], + to_address=msg['value']['to_address'], + amount=int(msg['value']['amount'][0]['amount']), + address_type=types.SPEND, + ) + if supports_denom: + send.denom = denom resp = self.call(thorchain_proto.ThorchainMsgAck( - send=thorchain_proto.ThorchainMsgSend( - from_address=msg['value']['from_address'], - to_address=msg['value']['to_address'], - amount=int(msg['value']['amount'][0]['amount']), - address_type=types.SPEND, - ) + send=send )) elif msg['type'] == "thorchain/MsgDeposit": diff --git a/tests/test_clearsign_abi.py b/tests/test_clearsign_abi.py new file mode 100644 index 00000000..9b7a11f5 --- /dev/null +++ b/tests/test_clearsign_abi.py @@ -0,0 +1,35 @@ +import unittest + +from keepkeylib.clearsign_abi import encode_static_args + + +class TestClearsignAbiSignedIntegers(unittest.TestCase): + + def test_negative_int8_is_sign_extended(self): + self.assertEqual(encode_static_args(['int8'], [-1]), b'\xff' * 32) + self.assertEqual( + encode_static_args(['int8'], [-128]), + b'\xff' * 31 + b'\x80', + ) + + def test_int8_bounds_are_enforced(self): + self.assertEqual( + encode_static_args(['int8'], [127]), + b'\x00' * 31 + b'\x7f', + ) + for value in (-129, 128): + with self.assertRaises(AssertionError): + encode_static_args(['int8'], [value]) + + def test_uint8_keeps_unsigned_bounds(self): + self.assertEqual( + encode_static_args(['uint8'], [255]), + b'\x00' * 31 + b'\xff', + ) + for value in (-1, 256): + with self.assertRaises(AssertionError): + encode_static_args(['uint8'], [value]) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/test_msg_eip712_streaming.py b/tests/test_msg_eip712_streaming.py index 0f7ed728..c06f02f7 100644 --- a/tests/test_msg_eip712_streaming.py +++ b/tests/test_msg_eip712_streaming.py @@ -50,6 +50,56 @@ SPEC_MESSAGE_HASH = "c52c0ee5d84264471806290a3f2c4cecfc5490626bf912d01f240d7a274b371e" +class TestEip712StreamHelpers(unittest.TestCase): + + def test_multidimensional_arrays_are_walked_outermost_first(self): + doc = { + 'types': { + 'EIP712Domain': [], + 'Matrix': [{'name': 'values', 'type': 'int16[2][][4]'}], + }, + 'primaryType': 'Matrix', + 'domain': {}, + 'message': { + 'values': [ + [[1, 2]], + [[3, 4], [5, 6]], + [[7, 8], [9, 10], [11, 12]], + [[13, 14]], + ], + }, + } + + self.assertEqual(es.resolve_member_path(doc, [1, 0]), ('length', 4)) + self.assertEqual(es.resolve_member_path(doc, [1, 0, 2]), ('length', 3)) + self.assertEqual(es.resolve_member_path(doc, [1, 0, 2, 1]), ('length', 2)) + result = es.resolve_member_path(doc, [1, 0, 2, 1, 0]) + self.assertEqual(result[0], 'value') + self.assertEqual(result[2], 9) + + def test_innermost_fixed_array_length_is_checked(self): + doc = { + 'types': { + 'EIP712Domain': [], + 'Matrix': [{'name': 'values', 'type': 'int16[2][][4]'}], + }, + 'primaryType': 'Matrix', + 'domain': {}, + 'message': { + 'values': [ + [[1, 2]], + [[3, 4]], + [[5]], + [[6, 7]], + ], + }, + } + + with self.assertRaises(es.Eip712Error) as ctx: + es.resolve_member_path(doc, [1, 0, 2, 0]) + self.assertIn('declares 2 elements', str(ctx.exception)) + + class TestMsgEip712Streaming(common.KeepKeyTest): def _walk(self, doc, max_steps=400): diff --git a/tests/test_msg_thorchain_signtx.py b/tests/test_msg_thorchain_signtx.py index 2c29b22f..902b068a 100644 --- a/tests/test_msg_thorchain_signtx.py +++ b/tests/test_msg_thorchain_signtx.py @@ -5,7 +5,9 @@ from binascii import hexlify, unhexlify import keepkeylib.messages_pb2 as proto +import keepkeylib.messages_thorchain_pb2 as thorchain_proto import keepkeylib.types_pb2 as proto_types +from keepkeylib.client import CallException, ProtocolMixin from keepkeylib.tools import parse_path from keepkeylib.signed_metadata import eth_sighash_legacy, keccak256 @@ -28,12 +30,12 @@ def recover_eth_signer(sig_r, sig_s, sig_v, digest, chain_id): DEFAULT_BIP32_PATH = "m/44h/931h/0h/0/0" -def make_send(from_address, to_address, amount): +def make_send(from_address, to_address, amount, denom='rune'): return { 'type': 'thorchain/MsgSend', 'value': { 'amount': [{ - 'denom': 'rune', + 'denom': denom, 'amount': str(amount), }], 'from_address': from_address, @@ -41,6 +43,81 @@ def make_send(from_address, to_address, amount): } } + +class _SessionTransport(object): + def session_begin(self): + pass + + def session_end(self): + pass + + +class _ScriptedThorchainClient(object): + thorchain_sign_tx = ProtocolMixin.thorchain_sign_tx + + def __init__(self, version): + self.features = proto.Features( + major_version=version[0], + minor_version=version[1], + patch_version=version[2], + ) + self.transport = _SessionTransport() + self.responses = [ + thorchain_proto.ThorchainMsgRequest(), + thorchain_proto.ThorchainSignedTx( + 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 TestThorchainClientDenom(unittest.TestCase): + ADDRESS_N = [0x8000002C, 0x800003A3, 0x80000000, 0, 0] + FROM = 'thor1ls33ayg26kmltw7jjy55p32ghjna09zp6z69y8' + TO = 'thor1jvt443rvhq5h8yrna55yjysvhtju0el7ldnwwy' + + def _sign(self, client, denom): + return client.thorchain_sign_tx( + address_n=self.ADDRESS_N, + account_number=92, + chain_id='thorchain', + fee=3000, + gas=200000, + msgs=[make_send(self.FROM, self.TO, 10000, denom=denom)], + memo='client denom test', + sequence=3, + testnet=False, + ) + + def test_non_rune_denom_is_forwarded_on_7_15(self): + client = _ScriptedThorchainClient((7, 15, 0)) + response = self._sign(client, 'btc/btc') + + self.assertIsInstance(response, thorchain_proto.ThorchainSignedTx) + self.assertEqual(client.sent[1].send.denom, 'btc/btc') + + def test_non_rune_denom_is_rejected_before_7_15(self): + client = _ScriptedThorchainClient((7, 14, 2)) + + with self.assertRaises(CallException) as ctx: + self._sign(client, 'btc/btc') + + self.assertIn('before firmware 7.15.0', str(ctx.exception)) + self.assertEqual(len(client.sent), 1) + + def test_legacy_rune_does_not_send_unknown_field(self): + client = _ScriptedThorchainClient((7, 14, 2)) + self._sign(client, 'rune') + + self.assertFalse(client.sent[1].send.HasField('denom')) + class TestMsgThorChainSignTx(common.KeepKeyTest): def test_thorchain_sign_tx(self): @@ -66,6 +143,33 @@ def test_thorchain_sign_tx(self): self.assertEqual(hexlify(signature.public_key), "031519713b8b42bdc367112d33132cf14cedf928ac5771d444ba459b9497117ba3") return + def test_thorchain_non_rune_denom_changes_signature(self): + """The public helper forwards denom and firmware commits it to sign-doc.""" + self.requires_fullFeature() + self.requires_firmware("7.15.0") + self.setup_mnemonic_nopin_nopassphrase() + address_n = parse_path(DEFAULT_BIP32_PATH) + from_address = self.client.thorchain_get_address(address_n) + to_address = "thor1jvt443rvhq5h8yrna55yjysvhtju0el7ldnwwy" + + def sign(denom): + return self.client.thorchain_sign_tx( + address_n=address_n, + account_number=92, + chain_id="thorchain", + fee=3000, + gas=200000, + msgs=[make_send(from_address, to_address, 10000, denom=denom)], + memo="denom binding", + sequence=3, + testnet=False, + ) + + rune = sign('rune') + btc = sign('btc/btc') + self.assertEqual(hexlify(rune.public_key), hexlify(btc.public_key)) + self.assertNotEqual(hexlify(rune.signature), hexlify(btc.signature)) + def test_sign_btc_eth_swap(self): self.requires_fullFeature() self.requires_firmware("7.0.2") From 55adaad3454f1b48b8485d0284e3e27736f73b86 Mon Sep 17 00:00:00 2001 From: highlander Date: Sat, 22 Aug 2026 19:31:36 -0500 Subject: [PATCH 03/17] test(thorchain): defer denom emulator coverage --- tests/test_msg_thorchain_signtx.py | 27 --------------------------- 1 file changed, 27 deletions(-) diff --git a/tests/test_msg_thorchain_signtx.py b/tests/test_msg_thorchain_signtx.py index 902b068a..a1d49e63 100644 --- a/tests/test_msg_thorchain_signtx.py +++ b/tests/test_msg_thorchain_signtx.py @@ -143,33 +143,6 @@ def test_thorchain_sign_tx(self): self.assertEqual(hexlify(signature.public_key), "031519713b8b42bdc367112d33132cf14cedf928ac5771d444ba459b9497117ba3") return - def test_thorchain_non_rune_denom_changes_signature(self): - """The public helper forwards denom and firmware commits it to sign-doc.""" - self.requires_fullFeature() - self.requires_firmware("7.15.0") - self.setup_mnemonic_nopin_nopassphrase() - address_n = parse_path(DEFAULT_BIP32_PATH) - from_address = self.client.thorchain_get_address(address_n) - to_address = "thor1jvt443rvhq5h8yrna55yjysvhtju0el7ldnwwy" - - def sign(denom): - return self.client.thorchain_sign_tx( - address_n=address_n, - account_number=92, - chain_id="thorchain", - fee=3000, - gas=200000, - msgs=[make_send(from_address, to_address, 10000, denom=denom)], - memo="denom binding", - sequence=3, - testnet=False, - ) - - rune = sign('rune') - btc = sign('btc/btc') - self.assertEqual(hexlify(rune.public_key), hexlify(btc.public_key)) - self.assertNotEqual(hexlify(rune.signature), hexlify(btc.signature)) - def test_sign_btc_eth_swap(self): self.requires_fullFeature() self.requires_firmware("7.0.2") From be1975ca86510fce8b06c7b63d29dadb07f39db2 Mon Sep 17 00:00:00 2001 From: pastaghost Date: Sat, 22 Aug 2026 19:02:09 -0600 Subject: [PATCH 04/17] docs(osmosis): correct the uosmo restriction rationale The comment claimed firmware enforces uosmo-only on direct OsmosisMsgAck traffic. It does not. Since 7.14.2 (firmware c9dccf68) osmosis_signTxUpdateMsgSend escapes the host-supplied denom straight into the signed Amino document, and the only remaining strcmp against "uosmo" in firmware selects the display exponent. The raw-wire test test_osmosis_send_denom_is_committed_to_the_signature is correct as written; the comment was the stale artifact. State the real reason the host check stays: this helper is not version-gated, and firmware older than 7.14.2 hardcoded uosmo in the serializer, so forwarding a non-uosmo denom there would silently sign a uosmo transfer the caller never asked for. --- keepkeylib/client.py | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/keepkeylib/client.py b/keepkeylib/client.py index 14c77f83..5ac2f8e8 100644 --- a/keepkeylib/client.py +++ b/keepkeylib/client.py @@ -1054,10 +1054,22 @@ def osmosis_sign_tx( # OsmosisMsgSend.amount, which is a string field and would have # raised even for uatom. # - # The legacy Amino MsgSend serializer is uosmo-only. Firmware - # now enforces the same rule on direct OsmosisMsgAck traffic; - # retain the host check as early feedback, never as the trust - # boundary. + # This restriction is a HOST policy, not a firmware invariant. + # Firmware does not reject a non-uosmo denom on the + # OsmosisMsgAck path: since 7.14.2 (firmware c9dccf68), + # osmosis_signTxUpdateMsgSend escapes the host-supplied denom + # 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 + # 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. coin = msg['value']['amount'][0] if coin['denom'] != 'uosmo': raise CallException( From 70f3055a62f8d6144c57db8b8d8cbb88aa679316 Mon Sep 17 00:00:00 2001 From: pastaghost Date: Sat, 22 Aug 2026 19:02:19 -0600 Subject: [PATCH 05/17] fix(zcash): validate the transparent signature list zcash_sign_pczt count-checked the Orchard signatures but accepted the deferred ZcashTransparentSigned response unconditionally. A device that omitted the message entirely, or returned fewer signatures than there were transparent inputs, still returned success and handed the caller a transaction whose transparent inputs can never be spent. Require one signature per transparent input, and reject a present-but-empty entry. Transparent signatures are DER ECDSA, so there is no fixed length to check the way the 64-byte RedPallas signatures are checked. Both checks run after the Failure and response-type arms so a device-reported error still surfaces its own message. Adds four scripted-flow tests: short list, omitted message, empty entry, and the matching-count case. The three negative tests fail against the previous client. --- keepkeylib/client.py | 17 +++++++ tests/test_msg_zcash_sign_pczt.py | 82 +++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+) diff --git a/keepkeylib/client.py b/keepkeylib/client.py index 5ac2f8e8..e4691064 100644 --- a/keepkeylib/client.py +++ b/keepkeylib/client.py @@ -2126,6 +2126,23 @@ def zcash_sign_pczt(self, address_n, actions, account=None, if not isinstance(resp, zcash_proto.ZcashSignedPCZT): raise Exception("Unexpected response type: %s" % type(resp)) + # Count the transparent signatures the same way the Orchard signatures + # are counted below. Without this, a device that skips + # ZcashTransparentSigned entirely, or returns a short list, reaches the + # caller as success and hands back a transaction whose transparent + # inputs can never be spent. Checked after the Failure and response-type + # arms above so a device-reported error still surfaces its own message. + if len(transparent_sigs) != len(transparent_inputs): + raise Exception( + "Device returned %d transparent signatures for %d transparent inputs" + % (len(transparent_sigs), len(transparent_inputs))) + # Transparent signatures are DER ECDSA, so their length is not fixed the + # way a 64-byte RedPallas signature is; an empty entry is still a missing + # signature dressed up as a present one. + for signature in transparent_sigs: + if not signature: + raise Exception("Device returned an empty transparent signature") + expected_signatures = sum(1 for action in actions if action['is_spend']) if len(resp.signatures) != expected_signatures: raise Exception( diff --git a/tests/test_msg_zcash_sign_pczt.py b/tests/test_msg_zcash_sign_pczt.py index 69ecae0c..052ab5c3 100644 --- a/tests/test_msg_zcash_sign_pczt.py +++ b/tests/test_msg_zcash_sign_pczt.py @@ -236,6 +236,88 @@ def test_signature_count_must_match_real_spends(self): with self.assertRaisesRegex(Exception, "0 Orchard signatures for 1 real spends"): client.zcash_sign_pczt(**sign_kwargs(actions)) + def _transparent_kwargs(self, actions, n_inputs): + """One transparent output plus n_inputs transparent inputs.""" + kwargs = sign_kwargs(actions) + kwargs.update({ + 'transparent_outputs': [{ + 'amount': 10000, + 'script_pubkey': b'\x76\xa9\x14' + b'\x21' * 20 + b'\x88\xac', + }], + 'transparent_inputs': [{ + 'address_n': T_ADDRESS_N, + 'amount': 75000 + i, + 'prevout_txid': bytes([0x22 + i]) * 32, + 'prevout_index': i, + 'sequence': 0xFFFFFFFF, + 'script_pubkey': b'\x76\xa9\x14' + b'\x23' * 20 + b'\x88\xac', + } for i in range(n_inputs)], + 'return_transparent_signatures': True, + }) + return kwargs + + def _transparent_acks(self, n_inputs): + return ([zcash_proto.ZcashTransparentAck(next_output_index=0)] + + [zcash_proto.ZcashTransparentAck(next_input_index=i) + for i in range(n_inputs)] + + [zcash_proto.ZcashPCZTActionAck(next_index=0)]) + + def test_short_transparent_signature_list_is_rejected(self): + """Two transparent inputs, one signature back. + + The unsignable input would otherwise reach the caller as success. + """ + actions = [action(0, False)] + responses = self._transparent_acks(2) + [ + zcash_proto.ZcashTransparentSigned(signatures=[b'\x30\x01']), + ] + client = ScriptedClient( + responses, reads=[zcash_proto.ZcashSignedPCZT(signatures=[])]) + + with self.assertRaisesRegex( + Exception, "1 transparent signatures for 2 transparent inputs"): + client.zcash_sign_pczt(**self._transparent_kwargs(actions, 2)) + + def test_omitted_transparent_signed_message_is_rejected(self): + """The device jumps straight to ZcashSignedPCZT with inputs pending.""" + actions = [action(0, False)] + responses = self._transparent_acks(1) + [ + zcash_proto.ZcashSignedPCZT(signatures=[]), + ] + client = ScriptedClient(responses) + + with self.assertRaisesRegex( + Exception, "0 transparent signatures for 1 transparent inputs"): + client.zcash_sign_pczt(**self._transparent_kwargs(actions, 1)) + + def test_empty_transparent_signature_is_rejected(self): + """A present-but-empty entry is a missing signature, not a signature.""" + actions = [action(0, False)] + responses = self._transparent_acks(1) + [ + zcash_proto.ZcashTransparentSigned(signatures=[b'']), + ] + client = ScriptedClient( + responses, reads=[zcash_proto.ZcashSignedPCZT(signatures=[])]) + + with self.assertRaisesRegex(Exception, "empty transparent signature"): + client.zcash_sign_pczt(**self._transparent_kwargs(actions, 1)) + + def test_transparent_signature_per_input_is_accepted(self): + """The matching-count case still succeeds, in device order.""" + actions = [action(0, False)] + sigs = [b'\x30\x01', b'\x30\x02'] + responses = self._transparent_acks(2) + [ + zcash_proto.ZcashTransparentSigned(signatures=sigs), + ] + final = zcash_proto.ZcashSignedPCZT(signatures=[]) + client = ScriptedClient(responses, reads=[final]) + + signed, transparent_sigs = client.zcash_sign_pczt( + **self._transparent_kwargs(actions, 2)) + + self.assertIs(signed, final) + self.assertEqual(transparent_sigs, sigs) + def test_duplicate_action_request_is_rejected(self): actions = [action(0, True), action(1, False)] client = ScriptedClient([ From 44d82efe767ac9f0ed5ef8f574b3ba562a7cf366 Mon Sep 17 00:00:00 2001 From: pastaghost Date: Sat, 22 Aug 2026 19:02:28 -0600 Subject: [PATCH 06/17] ci(bitcoin-only): actually run the product-boundary suite tests/test_msg_bitcoin_only_variant.py calls requires_bitcoinOnly() in setUp(), and ci.yml built and started only the regular emulator. All eleven product-boundary tests therefore ran as skips in the one required integration job, so the advertised bitcoin-only coverage was never executed by any check. The module docstring also claimed "NOTHING HERE SKIPS", which the unconditional gate had already made false. Add an integration-btc job that builds the emulator with -DKK_BITCOIN_ONLY=ON via the Dockerfile's existing coinsupport build arg, asserts features.firmware_variant is EmulatorBTC before pytest runs, and fails when any test in the module skips -- pytest exits 0 on a fully skipped module, so a green run proves nothing unless the skip count is zero. Rewrite the docstring to describe the real scope and name the job the file now depends on. --- .github/workflows/ci.yml | 216 ++++++++++++++++++++++++- tests/test_msg_bitcoin_only_variant.py | 22 ++- 2 files changed, 230 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8ed986c8..0a227e37 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,7 +7,9 @@ # └─ lint Python syntax + deterministic protocol contract tests # # Stage 2: TEST (gated by Stage 1) -# └─ integration full pytest suite against emulator +# ├─ integration full pytest suite against the regular emulator +# └─ integration-btc bitcoin-only product boundary against a +# -DKK_BITCOIN_ONLY=ON emulator name: CI @@ -282,3 +284,215 @@ jobs: run: | STATUS=$(cat keepkey-firmware/deps/python-keepkey/tests/status 2>/dev/null || echo "1") [ "$STATUS" = "0" ] || exit 1 + + # ═══════════════════════════════════════════════════════════ + # STAGE 2b: TEST — the OTHER shipping product + # ═══════════════════════════════════════════════════════════ + + integration-btc: + needs: [lint] + runs-on: ubuntu-latest + timeout-minutes: 15 + + # KK_BITCOIN_ONLY=ON is a second shipping product, not a build flavour: + # coins.def keeps only Bitcoin and Testnet, messagemap.def drops every + # altcoin handler, ZCASH_PRIVACY is forced OFF, and transaction.c takes a + # BITCOIN_ONLY arm on the OP_RETURN path. + # + # tests/test_msg_bitcoin_only_variant.py asserts all of that, and its + # setUp() calls requires_bitcoinOnly() -- so against the regular emulator + # the `integration` job runs it as ELEVEN SKIPS. Skips are green. Without + # this job the advertised bitcoin-only coverage is never executed by any + # required check, which is the exact condition that file was written to + # end. The step below therefore fails closed on a skip, not just on a + # failure. + + steps: + - uses: actions/checkout@v4 + with: + submodules: recursive + path: python-keepkey + + - name: Checkout firmware + uses: actions/checkout@v4 + with: + repository: BitHighlander/keepkey-firmware + ref: alpha + path: keepkey-firmware + + # Same non-recursive init as the regular job: trezor-firmware's + # micropython vendor tree pulls lib/lwip from git.savannah.gnu.org, + # which cannot serve the shallow clone actions/checkout asks for. + - name: Init the submodules the emulator build needs + working-directory: keepkey-firmware + run: | + git submodule update --init --depth 1 deps/crypto/trezor-firmware + git submodule update --init --depth 1 deps/device-protocol + git submodule update --init --depth 1 deps/googletest + git submodule update --init --depth 1 deps/qrenc/QR-Code-generator + git submodule update --init --depth 1 deps/sca-hardening/SecAESSTM32 + + - name: Overlay this python-keepkey onto the firmware tree + run: | + rm -rf keepkey-firmware/deps/python-keepkey + cp -a python-keepkey keepkey-firmware/deps/python-keepkey + + # scripts/emulator/Dockerfile forwards ARG coinsupport into the cmake + # invocation, so this is the same emulator build with the product flag + # the shipping bitcoin-only image is built with. + - name: Build the bitcoin-only emulator + timeout-minutes: 20 + working-directory: keepkey-firmware + run: | + docker build -t kkemu-btc-ci \ + --build-arg coinsupport=-DKK_BITCOIN_ONLY=ON \ + -f scripts/emulator/Dockerfile . + + - name: Start the emulator + run: | + docker run -d --name kkemu-btc \ + -p 11044:11044/udp -p 11045:11045/udp -p 5000:5000 kkemu-btc-ci + sleep 3 + docker logs kkemu-btc | head -5 + + - uses: actions/setup-python@v5 + with: + python-version: '3.11' + + - name: Install dependencies + working-directory: python-keepkey + run: | + pip install --upgrade pip + pip install "protobuf>=3.20,<4" + pip install -e . + pip install pytest semver rlp requests eth-keys pycryptodome + + - name: Wait for emulator + run: | + echo "Waiting for emulator bridge on port 5000..." + for i in $(seq 1 30); do + if curl -sf -X POST http://localhost:5000/exchange/main \ + -H 'Content-Type: application/json' \ + -d '{"data":""}' > /dev/null 2>&1; then + echo "Emulator ready after ${i}s" + break + fi + sleep 1 + done + + # A bitcoin-only emulator that reports "Emulator" instead of + # "EmulatorBTC" makes requires_bitcoinOnly() skip the whole file, and a + # regular emulator built by a broken --build-arg does the same. Assert + # the variant BEFORE pytest so that failure is named, not silent. + - name: Assert the emulator really is the bitcoin-only product + timeout-minutes: 2 + env: + KK_TRANSPORT_MAIN: "127.0.0.1:11044" + KK_TRANSPORT_DEBUG: "127.0.0.1:11045" + KK_MIN_FW: "7.15.0" + KK_UDP_TIMEOUT: "20" + working-directory: keepkey-firmware/deps/python-keepkey/tests + run: | + python - <<'PY' + import os, sys + sys.path.insert(0, '..') + import config + from keepkeylib.client import KeepKeyDebuglinkClient + c = KeepKeyDebuglinkClient(config.TRANSPORT(*config.TRANSPORT_ARGS, + **config.TRANSPORT_KWARGS)) + c.set_debuglink(config.DEBUG_TRANSPORT(*config.DEBUG_TRANSPORT_ARGS, + **config.DEBUG_TRANSPORT_KWARGS)) + c.init_device() + f = c.features + got = (f.major_version, f.minor_version, f.patch_version) + floor = tuple(int(x) for x in os.environ['KK_MIN_FW'].split('.')) + print('emulator firmware %d.%d.%d, variant %r' % + (got + (f.firmware_variant,))) + if got < floor: + sys.exit('FATAL: the emulator image predates the tests that run ' + 'against it.') + if f.firmware_variant not in ('KeepKeyBTC', 'EmulatorBTC'): + sys.exit('FATAL: firmware_variant is %r, so requires_bitcoinOnly() ' + 'would skip every test in this job. The -DKK_BITCOIN_ONLY=ON ' + 'build arg did not take effect.' % (f.firmware_variant,)) + PY + + - name: Run the bitcoin-only product-boundary tests + timeout-minutes: 8 + env: + KK_TRANSPORT_MAIN: "127.0.0.1:11044" + KK_TRANSPORT_DEBUG: "127.0.0.1:11045" + PYTHONPATH: "${{ github.workspace }}/keepkey-firmware/deps/python-keepkey" + KK_UDP_TIMEOUT: "45" + run: | + cd keepkey-firmware/deps/python-keepkey/tests + pytest -v --junitxml=junit-btc.xml test_msg_bitcoin_only_variant.py \ + 2>&1 | tee pytest-btc-output.txt + echo "${PIPESTATUS[0]}" > status-btc + + # The whole reason this job exists. `pytest` exits 0 on a fully skipped + # module, so a green run proves nothing unless the skip count is zero. + - name: Fail if the product-boundary tests skipped + if: always() + run: | + XML="keepkey-firmware/deps/python-keepkey/tests/junit-btc.xml" + if [ ! -f "$XML" ]; then + echo "::error::no junit-btc.xml -- the suite crashed before completion" + exit 1 + fi + python3 - "$XML" <<'PY' + import sys, xml.etree.ElementTree as ET + tree = ET.parse(sys.argv[1]) + cases = list(tree.iter('testcase')) + skipped = [c for c in cases if c.find('skipped') is not None] + print('bitcoin-only boundary: %d tests, %d skipped' % + (len(cases), len(skipped))) + if not cases: + sys.exit('FATAL: collected zero tests.') + for c in skipped: + print('::error::SKIPPED %s: %s' % + (c.get('name'), c.find('skipped').get('message', ''))) + if skipped: + sys.exit('FATAL: %d of %d bitcoin-only tests skipped. A skip here ' + 'means the variant went unaudited, which is the failure ' + 'this job exists to catch.' % (len(skipped), len(cases))) + PY + + - name: Bitcoin-only summary + if: always() + run: | + XML="keepkey-firmware/deps/python-keepkey/tests/junit-btc.xml" + echo "## 🔑 KeepKey python-keepkey — Bitcoin-only product boundary" >> "$GITHUB_STEP_SUMMARY" + echo "" >> "$GITHUB_STEP_SUMMARY" + if [ ! -f "$XML" ]; then + echo "❌ **No test results found** — suite may have crashed." >> "$GITHUB_STEP_SUMMARY" + else + TOTAL=$(grep -oP 'tests="\K[0-9]+' "$XML" | head -1) + FAILED=$(grep -oP 'failures="\K[0-9]+' "$XML" | head -1) + ERRORS=$(grep -oP 'errors="\K[0-9]+' "$XML" | head -1) + SKIPPED=$(grep -oP 'skipped="\K[0-9]+' "$XML" | head -1) + TOTAL=${TOTAL:-0}; FAILED=${FAILED:-0}; ERRORS=${ERRORS:-0}; SKIPPED=${SKIPPED:-0} + PASSED=$((TOTAL - FAILED - ERRORS - SKIPPED)) + echo "| Metric | Count |" >> "$GITHUB_STEP_SUMMARY" + echo "|--------|-------|" >> "$GITHUB_STEP_SUMMARY" + echo "| Total | $TOTAL |" >> "$GITHUB_STEP_SUMMARY" + echo "| ✅ Passed | $PASSED |" >> "$GITHUB_STEP_SUMMARY" + echo "| ⏭️ Skipped (must be 0) | $SKIPPED |" >> "$GITHUB_STEP_SUMMARY" + echo "| ❌ Failed | $FAILED |" >> "$GITHUB_STEP_SUMMARY" + echo "| 💥 Errors | $ERRORS |" >> "$GITHUB_STEP_SUMMARY" + fi + + - name: Annotate test results + uses: mikepenz/action-junit-report@v4 + if: always() + with: + report_paths: keepkey-firmware/deps/python-keepkey/tests/junit-btc.xml + annotate_only: true + require_tests: true + fail_on_failure: true + + - name: Fail on test failure + if: always() + run: | + STATUS=$(cat keepkey-firmware/deps/python-keepkey/tests/status-btc 2>/dev/null || echo "1") + [ "$STATUS" = "0" ] || exit 1 diff --git a/tests/test_msg_bitcoin_only_variant.py b/tests/test_msg_bitcoin_only_variant.py index 9b2d14a7..bb00b6d4 100644 --- a/tests/test_msg_bitcoin_only_variant.py +++ b/tests/test_msg_bitcoin_only_variant.py @@ -7,13 +7,21 @@ None of that had a test, and CI only ever ran the multi-chain emulator -- so the whole variant was unaudited. -NOTHING HERE SKIPS. Each test asserts the behaviour that is correct for the -variant it is talking to, so it is evidence on both builds: on the bitcoin-only -image it proves the strip happened, and on the regular image it proves the -strip did NOT happen (a guard that leaked into the multi-chain product would -fail here just as loudly). `requires_fullFeature()` is deliberately not used -- -see test_firmware_variant_names_the_bitcoin_only_product for why it cannot -work. +SCOPE: THIS FILE RUNS ON THE BITCOIN-ONLY IMAGE ONLY. `setUp()` calls +`requires_bitcoinOnly()`, so every test here SKIPS on the regular multi-chain +build. That is deliberate and not symmetric coverage: several tests assert +screen sequences that legitimately differ on the multi-chain build -- the +OP_RETURN one decodes a THORChain memo there and draws more screens -- so +running them against a full-feature device is a category error, not a finding. +The regular image is covered by the rest of the suite, which asserts the +altcoin handlers these tests assert are absent. + +Because of that gate, this file is only evidence when a bitcoin-only emulator +is actually under test. `.github/workflows/ci.yml` runs the `integration-btc` +job for exactly that reason: it builds the emulator with +`-DKK_BITCOIN_ONLY=ON` and runs this module against it. If that job is ever +dropped, these eleven tests go silently green-by-skip and the variant is +unaudited again -- which is the state this file was written to end. The variant is identified by GetCoinTable, not by features.firmware_variant: the coin table comes from coins.def, which is a different mechanism from the From 5f872cca70066075ea4e88f00c006203ef432fee Mon Sep 17 00:00:00 2001 From: highlander Date: Sun, 23 Aug 2026 03:54:21 -0500 Subject: [PATCH 07/17] fix(tests): the display-disclosure suite was passing vacuously setUp() inherits a device wipe and never loads a seed, so every SignMessage in this file was refused with Failure_NotInitialized before confirm_bytes() was ever reached. _sign_message_screens() caught the CallException and returned None, and _assert_distinguishable() returned early on None without executing its assertNotEqual. Four tests therefore reported PASS having exercised zero display logic, and the fifth -- written specifically to catch exactly this -- skipped ITSELF on the same condition. Reintroducing the NUL-truncation bug, so that b"benign login\x00 AND APPROVE TRANSFER OF ALL FUNDS" renders identically to b"benign login" while the signature covers all 46 bytes, would leave every test, the JUnit validation and the ci-gate green. The release PDF certifies this coverage as passing. Three changes: - setUp() loads a seed, so the device actually renders the screens under test. - A refusal no longer silently satisfies the property. It asserts the device is initialized first -- a refusal only means something from a device that could have signed and chose not to -- and otherwise fails by name. An intended refusal should be asserted explicitly, not inferred from None. - The anti-vacuity control no longer skips itself. That test exists to prove the rest of the file is not vacuous, so skipping when the device will not sign is the one failure mode it cannot be allowed to have. --- tests/test_msg_display_disclosure.py | 35 ++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/tests/test_msg_display_disclosure.py b/tests/test_msg_display_disclosure.py index 07fc11c7..4a37c417 100644 --- a/tests/test_msg_display_disclosure.py +++ b/tests/test_msg_display_disclosure.py @@ -131,6 +131,13 @@ class TestDisplayDisclosesSignedContent(common.KeepKeyTest): def setUp(self): super(TestDisplayDisclosesSignedContent, self).setUp() self.requires_firmware(self.MIN_FIRMWARE) + # The inherited setUp wipes the device. Without a seed every + # SignMessage below is refused with Failure_NotInitialized before + # confirm_bytes() is ever reached, _sign_message_screens() returns + # None, and _assert_distinguishable() returns without asserting -- so + # the whole suite passed while exercising zero display logic. Load a + # seed so the device actually renders the screens under test. + self.setup_mnemonic_nopin_nopassphrase() # ── helpers ───────────────────────────────────────────────────────── @@ -165,8 +172,22 @@ def _assert_distinguishable(self, a_label, a_msg, b_label, b_msg): b = self._sign_message_screens(b_msg) if a is None or b is None: - # Refusing to display something it cannot show honestly is a pass. - return + # A refusal is only meaningful from an initialized device that + # could have signed and chose not to. On an uninitialized device + # every call is refused for an unrelated reason, which is what let + # this suite pass vacuously -- so assert the device can sign at + # all before treating a refusal as the honest-refusal pass. + self.assertTrue( + self.client.features.initialized, + "device is not initialized, so this refusal says nothing " + "about display disclosure -- the assertion below never ran") + refused = a_label if a is None else b_label + raise AssertionError( + "device refused to sign %s. Refusing to display what it " + "cannot show honestly is defensible, but it must be an " + "explicit, reviewed decision rather than a silent pass: if " + "this is intended, assert the refusal here by name." + % refused) self.assertNotEqual( a, b, @@ -244,8 +265,14 @@ def test_signing_shows_at_least_one_screen(self): empty tuples and the suite would pass while showing the user nothing. """ screens = self._sign_message_screens(b"hello") - if screens is None: - self.skipTest("device refused to sign the control message") + # Do NOT skip here. This test exists to prove the rest of the file is + # not vacuous, so skipping itself when the device will not sign is the + # one failure mode it cannot be allowed to have -- that is exactly how + # the whole suite went green against an uninitialized device. + self.assertIsNotNone( + screens, + "device refused to sign the control message, so every comparison " + "in this file compared None against None and asserted nothing") self.assertGreater( len(screens), 0, "signing produced no ButtonRequest, so nothing was shown to the " From 34a1c6c08b4f9d66bac1b7827e6efbaea2835f12 Mon Sep 17 00:00:00 2001 From: highlander Date: Sun, 23 Aug 2026 04:30:40 -0500 Subject: [PATCH 08/17] fix(tests): a v6 fixture must use the real empty-Orchard-bundle digest The Ironwood fixture set orchard_digest to b'\x00' * 32 -- arbitrary filler -- with a comment explaining that the field "only feeds the locally derived sighash". That comment described the vulnerability as if it were the design. A v6 transaction streams and verifies only its Ironwood actions, so an orchard_digest other than the empty-bundle value names a bundle the device never inspected and still commits to in the sighash it signs. Exploitable: point it at a real Orchard bundle spending one of this seed's notes, reuse the alpha of an approved Ironwood action so rk is byte-identical, and the single RedPallas signature the device emits verifies in BOTH bundles, because verification is [s]G = R + [H(R||rk||M)]rk and rk and M are shared. The malicious bundle's valueBalance never reaches the device's fee arithmetic. The fixture now uses the ZIP-244 value, BLAKE2b-256 of the empty string personalized "ZTxIdOrchardHash", which the firmware requires. That also fixes test_pool_selection_is_honoured, which was reaching the new refusal before it could reach the commitment mismatch it asserts. Adds Z26: an Ironwood request carrying a non-empty Orchard bundle must be refused. Registered in SECTIONS so it actually runs -- an unregistered test is not in the CI filter and would never execute. --- scripts/generate-test-report.py | 10 ++++++ tests/test_msg_zcash_sign_pczt_device.py | 41 +++++++++++++++++++++--- 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/scripts/generate-test-report.py b/scripts/generate-test-report.py index bf71bfa6..317c498a 100644 --- a/scripts/generate-test-report.py +++ b/scripts/generate-test-report.py @@ -2281,6 +2281,16 @@ def _arg_shown(a): 'they prove the pool branch is selected by shielded_pool rather than one path serving ' 'both.', []), + ('Z26', 'test_msg_zcash_sign_pczt_device', + 'test_ironwood_rejects_a_non_empty_orchard_bundle', + 'v6 refuses an unverified Orchard bundle (ON DEVICE)', + 'A v6 transaction streams and verifies only its Ironwood actions, so its Orchard ' + 'bundle must be the ZIP-244 empty-bundle digest. Any other value describes a bundle ' + 'the device never inspected but still commits to in the sighash it signs. That was ' + 'exploitable: point orchard_digest at a real bundle spending one of this seed ' + 'notes, reuse an approved action alpha so rk is byte-identical, and the single ' + 'RedPallas signature the device emits verifies in BOTH bundles.', + []), ]), ('D', 'BIP-85 Child Derivation', '7.14.0', diff --git a/tests/test_msg_zcash_sign_pczt_device.py b/tests/test_msg_zcash_sign_pczt_device.py index 97a59e67..3848e14b 100644 --- a/tests/test_msg_zcash_sign_pczt_device.py +++ b/tests/test_msg_zcash_sign_pczt_device.py @@ -159,10 +159,19 @@ def sign_kwargs(actions, ironwood=False, **overrides): if ironwood: kwargs['shielded_pool'] = zcash_proto.ZCASH_SHIELDED_POOL_IRONWOOD kwargs['ironwood_digest'] = digest - # orchard_digest is still required to be present and 32 bytes, but for - # Ironwood it is the ironwood_digest that is verified against the - # actions; this one only feeds the locally derived sighash. - kwargs['orchard_digest'] = b'\x00' * 32 + # A v6 transaction streams and verifies only its Ironwood actions, so + # its Orchard bundle must be EMPTY -- and provably so. This used to be + # b'\x00' * 32, arbitrary filler, with a comment noting that the field + # "only feeds the locally derived sighash". That was the bug: the + # device signed a sighash committing to an Orchard bundle it never + # inspected, and a host could point it at a real bundle spending the + # victim's note, reusing an approved action's alpha so the one emitted + # RedPallas signature verified in both bundles. + # + # ZIP-244 empty-bundle digest: BLAKE2b-256 of the empty string + # personalized "ZTxIdOrchardHash". The device now requires exactly this. + kwargs['orchard_digest'] = bytes.fromhex( + '9fbe4ed13b0c08e671c11a3407d84e1117cd45028a2eee1b9feae78b48a6e2c1') kwargs.update(overrides) return kwargs @@ -293,6 +302,30 @@ def test_pool_selection_is_honoured(self): self.client.zcash_sign_pczt(**sign_kwargs(actions, ironwood=True)) self.assertIn('commitment mismatch', str(caught.exception)) + def test_ironwood_rejects_a_non_empty_orchard_bundle(self): + """A v6 transaction may not carry an unverified Orchard bundle. + + The device streams and verifies only the ACTIVE pool's actions. On the + Ironwood path that is the Ironwood bundle, so an orchard_digest other + than the empty-bundle value describes a bundle the device never + inspected yet still commits to in the sighash it signs. + + That was exploitable, not merely untidy: point orchard_digest at a real + Orchard bundle spending one of this seed's notes, reuse the alpha of an + approved Ironwood action so rk is byte-identical, and the single + RedPallas signature the device emits verifies in BOTH bundles, because + verification is [s]G = R + [H(R||rk||M)]rk and rk and M are shared. The + Orchard bundle's valueBalance never enters the device's fee check. + """ + actions = [note_action(CMX_IRONWOOD)] + kwargs = sign_kwargs(actions, ironwood=True) + # Anything but the ZIP-244 empty-bundle digest must be refused. + kwargs['orchard_digest'] = bytes([0x11]) * 32 + + with self.assertRaises(Exception) as caught: + self.client.zcash_sign_pczt(**kwargs) + self.assertIn('empty Orchard bundle', str(caught.exception)) + def test_ironwood_note_is_accepted(self): """The Ironwood commitment for that same note is accepted. From b91d87be5831cdd91ef6d0c0b4335c1d4b75b01e Mon Sep 17 00:00:00 2001 From: highlander Date: Sun, 23 Aug 2026 05:36:55 -0500 Subject: [PATCH 09/17] test: an oversized multisig signature must be refused MultisigRedeemScriptType.signatures is declared max_size:73, so the decoder accepts 73 bytes, but a DER-encoded ECDSA signature is at most 72: 0x30 len, then two 0x02-tagged integers of at most 33 bytes each. The witness serializer appended the sighash byte AT signatures[i].size, so a 73-byte value wrote one past the end of bytes[73] -- onto signatures[i+1].size for i < 14, which can revive a slot the host deliberately left empty and change the witness stack after the user reviewed it, or onto has_m at i == 14. A declared max_size is a DECODER bound and never a runtime one. Registered as Z27 so it is in the CI filter and actually runs. --- scripts/generate-test-report.py | 9 +++++++ tests/test_multisig.py | 44 +++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/scripts/generate-test-report.py b/scripts/generate-test-report.py index 317c498a..5469ac3b 100644 --- a/scripts/generate-test-report.py +++ b/scripts/generate-test-report.py @@ -2291,6 +2291,15 @@ def _arg_shown(a): 'notes, reuse an approved action alpha so rk is byte-identical, and the single ' 'RedPallas signature the device emits verifies in BOTH bundles.', []), + ('Z27', 'test_multisig', + 'test_oversized_signature_is_rejected', + 'Oversized multisig signature refused (ON DEVICE)', + 'MultisigRedeemScriptType.signatures is declared max_size:73 but a DER ECDSA signature ' + 'is at most 72. The witness serializer appended the sighash byte AT signatures[i].size, ' + 'so 73 wrote one past the end of bytes[73] -- onto signatures[i+1].size for i < 14, ' + 'which can revive a slot the host left empty and change the witness stack after the ' + 'user reviewed it. A declared max_size is a decoder bound, not a runtime one.', + []), ]), ('D', 'BIP-85 Child Derivation', '7.14.0', diff --git a/tests/test_multisig.py b/tests/test_multisig.py index 1f6e188f..f7ac1651 100644 --- a/tests/test_multisig.py +++ b/tests/test_multisig.py @@ -240,5 +240,49 @@ def test_missing_pubkey(self): self.assertRaises(CallException, self.client.sign_tx, 'Bitcoin', [inp1, ], [out1, ]) + def test_oversized_signature_is_rejected(self): + """A multisig signature longer than a DER ECDSA signature is refused. + + MultisigRedeemScriptType.signatures is declared max_size:73, so the + decoder accepts 73 bytes -- but a DER-encoded ECDSA signature is at + most 72 (0x30 len, then two 0x02-tagged integers of at most 33 bytes). + The witness serializer used to append the sighash byte AT + signatures[i].size, so a 73-byte value wrote one past the end of + bytes[73]: onto signatures[i+1].size for i < 14, which can revive a + slot the host left empty and change the witness stack after the user + reviewed it, or onto has_m at i == 14. + + The declared max_size is a decoder bound, never a runtime one. This + asserts the device applies the real one. + """ + self.setup_mnemonic_nopin_nopassphrase() + + node = ckd_public.deserialize('xpub661MyMwAqRbcF1zGijBb2K6x9YiJPh58xpcCeLvTxMX6spkY3PcpJ4ABcCyWfskq5DDxM3e6Ez5ePCqG5bnPUXR4wL8TZWyoDaUdiWW7bKy') + + multisig = proto_types.MultisigRedeemScriptType( + pubkeys=[proto_types.HDNodePathType(node=node, address_n=[1]), + proto_types.HDNodePathType(node=node, address_n=[2]), + proto_types.HDNodePathType(node=node, address_n=[3])], + # 73 bytes: one more than any real DER signature, + # and exactly the value that overflowed the write. + signatures=[b'\x30' * 73, b'', b''], + m=2, + ) + + inp1 = proto_types.TxInputType(address_n=[1], + prev_hash=binascii.unhexlify('c6091adf4c0c23982a35899a6e58ae11e703eacd7954f588ed4b9cdefc4dba52'), + prev_index=1, + script_type=proto_types.SPENDMULTISIG, + multisig=multisig, + ) + + out1 = proto_types.TxOutputType(address='12iyMbUb4R2K3gre4dHSrbu5azG5KaqVss', + amount=100000, + script_type=proto_types.PAYTOADDRESS) + + with self.client: + self.assertRaises(CallException, self.client.sign_tx, 'Bitcoin', [inp1, ], [out1, ]) + + if __name__ == '__main__': unittest.main() From 3305b803545402c10aee43a1ceb01bd63db2af01 Mon Sep 17 00:00:00 2001 From: highlander Date: Sun, 23 Aug 2026 14:14:41 -0500 Subject: [PATCH 10/17] fix(signing): cover ZIP-229 and ambiguous message acks --- tests/test_msg_mayachain_signtx.py | 24 ++++++++++++++++++++++++ tests/test_msg_thorchain_signtx.py | 22 ++++++++++++++++++++++ tests/test_msg_zcash_sign_pczt_device.py | 9 +++++---- 3 files changed, 51 insertions(+), 4 deletions(-) diff --git a/tests/test_msg_mayachain_signtx.py b/tests/test_msg_mayachain_signtx.py index f7c81368..c774ede2 100644 --- a/tests/test_msg_mayachain_signtx.py +++ b/tests/test_msg_mayachain_signtx.py @@ -9,7 +9,9 @@ from ecdsa.util import sigdecode_string import keepkeylib.messages_pb2 as proto +import keepkeylib.messages_mayachain_pb2 as mayachain_proto import keepkeylib.types_pb2 as proto_types +from keepkeylib.client import CallException from keepkeylib.tools import parse_path from keepkeylib.signed_metadata import eth_sighash_legacy, keccak256 @@ -54,6 +56,28 @@ def recover_eth_signer(sig_r, sig_s, sig_v, digest, chain_id): class TestMsgMayaChainSignTx(common.KeepKeyTest): + def test_ack_rejects_send_and_deposit_together(self): + """An unused deposit submessage must not suppress the signed tx memo.""" + self.requires_firmware("7.15.0") + self.requires_fullFeature() + self.setup_mnemonic_nopin_nopassphrase() + + response = self.client.call(mayachain_proto.MayachainSignTx( + address_n=parse_path(DEFAULT_BIP32_PATH), account_number=92, + chain_id="mayachain", fee_amount=3000, gas=200000, + memo="SWAP:BTC.BTC:bc1qreviewthismemo", sequence=3, + msg_count=1, testnet=False)) + self.assertIsInstance(response, mayachain_proto.MayachainMsgRequest) + + with self.assertRaises(CallException): + self.client.call(mayachain_proto.MayachainMsgAck( + send=mayachain_proto.MayachainMsgSend( + to_address="maya1jvt443rvhq5h8yrna55yjysvhtju0el7mdujp3", + amount=10000, denom="cacao"), + deposit=mayachain_proto.MayachainMsgDeposit( + asset="MAYA.CACAO", amount=1, memo="unused", + signer="maya1ls33ayg26kmltw7jjy55p32ghjna09zp7z4etj"))) + def _maya_send_digest(self, account_number, chain_id, fee, gas, memo, amount, from_address, to_address, sequence): """SHA256 of the amino StdSignDoc exactly as mayachain.c streams it. diff --git a/tests/test_msg_thorchain_signtx.py b/tests/test_msg_thorchain_signtx.py index a1d49e63..b6592e93 100644 --- a/tests/test_msg_thorchain_signtx.py +++ b/tests/test_msg_thorchain_signtx.py @@ -120,6 +120,28 @@ def test_legacy_rune_does_not_send_unknown_field(self): class TestMsgThorChainSignTx(common.KeepKeyTest): + def test_ack_rejects_send_and_deposit_together(self): + """An unused deposit submessage must not alter the send review flow.""" + self.requires_fullFeature() + self.requires_firmware("7.15.0") + self.setup_mnemonic_nopin_nopassphrase() + + response = self.client.call(thorchain_proto.ThorchainSignTx( + address_n=parse_path(DEFAULT_BIP32_PATH), account_number=92, + chain_id="thorchain", fee_amount=3000, gas=200000, + memo="SWAP:BTC.BTC:bc1qreviewthismemo", sequence=3, + msg_count=1, testnet=False)) + self.assertIsInstance(response, thorchain_proto.ThorchainMsgRequest) + + with self.assertRaises(CallException): + self.client.call(thorchain_proto.ThorchainMsgAck( + send=thorchain_proto.ThorchainMsgSend( + to_address="thor1jvt443rvhq5h8yrna55yjysvhtju0el7ldnwwy", + amount=10000, denom="rune"), + deposit=thorchain_proto.ThorchainMsgDeposit( + asset="THOR.RUNE", amount=1, memo="unused", + signer="thor1ls33ayg26kmltw7jjy55p32ghjna09zp6z69y8"))) + def test_thorchain_sign_tx(self): self.requires_fullFeature() self.requires_firmware("7.0.2") diff --git a/tests/test_msg_zcash_sign_pczt_device.py b/tests/test_msg_zcash_sign_pczt_device.py index 3848e14b..c29978e2 100644 --- a/tests/test_msg_zcash_sign_pczt_device.py +++ b/tests/test_msg_zcash_sign_pczt_device.py @@ -168,10 +168,11 @@ def sign_kwargs(actions, ironwood=False, **overrides): # victim's note, reusing an approved action's alpha so the one emitted # RedPallas signature verified in both bundles. # - # ZIP-244 empty-bundle digest: BLAKE2b-256 of the empty string - # personalized "ZTxIdOrchardHash". The device now requires exactly this. + # ZIP-229 v6 empty-bundle digest: BLAKE2b-256 of the empty string + # personalized "ZTxIdOrchardH_v6". The v5/ZIP-244 + # "ZTxIdOrchardHash" value is a different digest. kwargs['orchard_digest'] = bytes.fromhex( - '9fbe4ed13b0c08e671c11a3407d84e1117cd45028a2eee1b9feae78b48a6e2c1') + 'a3367d2fdea2910159fc5026e9bf1fccd3e28ce5e6de46bfb71587230eea9515') kwargs.update(overrides) return kwargs @@ -319,7 +320,7 @@ def test_ironwood_rejects_a_non_empty_orchard_bundle(self): """ actions = [note_action(CMX_IRONWOOD)] kwargs = sign_kwargs(actions, ironwood=True) - # Anything but the ZIP-244 empty-bundle digest must be refused. + # Anything but the ZIP-229 v6 empty-bundle digest must be refused. kwargs['orchard_digest'] = bytes([0x11]) * 32 with self.assertRaises(Exception) as caught: From e79c6b810dd971c9f3b823205f2fdcfecead3652 Mon Sep 17 00:00:00 2001 From: highlander Date: Wed, 26 Aug 2026 17:18:39 -0600 Subject: [PATCH 11/17] test(bitcoin-only): gate unsupported 7.15 handlers --- tests/test_msg_bip85.py | 1 + tests/test_msg_mayachain_signtx.py | 1 + 2 files changed, 2 insertions(+) diff --git a/tests/test_msg_bip85.py b/tests/test_msg_bip85.py index 4a0b2b89..1020d280 100644 --- a/tests/test_msg_bip85.py +++ b/tests/test_msg_bip85.py @@ -20,6 +20,7 @@ class TestMsgBip85(common.KeepKeyTest): def setUp(self): super().setUp() self.requires_firmware("7.15.0") + self.requires_fullFeature() def test_bip85_12word_flow(self): """12-word derivation: verify device goes through display flow and returns Success.""" diff --git a/tests/test_msg_mayachain_signtx.py b/tests/test_msg_mayachain_signtx.py index c774ede2..3d8952fc 100644 --- a/tests/test_msg_mayachain_signtx.py +++ b/tests/test_msg_mayachain_signtx.py @@ -264,6 +264,7 @@ def test_mayachain_sign_tx_memos(self): signs, and each signature is bound to its exact memo bytes — a memo substitution changes the sign-doc digest and fails verification.""" self.requires_firmware("7.9.1") + self.requires_fullFeature() self.setup_mnemonic_nopin_nopassphrase() memos = [ From c697a25115ea859ab5b0a89f77dd2c77e61ab889 Mon Sep 17 00:00:00 2001 From: highlander Date: Wed, 26 Aug 2026 17:49:31 -0600 Subject: [PATCH 12/17] test(report): respect Bitcoin-only feature boundaries --- scripts/generate-test-report.py | 21 ++++++++++-- tests/test_report_variant_validation.py | 45 +++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 3 deletions(-) create mode 100644 tests/test_report_variant_validation.py diff --git a/scripts/generate-test-report.py b/scripts/generate-test-report.py index 5469ac3b..bd4ddf85 100644 --- a/scripts/generate-test-report.py +++ b/scripts/generate-test-report.py @@ -3243,6 +3243,13 @@ def screenshot_filter(fw_version): 'test_msg_solana_lut_attestation': '7.15.0', } +# These modules are mandatory only on the multi-chain product. Their handlers +# are intentionally absent from KK_BITCOIN_ONLY, so a capability-gated skip is +# evidence of the product boundary there, not missing release coverage. +FULL_FEATURE_ONLY_MUST_RUN_MODULES = { + 'test_msg_solana_lut_attestation', +} + def screenshot_audit(fw_version, screenshot_root, junit_path=None): """Which SECTIONS tests DECLARED screens but captured none? @@ -3283,7 +3290,7 @@ def screenshot_audit(fw_version, screenshot_root, junit_path=None): return (len(missing) == 0, missing) -def validate_junit(fw_version, results): +def validate_junit(fw_version, results, variant='full'): """Check SECTIONS tests against JUnit results. Returns (passed, failed_list). A test is considered failed if it appears in SECTIONS for this firmware version @@ -3299,7 +3306,12 @@ def validate_junit(fw_version, results): status = _lookup(results, mod, meth) if status in ('fail', 'error'): failures.append((tid, mod, meth, status)) - elif status == 'skip' and ver_ge(fw_version, MUST_RUN_MODULES.get(mod, '99.0.0')): + must_run = not ( + variant == 'bitcoin-only' and + mod in FULL_FEATURE_ONLY_MUST_RUN_MODULES + ) + if (status == 'skip' and must_run and + ver_ge(fw_version, MUST_RUN_MODULES.get(mod, '99.0.0'))): failures.append((tid, mod, meth, 'skipped-but-required')) elif not status: failures.append((tid, mod, meth, 'missing')) @@ -3320,6 +3332,9 @@ def main(): help='Print pytest -k expression for tests needing screenshots, then exit') p.add_argument('--validate-junit', action='store_true', help='Validate JUnit results against SECTIONS, exit non-zero on failures') + p.add_argument('--variant', choices=('full', 'bitcoin-only'), + default=os.environ.get('KK_FIRMWARE_VARIANT', 'full'), + help='Product variant whose required report coverage is validated') args = p.parse_args() fw = args.fw_version @@ -3347,7 +3362,7 @@ def main(): print('ERROR: --validate-junit requires --junit=', file=sys.stderr) sys.exit(2) results = parse_junit(args.junit) - ok, failures = validate_junit(fw, results) + ok, failures = validate_junit(fw, results, args.variant) if ok: print(f'SECTIONS validation passed: all tests for fw {fw} are pass or skip') sys.exit(0) diff --git a/tests/test_report_variant_validation.py b/tests/test_report_variant_validation.py new file mode 100644 index 00000000..73b25c08 --- /dev/null +++ b/tests/test_report_variant_validation.py @@ -0,0 +1,45 @@ +import importlib.util +import os +import unittest + + +REPORT_SCRIPT = os.path.join( + os.path.dirname(__file__), '..', 'scripts', 'generate-test-report.py') +SPEC = importlib.util.spec_from_file_location('generate_test_report', + REPORT_SCRIPT) +REPORT = importlib.util.module_from_spec(SPEC) +SPEC.loader.exec_module(REPORT) + + +def catalog_results_with_solana_lut_skipped(): + results = {} + for _, _, min_fw, _, _, tests in REPORT.SECTIONS: + if not REPORT.ver_ge('7.15.0', min_fw): + continue + for _, module, method, _, _, _ in tests: + results['%s::%s' % (module, method)] = 'pass' + for key in list(results): + if key.startswith('test_msg_solana_lut_attestation::'): + results[key] = 'skip' + return results + + +class TestReportVariantValidation(unittest.TestCase): + + def test_full_product_requires_solana_lut_coverage(self): + ok, failures = REPORT.validate_junit( + '7.15.0', catalog_results_with_solana_lut_skipped(), 'full') + self.assertFalse(ok) + self.assertEqual(4, len(failures)) + self.assertTrue(all(item[3] == 'skipped-but-required' + for item in failures)) + + def test_bitcoin_only_accepts_absent_solana_lut_handlers(self): + result = REPORT.validate_junit( + '7.15.0', catalog_results_with_solana_lut_skipped(), + 'bitcoin-only') + self.assertEqual((True, []), result) + + +if __name__ == '__main__': + unittest.main() From 7d32a3916c0ee39e89b6576c7a557dcee1318a2c Mon Sep 17 00:00:00 2001 From: highlander Date: Thu, 27 Aug 2026 14:03:08 -0600 Subject: [PATCH 13/17] ci: run reconciliation branches --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d7c82ec0..20a6eab1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,7 +15,7 @@ name: CI on: push: - branches: [master, develop, reconcile/upstream-sync, 'feature/**', 'fix/**', 'hotfix/**'] + branches: [master, develop, 'reconcile/**', 'feature/**', 'fix/**', 'hotfix/**'] pull_request: branches: [master, develop, reconcile/upstream-sync] From 68173d88c53d074e393eb6a2264d1e2623cf2fe3 Mon Sep 17 00:00:00 2001 From: highlander Date: Thu, 27 Aug 2026 16:34:37 -0600 Subject: [PATCH 14/17] fix(7.15): preserve session policy and valid auth fixtures --- keepkeylib/client.py | 6 +++++- tests/test_msg_authenticator_boundaries.py | 4 +++- tests/test_msg_ping.py | 3 ++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/keepkeylib/client.py b/keepkeylib/client.py index 1fe1091c..8d4f7de2 100644 --- a/keepkeylib/client.py +++ b/keepkeylib/client.py @@ -1473,7 +1473,11 @@ def apply_policy(self, policy_name, enabled): apply_policies = proto.ApplyPolicies(policy=[policy]) out = self.call(apply_policies) - self.init_device() # Reload Features + # AdvancedMode is intentionally session-scoped on firmware 7.15+. + # Initialize is a session-boundary message, so issuing it here to + # refresh Features immediately revokes the policy this method just + # applied. Callers that explicitly need fresh Features can initialize + # after they are done with the policy-gated operation. return out @field('message') diff --git a/tests/test_msg_authenticator_boundaries.py b/tests/test_msg_authenticator_boundaries.py index e5ffe3e5..8f0e8417 100644 --- a/tests/test_msg_authenticator_boundaries.py +++ b/tests/test_msg_authenticator_boundaries.py @@ -17,7 +17,9 @@ class TestAuthenticatorBoundaries(common.KeepKeyTest): - ADD_ACCOUNT = '\x15initializeAuth:example:alice:JBSWY3DPEHPK3PXP' + # 7.15 enforces the RFC-recommended 128-bit minimum for TOTP secrets. + ADD_ACCOUNT = ('\x15initializeAuth:example:alice:' + 'JBSWY3DPEHPK3PXPJBSWY3DPEHPK3PXP') GET_ACCOUNT = '\x17getAccount:0' WIPE_ACCOUNTS = '\x19wipeAuthdata:' diff --git a/tests/test_msg_ping.py b/tests/test_msg_ping.py index 4cf55a89..507c197a 100644 --- a/tests/test_msg_ping.py +++ b/tests/test_msg_ping.py @@ -142,7 +142,8 @@ def test_authenticator_passphrase_cancel_is_terminal(self): # local cache. This is the precondition that made the stale-data path # reachable after ClearSession. self.client.ping('\x19wipeAuthdata:') - init_auth = '\x15initializeAuth:example.com:alice:JBSWY3DPEHPK3PXP' + init_auth = ('\x15initializeAuth:example.com:alice:' + 'JBSWY3DPEHPK3PXPJBSWY3DPEHPK3PXP') self.client.ping(init_auth) self.client.clear_session() # The wipe/add-account confirmations establish the stale-cache From 96c5805e405fe74c2779cbe9a4741646bdbe7ff6 Mon Sep 17 00:00:00 2001 From: highlander Date: Thu, 27 Aug 2026 16:39:38 -0600 Subject: [PATCH 15/17] test(7.15): align Solana wire case and OLED baselines --- tests/test_msg_ethereum_signtx.py | 4 ++-- tests/test_msg_ethereum_signtx_xfer.py | 2 +- tests/test_msg_solana_signtx.py | 9 +++++---- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/tests/test_msg_ethereum_signtx.py b/tests/test_msg_ethereum_signtx.py index 85c566f0..0c9de301 100644 --- a/tests/test_msg_ethereum_signtx.py +++ b/tests/test_msg_ethereum_signtx.py @@ -47,13 +47,13 @@ def test_ethereum_native_pseudo_address_is_unknown_off_mainnet(self): "transfer", binascii.unhexlify("a9059cbb" + "00" * 12) + recipient + int_to_big_endian(1).rjust(32, b"\x00"), - "85d9054ee56836c1784c90dd777fc89444bf82b840d0818a59c73aa5b57ee35d", + "7910ca5cdea6e4f6870dad52fde79fd55891fd38fe2ad5d3295502fdf578dfe7", ), ( "approve", binascii.unhexlify("095ea7b3" + "00" * 12) + recipient + int_to_big_endian(1).rjust(32, b"\x00"), - "ab30156ff400957ffa9146ea827318bf878614e6ab4ae7dd731824e285fa5da6", + "e8e44436251ef16cb00192f23adcc86f843201d676d1a3d2377a1e8ae6330c01", ), ) diff --git a/tests/test_msg_ethereum_signtx_xfer.py b/tests/test_msg_ethereum_signtx_xfer.py index 8c78201c..0248e47e 100644 --- a/tests/test_msg_ethereum_signtx_xfer.py +++ b/tests/test_msg_ethereum_signtx_xfer.py @@ -61,7 +61,7 @@ def test_native_pseudo_address_transfer_is_unknown_off_mainnet(self): self.assertGreaterEqual(len(recorder.screens), 2) self.assertEqual( hashlib.sha256(recorder.screens[0]).hexdigest(), - "b0a3026e7af1778ebd71a968ace25c03945cccf2d8abc951e5dd65abc04e914e", + "3915d325da0a0e9842d7eb3eaa6e01ef0bbf7e010790af883ca1a7f30770ae8f", ) finally: self.client.apply_policy('AdvancedMode', 0) diff --git a/tests/test_msg_solana_signtx.py b/tests/test_msg_solana_signtx.py index 1dfe18af..926a0512 100644 --- a/tests/test_msg_solana_signtx.py +++ b/tests/test_msg_solana_signtx.py @@ -659,12 +659,13 @@ def test_solana_sign_token_transfer_with_metadata(self): 0x7c, 0xa6, 0x02, 0x03, 0x45, 0x2f, 0x5d, 0x61, ]) - # TransferChecked signs the mint and decimals. Unchecked Transfer is - # deliberately opaque because it carries neither. - instr_data = bytes([12]) + struct.pack(' Date: Thu, 27 Aug 2026 16:47:33 -0600 Subject: [PATCH 16/17] ci(7.15): test against the matching firmware branch --- .circleci/config.yml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index f7133ff2..d19a1f6d 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -32,9 +32,10 @@ jobs: # Move python-keepkey out of the way mv .pykk ../ - # This companion branch gates firmware PR #604, not the default - # firmware branch. Keep the target explicit and fail if it moves. - git clone --depth 1 -b release/7.14.3-bitcoin-only \ + # This companion branch gates the fork's 7.15 firmware PR, not the + # default firmware branch. Keep the target explicit; the checkout + # then replaces its python-keepkey submodule with CIRCLE_SHA1 below. + git clone --depth 1 -b release/7.15 \ https://github.com/BitHighlander/keepkey-firmware.git . # Match firmware CI's build set. A recursive init reaches optional From c4fb8bf55197e6068f48a22932247c6c73beada6 Mon Sep 17 00:00:00 2001 From: highlander Date: Thu, 27 Aug 2026 16:56:20 -0600 Subject: [PATCH 17/17] test(7.15): assert fail-closed signing contracts --- tests/test_msg_ethereum_clear_signing.py | 15 +++++++++++++-- tests/test_msg_osmosis_validation.py | 9 ++++++--- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/tests/test_msg_ethereum_clear_signing.py b/tests/test_msg_ethereum_clear_signing.py index f1775816..70cf304d 100644 --- a/tests/test_msg_ethereum_clear_signing.py +++ b/tests/test_msg_ethereum_clear_signing.py @@ -1012,8 +1012,8 @@ def test_binding_happy_path_signs_and_recovers(self): def _clearsign_flow(self, flow, chain_id=1): """Run one catalog flow END-TO-END with AdvancedMode ON: real tx, per-tx-bound metadata, who/what/why annotation plus the ordinary raw - review (auto-acked), sign, and assert the signature recovers to the - device signer over this exact digest.""" + review (auto-acked), then either sign and recover the exact digest or + assert the release policy's explicit fail-closed rejection.""" n = parse_path(DEVICE_PATH) tx_hash = flow_tx_hash(flow, chain_id) resp = self.client.ethereum_send_tx_metadata( @@ -1021,6 +1021,17 @@ def _clearsign_flow(self, flow, chain_id=1): metadata_version=1, key_id=TEST_KEY_ID) self.assertEqual(resp.classification, CLASSIFICATION_VERIFIED) + if flow['key'] == 'erc20-approve-unlimited': + with self.assertRaises(CallException) as ctx: + self.client.ethereum_sign_tx( + n=n, nonce=FLOW_NONCE, gas_price=FLOW_GAS_PRICE, + gas_limit=FLOW_GAS_LIMIT, to=flow['to'], + value=flow['value'], data=flow['data'], + chain_id=chain_id) + self.assertIn('Unlimited ERC20 approval is disabled', + str(ctx.exception)) + return + sig_v, sig_r, sig_s = self.client.ethereum_sign_tx( n=n, nonce=FLOW_NONCE, gas_price=FLOW_GAS_PRICE, gas_limit=FLOW_GAS_LIMIT, to=flow['to'], value=flow['value'], diff --git a/tests/test_msg_osmosis_validation.py b/tests/test_msg_osmosis_validation.py index 96eff418..83f381d9 100644 --- a/tests/test_msg_osmosis_validation.py +++ b/tests/test_msg_osmosis_validation.py @@ -31,7 +31,7 @@ def _assert_missing_parameter_failure(self, ack): self.assertEqual(ret.code, proto_types.Failure_FirmwareError) self.assertEndsWith(ret.message, "missing required parameters") - def test_present_but_empty_amount_is_rejected_before_review(self): + def test_present_but_empty_amount_is_rejected_as_invalid(self): self._start_signing() send = osmosis_proto.OsmosisMsgSend( to_address="osmo1g9el7lzjwh9yun2c4jjzhy09j98vkhfx8tzcpt", @@ -39,8 +39,11 @@ def test_present_but_empty_amount_is_rejected_before_review(self): denom="uosmo", ) self.assertTrue(send.HasField("amount")) - self._assert_missing_parameter_failure( - osmosis_proto.OsmosisMsgAck(send=send)) + ret = self.client.call_raw(osmosis_proto.OsmosisMsgAck(send=send)) + self.assertIsInstance(ret, proto.Failure) + self.assertEqual(ret.code, proto_types.Failure_SyntaxError) + self.assertEndsWith(ret.message, + "Invalid Osmosis amount or denomination") def test_ibc_omitted_amount_and_receiver_are_rejected_before_review(self): self._start_signing()