Skip to content

Commit 5c97c58

Browse files
committed
Merge upstream/master (#216) into the 7.15 harness (#197)
#216 landed as one squashed commit whose content had already reached #197 by another route, so most of the 31 conflicts are the two branches doing the same thing in different words. Resolved by hand, hunk by hunk; no --ours/--theirs. Every #216 addition is verified present in the merged tree, including the files that never conflicted (test_msg_display_disclosure.py, the eip155 vectors, the ripple/ping/tron/eth-signtx disclosure tests, and the `chain_id is not None` fix in keepkeylib/client.py -- that one matters, it is what lets an explicit chain_id=0 reach the wire to be refused). tests/test_sign_typed_data.py (1 hunk) -- #197 Same policy call on both sides; the only difference is `with open(...)` vs open/close and one extra clause of comment. Nothing asserted differs. tests/test_msg_thorchain_signtx.py (2 hunks) -- #197, and this one is a real divergence, not a restatement. #197 repointed test_sign_eth_add_liquidity at the firmware-pinned THORChain router (to=0xd37bbe57...); that line auto-merged, so it IS the transaction the merged test signs. #216 regenerated exact r/s against the OLD to=0x41e55600.... `to` is an RLP field of the legacy sighash, so #216's vectors do not describe the transaction under test and would fail. #197's structural assertions win on those grounds -- NOT because structural is preferable. They are genuinely weaker than what #216 had, and the superseded vectors are recorded in a comment at the assertion so the gap is visible and regenerable on-device. Same treatment for the memo-length hunk: both sides independently corrected 0x3b -> 0x3a (the memo is 58 bytes; the 59th was ABI padding), so the comments were combined rather than one dropped. tests/test_msg_mayachain_signtx.py (2 hunks) -- #197 Same router story (to=0xe3985e6b..., Maya router v4), so #216's exact r/s -- byte-identical to the thorchain pair, same base fixture -- are stale here too. #197 does not merely relax: it reconstructs the sighash host-side and recovers the signer, so a wrong digest, wrong calldata or wrong key fails. Taking #216 would also have re-added a stray `)` and restored @unittest.skip on test_mayachain_remove_liquidity, which #197 unskipped. Raw assertion count in this file drops 26 -> 13 because seven frozen-vector memo cases became one loop over the same seven memos and three skipped tests now actually run; memo coverage is unchanged at nine cases and none of them are skipped any more. tests/test_msg_ton_signtx.py (9 hunks) -- union Both sides added the identical AdvancedMode opt-in to nine tests; #216 also carried the four-line reason. Kept #216's comment and call on all nine. #197 additionally gated three tests #216 did not -- the reject-path tests -- and those auto-merged; annotated them, because opening the gate is what stops them passing for the wrong reason (with the gate shut the firmware refuses every TonSignTx and the raises-assertion is satisfied without the parser ever validating a field). scripts/generate-test-report.py (17 hunks) -- #197 Fifteen hunks are section-letter collisions: #216 filed Display Binding under 'S' and Display Disclosure under 'D', but 'S' is already Solana and 'D' is already BIP-85 Child Derivation, so #216's own catalog carries each letter twice. #197 assigns 'J' and 'Q' and does not collide; _audit_catalog() asserts letter and id uniqueness and would have tripped on #216's scheme. Entries are otherwise identical -- same module, same test, same prose -- so nothing #216 catalogued is lost, only relettered. Fixed one cross-reference #197's rename left dangling: J13's prose said "the control for S12", which now points at a Solana test; it means J12. The other two hunks are #197-only additions (the F, I, L, U, TD sections and MUST_RUN_MODULES) against an empty #216 side. tests/test_msg_ethereum_erc20_0x_signtx.py -- union, and git got this one wrong Not one of the 31: it merged clean and merged badly. Both sides inserted an AdvancedMode opt-in into test__sign_transformERC20 at different offsets, so the merged file called apply_policy twice in a row. Deduped to one call carrying both rationales (#216's 1480-bytes-past-the-1024-chunk mechanism, which is what atlas J1 actually claims, plus #197's no-recognised-handler note). Folded in the comment #197 left three lines above it saying "No AdvancedMode policy is set here on purpose", which contradicted its own apply_policy call and both sides' findings; kept as an explicitly superseded note rather than deleted, so the behaviour change stays legible. Verified: 0 conflict markers; every .py under tests/, scripts/ and keepkeylib/ parses; _audit_catalog() passes and the report renders 27 sections / 381 distinct tests; all 383 catalog rows resolve to a test that exists (V17-V67 against CLEARSIGN_FLOWS, which generates those methods by the same transform; K3-K11 are native gtest suites the JUnit parser handles by bare classname). Not verified: no test was executed against firmware. The one suite that runs offline, test_storage_version_gate.py, needs the firmware sources checked out above this tree and errors in setUp without them -- unchanged by this merge.
2 parents 999e776 + 598097e commit 5c97c58

5 files changed

Lines changed: 89 additions & 24 deletions

File tree

scripts/generate-test-report.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -579,7 +579,7 @@ def _arg_shown(a):
579579
['Numbered page screens covering the whole body']),
580580
('J13', 'test_msg_ping', 'test_ping_short_body_is_not_paged',
581581
'A body that fits is not paged',
582-
'The control for S12. A fitting body must still take exactly one screen with an '
582+
'The control for J12. A fitting body must still take exactly one screen with an '
583583
'unnumbered title - otherwise a pager that numbered every confirmation, making '
584584
'ordinary approvals cost extra presses, would pass unnoticed.',
585585
['Single unnumbered confirmation screen']),

