Skip to content

test: Hive phase-2/3 screenshot capture + first Osmosis device tests #199

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
BitHighlander merged 7 commits into from
Jul 25, 2026
Merged

test: Hive phase-2/3 screenshot capture + first Osmosis device tests #199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a010285
test(report): put the phase-2/3 Hive ops in SECTIONS so they get scre…
BitHighlander Jul 21, 2026
59f7c85
test(report): G36 is rejection-only — drop its screenshot hint
BitHighlander Jul 21, 2026
b257df2
test(osmosis): device tests for the confirm screens, with screenshot …
BitHighlander Jul 21, 2026
3a9e1b7
fix(osmosis): the MsgSend client path was dead code with three bugs
BitHighlander Jul 21, 2026
0594d57
test(osmosis): derive the osmo1 address from the device, not a literal
BitHighlander Jul 21, 2026
52e1fb5
test(osmosis): osmosis_get_address already returns the string
BitHighlander Jul 21, 2026
1a69741
fix(osmosis): fence MsgSend to uosmo — the firmware signs a denom it …
BitHighlander Jul 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 27 additions & 5 deletions keepkeylib/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -999,15 +999,37 @@ def osmosis_sign_tx(
if len(msg['value']['amount']) != 1:
raise CallException("Osmosis.MsgSend", "Multiple amounts per msg not supported")

denom = msg['value']['amount'][0]['denom']
if denom != 'uatom':
raise CallException("Osmosis.MsgSend", "Unsupported denomination: " + denom)

# This branch had never executed. It whitelisted 'uatom' — the
# COSMOS denom, so a native OSMO send was impossible — dropped
# the denom instead of forwarding it, and assigned an int to
# OsmosisMsgSend.amount, which is a string field and would have
# raised even for uatom.
#
# The denom IS now forwarded (the firmware needs it to decide
# whether to scale), but MsgSend is still fenced to uosmo, and
# not for the reason the old whitelist implied:
# osmosis_signTxUpdateMsgSend takes only (amount, to_address)
# and HARDCODES "denom":"uosmo" into the amino JSON it hashes,
# while fsm_msgOsmosisMsgAck renders whatever denom arrives.
# Forwarding a different one therefore DISPLAYS one asset and
# SIGNS another — e.g. a large amount of some worthless ibc/...
# token on screen, a large amount of OSMO in the signature.
# osmosis_signTxUpdateMsgDelegate already takes a denom, so
# MsgSend is the outlier. Lift this fence only once the
# firmware serializer accepts a denom.
coin = msg['value']['amount'][0]
if coin['denom'] != 'uosmo':
raise CallException(
"Osmosis.MsgSend",
"Only uosmo is signable: the firmware MsgSend serializer "
"hardcodes uosmo in the sighash, so any other denom would "
"be displayed but not signed (got %s)" % coin['denom'])
resp = self.call(osmosis_proto.OsmosisMsgAck(
send=osmosis_proto.OsmosisMsgSend(
from_address=msg['value']['from_address'],
to_address=msg['value']['to_address'],
amount=int(msg['value']['amount'][0]['amount']),
denom=coin['denom'],
amount=str(coin['amount']),
address_type=types.SPEND,
)
))
Expand Down
125 changes: 125 additions & 0 deletions scripts/generate-test-report.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -962,6 +962,56 @@ def _arg_shown(a):
'Sign Cosmos with memo', 'Memo field displayed for exchange deposit tags.', []),
]),

