From 249be9c36768959e14477b555b864d3a443a33d9 Mon Sep 17 00:00:00 2001 From: venugopalanvip <102638505+venugopalanvip@users.noreply.github.com> Date: Wed, 16 Sep 2026 10:17:50 +0530 Subject: [PATCH] feat(lint): report which deployment the ABI checks covered A v2 contract descriptor is validated against one reference ABI: the linter fetches deployments in order and stops at the first that yields one. Nothing said so. On a successful run it emitted no output at all, so a descriptor with three deployments compared against one looked exactly like a descriptor compared against all three. `erc7730 lint` now reports, for a contract context: - the reference it validated against, by explorer URL; - the deployments it never compared, because an earlier one supplied the ABI; - the deployments whose ABI was unavailable, each with the reason. When no deployment yields an ABI at all, the per-deployment fetch warnings are replaced by one warning naming every deployment tried and stating which checks were therefore skipped -- field paths, parameter coverage and selector exhaustiveness. Previously that final give-up was a bare `return`, so a client returning no ABI without raising produced a clean "no issue found". Severity is unchanged: this is still a warning, not an error. Whether an unavailable ABI should block a merge is a registry policy decision, not one this library should make. Asked for in ethereum/clear-signing-erc7730-registry#2871. Co-Authored-By: Claude Opus 5 --- .../lint/v2/lint_validate_display_fields.py | 51 +++++++++++++++---- .../lint/test_lint_validate_display_fields.py | 37 ++++++++++++++ 2 files changed, 79 insertions(+), 9 deletions(-) diff --git a/src/erc7730/lint/v2/lint_validate_display_fields.py b/src/erc7730/lint/v2/lint_validate_display_fields.py index 86d6e82e..a9e318fe 100644 --- a/src/erc7730/lint/v2/lint_validate_display_fields.py +++ b/src/erc7730/lint/v2/lint_validate_display_fields.py @@ -38,6 +38,23 @@ class ValidateDisplayFieldsLinter(ERC7730Linter): - No schema available in v2 resolved model, so no validation is performed """ + @staticmethod + def _coverage_message(source: str | None, unavailable: list[str], not_reached: list[str]) -> str: + """State which deployment the ABI checks ran against, and which they did not cover. + + One reference ABI validates the display fields, so a descriptor with several deployments + is only ever compared against one of them. Saying so keeps "validated" from reading as + "validated against every deployment". + """ + parts = [f"Display fields validated against the ABI of {source}."] + if unavailable: + parts.append(f"No ABI available for {len(unavailable)} earlier deployment(s): " + f"{'; '.join(unavailable)}.") + if not_reached: + parts.append(f"Not compared against {len(not_reached)} further deployment(s): " + f"{', '.join(not_reached)}.") + return " ".join(parts) + @override def lint( self, input_descriptor: InputERC7730Descriptor, descriptor: ResolvedERC7730Descriptor, out: OutputAdder @@ -59,30 +76,41 @@ def _validate_contract_display_fields( if (deployments := context.contract.deployments) is None: return - # Try to fetch ABI from Sourcify or Etherscan for the first deployment that succeeds + # Fetch the reference ABI from the first deployment that has one, recording the outcome + # of every deployment so the result says what was checked and what could not be. reference_abis = None explorer_url = None + unavailable: list[str] = [] + not_reached: list[str] = [] for deployment in deployments: + where = f"chain id {deployment.chainId} address {deployment.address}" + if reference_abis is not None: + # one reference ABI is enough to validate against, so do not fetch the rest + not_reached.append(where) + continue try: if (abis := client.get_contract_abis(deployment.chainId, deployment.address)) is None: + unavailable.append(f"{where}: no verified source found") continue except Exception as e: - out.warning( - title="Could not fetch ABI", - message=f"Fetching reference ABI for chain id {deployment.chainId} failed, display fields will " - f"not be validated against ABI: {e}", - ) + unavailable.append(f"{where}: {e}") continue reference_abis = get_functions(abis) try: explorer_url = client.get_contract_explorer_url(deployment.chainId, deployment.address) except NotImplementedError: - explorer_url = f"" - break + explorer_url = f"<{where}>" if reference_abis is None: - return + # Say so explicitly. A silent return is indistinguishable from a descriptor whose + # display fields were checked against an ABI and found correct. + return out.warning( + title="Display fields not validated against an ABI", + message="No deployment yielded a reference ABI, so display field paths, parameter coverage " + f"and selector exhaustiveness were not checked. Tried {len(unavailable)} deployment(s): " + + "; ".join(unavailable), + ) if reference_abis.proxy: return out.info( @@ -90,6 +118,11 @@ def _validate_contract_display_fields( message=f"Contract {explorer_url} is likely to be a proxy, validation of display fields skipped", ) + out.info( + title="Reference ABI", + message=cls._coverage_message(explorer_url, unavailable, not_reached), + ) + # Build ABI paths by selector abi_paths_by_selector: dict[str, set[DataPath]] = {} for selector, abi in reference_abis.functions.items(): diff --git a/tests/v2/lint/test_lint_validate_display_fields.py b/tests/v2/lint/test_lint_validate_display_fields.py index a3550b84..0c447c06 100644 --- a/tests/v2/lint/test_lint_validate_display_fields.py +++ b/tests/v2/lint/test_lint_validate_display_fields.py @@ -38,3 +38,40 @@ def test_every_declared_name_is_accepted_when_the_reference_abi_names_nothing() def test_no_name_is_accepted_when_the_format_key_is_a_selector() -> None: assert unnamed_parameter_names(reference_abi("", "", ""), declared_abi=None) == set() + + +def coverage(unavailable: list[str] | None = None, not_reached: list[str] | None = None) -> str: + return ValidateDisplayFieldsLinter._coverage_message( + "https://etherscan.io//address/0xabc#code", unavailable or [], not_reached or [] + ) + + +def test_coverage_names_the_reference_it_validated_against() -> None: + assert coverage() == "Display fields validated against the ABI of https://etherscan.io//address/0xabc#code." + + +def test_coverage_reports_deployments_it_never_compared() -> None: + """One ABI validates the fields, so the other deployments are not covered and must be named.""" + message = coverage(not_reached=["chain id 137 address 0xdead", "chain id 10 address 0xbeef"]) + + assert "Not compared against 2 further deployment(s)" in message + assert "chain id 137 address 0xdead" in message + assert "chain id 10 address 0xbeef" in message + + +def test_coverage_keeps_the_reason_an_earlier_deployment_had_no_abi() -> None: + """The reason is what tells a reviewer whether to go and verify the contract.""" + message = coverage(unavailable=["chain id 1 address 0xaaa: no verified source found"]) + + assert "No ABI available for 1 earlier deployment(s)" in message + assert "no verified source found" in message + + +def test_coverage_reports_both_kinds_of_gap_together() -> None: + message = coverage( + unavailable=["chain id 1 address 0xaaa: no verified source found"], + not_reached=["chain id 10 address 0xbeef"], + ) + + assert "No ABI available" in message + assert "Not compared against" in message