diff --git a/src/erc7730/lint/v2/__init__.py b/src/erc7730/lint/v2/__init__.py index af80328d..ce7512b6 100644 --- a/src/erc7730/lint/v2/__init__.py +++ b/src/erc7730/lint/v2/__init__.py @@ -2,6 +2,7 @@ from typing import final, override from erc7730.common.output import OutputAdder +from erc7730.model.input.v2.descriptor import InputERC7730Descriptor from erc7730.model.resolved.v2.descriptor import ResolvedERC7730Descriptor @@ -10,11 +11,16 @@ class ERC7730Linter(ABC): Linter for ERC-7730 v2 descriptors, inspects a (structurally valid) resolved v2 descriptor and emits notes, warnings, or errors. + The input descriptor it was resolved from is also provided, as resolution drops information some checks need + (most notably, contract format keys are reduced to selectors, losing the declared parameter names). + A linter may emit false positives or false negatives. It is up to the user to interpret the output. """ @abstractmethod - def lint(self, descriptor: ResolvedERC7730Descriptor, out: OutputAdder) -> None: + def lint( + self, input_descriptor: InputERC7730Descriptor, descriptor: ResolvedERC7730Descriptor, out: OutputAdder + ) -> None: raise NotImplementedError() @@ -26,6 +32,8 @@ def __init__(self, linters: list[ERC7730Linter]): self.linters = linters @override - def lint(self, descriptor: ResolvedERC7730Descriptor, out: OutputAdder) -> None: + def lint( + self, input_descriptor: InputERC7730Descriptor, descriptor: ResolvedERC7730Descriptor, out: OutputAdder + ) -> None: for linter in self.linters: - linter.lint(descriptor, out) + linter.lint(input_descriptor, descriptor, out) diff --git a/src/erc7730/lint/v2/lint.py b/src/erc7730/lint/v2/lint.py index 5f0cd630..32990d76 100644 --- a/src/erc7730/lint/v2/lint.py +++ b/src/erc7730/lint/v2/lint.py @@ -87,4 +87,4 @@ def lint_file(path: Path, linter: ERC7730Linter, out: OutputAdder, show_as: Path input_descriptor = InputERC7730Descriptor.load(path) resolved_descriptor = ERC7730InputToResolved().convert(input_descriptor, out) if resolved_descriptor is not None: - linter.lint(resolved_descriptor, out) + linter.lint(input_descriptor, resolved_descriptor, out) diff --git a/src/erc7730/lint/v2/lint_transaction_type_classifier.py b/src/erc7730/lint/v2/lint_transaction_type_classifier.py index 3486b79a..4b58971d 100644 --- a/src/erc7730/lint/v2/lint_transaction_type_classifier.py +++ b/src/erc7730/lint/v2/lint_transaction_type_classifier.py @@ -13,6 +13,7 @@ from erc7730.lint.classifier import TxClass from erc7730.lint.classifier.abi_classifier import ABIClassifier from erc7730.lint.v2 import ERC7730Linter +from erc7730.model.input.v2.descriptor import InputERC7730Descriptor from erc7730.model.resolved.v2.context import ResolvedContractContext, ResolvedEIP712Context from erc7730.model.resolved.v2.descriptor import ResolvedERC7730Descriptor from erc7730.model.resolved.v2.display import ResolvedDisplay, ResolvedField, ResolvedFieldDescription, ResolvedFormat @@ -28,7 +29,9 @@ class ClassifyTransactionTypeLinter(ERC7730Linter): """ @override - def lint(self, descriptor: ResolvedERC7730Descriptor, out: OutputAdder) -> None: + def lint( + self, input_descriptor: InputERC7730Descriptor, descriptor: ResolvedERC7730Descriptor, out: OutputAdder + ) -> None: if (tx_class := self._determine_tx_class(descriptor)) is None: return None DisplayFormatChecker(tx_class, descriptor.display).check(out) diff --git a/src/erc7730/lint/v2/lint_validate_display_fields.py b/src/erc7730/lint/v2/lint_validate_display_fields.py index 21a15c79..d71a0a27 100644 --- a/src/erc7730/lint/v2/lint_validate_display_fields.py +++ b/src/erc7730/lint/v2/lint_validate_display_fields.py @@ -10,11 +10,13 @@ from typing import final, override from erc7730.common import client -from erc7730.common.abi import compute_signature, get_functions +from erc7730.common.abi import compute_signature, get_functions, parse_signature, signature_to_selector from erc7730.common.output import OutputAdder from erc7730.lint.v2 import ERC7730Linter from erc7730.lint.v2.path_schemas import compute_format_schema_paths -from erc7730.model.paths import DataPath +from erc7730.model.abi import Function +from erc7730.model.input.v2.descriptor import InputERC7730Descriptor +from erc7730.model.paths import DataPath, Field from erc7730.model.paths.path_ops import data_path_starts_with from erc7730.model.paths.path_schemas import compute_abi_schema_paths from erc7730.model.resolved.v2.context import ResolvedContractContext, ResolvedEIP712Context @@ -37,15 +39,19 @@ class ValidateDisplayFieldsLinter(ERC7730Linter): """ @override - def lint(self, descriptor: ResolvedERC7730Descriptor, out: OutputAdder) -> None: + def lint( + self, input_descriptor: InputERC7730Descriptor, descriptor: ResolvedERC7730Descriptor, out: OutputAdder + ) -> None: match descriptor.context: case ResolvedEIP712Context(): pass # no schema to validate against in v2 case ResolvedContractContext(): - self._validate_contract_display_fields(descriptor, out) + self._validate_contract_display_fields(input_descriptor, descriptor, out) @classmethod - def _validate_contract_display_fields(cls, descriptor: ResolvedERC7730Descriptor, out: OutputAdder) -> None: + def _validate_contract_display_fields( + cls, input_descriptor: InputERC7730Descriptor, descriptor: ResolvedERC7730Descriptor, out: OutputAdder + ) -> None: context = descriptor.context if not isinstance(context, ResolvedContractContext): return @@ -89,6 +95,9 @@ def _validate_contract_display_fields(cls, descriptor: ResolvedERC7730Descriptor for selector, abi in reference_abis.functions.items(): abi_paths_by_selector[selector] = compute_abi_schema_paths(abi) + # Parse the input format keys, which carry the parameter names resolution reduced to selectors + declared_abis_by_selector = cls._parse_declared_abis(input_descriptor) + # Validate display field paths against ABI paths for selector, fmt in descriptor.display.formats.items(): if selector not in abi_paths_by_selector: @@ -101,12 +110,17 @@ def _validate_contract_display_fields(cls, descriptor: ResolvedERC7730Descriptor format_paths = compute_format_schema_paths(fmt) abi_paths = abi_paths_by_selector[selector] + unnamed_parameter_names = cls._unnamed_parameter_names( + reference_abis.functions[selector], declared_abis_by_selector.get(selector) + ) # Check for display fields referencing non-existent ABI paths. # A format path is valid if it matches an ABI path exactly OR is a prefix of one # (e.g. defining a field for an array root covers all nested elements). for path in format_paths.data_paths - abi_paths: if not any(data_path_starts_with(abi_path, path) for abi_path in abi_paths): + if cls._root_name(path) in unnamed_parameter_names: + continue out.error( title="Invalid display field", message=f"A display field is defined for `{path}`, but it does not exist in function " @@ -132,3 +146,59 @@ def _validate_contract_display_fields(cls, descriptor: ResolvedERC7730Descriptor message=f"Function {compute_signature(abi)} (selector: {selector}) exists in reference ABI " f"(see {explorer_url}) but has no display format defined in the descriptor.", ) + + @classmethod + def _parse_declared_abis(cls, input_descriptor: InputERC7730Descriptor) -> dict[str, Function]: + """ + Parse the function signatures declared as display format keys, indexed by selector. + + Format keys that are already selectors declare no parameter name, and are skipped. + + :param input_descriptor: input descriptor the linted descriptor was resolved from + :return: declared ABI functions, by selector + """ + declared_abis: dict[str, Function] = {} + for format_id in input_descriptor.display.formats: + if format_id.startswith("0x"): + continue + try: + declared_abi = parse_signature(format_id) + except ValueError: + continue # invalid signatures are reported by the resolution step + declared_abis[signature_to_selector(compute_signature(declared_abi))] = declared_abi + return declared_abis + + @classmethod + def _unnamed_parameter_names(cls, abi: Function, declared_abi: Function | None) -> set[str]: + """ + Get the names the descriptor gave to the parameters the reference ABI left unnamed. + + Unnamed parameters carry an empty name in the reference ABI, so no path is computed for them. The descriptor + names them in its format key, and such a name is only valid at the position it is declared at. + + :param abi: reference ABI of the function, as fetched from the explorer + :param declared_abi: ABI declared by the descriptor format key, if it was a signature and not a selector + :return: declared names of the parameters the reference ABI leaves unnamed + """ + if declared_abi is None: + return set() + parameters, declared_parameters = abi.inputs or [], declared_abi.inputs or [] + if len(parameters) != len(declared_parameters): + return set() # not expected, as both share the same selector + return { + declared_parameter.name + for parameter, declared_parameter in zip(parameters, declared_parameters, strict=True) + if not parameter.name and declared_parameter.name + } + + @classmethod + def _root_name(cls, path: DataPath) -> str | None: + """ + Get the identifier of the first element of a data path, if it is a field. + + :param path: data path to inspect + :return: the top-level field identifier, or None if the path does not start with a field + """ + if path.elements and isinstance(root := path.elements[0], Field): + return root.identifier + return None diff --git a/src/erc7730/lint/v2/lint_validate_max_length.py b/src/erc7730/lint/v2/lint_validate_max_length.py index 3ec898b6..5dff6c35 100644 --- a/src/erc7730/lint/v2/lint_validate_max_length.py +++ b/src/erc7730/lint/v2/lint_validate_max_length.py @@ -17,6 +17,7 @@ ) from erc7730.common.output import OutputAdder from erc7730.lint.v2 import ERC7730Linter +from erc7730.model.input.v2.descriptor import InputERC7730Descriptor from erc7730.model.resolved.v2.descriptor import ResolvedERC7730Descriptor from erc7730.model.resolved.v2.display import ResolvedField, ResolvedFieldDescription, ResolvedFieldGroup @@ -28,7 +29,9 @@ class ValidateMaxLengthLinter(ERC7730Linter): """ @override - def lint(self, descriptor: ResolvedERC7730Descriptor, out: OutputAdder) -> None: + def lint( + self, input_descriptor: InputERC7730Descriptor, descriptor: ResolvedERC7730Descriptor, out: OutputAdder + ) -> None: self._validate_metadata_lengths(descriptor, out) self._validate_display_lengths(descriptor, out) self._validate_enum_lengths(descriptor, out) diff --git a/tests/v2/lint/__init__.py b/tests/v2/lint/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/v2/lint/test_lint_validate_display_fields.py b/tests/v2/lint/test_lint_validate_display_fields.py new file mode 100644 index 00000000..a3550b84 --- /dev/null +++ b/tests/v2/lint/test_lint_validate_display_fields.py @@ -0,0 +1,40 @@ +from erc7730.common.abi import parse_signature +from erc7730.lint.v2.lint_validate_display_fields import ValidateDisplayFieldsLinter +from erc7730.model.abi import Function, InputOutput + +# what the descriptor declares as its format key, naming every parameter +DECLARED = parse_signature("depositETH(address pool, address onBehalfOf, uint16 referralCode)") + + +def reference_abi(*names: str) -> Function: + """Build the reference ABI of depositETH, in which unnamed parameters carry an empty name.""" + types = ["address", "address", "uint16"] + return Function( + name="depositETH", + inputs=[InputOutput(name=name, type=type) for name, type in zip(names, types, strict=True)], + ) + + +def unnamed_parameter_names(abi: Function, declared_abi: Function | None = DECLARED) -> set[str]: + return ValidateDisplayFieldsLinter._unnamed_parameter_names(abi, declared_abi) + + +def test_name_declared_at_an_unnamed_position_is_accepted() -> None: + assert unnamed_parameter_names(reference_abi("", "onBehalfOf", "referralCode")) == {"pool"} + + +def test_name_declared_at_a_named_position_is_not_accepted() -> None: + """`pool` is declared at position 0, which the reference ABI names `p`, so `#.pool` stays invalid.""" + assert unnamed_parameter_names(reference_abi("p", "", "referralCode")) == {"onBehalfOf"} + + +def test_no_name_is_accepted_when_the_reference_abi_names_every_parameter() -> None: + assert unnamed_parameter_names(reference_abi("pool", "onBehalfOf", "referralCode")) == set() + + +def test_every_declared_name_is_accepted_when_the_reference_abi_names_nothing() -> None: + assert unnamed_parameter_names(reference_abi("", "", "")) == {"pool", "onBehalfOf", "referralCode"} + + +def test_no_name_is_accepted_when_the_format_key_is_a_selector() -> None: + assert unnamed_parameter_names(reference_abi("", "", ""), declared_abi=None) == set()