('P', 'Osmosis', '7.15.0',
'Osmosis is the Cosmos-ecosystem DEX, signed with the same amino encoding as Cosmos Hub and '
'derived from the same coin type (118). 7.15.0 CHANGED how every Osmosis amount is drawn: the '
'confirm screens formatted with atof() + "%.6f", and a float carries only ~7 significant '
'decimal digits, so a large transfer was displayed ROUNDED on the screen the user approves '
'(123456789.123456 OSMO rendered as 123456792.000000). The signature was always over the '
'correct amount — the error was confined to the display, which is the half a hardware wallet '
'exists to get right. Amounts now format through bn_format_uint64 in integer math, exact at '
'any magnitude, and the same change removed the newlib floating-point engine from the build.',
[
'SEND: recipient + OSMO amount rendered from integer base units, never a float',
'PRECISION: 15-significant-digit amounts display exactly, not rounded to 7',
'UNKNOWN DENOM: shown as raw base units — the device does not guess a decimal point',
],
[
('P1', 'test_msg_osmosis_signtx', 'test_osmosis_sign_tx',
'Sign Osmosis send',
'Baseline MsgSend: recipient and a whole-OSMO amount on the confirm screen.',
['OSMO send']),
('P2', 'test_msg_osmosis_signtx', 'test_osmosis_send_amount_beyond_float_precision',
'Amount beyond float precision displays exactly',
'The regression this section exists for: 123456789123456 uosmo needs 15 significant '
'digits. The old float path drew 123456792.000000 OSMO over a transaction moving '
'123456789.123456 OSMO. The captured frame is the evidence.',
['Exact large amount']),
('P3', 'test_msg_osmosis_signtx', 'test_osmosis_send_subunit_amount',
'Sub-unit amount keeps its tail',
'500 uosmo is 0.000500 OSMO — no integer part and six decimals; it must not collapse '
'to 0 or lose the trailing digits.',
['Sub-unit amount']),
('P4', 'test_msg_osmosis_signtx', 'test_osmosis_send_non_uosmo_denom_is_refused',
'Non-uosmo MsgSend is refused, not displayed',
'osmosis_signTxUpdateMsgSend hardcodes "denom":"uosmo" into the amino JSON it '
'hashes while the confirm screen renders whatever denom arrived, so any other denom '
'would display one asset and sign another. Refused client-side until the firmware '
'serializer accepts a denom.',
# Refusal happens before the device is reached — no frame to capture.
[]),
('P5', 'test_msg_osmosis_signtx', 'test_osmosis_amount_is_committed_to_the_signature',
'Displayed amount is in the digest',
'Two sends differing only in amount produce different signatures, so the confirm '
'screen is bound to what is signed rather than decorative.',
[]),
('P6', 'test_msg_osmosis_signtx', 'test_osmosis_signing_is_deterministic',
'Deterministic nonces (RFC6979)',
'Identical input yields an identical signature; a mismatch is a key-recovery risk, '
'not a cosmetic one.',
[]),
]),

('H', 'THORChain', '7.0.0',
'THORChain is a decentralized cross-chain liquidity protocol. Native RUNE transactions use amino '
'encoding with thor1... bech32 addresses. The memo field is the critical security element - it '
Expand Down Expand Up @@ -1456,6 +1506,81 @@ def _arg_shown(a):
'Account authority ops require owner',
'account_create / account_update sign only under the owner role.',
[]),
# ── Phase 2/3 op table (fw #315) ────────────────────────────────
# These ran green in the full suite from the day they landed, but had
# no SECTIONS entry, so screenshot_filter() never selected them and
# eleven newly clear-signed ops shipped with zero OLED proof. A
# correct signature over bytes the user was shown something else for
# is the exact failure the clear-sign table exists to prevent, so
# every op that renders a confirm screen gets a non-empty hint.
('G29', 'test_msg_hive', 'test_hive_sign_ops_limit_order_create',
'Internal market: limit_order_create',
'The op that motivated phase 3 — a HIVE->HBD market swap. Both sides of the order '
'are shown with their symbols pinned (a swapped symbol hides a ~2000x value '
'difference behind an identical-looking number), and order id / fill-or-kill / '
'expiry get their own screen so they cannot be crowded off the first.',
['Sell and receive amounts', 'Order terms screen']),
('G30', 'test_msg_hive', 'test_hive_sign_ops_limit_order_cancel',
'Internal market: limit_order_cancel',
'Cancelling names the order id and the owner. No recipient row is forged — the op '
'acts on the signer\'s own book entry.',
['Cancel order screen']),
('G31', 'test_msg_hive', 'test_hive_sign_ops_active_tier_value_ops',
'Active-tier value ops',
'transfer_to_vesting, convert, transfer_to/from_savings, delegate_vesting_shares '
'and withdraw_vesting all move or lock value, so all six sign only under active. '
'Each renders its own amount + counterparty.',
['Power up', 'Convert', 'Savings deposit/withdraw', 'Delegation', 'Power down']),
('G32', 'test_msg_hive', 'test_hive_sign_ops_posting_tier_ops',
'claim_reward_balance is posting tier',
'Claiming is not spending, so it signs under posting. Three reward assets across '
'two screens (the OLED body fits three rows; a fourth would be signed but never '
'shown).',
['Claim rewards screens']),
('G33', 'test_msg_hive', 'test_hive_sign_ops_zero_amount_semantics',
'Zero means something for two ops, nothing for the rest',
'0 VESTS stops a power-down and removes a delegation — both legitimate, so zero is '
'NOT rejected there and the screen must say which action it is. Everywhere else a '
'zero amount is a no-op and refused.',
['Stop power down', 'Remove delegation']),
('G34', 'test_msg_hive', 'test_hive_sign_ops_asset_symbol_and_precision_pinned',
'Asset symbol pinned to its protocol precision',
'HIVE/HBD are 3-decimal, VESTS is 6. The parser refuses any other pairing: a wrong '
'precision moves the decimal point on the confirmation screen relative to what the '
'chain applies.',
[]),
('G35', 'test_msg_hive', 'test_hive_sign_ops_comment_options_binds_to_its_comment',
'comment_options binds to its own comment',
'Payout redirection is only accepted immediately after a comment op with the same '
'author and permlink. Standing alone it could attach beneficiaries to a post the '
'user published earlier and is not reviewing on this screen.',
['Payout options screens']),
('G36', 'test_msg_hive', 'test_hive_sign_ops_comment_options_beneficiary_rules',
'Beneficiary ordering, uniqueness and total enforced on-device',
# Scoped to exactly what the mapped test asserts. It covers three
# rejections — unsorted, duplicate, and weights summing over 100%.
# The extension-count cap, the 1-8 count bound and per-beneficiary
# weight range are enforced by the parser but are NOT exercised here,
# so the entry must not claim them.
'Beneficiaries must be strictly ascending by account (which also makes them unique) '
'and their weights must sum to no more than 10000 bp. Unsorted, duplicate and '
'over-100% lists are each refused.',
# Rejection-only: every case here is _assert_ops_fails, so the device
# refuses before drawing anything and the capture would be three
# frames of the idle home screen — a report entry that LOOKS like
# visual proof and is not. The per-beneficiary confirm screens are
# captured by G35, which actually signs a two-beneficiary payout.
[]),
('G37', 'test_msg_hive', 'test_hive_sign_ops_account_update2_rejects_authority_change',
'account_update2 cannot rotate keys',
'Only the profile-metadata form is in the table. Any owner/active/posting/memo_key '
'field present is a hard reject — the op-9/10 device-derived-keys invariant applied '
'field-level.',
[]),
('G38', 'test_msg_hive', 'test_hive_sign_ops_truncated_bodies_rejected',
'Truncated op bodies refused',
'A body cut short mid-field is a parse failure, not sign-what-you-can.',
[]),
]),

