Skip to content

fix(lint): stop reporting lengths that can never truncate - #7

Open
venugopalanvip wants to merge 1 commit into
sourcifyeth:mainfrom
venugopalanvip:fix-lint-false-positives
Open

venugopalanvip wants to merge 1 commit into
sourcifyeth:mainfrom
venugopalanvip:fix-lint-false-positives

Conversation

@venugopalanvip

Copy link
Copy Markdown

Two false positives in ValidateMaxLengthLinter, both the same shape: it measures a string that never reaches a screen against the limit for strings that do.

This matters here rather than only upstream because this is the library clear-signing-erc7730-registry installs for its validate descriptors job since ethereum/clear-signing-erc7730-registry#3008, so the noise lands on every contributor there.

1. A hidden label cannot be truncated

_collect_long_labels measured every label, including fields the device never draws:

$ erc7730 lint registry/p2p/calldata-P2pSsvProxyFactory.json
🟠 Display label too long: `Referrer Config Basis Points` exceeds 20 characters
   and may be truncated on Ledger devices.

That field is "visible": "never". Descriptors routinely spell out hidden struct leaves in full — it is how a reader tells #._referrerConfig.basisPoints from #._clientConfig.basisPoints.

rule drawn? measured?
always yes yes
optional when the wallet can yes
unset yes yes
ifNotIn when the value is not in the list yes
never no no
mustMatch no — checked, not shown no

mustMatch is worth spelling out: it is a constraint the device enforces without displaying the field, which is exactly why convert_erc7730_v2_input_to_calldata already lets such a field omit its label —

# A mustMatch field is never displayed, so the descriptor is allowed to omit its label, but
# the FIELD struct still requires a NAME tag.

A linter that permits no label while warning about a long one contradicts the converter.

2. {path} is template syntax, not displayed text

interpolatedIntent was measured as written, so the braces and the path inside them counted toward the 30-character limit. Neither is displayed, and the length of the value replacing them is not knowable here — an amount may render as 1 ETH or as eighteen decimal places.

The clearest case in the registry:

Swap {execution.desc.amount} for at least {execution.desc.minReturnAmount} to recipient {execution.desc.dstReceiver}

116 characters as written, 33 of literal text. {execution.desc.minReturnAmount} alone is 32 characters of path that never reaches a device.

Measured across the registry, at 0fbe25b

count
long labels on hidden fields — cannot truncate 83
long labels on visible fields — all still reported 24
interpolatedIntent over 30 as written 111
— of those, placeholder text only 87
— genuinely too long, all still reported 24

So 170 of the 218 warnings in this family are about characters that never reach a device, and all 48 real ones survive.

Only the literal text is certain to reach the screen, so that is what is measured now. Over the limit on that alone means the intent truncates whatever the values render to — conservative, and it keeps every genuine case.

Scope and tests

Both remain warnings; nothing changes an exit code. Neither touches intent, id, metadata or enum lengths, which are literal strings and were always measured correctly.

tests/v2/lint/test_lint_validate_max_length.py is new: all six visibility rules, a ResolvedFieldGroup case proving a hidden child does not hide its visible sibling's label, and the literal-length measure including a 32-character path inside a placeholder and a literal that is genuinely over.

Also raised upstream as LedgerHQ/python-erc7730#341, unreviewed. Sending it here too since this is the copy the registry runs.

Two false positives in ValidateMaxLengthLinter, both the same shape: it
measures a string that never reaches a screen against the limit for strings
that do. This repository is the library clear-signing-erc7730-registry now
installs for its `validate descriptors` job, so the noise lands on every
contributor there.

1. A hidden label cannot be truncated. `_collect_long_labels` measured every
   label, including fields the device never draws. Descriptors routinely spell
   out hidden struct leaves in full -- it is how a reader tells
   `#._referrerConfig.basisPoints` from `#._clientConfig.basisPoints`.

   `never` and `mustMatch` are skipped. `ifNotIn` and `optional` are not: both
   reach a screen at least some of the time. `mustMatch` is worth spelling out
   because convert_erc7730_v2_input_to_calldata already lets such a field omit
   its label entirely, so warning about a long one contradicted the converter.

2. `{path}` is template syntax, not displayed text. `interpolatedIntent` was
   measured as written, so the braces and the path inside them counted toward
   the 30 character limit -- `{execution.desc.minReturnAmount}` is 32 on its
   own. Only the literal text is certain to reach the screen, so that is what
   is measured now. Over the limit on that alone means it truncates whatever
   the values render to.

Measured across every descriptor in the registry at 0fbe25b:

  long labels on hidden fields     83 warnings, none of which can truncate
  long labels on visible fields    24 warnings, all kept
  interpolatedIntent over 30       111, of which 87 are placeholder text only
                                   and 24 genuinely too long, all kept

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant