Skip to content

Commit

Permalink
fix: improve ABI paths discovery and signature reduction (#87)
Browse files Browse the repository at this point in the history
  • Loading branch information
fsamier authored Oct 15, 2024
1 parent 0ebbefb commit 1b4234f
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 49 deletions.
28 changes: 14 additions & 14 deletions src/erc7730/common/abi.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@

from erc7730.model.abi import ABI, Component, Function, InputOutput

_SOLIDITY_IDENTIFIER = r"[a-zA-Z$_][a-zA-Z$_0-9]*"
_SOLIDITY_PARAMETER_LIST = r"(" + _SOLIDITY_IDENTIFIER + r" *(" + _SOLIDITY_IDENTIFIER + r") *,? *)*"
_SOLIDITY_FUNCTION = r" *(" + _SOLIDITY_IDENTIFIER + r") *\((" + _SOLIDITY_PARAMETER_LIST + r")\)"
_SOLIDITY_FUNCTION_RE = re.compile(_SOLIDITY_FUNCTION)
_SIGNATURE_SPACES_PRE_CLEANUP = r"(,|\()( +)"
_SIGNATURE_CLEANUP = r"( +[^,\(\)]*)(\(|,|\))"

_SIGNATURE_SPACES_PRE_CLEANUP_RE = re.compile(_SIGNATURE_SPACES_PRE_CLEANUP)
_SIGNATURE_CLEANUP_RE = re.compile(_SIGNATURE_CLEANUP)


def _append_path(root: str, path: str) -> str:
Expand All @@ -23,10 +24,11 @@ def compute_paths(abi: Function) -> set[str]:
def append_paths(path: str, params: list[InputOutput] | list[Component] | None, paths: set[str]) -> None:
if params:
for param in params:
name = param.name + ".[]" if param.type.endswith("[]") else param.name
if param.components:
append_paths(_append_path(path, param.name), param.components, paths) # type: ignore
append_paths(_append_path(path, name), param.components, paths) # type: ignore
else:
paths.add(_append_path(path, param.name))
paths.add(_append_path(path, name))

paths: set[str] = set()
append_paths("", abi.inputs, paths)
Expand All @@ -39,14 +41,12 @@ def compute_signature(abi: Function) -> str:
return abi_to_signature(abi_function)


def reduce_signature(signature: str) -> str | None:
"""Remove parameter names from a function signature. return None if the signature is invalid."""
if match := _SOLIDITY_FUNCTION_RE.fullmatch(signature):
function_name = match.group(1)
parameters = match.group(2).split(",")
parameters_types = [param.strip().partition(" ")[0] for param in parameters]
return f"{function_name}({','.join(parameters_types)})"
return None
def reduce_signature(signature: str) -> str:
"""Remove parameter names and spaces from a function signature. Behaviour is undefined on invalid signature."""
# Note: Implementation is hackish, but as parameter types can be tuples that can be nested,
# it would require a full parser to do it properly.
# Test coverage should be enough to ensure it works as expected on valid signatures.
return re.sub(_SIGNATURE_CLEANUP_RE, r"\2", re.sub(_SIGNATURE_SPACES_PRE_CLEANUP_RE, r"\1", signature))


def signature_to_selector(signature: str) -> str:
Expand Down
21 changes: 14 additions & 7 deletions src/erc7730/lint/common/paths.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import re
from dataclasses import dataclass
from typing import cast

Expand All @@ -10,15 +11,20 @@
TokenAmountParameters,
)

ARRAY_SUFFIX = "[]"
_ARRAY_SUFFIX = "[]"

_INDICE_ARRAY = re.compile(r"\[-?\d+\]")
_SLICE_ARRAY = re.compile(r"\.\[-?\d+:-?\d+\]")


def _append_path(root: str, path: str) -> str:
return f"{root}.{path}" if root else path


def _remove_slicing(token_path: str) -> str:
return token_path.split("[")[0]
def _cleanup_brackets(token_path: str) -> str:
without_slices = re.sub(_SLICE_ARRAY, "", token_path) # remove slicing syntax
without_indices = re.sub(_INDICE_ARRAY, _ARRAY_SUFFIX, without_slices) # keep only array syntax
return without_indices