tests/test_msg_ethereum_erc20_0x_signtx.py

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -164,17 +164,26 @@ def test_sign_longdata_swap(self):
164164
# test transformERC20
165165
def test__sign_transformERC20(self):
166166
self.requires_fullFeature()
167-
# transformERC20 is pinned to the 0x ExchangeProxy and bounded by its
168-
# displayed input/min-output amounts, so it clear-signs WITHOUT
169-
# AdvancedMode at any calldata size (the transformations[] tail exceeds
170-
# one chunk). No AdvancedMode policy is set here on purpose.
171167
self.requires_firmware("7.1.5")
172168
self.setup_mnemonic_nopin_nopassphrase()
173-
# transformERC20 to the 0x Exchange Proxy is blind contract data (no
174-
# recognized token / contract handler). Since 7.15.0 the device
175-
# hard-rejects blind contract data unless AdvancedMode is on (Insight
176-
# clear-signing policy) — same as test_sign_longdata_swap above. This
177-
# test checks signing correctness, so run it in expert mode.
169+
# This payload is 1480 bytes, so it exceeds one 1024-byte chunk and the
170+
# 0x decoder no longer claims it: the bytes past the initial chunk are
171+
# hashed without being decoded, so describing them as a token swap would
172+
# be a screen the device cannot vouch for. It falls to the generic
173+
# contract-data path (blind contract data, no recognized token /
174+
# contract handler), which since 7.15.0 the device hard-rejects unless
175+
# AdvancedMode is on (Insight clear-signing policy) — same as
176+
# test_sign_longdata_swap above.
177+
#
178+
# This test is about SIGNING CORRECTNESS, not about the gate, so enable
179+
# the policy and keep asserting the signature. The gate itself is
180+
# covered by test_msg_ethereum_signing_guards.
181+
#
182+
# SUPERSEDED, kept so the behaviour change stays visible: transformERC20
183+
# was once pinned to the 0x ExchangeProxy and bounded by its displayed
184+
# input/min-output amounts, and on that basis clear-signed WITHOUT
185+
# AdvancedMode at any calldata size (the transformations[] tail exceeds
186+
# one chunk). That claim was withdrawn for payloads past the first chunk.
178187
self.client.apply_policy("AdvancedMode", 1)
179188

