fix: EIP-1559 chainId hashing broken for chainId >= 256#427
Merged
pastaghost merged 4 commits intoMay 23, 2026
Conversation
hash_rlp_field((uint8_t*)&chain_id, 1) only hashed the least-significant byte of chain_id on little-endian ARM. For Base (8453=0x2105), this hashed 0x05 instead of the correct 3-byte RLP 0x82 0x21 0x05, producing a wrong keccak pre-image — the signature recovered to a random address with 0 ETH. The RLP length calculation already used rlp_calculate_number_length(chain_id) correctly. The legacy EIP-155 path also used hash_rlp_number correctly. Only the EIP-1559 hash step was wrong. Fix: replace hash_rlp_field((uint8_t*)&chain_id, sizeof(uint8_t)) with hash_rlp_number(chain_id) which strips leading zeros and encodes all chain IDs correctly. Affected chains (chainId >= 256): Base (8453), Arbitrum (42161), Avalanche (43114). Unaffected: ETH (1), OP (10), BSC (56), Polygon (137).
This was referenced May 21, 2026
There was a problem hiding this comment.
Pull request overview
Fixes EIP-1559 transaction signing on chains with chainId >= 256 by ensuring the chain ID is RLP-encoded and hashed correctly in the signing preimage, matching the legacy EIP-155 path’s handling.
Changes:
- Replace incorrect 1-byte
hash_rlp_field((uint8_t*)&chain_id, 1)withhash_rlp_number(chain_id)for EIP-1559 chain ID hashing. - Remove outdated comments implying EIP-1559 chain IDs are “only one byte for now.”
- Add an explanatory comment documenting the endianness/LSB bug and affected chain IDs.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Comment on lines
+863
to
+868
| // Arbitrum=42161). For chain_id == 0, hash_rlp_number is a no-op, so | ||
| // explicitly hash the single-byte RLP encoding for zero to keep the | ||
| // preimage consistent with rlp_calculate_number_length(chain_id). | ||
| if (chain_id == 0) { | ||
| uint8_t zero_rlp[1] = {0x80}; | ||
| hash_data(zero_rlp, sizeof(zero_rlp)); |
Comment on lines
+861
to
+871
| // chain_id is a uint32; hash_rlp_number strips leading zeros and encodes | ||
| // correctly for all chain IDs including multi-byte ones (Base=8453, | ||
| // Arbitrum=42161). For chain_id == 0, hash_rlp_number is a no-op, so | ||
| // explicitly hash the single-byte RLP encoding for zero to keep the | ||
| // preimage consistent with rlp_calculate_number_length(chain_id). | ||
| if (chain_id == 0) { | ||
| uint8_t zero_rlp[1] = {0x80}; | ||
| hash_data(zero_rlp, sizeof(zero_rlp)); | ||
| } else { | ||
| hash_rlp_number(chain_id); | ||
| } |
EIP-1559 requires a chain_id per the spec. Explicitly reject signing when chain_id is absent/zero with a SyntaxError — this avoids the preimage corruption that would result from hash_rlp_number() being a no-op for 0, and removes the need for the raw 0x80 literal that Copilot flagged as bypassing the RLP helpers. After the guard, hash_rlp_number(chain_id) is safe for all valid chain IDs (>= 1) and no special-casing is needed.
pastaghost
force-pushed
the
fix/eip1559-chainid-multibyte
branch
from
5c24fd8 to
e86e44a
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
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.
Problem
ethereum_signing_init()inlib/firmware/ethereum.chashes the wrong pre-image for EIP-1559 transactions on chains withchainId >= 256(Base=8453, Arbitrum=42161, Avalanche=43114).The bug:
chain_idis auint32_t. On ARM little-endian,(uint8_t*)&chain_idpoints to the least-significant byte. For Base (8453 = 0x2105), this feeds0x05to keccak instead of the correct 3-byte RLP encoding0x82 0x21 0x05.The RLP length calculation already used
rlp_calculate_number_length(chain_id)correctly, so the list header was sized for 3 bytes but only 1 byte was hashed — corrupting the keccak pre-image. The resulting signature recovered to a random address with 0 ETH, causing every EIP-1559 transaction on those chains to fail with "insufficient funds".The legacy EIP-155 signing path (
send_signature()) already usedhash_rlp_number(chain_id)correctly. Only the EIP-1559 path had this copy-paste bug.Fix
One line changed.
hash_rlp_number()converts the uint32 to big-endian, strips leading zeros, and callshash_rlp_fieldwith the correct byte count — consistent with how the legacy path handles it.Affected chains
0x05was hashed0x11was hashed0x6awas hashedTest vector (Base, chainId=8453)
Correct EIP-1559 signing pre-image for chainId=8453 must include RLP bytes
82 21 05for the chain ID field. Before this fix the firmware fed only05, producing a different keccak hash and a signature that recovers to a wrong address.