def compute_eip712_paths(schema: EIP712JsonSchema) -> set[str]:
Expand All @@ -30,9 +36,9 @@ def append_paths(
for domain in current_type:
new_path = _append_path(path, domain.name)
domain_type = domain.type
if domain_type.endswith(ARRAY_SUFFIX):
domain_type = _remove_slicing(domain_type)
new_path += f".{ARRAY_SUFFIX}"
if domain_type.endswith(_ARRAY_SUFFIX):
domain_type = domain_type[: -len(_ARRAY_SUFFIX)]
new_path += f".{_ARRAY_SUFFIX}"
if domain_type in types:
append_paths(new_path, types[domain_type], types, paths)
else:
Expand All @@ -57,6 +63,7 @@ def compute_format_paths(format: ResolvedFormat) -> FormatPaths:
paths = FormatPaths(data_paths=set(), format_paths=set(), container_paths=set())

def add_path(root: str, path: str) -> None:
path = _cleanup_brackets(path)
if path.startswith("@."):
paths.container_paths.add(path[2:])
elif path.startswith("$."):
Expand All @@ -76,7 +83,7 @@ def append_paths(path: str, field: ResolvedField | None) -> None:
and isinstance(params, TokenAmountParameters)
and (token_path := params.tokenPath) is not None
):
add_path(path, _remove_slicing(token_path).strip("."))
add_path(path, token_path)
case ResolvedNestedFields():
for nested_field in field.fields:
append_paths(_append_path(path, field.path), cast(ResolvedField, nested_field))
Expand Down
58 changes: 36 additions & 22 deletions tests/common/test_abi.py
Original file line number Diff line number Diff line change
@@ -1,30 +1,44 @@
import pytest

from erc7730.common.abi import compute_paths, compute_signature, reduce_signature, signature_to_selector
from erc7730.model.abi import Component, Function, InputOutput


def test_reduce_signature_no_params() -> None:
signature = "transfer()"
expected = "transfer()"
assert reduce_signature(signature) == expected


def test_reduce_signature_without_names() -> None:
signature = "transfer(address)"
expected = "transfer(address)"
@pytest.mark.parametrize(
"signature, expected",
[
# no param, no space
("transfer()", "transfer()"),
# one param, no space
("transfer(address)", "transfer(address)"),
# multiple params, expected spaces, no names
("mintToken(uint256, uint256, address, uint256, bytes)", "mintToken(uint256,uint256,address,uint256,bytes)"),
# multiple params, expected spaces, names
(
"mintToken(uint256 eventId, uint256 tokenId, address receiver, uint256 expirationTime, bytes signature)",
"mintToken(uint256,uint256,address,uint256,bytes)",
),
# multiple params, spaces everywhere, names, end with tuple
(
"f1( uint256[] _a , address _o , ( uint256 v , uint256 d ) _p )",
"f1(uint256[],address,(uint256,uint256))",
),
# multiple params, spaces everywhere, names, tuple in middle
(
"f2( uint256[] _a , ( uint256 v , uint256 d ) _p , address _o )",
"f2(uint256[],(uint256,uint256),address)",
),
# multiple params, spaces everywhere, names, nested tuples
(
"f3( uint256[] _a , ( uint256 v , ( bytes s , address a ) , uint256 d ) _p , address _o )",
"f3(uint256[],(uint256,(bytes,address),uint256),address)",
),
],
)
def test_reduce_signature(signature: str, expected: str) -> None:
assert reduce_signature(signature) == expected


def test_reduce_signature_with_names_and_spaces() -> None:
signature = "mintToken(uint256 eventId, uint256 tokenId, address receiver, uint256 expirationTime, bytes signature)"
expected = "mintToken(uint256,uint256,address,uint256,bytes)"
assert reduce_signature(signature) == expected


def test_reduce_signature_invalid() -> None:
signature = "invalid_signature"
assert reduce_signature(signature) is None


def test_compute_signature_no_params() -> None:
abi = Function(name="transfer", inputs=[])
expected = "transfer()"
Expand Down Expand Up @@ -99,10 +113,10 @@ def test_compute_paths_with_multiple_nested_params() -> None:
components=[
Component(name="baz", type="uint256"),
Component(name="qux", type="address"),
Component(name="nested", type="tuple", components=[Component(name="deep", type="string")]),
Component(name="nested", type="tuple[]", components=[Component(name="deep", type="string")]),
],
)
],
)
expected = {"bar.baz", "bar.qux", "bar.nested.deep"}
expected = {"bar.baz", "bar.qux", "bar.nested.[].deep"}
assert compute_paths(abi) == expected
5 changes: 0 additions & 5 deletions tests/lint/test_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,4 @@ def test_registry_files(input_file: Path) -> None:
"""
Test linting ERC-7730 registry files, which should all be valid at all times.
"""

# TODO: invalid files in registry
if input_file.name in {"calldata-AugustusSwapper.json"}:
pytest.skip("addressName `type` must be changed to `types`")

assert lint_all_and_print_errors([input_file])

0 comments on commit 1b4234f

Please sign in to comment.