180189
sig_v, sig_r, sig_s = self.client.ethereum_sign_tx(

tests/test_msg_mayachain_signtx.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ def test_sign_eth_add_liquidity(self):
205205
'0000000000000000000000000000000000000000000000000000000000000000' +
206206
'0000000000000000000000000000000000000000000000000000000000000000' +
207207
'0000000000000000000000000000000000000000000000000000000000000080' + # offset of memo string from 4
208-
'000000000000000000000000000000000000000000000000000000000000003a' + # length of memo string in bytes (58, not 59: the 59th byte is ABI padding)
208+
'000000000000000000000000000000000000000000000000000000000000003a' + # length of memo string in bytes (58: ADD:ETH.ETH:<addr>:420; the 59th byte the old 0x3b counted was ABI padding)
209209
# ADD:ETH.ETH:0xc5b2608927ea95ed43f842f553e3a27b09c050e8:420
210210
'4144443a4554482e4554483a3078633562323630383932376561393565643433' +
211211
'663834326635353365336132376230396330353065383a343230000000000000')

tests/test_msg_thorchain_signtx.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ def test_sign_eth_add_liquidity(self):
137137
'0000000000000000000000000000000000000000000000000000000000000000' +
138138
'0000000000000000000000000000000000000000000000000000000000000000' +
139139
'0000000000000000000000000000000000000000000000000000000000000080' + # offset of memo string from 4
140-
'000000000000000000000000000000000000000000000000000000000000003a' + # length of memo string in bytes (58, not 59: the 59th byte is ABI padding)
140+
'000000000000000000000000000000000000000000000000000000000000003a' + # length of memo string in bytes (58: ADD:ETH.ETH:<addr>:420; the 59th byte the old 0x3b counted was ABI padding)
141141
# ADD:ETH.ETH:0xc5b2608927ea95ed43f842f553e3a27b09c050e8:420
142142
'4144443a4554482e4554483a3078633562323630383932376561393565643433' +
143143
'663834326635353365336132376230396330353065383a343230000000000000')
@@ -146,6 +146,14 @@ def test_sign_eth_add_liquidity(self):
146146
# `to` updated to the firmware-pinned THORChain router; exact r/s
147147
# change with it, so assert structure here and regenerate exact vectors
148148
# on-device.
149+
#
150+
# 7.14.2 regenerated exact vectors for this calldata, but against the
151+
# OLD `to` (0x41e5560054824ea6b0732e656e3ad64e20e94e45). `to` is an RLP
152+
# field of the sighash, so they do not describe the tx signed above.
153+
# Retained as the oracle for that superseded fixture:
154+
# sig_v 37
155+
# r 7adc5bda6e66b37a81962557c844509c4bfaa1e9217fc6d05968286d60b67dbf
156+
# s 613479150c4cfbcdc8243055aa5137afc89826c4176c420a60409f139171831b
149157
self.assertIn(sig_v, [37, 38]) # EIP-155 chain_id=1
150158
self.assertEqual(len(sig_r), 32)
151159
self.assertEqual(len(sig_s), 32)

tests/test_msg_ton_signtx.py

