fix(calldata): accept boolean enum keys instead of failing the descriptor - #345
Open
venugopalanvip wants to merge 1 commit into
Open
venugopalanvip wants to merge 1 commit into
venugopalanvip wants to merge 1 commit into
Conversation
…ptor
An enum key is the field value it matches, and a bool holds 0 or 1 in calldata,
so `true` and `false` are the keys a descriptor writes for a bool field. JSON
has no boolean object key, so they arrive as text.
`convert_enums` ran `int(ordinal)` on them. That raises, and the exception is
caught only at the top of the conversion, so one bool enum discards every
format and every deployment of the descriptor and returns an empty list.
Two descriptors in the clear-signing registry produce nothing today:
registry/flyingtulip/calldata-PftNft.json {"True": …, "False": …} 0 -> 6
registry/ekubo/calldata-MEVCaptureRouter.json {"false": …, "true": …} 0 -> 168
The casing differs because nothing constrains it: the v2 schema says
"enumeration keys are the field values" and allows any string.
Boolean literals now map to the value the bool holds, case-insensitively.
A key that is neither decimal nor boolean still raises, because silently
mapping it to a number would build an entry the device can never match.
Co-Authored-By: Claude Opus 5 <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.
On the v2 path:
convert_erc7730_v2_input_to_calldatacallsconvert_enumsat line 231.calldata/v1here is the device TLV protocol version, not ERC-7730 v1.The bug
An enum key is the field value it matches — the v2 schema is explicit:
A bool holds
0or1in calldata, sotrueandfalseare the keys a descriptor writes for a bool field. JSON has no boolean object key, so they arrive as text.convert_enumsranint(ordinal)on them:int("true")raises, and nothing catches it until the top of the conversion, where the handler discards the entire descriptor — every format, every deployment — and returns an empty list:One bool enum is enough. The descriptor still passes the registry's own CI, because neither reference test runner goes through this converter — so the failure only appears when device descriptors are generated.
What it costs today
Two descriptors on
masterof the clear-signing registry produce nothing:registry/flyingtulip/calldata-PftNft.json{"True": "Grant all", "False": "Deny all"}registry/ekubo/calldata-MEVCaptureRouter.json{"false": "Token 0", "true": "Token 1"}I found them by converting all 280 calldata descriptors in the registry: three produced nothing, and these were two of the three. (The third,
registry/igra/calldata-KasExitBridge.json, is unrelated — its only deployment is on a chainledger_network_iddoes not know. That is a separate matter I am writing up on its own.)The casing differs between the two because nothing constrains it — the schema allows any string key, and each author wrote what looked natural.
It is also about to get more common. ethereum/clear-signing-erc7730-registry#2912 proposes a bool enum for the DAI permit
allowedflag, and my own #2989 adds one for Lido'ssetApprovalForAll— that branch converts to 0 descriptors today, against 11 on master, which is how I noticed.The fix
Boolean literals map to the value the bool holds, case-insensitively and ignoring surrounding space:
This matches how the constraint encoder in the same package already treats a bool value:
A key that is neither decimal nor boolean still raises. Silently mapping it to a number would build an enum entry the device can never match, which is worse than the loud failure.
Tests
tests/convert/calldata/v1/test_enum_ordinal.py: decimal keys including a negative, the six boolean spellings plus a padded one, and five keys that must still raise.Verified against the registry: the two descriptors above now convert, numeric-enum descriptors are unchanged (
safe/calldata-Safe-1.4.1.jsonstill 49), and descriptors producing zero output across the whole registry go from 3 to 1.One thing I did not change
convert_enumstakes noOutputAdder, so a bad key can only raise, and the handler that catches it cannot attribute the failure to an enum — the message above never names the descriptor, the enum id, or the key. Threadingoutthrough would let a single unusable entry be reported and skipped rather than costing the whole file. That is a wider change than this fix needs, so I have left it; happy to follow up if it is wanted.