From edb376cd26d46fa9b129db7c9cd3a9d6fb098bc7 Mon Sep 17 00:00:00 2001 From: Julien Nicoulaud Date: Mon, 28 Oct 2024 14:00:54 +0100 Subject: [PATCH 1/3] feat: implement resolution of selectors --- .../convert_erc7730_input_to_resolved.py | 55 ++++++++++++++++--- src/erc7730/model/resolved/display.py | 5 +- src/erc7730/model/types.py | 11 ++++ .../data/minimal_contract_resolved.json | 2 +- .../test_convert_input_to_resolved.py | 2 +- 5 files changed, 64 insertions(+), 11 deletions(-) diff --git a/src/erc7730/convert/resolved/convert_erc7730_input_to_resolved.py b/src/erc7730/convert/resolved/convert_erc7730_input_to_resolved.py index bca29888..95998acd 100644 --- a/src/erc7730/convert/resolved/convert_erc7730_input_to_resolved.py +++ b/src/erc7730/convert/resolved/convert_erc7730_input_to_resolved.py @@ -1,8 +1,10 @@ from typing import assert_never, final, override +from eip712.model.schema import EIP712Type from pydantic_string_url import HttpUrl from erc7730.common import client +from erc7730.common.abi import reduce_signature, signature_to_selector from erc7730.common.output import OutputAdder from erc7730.convert import ERC7730Converter from erc7730.convert.resolved.constants import ConstantProvider, DefaultConstantProvider @@ -43,7 +45,7 @@ ResolvedNestedFields, ) from erc7730.model.resolved.metadata import ResolvedMetadata -from erc7730.model.types import Id +from erc7730.model.types import Id, Selector @final @@ -57,7 +59,7 @@ class ERC7730InputToResolved(ERC7730Converter[InputERC7730Descriptor, ResolvedER - References have been inlined - Constants have been inlined - Field definitions have been inlined - - Selectors have been converted to 4 bytes form (TODO not implemented) + - Selectors have been converted to 4 bytes form """ @override @@ -68,7 +70,9 @@ def convert(self, descriptor: InputERC7730Descriptor, out: OutputAdder) -> Resol return None if (metadata := self._resolve_metadata(descriptor.metadata, out)) is None: return None - if (display := self._resolve_display(descriptor.display, metadata.enums or {}, constants, out)) is None: + if ( + display := self._resolve_display(descriptor.display, context, metadata.enums or {}, constants, out) + ) is None: return None return ResolvedERC7730Descriptor(context=context, metadata=metadata, display=display) @@ -199,14 +203,27 @@ def _resolve_schema(cls, schema: EIP712JsonSchema | HttpUrl, out: OutputAdder) - @classmethod def _resolve_display( - cls, display: InputDisplay, enums: dict[Id, EnumDefinition], constants: ConstantProvider, out: OutputAdder + cls, + display: InputDisplay, + context: ResolvedContractContext | ResolvedEIP712Context, + enums: dict[Id, EnumDefinition], + constants: ConstantProvider, + out: OutputAdder, ) -> ResolvedDisplay | None: formats = {} - for format_key, format in display.formats.items(): + for format_id, format in display.formats.items(): + if (resolved_format_id := cls._resolve_format_id(format_id, context, out)) is None: + return None if ( resolved_format := cls._resolve_format(format, display.definitions or {}, enums, constants, out) - ) is not None: - formats[format_key] = resolved_format + ) is None: + return None + if resolved_format_id in formats: + return out.error( + title="Duplicate format", + message=f"Descriptor contains 2 formats sections for {resolved_format_id}", + ) + formats[resolved_format_id] = resolved_format return ResolvedDisplay(formats=formats) @@ -254,6 +271,30 @@ def _resolve_field_description( } ) + @classmethod + def _resolve_format_id( + cls, + format_id: str, + context: ResolvedContractContext | ResolvedEIP712Context, + out: OutputAdder, + ) -> EIP712Type | Selector | None: + match context: + case ResolvedContractContext(): + if format_id.startswith("0x"): + return Selector(format_id) + + if (reduced_signature := reduce_signature(format_id)) is not None: + return Selector(signature_to_selector(reduced_signature)) + + return out.error( + title="Invalid selector", + message=f""""{format_id}" is not a valid function signature or selector.""", + ) + case ResolvedEIP712Context(): + return format_id + case _: + assert_never(context) + @classmethod def _resolve_format( cls, diff --git a/src/erc7730/model/resolved/display.py b/src/erc7730/model/resolved/display.py index ab431c14..d73d436a 100644 --- a/src/erc7730/model/resolved/display.py +++ b/src/erc7730/model/resolved/display.py @@ -1,5 +1,6 @@ from typing import Annotated, ForwardRef +from eip712.model.schema import EIP712Type from pydantic import Discriminator, Field, Tag from erc7730.model.base import Model @@ -12,7 +13,7 @@ ) from erc7730.model.paths import ContainerPath, DataPath from erc7730.model.resolved.path import ResolvedPath -from erc7730.model.types import Address, HexStr, Id +from erc7730.model.types import Address, HexStr, Id, Selector from erc7730.model.unions import field_discriminator, field_parameters_discriminator # ruff: noqa: N815 - camel case field names are tolerated to match schema @@ -252,7 +253,7 @@ class ResolvedDisplay(Model): Display Formatting Info Section. """ - formats: dict[str, ResolvedFormat] = Field( + formats: dict[EIP712Type | Selector, ResolvedFormat] = Field( title="List of field formats", description="The list includes formatting info for each field of a structure. This list is indexed by a key" "identifying uniquely the message's type in the abi. For smartcontracts, it is the selector of the" diff --git a/src/erc7730/model/types.py b/src/erc7730/model/types.py index 3619cc88..bc746cfa 100644 --- a/src/erc7730/model/types.py +++ b/src/erc7730/model/types.py @@ -30,6 +30,17 @@ ), ] +Selector = Annotated[ + str, + Field( + title="Selector", + description="An Ethereum contract function identifier, in 4 bytes, hex encoded form.", + min_length=10, + max_length=10, + pattern=r"^0x[a-z0-9]+$", + ), +] + HexStr = Annotated[ str, Field( diff --git a/tests/convert/resolved/data/minimal_contract_resolved.json b/tests/convert/resolved/data/minimal_contract_resolved.json index 487b1798..e31f5f4e 100644 --- a/tests/convert/resolved/data/minimal_contract_resolved.json +++ b/tests/convert/resolved/data/minimal_contract_resolved.json @@ -32,7 +32,7 @@ }, "display": { "formats": { - "function1(bytes4)": { + "0x5ca8f297": { "fields": [ { "path": { diff --git a/tests/convert/resolved/test_convert_input_to_resolved.py b/tests/convert/resolved/test_convert_input_to_resolved.py index 3abed90a..e6f4d3fa 100644 --- a/tests/convert/resolved/test_convert_input_to_resolved.py +++ b/tests/convert/resolved/test_convert_input_to_resolved.py @@ -12,7 +12,7 @@ from tests.skip import single_or_skip DATA = Path(__file__).resolve().parent / "data" -UPDATE_REFERENCES = False +UPDATE_REFERENCES = True @pytest.mark.parametrize("input_file", ERC7730_DESCRIPTORS, ids=path_id) From b93ce9adf2063b863a35415f2e7a90efc9eddfda Mon Sep 17 00:00:00 2001 From: Julien Nicoulaud Date: Mon, 28 Oct 2024 14:01:35 +0100 Subject: [PATCH 2/3] UPDATE_REFERENCES = False --- tests/convert/resolved/test_convert_input_to_resolved.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/convert/resolved/test_convert_input_to_resolved.py b/tests/convert/resolved/test_convert_input_to_resolved.py index e6f4d3fa..3abed90a 100644 --- a/tests/convert/resolved/test_convert_input_to_resolved.py +++ b/tests/convert/resolved/test_convert_input_to_resolved.py @@ -12,7 +12,7 @@ from tests.skip import single_or_skip DATA = Path(__file__).resolve().parent / "data" -UPDATE_REFERENCES = True +UPDATE_REFERENCES = False @pytest.mark.parametrize("input_file", ERC7730_DESCRIPTORS, ids=path_id) From 3528dede56b06cf339f7c860e405e78c8ea55801 Mon Sep 17 00:00:00 2001 From: Julien Nicoulaud Date: Mon, 28 Oct 2024 14:37:57 +0100 Subject: [PATCH 3/3] update linter --- .../lint/lint_validate_display_fields.py | 30 +++++-------------- 1 file changed, 7 insertions(+), 23 deletions(-) diff --git a/src/erc7730/lint/lint_validate_display_fields.py b/src/erc7730/lint/lint_validate_display_fields.py index 00f7212b..0c9e7a90 100644 --- a/src/erc7730/lint/lint_validate_display_fields.py +++ b/src/erc7730/lint/lint_validate_display_fields.py @@ -1,6 +1,6 @@ from typing import final, override -from erc7730.common.abi import function_to_selector, reduce_signature, signature_to_selector +from erc7730.common.abi import function_to_selector from erc7730.common.output import OutputAdder from erc7730.lint import ERC7730Linter from erc7730.model.paths import DataPath, Field @@ -99,10 +99,6 @@ def _validate_eip712_paths(cls, descriptor: ResolvedERC7730Descriptor, out: Outp f"valid according to the EIP-712 schema.", ) - @classmethod - def _display(cls, selector: str, keccak: str) -> str: - return selector if selector == keccak else f"`{keccak}/{selector}`" - @classmethod def _validate_abi_paths(cls, descriptor: ResolvedERC7730Descriptor, out: OutputAdder) -> None: if isinstance(descriptor.context, ResolvedContractContext): @@ -112,32 +108,20 @@ def _validate_abi_paths(cls, descriptor: ResolvedERC7730Descriptor, out: OutputA abi_paths_by_selector[function_to_selector(abi)] = compute_abi_schema_paths(abi) for selector, fmt in descriptor.display.formats.items(): - keccak = selector - if not selector.startswith("0x"): - if (reduced_signature := reduce_signature(selector)) is not None: - keccak = signature_to_selector(reduced_signature) - else: - out.error( - title="Invalid selector", - message=f"Selector {cls._display(selector, keccak)} is not a valid function signature.", - ) - continue - if keccak not in abi_paths_by_selector: + if selector not in abi_paths_by_selector: out.error( title="Invalid selector", - message=f"Selector {cls._display(selector, keccak)} not found in ABI.", + message=f"Selector {selector} not found in ABI.", ) continue format_paths = compute_format_schema_paths(fmt).data_paths - abi_paths = abi_paths_by_selector[keccak] + abi_paths = abi_paths_by_selector[selector] if (excluded := fmt.excluded) is not None: excluded_paths = [to_absolute(path) for path in excluded] else: excluded_paths = [] - function = cls._display(selector, keccak) - for path in abi_paths - format_paths: if any(path_starts_with(path, excluded_path) for excluded_path in excluded_paths): continue @@ -145,17 +129,17 @@ def _validate_abi_paths(cls, descriptor: ResolvedERC7730Descriptor, out: OutputA if any(data_path_ends_with(path, allowed) for allowed in AUTHORIZED_MISSING_DISPLAY_FIELDS): out.debug( title="Optional Display field missing", - message=f"Display field for path `{path}` is missing for selector {function}. If " + message=f"Display field for path `{path}` is missing for selector {selector}. If " f"intentionally excluded, please add it to `excluded` list to avoid this warning.", ) else: out.warning( title="Missing Display field", - message=f"Display field for path `{path}` is missing for selector {function}. If " + message=f"Display field for path `{path}` is missing for selector {selector}. If " f"intentionally excluded, please add it to `excluded` list to avoid this warning.", ) for path in format_paths - abi_paths: out.error( title="Invalid Display field", - message=f"Display field for path `{path}` is not in selector {function}.", + message=f"Display field for path `{path}` is not in selector {selector}.", )