test(eth): EIP-1559 chunked-data regression guard (firmware 7.14.1+)#193
Merged
Merged
Conversation
… ≤ 7.14.0)
Pairs the device, signs a 1550-byte EIP-1559 transaction with the
all-all-all test mnemonic, and asserts that ECDSA recovery against the
canonical type-2 pre-image yields the device's own address.
Catches a firmware/ethereum.c ordering bug present in 7.x.0 .. 7.14.0
where the empty access-list byte (0xC0) — which closes the EIP-1559 RLP
body and must be the last byte fed to keccak before signing — was being
hashed inside ethereum_signing_init() right after the initial 1024-byte
data chunk, BEFORE the host had a chance to send the remaining
EthereumTxAck frames. For any tx whose data exceeded the single-chunk
threshold, the resulting pre-image was:
keccak( ...header...
|| data_len_prefix
|| data[0..1024]
|| 0xC0 (bug: should be after ALL data)
|| data[1024..end] )
The signature was mathematically valid for that mangled hash so RPCs
accepted the broadcast, but the recovered signer was a wrong-but-
deterministic address. The mempool dropped the tx because the recovered
"from" had no balance / wrong nonce. Production symptom: every Uniswap
Universal Router swap, Permit2 batch, and large multicall hung at
"Confirm in wallet."
Single-chunk transactions (<= 1024 bytes) escaped the bug only by
accident — the misplaced 0xC0 happened to land at the end anyway.
Recovery-based assertion (eth-keys, eth-utils.keccak) — works on any
seed, no golden vectors to capture, the test asserts the actual
invariant: "signature recovers to the signer." Fails on broken
firmware, passes on 7.14.1+.
CI: eth-keys added to the existing pip install line; ships a pure-Python
keccak via eth-utils so no native deps are required.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
requires_message("EthereumTxAck") sends an empty EthereumTxAck as a
discovery probe. The firmware (correctly) rejects that with
Failure_UnexpectedMessage because we're not mid-sign, which skips the
test before the actual assertion runs.
requires_firmware("7.2.1") is sufficient — EthereumTxAck has been part
of the protocol since EIP-1559 support landed in 7.2.1.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
eth-utils ships keccak via the eth-hash adapter, which auto-selects between pycryptodome and pysha3 at import time. Without either backend installed, importing keccak raises: ImportError: None of these hashing backends are installed: ['pycryptodome', 'pysha3']. The new EIP-1559 chunked-data regression test imports keccak from eth_utils to build the canonical type-2 pre-image, so it failed at import rather than at the recovery assertion. Adding pycryptodome to the existing pip-install line fixes it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
KeepKeyTest overrides unittest's assertEqual with a 2-arg version (common.py:104) that doesn't accept the optional msg parameter — passing one raises: TypeError: KeepKeyTest.assertEqual() takes 3 positional arguments but 4 were given Print the regression diagnostic before asserting instead. Pytest captures stdout on failure, so the divergence (expected vs recovered, canonical hash, sig values) still surfaces in the failure report. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Upstreaming this test as a permanent regression guard rather than a one-shot bug catcher. Bumping requires_firmware from 7.2.1 (the version where EIP-1559 support originally landed) to 7.14.1 (the first version where the access-list ordering bug is fixed) so CI on broken builds skips this test instead of flagging a known-broken state as a new regression. The header comment already documents the affected range (7.x.0 .. 7.14.0) and the fix landing in 7.14.1. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pastaghost
approved these changes
Apr 29, 2026
BitHighlander
added a commit
to BitHighlander/keepkey-firmware
that referenced
this pull request
Apr 29, 2026
Pulls in the EIP-1559 chunked-data regression test merged in keepkey/python-keepkey#193, which gates on requires_firmware("7.14.1"). Combined with the version bump in this branch, CI will now actually run (not skip) the test that asserts the access-list ordering fix. Also includes #190 (7.15-expanded-tests) carried in from master. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Permanent regression guard for the EIP-1559 chunked-data signing bug that shipped in firmware 7.x.0 .. 7.14.0 and was fixed in 7.14.1.
The bug:
firmware/ethereum.chashed the empty access-list byte (0xC0) — which closes the EIP-1559 RLP body and must be the last byte fed to keccak before signing — insideethereum_signing_init()immediately after the initial 1024-byte data chunk, before the host had a chance to send the remainingEthereumTxAckframes. For any tx whosedataexceeded the single-USB-chunk threshold (1024 B), the firmware produced a non-canonical pre-image:The signature was mathematically valid for the mangled hash so RPCs accepted the broadcast, but the recovered signer was a wrong-but-deterministic address. The mempool dropped the tx because the recovered
fromhad no balance / wrong nonce. Production symptom: every Uniswap Universal Router swap, Permit2 batch, and large multicall on affected firmware hung at "Confirm in wallet."Single-chunk transactions (≤ 1024 B) escaped the bug only by accident — the misplaced
0xC0happened to land at the end anyway.What this PR adds
tests/test_msg_ethereum_signtx_chunked_data_eip1559.py— recovery-based, seed-agnostic regression. Pairs the device with the all-all-all test mnemonic, signs a 1550-byte EIP-1559 transaction (same size class as the captured production failure), builds the canonical type-2 unsigned-envelope keccak hash, performs ECDSA recovery, and asserts the recovered address equals the device's own EOA. No golden vectors to maintain — asserts the actual invariant: "signature recovers to the signer.".github/workflows/ci.yml— addseth-keys+pycryptodometo the existingpip installline. Pure-Python keccak viaeth-utils, no native deps.scripts/generate-test-report.py— adds anE5brow so the test surfaces in the test report.Difference from #191
Previous PR (#191, closed) gated on
requires_firmware("7.2.1")— the version EIP-1559 support landed — so it was intentionally RED on every emulator pinned to ≤ 7.14.0. That made sense as a one-shot bug catcher but is the wrong shape for an ongoing CI guard.This PR gates on
requires_firmware("7.14.1")so the test skips on broken firmware and runs+passes on fixed firmware. CI stays green; the test still fails loudly on any future regression.Test plan
pytest tests/test_msg_ethereum_signtx_chunked_data_eip1559.pypasses.