Skip to content

fix(calldata): always emit DECIMALS in PARAM_UNIT - #353

Merged
paoun-ledger merged 1 commit into
mainfrom
fix/always-emit-unit-decimals
Oct 9, 2026
Merged

paoun-ledger merged 1 commit into
mainfrom
fix/always-emit-unit-decimals

Conversation

@paoun-ledger

Copy link
Copy Markdown
Collaborator

Problem

app-ethereum commit 21d2695f (shipped since 1.22.4) made DECIMALS mandatory in PARAM_UNIT, while the app's own spec documents it as optional with a default of 0. python-erc7730 only emitted the tag when decimals was set, so any unit field without decimals is rejected with 6a80 and the whole transaction falls back to blind signing on released devices (e.g. opencover's "Cover duration").

Fix

Always serialize DECIMALS in tlv_param_unit, defaulting to 0 — the exact value the app is specified to assume when the tag is absent, so display is unchanged. The calldata model (decimals: int | None) is left as is.

The app-side check should still be relaxed (or the spec updated) so other descriptor producers don't hit the same issue.

Tests

  • New test_convert_unit_always_serializes_decimals (unset → 0, set → value); the unset case fails without the fix.
  • prek run --all-files and pytest tests pass locally.

🤖 Generated with Claude Code

app-ethereum >= 1.22.4 rejects PARAM_UNIT structs without DECIMALS (6a80),
although its spec documents the tag as optional with a default of 0. Unit
fields without decimals (e.g. opencover "Cover duration") therefore fall
back to blind signing on released devices.

Always serialize DECIMALS, defaulting to 0, which is byte-for-byte the
value the app is specified to assume.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@paoun-ledger
paoun-ledger requested a review from a team as a code owner October 9, 2026 12:27
@paoun-ledger
paoun-ledger merged commit e88b2a6 into main Oct 9, 2026
13 checks passed
@paoun-ledger
paoun-ledger deleted the fix/always-emit-unit-decimals branch October 9, 2026 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants