From e5d174e4475cfb8260326e43fa2ac4d4dfe3684c Mon Sep 17 00:00:00 2001 From: venugopalanvip <102638505+venugopalanvip@users.noreply.github.com> Date: Tue, 15 Sep 2026 15:48:37 +0530 Subject: [PATCH 1/2] fix(lint): do not warn that a hidden label may be truncated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_collect_long_labels` measured every label against the 20-character device limit, including labels on fields the device never draws. Those cannot be truncated, because they never reach a screen. It fires today. `erc7730 lint` on `registry/p2p/calldata-P2pSsvProxyFactory.json` in the clear-signing registry reports "Display label too long" for `Referrer Config Basis Points`, a field carrying `"visible": "never"`. Descriptors routinely name hidden struct leaves in full, so the warning is noise that competes with the real ones. Two rules are skipped now: - `never` — never drawn. - `mustMatch` — a constraint the device enforces without displaying the field, which is why `convert_erc7730_v2_input_to_calldata` lets such a field omit its label entirely. `ifNotIn` and `optional` are not skipped: both put the field on a screen at least some of the time, so their labels can still truncate. Co-Authored-By: Claude Opus 5 --- .../lint/v2/lint_validate_max_length.py | 24 +++++++- .../v2/lint/test_lint_validate_max_length.py | 60 +++++++++++++++++++ 2 files changed, 83 insertions(+), 1 deletion(-) create mode 100644 tests/v2/lint/test_lint_validate_max_length.py diff --git a/src/erc7730/lint/v2/lint_validate_max_length.py b/src/erc7730/lint/v2/lint_validate_max_length.py index 8a826899..8e79387a 100644 --- a/src/erc7730/lint/v2/lint_validate_max_length.py +++ b/src/erc7730/lint/v2/lint_validate_max_length.py @@ -18,8 +18,15 @@ from erc7730.common.output import OutputAdder from erc7730.lint.v2 import ERC7730Linter from erc7730.model.input.v2.descriptor import InputERC7730Descriptor +from erc7730.model.input.v2.format import VisibilityRule from erc7730.model.resolved.v2.descriptor import ResolvedERC7730Descriptor -from erc7730.model.resolved.v2.display import ResolvedField, ResolvedFieldDescription, ResolvedFieldGroup +from erc7730.model.resolved.v2.display import ( + ResolvedField, + ResolvedFieldDescription, + ResolvedFieldGroup, + ResolvedVisibilityConditions, + ResolvedVisibilityRules, +) @final @@ -106,10 +113,25 @@ def _validate_display_lengths(cls, descriptor: ResolvedERC7730Descriptor, out: O f"{FIELD_NAME_MAX_LENGTH} characters and may be truncated on Ledger devices.", ) + @staticmethod + def _is_never_displayed(visible: ResolvedVisibilityRules | None) -> bool: + """Whether the field is only ever checked, never put on a screen. + + A ``mustMatch`` field is a constraint the device enforces without displaying it, which + is why the descriptor may omit its label altogether. ``ifNotIn`` and ``optional`` both + do reach a screen, so their labels still count. + """ + if visible == VisibilityRule.NEVER: + return True + return isinstance(visible, ResolvedVisibilityConditions) and visible.mustMatch is not None + @classmethod def _collect_long_labels(cls, field: ResolvedField, too_long: set[str]) -> None: match field: case ResolvedFieldDescription(): + # a label that never reaches a screen cannot be truncated on one + if cls._is_never_displayed(field.visible): + return if field.label is not None and len(field.label) > FIELD_NAME_MAX_LENGTH: too_long.add(field.label) case ResolvedFieldGroup(): diff --git a/tests/v2/lint/test_lint_validate_max_length.py b/tests/v2/lint/test_lint_validate_max_length.py new file mode 100644 index 00000000..4591fa4a --- /dev/null +++ b/tests/v2/lint/test_lint_validate_max_length.py @@ -0,0 +1,60 @@ +import pytest + +from erc7730.lint.v2.lint_validate_max_length import ValidateMaxLengthLinter +from erc7730.model.input.v2.format import VisibilityRule +from erc7730.model.paths.path_parser import to_path +from erc7730.model.resolved.v2.display import ( + ResolvedFieldDescription, + ResolvedFieldGroup, + ResolvedVisibilityConditions, + ResolvedVisibilityRules, +) + +# 28 characters, against the 20-character field name limit +LONG = "Referrer Config Basis Points" + + +def field(visible: ResolvedVisibilityRules | None) -> ResolvedFieldDescription: + return ResolvedFieldDescription(label=LONG, path=to_path("#.referrerConfig.basisPoints"), visible=visible) + + +def collected(visible: ResolvedVisibilityRules | None) -> set[str]: + out: set[str] = set() + ValidateMaxLengthLinter._collect_long_labels(field(visible), out) + return out + + +@pytest.mark.parametrize( + "visible", + [ + pytest.param(VisibilityRule.ALWAYS, id="always"), + pytest.param(VisibilityRule.OPTIONAL, id="optional"), + pytest.param(None, id="unset"), + pytest.param(ResolvedVisibilityConditions(ifNotIn=["0"]), id="ifNotIn"), + ], +) +def test_label_that_can_reach_a_screen_is_reported(visible: ResolvedVisibilityRules | None) -> None: + """always, optional, unset and ifNotIn all display the field at least sometimes.""" + assert collected(visible) == {LONG} + + +@pytest.mark.parametrize( + "visible", + [ + pytest.param(VisibilityRule.NEVER, id="never"), + pytest.param(ResolvedVisibilityConditions(mustMatch=["0"]), id="mustMatch"), + ], +) +def test_label_that_never_reaches_a_screen_is_not_reported(visible: ResolvedVisibilityRules) -> None: + """A hidden label cannot truncate, and a mustMatch field may legitimately carry no label.""" + assert collected(visible) == set() + + +def test_a_group_does_not_hide_the_labels_inside_it() -> None: + """ResolvedFieldGroup carries no visibility of its own, so each child decides for itself.""" + group = ResolvedFieldGroup(fields=[field(VisibilityRule.NEVER), field(VisibilityRule.ALWAYS)]) + + out: set[str] = set() + ValidateMaxLengthLinter._collect_long_labels(group, out) + + assert out == {LONG} From 67d66b21fc9289c5f147c2ee2f96693ccba0f972 Mon Sep 17 00:00:00 2001 From: venugopalanvip <102638505+venugopalanvip@users.noreply.github.com> Date: Wed, 16 Sep 2026 13:31:57 +0530 Subject: [PATCH 2/2] fix(lint): measure an interpolated intent by the text it displays `interpolatedIntent` was measured as written, so `{path}` counted toward the 30-character limit. The braces and the path inside them are template syntax and never reach a screen, and the length of the value that replaces them is not known here -- an amount may render as "1 ETH" or as eighteen decimal places. Across the clear-signing registry, 109 interpolated intents are longer than 30 characters as written. Only 24 have literal text that still exceeds the limit. The other 85 warnings measure `{execution.desc.minReturnAmount}` and the like: 32 characters of path that is never displayed. Only the literal text around the placeholders 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, which keeps the check conservative and keeps the 24 real cases. Co-Authored-By: Claude Opus 5 --- .../lint/v2/lint_validate_max_length.py | 19 ++++++++++-- .../v2/lint/test_lint_validate_max_length.py | 29 +++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/src/erc7730/lint/v2/lint_validate_max_length.py b/src/erc7730/lint/v2/lint_validate_max_length.py index 8e79387a..40cbb9fc 100644 --- a/src/erc7730/lint/v2/lint_validate_max_length.py +++ b/src/erc7730/lint/v2/lint_validate_max_length.py @@ -4,6 +4,7 @@ Adapted from v1 ValidateMaxLengthLinter to use v2 model types. """ +import re from typing import final, override from erc7730.common.ledger import ( @@ -86,8 +87,8 @@ def _validate_display_lengths(cls, descriptor: ResolvedERC7730Descriptor, out: O for fmt in descriptor.display.formats.values(): if fmt.intent is not None and isinstance(fmt.intent, str) and len(fmt.intent) > OPERATION_TYPE_MAX_LENGTH: too_long_intents.add(fmt.intent) - if fmt.interpolatedIntent is not None and len(fmt.interpolatedIntent) > OPERATION_TYPE_MAX_LENGTH: - too_long_intents.add(fmt.interpolatedIntent) + if (ii := fmt.interpolatedIntent) is not None and cls._literal_length(ii) > OPERATION_TYPE_MAX_LENGTH: + too_long_intents.add(ii) if fmt.id is not None and len(fmt.id) > OPERATION_TYPE_MAX_LENGTH: too_long_ids.add(fmt.id) @@ -125,6 +126,20 @@ def _is_never_displayed(visible: ResolvedVisibilityRules | None) -> bool: return True return isinstance(visible, ResolvedVisibilityConditions) and visible.mustMatch is not None + _PLACEHOLDER = re.compile(r"\{[^}]*\}") + + @classmethod + def _literal_length(cls, interpolated_intent: str) -> int: + """Length of the text an interpolated intent is guaranteed to display. + + `{path}` is template syntax. Neither the braces nor the path inside them reaches a + screen, and the length of the value that replaces them is not known here -- an amount + may render as "1 ETH" or as eighteen decimal places. Only the literal text around the + placeholders is certain to be displayed, so it is the only part that can be measured. + Over the limit on that alone means the intent truncates whatever the values are. + """ + return len(cls._PLACEHOLDER.sub("", interpolated_intent)) + @classmethod def _collect_long_labels(cls, field: ResolvedField, too_long: set[str]) -> None: match field: diff --git a/tests/v2/lint/test_lint_validate_max_length.py b/tests/v2/lint/test_lint_validate_max_length.py index 4591fa4a..75025fad 100644 --- a/tests/v2/lint/test_lint_validate_max_length.py +++ b/tests/v2/lint/test_lint_validate_max_length.py @@ -58,3 +58,32 @@ def test_a_group_does_not_hide_the_labels_inside_it() -> None: ValidateMaxLengthLinter._collect_long_labels(group, out) assert out == {LONG} + + +@pytest.mark.parametrize( + "intent,expected", + [ + pytest.param("Withdraw {_amounts.[]}", 9, id="one_placeholder"), + pytest.param("Send {sharesAmount} to {recipient}", 9, id="two_placeholders"), + pytest.param("Stake ETH with SSV", 18, id="no_placeholder"), + pytest.param("{_approved} unstETH NFTs", 13, id="leading_placeholder"), + ], +) +def test_literal_length_counts_only_what_is_certain_to_be_displayed(intent: str, expected: int) -> None: + """A `{path}` is template syntax, and the value replacing it has no length known here.""" + assert ValidateMaxLengthLinter._literal_length(intent) == expected + + +def test_a_long_path_inside_a_placeholder_does_not_make_the_intent_long() -> None: + """`{execution.desc.minReturnAmount}` is 32 characters of path that never reaches a screen.""" + intent = "Swap for at least {execution.desc.minReturnAmount}" + + assert len(intent) > 30 + assert ValidateMaxLengthLinter._literal_length(intent) == 18 + + +def test_literal_text_over_the_limit_is_still_measured() -> None: + """Literal text alone over the limit truncates whatever the values render to.""" + intent = "Authorize decryption on behalf of {delegator} for {contracts} for {days} days" + + assert ValidateMaxLengthLinter._literal_length(intent) > 30