fix: EIP-1559 fee display and RLP integer encoding#236
Open
BitHighlander wants to merge 4 commits into
Open
Conversation
…_gas field Bug 1 (display): formatEthereumFeeEIP1559 computed fee rate as max_fee_per_gas + max_priority_fee_per_gas before multiplying by gas_limit. The priority fee is a tip contained within max_fee_per_gas, not additive — the user's maximum cost is max_fee_per_gas * gas_limit. This caused the displayed fee to be overstated. Bug 2 (signing): max_priority_fee_per_gas was only included in the RLP length calculation and hash when has_max_priority_fee_per_gas was set. EIP-1559 always requires this field (9-field list). If the host omitted it, the firmware produced an 8-field RLP list — valid length header but wrong field count — resulting in a transaction that would be rejected at broadcast. nanopb zero-initialises unset fields, so removing the guard is safe: size=0 encodes as 0x80 (integer zero).
…llow paper RLP-encoded integer fields (nonce, gas, value, fees) must not carry leading zero bytes. hash_rlp_field preserved them verbatim, producing non-canonical transactions that are rejected on broadcast for any field whose big-endian representation has a leading zero (common for small nonces, zero values, etc.). Added hash_rlp_bytes_stripped() and applied it to nonce, gas_price, gas_limit, max_fee_per_gas, max_priority_fee_per_gas, and value. The to address field is intentionally left using hash_rlp_field — addresses are not integers.
hash_rlp_field(..., sizeof(uint8_t)) only hashed the low byte of chain_id (little-endian uint32). Chain IDs above 255 — Base (8453), Arbitrum (42161), Avalanche C-Chain (43114) — were all hashed as their low byte only, producing a signature that recovers the wrong signer on these networks. Use hash_rlp_number(chain_id) which converts to big-endian and strips leading zeros, matching the Ethereum yellow paper.
deps/device-protocol → 8ef74da (feat(ripple): memo + THORChain routing) deps/python-keepkey → bf870e6 (7.14.2: XRP memo + EVM depositWithExpiry + msg-signing)
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
Two EIP-1559 correctness fixes:
Fee display:
formatEthereumFeeEIP1559was addingmax_priority_fee_per_gason top ofmax_fee_per_gasbefore multiplying bygas_limit. The priority fee is a tip contained within max_fee, not additive — displayed fee was overstated.RLP encoding: Integer fields (nonce, gas, value, fees) must not carry leading zero bytes per the Ethereum yellow paper.
hash_rlp_fieldpreserved them verbatim. Addedhash_rlp_bytes_stripped()and applied it to all integer fields. Thetoaddress field is intentionally unchanged.Test plan