Lines changed: 60 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,11 @@ def test_ton_sign_structured(self):
7575
"""
7676
self.requires_fullFeature()
7777
self.setup_mnemonic_allallall()
78-
self.client.apply_policy('AdvancedMode', True)
78+
# 7.14.2 gates TonSignTx behind AdvancedMode: the device cannot parse
79+
# raw_tx, so every TonSignTx is a blind signature and must be disclosed
80+
# as one. This test exercises signing correctness, so it opts in
81+
# explicitly rather than the firmware relaxing the gate.
82+
self.client.apply_policy("AdvancedMode", 1)
7983

8084
dest_addr = make_ton_address()
8185

@@ -101,7 +105,11 @@ def test_ton_sign_with_memo(self):
101105
"""Test TON transfer with a text memo (blind-sign path)."""
102106
self.requires_fullFeature()
103107
self.setup_mnemonic_allallall()
104-
self.client.apply_policy('AdvancedMode', True)
108+
# 7.14.2 gates TonSignTx behind AdvancedMode: the device cannot parse
109+
# raw_tx, so every TonSignTx is a blind signature and must be disclosed
110+
# as one. This test exercises signing correctness, so it opts in
111+
# explicitly rather than the firmware relaxing the gate.
112+
self.client.apply_policy("AdvancedMode", 1)
105113

106114
dest_addr = make_ton_address()
107115

@@ -125,7 +133,11 @@ def test_ton_sign_legacy_raw_tx(self):
125133
"""Test legacy blind-sign with raw_tx field."""
126134
self.requires_fullFeature()
127135
self.setup_mnemonic_allallall()
128-
self.client.apply_policy('AdvancedMode', True)
136+
# 7.14.2 gates TonSignTx behind AdvancedMode: the device cannot parse
137+
# raw_tx, so every TonSignTx is a blind signature and must be disclosed
138+
# as one. This test exercises signing correctness, so it opts in
139+
# explicitly rather than the firmware relaxing the gate.
140+
self.client.apply_policy("AdvancedMode", 1)
129141

130142
raw_tx = b'\x00' * 64
131143

@@ -141,7 +153,11 @@ def test_ton_sign_missing_fields_rejected(self):
141153
"""Test that incomplete structured fields are rejected."""
142154
self.requires_fullFeature()
143155
self.setup_mnemonic_allallall()
144-
self.client.apply_policy('AdvancedMode', True)
156+
# Same 7.14.2 AdvancedMode gate as the signing tests above, but this is
157+
# a reject-path test: with the gate CLOSED the firmware refuses every
158+
# TonSignTx, so the assertion below would pass without the parser ever
159+
# validating anything. Opt in so the rejection proves what it claims.
160+
self.client.apply_policy("AdvancedMode", 1)
145161

146162
msg = ton_messages.TonSignTx(
147163
address_n=parse_path(TON_PATH),
@@ -155,7 +171,11 @@ def test_ton_sign_deterministic(self):
155171
"""Test that signing the same message produces same signature."""
156172
self.requires_fullFeature()
157173
self.setup_mnemonic_allallall()
158-
self.client.apply_policy('AdvancedMode', True)
174+
# 7.14.2 gates TonSignTx behind AdvancedMode: the device cannot parse
175+
# raw_tx, so every TonSignTx is a blind signature and must be disclosed
176+
# as one. This test exercises signing correctness, so it opts in
177+
# explicitly rather than the firmware relaxing the gate.
178+
self.client.apply_policy("AdvancedMode", 1)
159179

160180
dest_addr = make_ton_address()
161181
raw_tx = hashlib.sha256(b'test-ton-deterministic').digest() * 2 # 64 bytes
@@ -186,7 +206,11 @@ def test_ton_sign_empty_raw_tx(self):
186206
"""Empty raw_tx (0 bytes) should be rejected by firmware."""
187207
self.requires_fullFeature()
188208
self.setup_mnemonic_allallall()
189-
self.client.apply_policy('AdvancedMode', True)
209+
# Same 7.14.2 AdvancedMode gate as the signing tests above, but this is
210+
# a reject-path test: with the gate CLOSED the firmware refuses every
211+
# TonSignTx, so the assertion below would pass without the parser ever
212+
# validating anything. Opt in so the rejection proves what it claims.
213+
self.client.apply_policy("AdvancedMode", 1)
190214

191215
msg = ton_messages.TonSignTx(
192216
address_n=parse_path(TON_PATH),
@@ -200,7 +224,11 @@ def test_ton_sign_oversized_raw_tx(self):
200224
"""raw_tx of 1025 bytes exceeds proto max (1024) and should be rejected."""
201225
self.requires_fullFeature()
202226
self.setup_mnemonic_allallall()
203-
self.client.apply_policy('AdvancedMode', True)
227+
# Same 7.14.2 AdvancedMode gate as the signing tests above, but this is
228+
# a reject-path test: with the gate CLOSED the firmware refuses every
229+
# TonSignTx, so the assertion below would pass without the parser ever
230+
# validating anything. Opt in so the rejection proves what it claims.
231+
self.client.apply_policy("AdvancedMode", 1)
204232

205233
raw_tx = b'\xAB' * 1025
206234

@@ -216,7 +244,11 @@ def test_ton_sign_with_empty_memo(self):
216244
"""Empty memo string should be accepted (memo is optional text)."""
217245
self.requires_fullFeature()
218246
self.setup_mnemonic_allallall()
219-
self.client.apply_policy('AdvancedMode', True)
247+
# 7.14.2 gates TonSignTx behind AdvancedMode: the device cannot parse
248+
# raw_tx, so every TonSignTx is a blind signature and must be disclosed
249+
# as one. This test exercises signing correctness, so it opts in
250+
# explicitly rather than the firmware relaxing the gate.
251+
self.client.apply_policy("AdvancedMode", 1)
220252

221253
dest_addr = make_ton_address()
222254
raw_tx = hashlib.sha256(b'test-ton-empty-memo').digest() * 2 # 64 bytes
@@ -238,7 +270,11 @@ def test_ton_sign_with_long_memo(self):
238270
"""Memo of 120 characters (near max_size 121) should be accepted."""
239271
self.requires_fullFeature()
240272
self.setup_mnemonic_allallall()
241-
self.client.apply_policy('AdvancedMode', True)
273+
# 7.14.2 gates TonSignTx behind AdvancedMode: the device cannot parse
274+
# raw_tx, so every TonSignTx is a blind signature and must be disclosed
275+
# as one. This test exercises signing correctness, so it opts in
276+
# explicitly rather than the firmware relaxing the gate.
277+
self.client.apply_policy("AdvancedMode", 1)
242278

243279
dest_addr = make_ton_address()
244280
raw_tx = hashlib.sha256(b'test-ton-long-memo').digest() * 2 # 64 bytes
@@ -261,7 +297,11 @@ def test_ton_sign_workchain_zero(self):
261297
"""Explicit workchain=0 (basechain) in TonSignTx."""
262298
self.requires_fullFeature()
263299
self.setup_mnemonic_allallall()
264-
self.client.apply_policy('AdvancedMode', True)
300+
# 7.14.2 gates TonSignTx behind AdvancedMode: the device cannot parse
301+
# raw_tx, so every TonSignTx is a blind signature and must be disclosed
302+
# as one. This test exercises signing correctness, so it opts in
303+
# explicitly rather than the firmware relaxing the gate.
304+
self.client.apply_policy("AdvancedMode", 1)
265305

266306
dest_addr = make_ton_address()
267307
raw_tx = hashlib.sha256(b'test-ton-workchain-zero').digest() * 2 # 64 bytes
@@ -289,7 +329,11 @@ def test_ton_sign_workchain_default(self):
289329
"""
290330
self.requires_fullFeature()
291331
self.setup_mnemonic_allallall()
292-
self.client.apply_policy('AdvancedMode', True)
332+
# 7.14.2 gates TonSignTx behind AdvancedMode: the device cannot parse
333+
# raw_tx, so every TonSignTx is a blind signature and must be disclosed
334+
# as one. This test exercises signing correctness, so it opts in
335+
# explicitly rather than the firmware relaxing the gate.
336+
self.client.apply_policy("AdvancedMode", 1)
293337

294338
dest_addr = make_ton_address()
295339
raw_tx = hashlib.sha256(b'test-ton-workchain-default').digest() * 2 # 64 bytes
@@ -326,7 +370,11 @@ def test_ton_sign_different_accounts(self):
326370
"""Signing with different account paths must produce different signatures."""
327371
self.requires_fullFeature()
328372
self.setup_mnemonic_allallall()
329-
self.client.apply_policy('AdvancedMode', True)
373+
# 7.14.2 gates TonSignTx behind AdvancedMode: the device cannot parse
374+
# raw_tx, so every TonSignTx is a blind signature and must be disclosed
375+
# as one. This test exercises signing correctness, so it opts in
376+
# explicitly rather than the firmware relaxing the gate.
377+
self.client.apply_policy("AdvancedMode", 1)
330378

331379
dest_addr = make_ton_address()
332380
raw_tx = hashlib.sha256(b'test-ton-different-accounts').digest() * 2 # 64 bytes

0 commit comments

Comments
 (0)