feat(7.14.2): XRP THORChain memo + EVM depositWithExpiry tests#18
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>
…gnition - Bump device-protocol submodule to 8f80bcd (adds memo field to RippleSignTx) - Update messages_ripple_pb2.py with memo field (field 7, optional string) compatible with protobuf==3.20.3 (old-format serialized_pb descriptor) - Add test_sign_with_thorchain_memo in test_msg_ripple_sign_tx.py: verifies serialized XRPL tx ends with canonical Memos array binary (F9 EA 7D <len> <memo> E1 F1), requires firmware 7.14.2 - Add test_msg_ethereum_thorchain_deposit.py: covers legacy deposit() 0x1fece7b4 selector, new depositWithExpiry() 0x44bc937b selector (requires 7.14.2, no AdvancedMode), and verifies non-THORChain addresses are still blocked without AdvancedMode
BitHighlander
force-pushed
the
release/7.14.2-python-keepkey
branch
from
May 15, 2026 19:04
8159a7f to
297cba3
Compare
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
1 participant
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
messages_ripple_pb2.pyto addmemofield (field 7) toRippleSignTx— old-format protobuf descriptor compatible withprotobuf==3.20.3device-protocolsubmodule to8f80bcdwhich addsoptional string memo = 7to the prototest_sign_with_thorchain_memointest_msg_ripple_sign_tx.py: signs an XRP payment with a THORChain routing memo and verifies the serialized XRPL tx ends with the canonical Memos binary encoding (F9 EA 7D <len> <utf8-memo> E1 F1)test_msg_ethereum_thorchain_deposit.pywith three tests for the EVMdepositWithExpiry()fix:deposit()selector0x1fece7b4recognized without AdvancedMode (≥7.5.0)depositWithExpiry()selector0x44bc937brecognized without AdvancedMode (≥7.14.2)depositWithExpiry()selector is still blockedContext
Part of firmware release 7.14.2:
Memosarray. Without the protomemofield, swaps from XRP to other chains were completely broken.depositWithExpiry()(selector0x44bc937b). Firmware pre-7.14.2 only recognized the legacydeposit()selector, causing every EVM→THORChain swap to fail unless AdvancedMode was enabled.Test plan
python -m pytest tests/test_msg_ripple_sign_tx.py::TestMsgRippleSignTx::test_sign_with_thorchain_memopasses on 7.14.2 devicepython -m pytest tests/test_msg_ethereum_thorchain_deposit.pyall 3 tests passtest_signstill passes (no regression)PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=python python -c "from keepkeylib import messages_ripple_pb2; m=messages_ripple_pb2.RippleSignTx(); m.memo='test'; assert m.memo=='test'"passes