('S', 'Solana', '7.14.0',
Expand Down
177 changes: 177 additions & 0 deletions tests/test_msg_osmosis_signtx.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
"""Osmosis MsgSend signing — with the confirm-screen amount as the point.

Osmosis had NO device tests at all: the confirm screens that render amounts
were covered only by host-side unit tests of the formatter in isolation. That
matters more than it sounds, because 7.15.0 CHANGED how every Osmosis amount
is drawn.

Before, fsm_msg_osmosis.h rendered amounts with atof() + "%.6f". A float
carries ~7 significant decimal digits, so a large amount was displayed
ROUNDED on the very screen the user approves:

123456789123456 uosmo -> shown as "123456792.000000 OSMO"
actual 123456789.123456 OSMO

The signature was over the correct amount either way — the lie was only on
the screen, which is the half a hardware wallet exists to get right. It now
formats with bn_format_uint64 in integer math.

KNOWN LIMIT, do not overstate this: osmosis_formatAmount converts with an
unchecked strtoull(), so it is exact only for a CANONICAL decimal uint64.
strtoull saturates past UINT64_MAX and also accepts leading whitespace, a
sign, and 0x — so "18446744073709551616" or "-1" display as
18446744073.709551615 OSMO while the original string is what gets hashed.
That is the same display/signature divergence in a different disguise and
needs a firmware-side canonical-range check; these tests deliberately do not
claim otherwise.

These tests are paired with SECTIONS entries carrying screenshot hints, so
the rendered frame is captured as evidence. A test asserting only "it signed"
cannot prove what the OLED drew.

pyk's osmosis_sign_tx currently implements osmosis-sdk/MsgSend only; the
delegate/undelegate/LP/swap/IBC screens share the same formatter but are not
reachable from here until the client learns those message types.
"""
import unittest
import common

from binascii import hexlify

from keepkeylib.tools import parse_path

# Osmosis uses the Cosmos coin type (118), not one of its own.
DEFAULT_BIP32_PATH = "m/44h/118h/0h/0/0"


def make_send(from_address, to_address, amount, denom='uosmo'):
return {
'type': 'osmosis-sdk/MsgSend',
'value': {
'from_address': from_address,
'to_address': to_address,
'amount': [{'denom': denom, 'amount': str(amount)}],
},
}


class TestMsgOsmosisSignTx(common.KeepKeyTest):

def _address(self):
"""Ask the device for its own osmo1 address.

Deliberately NOT a hardcoded constant: the firmware bech32-decodes
to_address and refuses a bad checksum, so a literal invented by
swapping a cosmos1 prefix for osmo1 fails with the opaque "Failed to
include send message in transaction". Deriving it keeps the fixture
honest and makes these self-sends.
"""
# osmosis_get_address is decorated @field('address'), so it already
# returns the string rather than the OsmosisAddress message.
return self.client.osmosis_get_address(
address_n=parse_path(DEFAULT_BIP32_PATH)
)

def _sign(self, amount, denom='uosmo'):
addr = self._address()
return self.client.osmosis_sign_tx(
address_n=parse_path(DEFAULT_BIP32_PATH),
account_number=16359,
chain_id="osmosis-1",
fee=800,
gas=290000,
msgs=[make_send(addr, addr, amount, denom)],
memo="",
sequence=17,
)

def test_osmosis_sign_tx(self):
"""Baseline: a whole-OSMO send signs and returns a well-formed
secp256k1 signature + compressed pubkey."""
self.requires_fullFeature()
self.requires_firmware("7.15.0")
self.setup_mnemonic_nopin_nopassphrase()

sig = self._sign(1500000) # 1.500000 OSMO
self.assertEqual(len(sig.signature), 64)
self.assertEqual(len(sig.public_key), 33)
self.assertIn(hexlify(sig.public_key)[:2], (b'02', b'03'))

def test_osmosis_send_amount_beyond_float_precision(self):
"""THE regression. 123456789123456 uosmo needs 15 significant digits;
a float holds ~7, so the old atof()+"%.6f" path drew
"123456792.000000 OSMO" over a transaction that actually moves
123456789.123456 OSMO. The captured frame is the proof — assert here
only that the device signs it, and read the amount off the screenshot.
"""
self.requires_fullFeature()
self.requires_firmware("7.15.0")
self.setup_mnemonic_nopin_nopassphrase()

sig = self._sign(123456789123456)
self.assertEqual(len(sig.signature), 64)

def test_osmosis_send_subunit_amount(self):
"""500 uosmo is 0.000500 OSMO — six decimal places, no integer part.
The formatter must not collapse it to "0" or drop the tail."""
self.requires_fullFeature()
self.requires_firmware("7.15.0")
self.setup_mnemonic_nopin_nopassphrase()

sig = self._sign(500)
self.assertEqual(len(sig.signature), 64)

def test_osmosis_send_non_uosmo_denom_is_refused(self):
"""A non-uosmo MsgSend must not reach the device at all.

osmosis_signTxUpdateMsgSend takes only (amount, to_address) and
hardcodes "denom":"uosmo" into the amino JSON it hashes, while
fsm_msgOsmosisMsgAck renders whatever denom arrived. Sending uatom
therefore DISPLAYS "1500000 uatom" and SIGNS 1500000 uosmo — the
display/signature divergence a hardware wallet exists to prevent, and
exploitable in the large: a big number of some worthless ibc/... token
on screen, a big number of OSMO in the signature.

This asserts the fence, NOT that arbitrary denoms work. When the
firmware serializer takes a denom, replace this with the real check:
otherwise-identical uosmo and uatom transactions must produce
DIFFERENT signatures.
"""
self.requires_fullFeature()
self.requires_firmware("7.15.0")
self.setup_mnemonic_nopin_nopassphrase()

with self.assertRaises(Exception) as ctx:
self._sign(1500000, denom='uatom')
self.assertIn('uosmo', str(ctx.exception))

def test_osmosis_amount_is_committed_to_the_signature(self):
"""Guards the pairing between what is shown and what is signed: two
sends differing ONLY in amount must produce different signatures. If
they matched, the amount would not be in the digest and the confirm
screen would be decorative."""
self.requires_fullFeature()
self.requires_firmware("7.15.0")
self.setup_mnemonic_nopin_nopassphrase()

a = self._sign(1500000)
b = self._sign(1500001)
self.assertNotEqual(hexlify(a.signature), hexlify(b.signature))
# Same key throughout — only the message differed.
self.assertEqual(hexlify(a.public_key), hexlify(b.public_key))

def test_osmosis_signing_is_deterministic(self):
"""RFC6979: identical input must yield an identical signature. A
mismatch here means nonce generation is not deterministic, which is a
key-recovery risk long before it is a display problem."""
self.requires_fullFeature()
self.requires_firmware("7.15.0")
self.setup_mnemonic_nopin_nopassphrase()

first = self._sign(1500000)
second = self._sign(1500000)
self.assertEqual(hexlify(first.signature), hexlify(second.signature))


if __name__ == '__main__':
unittest.main()