From 9ac28cdbac0483642cc7f14653b9b1b73080fe76 Mon Sep 17 00:00:00 2001 From: Julien Nicoulaud Date: Fri, 20 Sep 2024 10:01:01 +0200 Subject: [PATCH] cleanups/doc --- src/erc7730/linter/__init__.py | 11 ++++- .../linter/common/display_format_checker.py | 22 +++++---- src/erc7730/linter/lint.py | 16 ++++--- src/erc7730/linter/linter_base.py | 8 ++-- .../linter_transaction_type_classifier_ai.py | 8 ++-- src/erc7730/linter/linter_validate_abi.py | 22 ++++----- .../linter/linter_validate_display_fields.py | 46 +++++++++---------- src/erc7730/main.py | 14 +++--- 8 files changed, 82 insertions(+), 65 deletions(-) diff --git a/src/erc7730/linter/__init__.py b/src/erc7730/linter/__init__.py index 9857258..f798960 100644 --- a/src/erc7730/linter/__init__.py +++ b/src/erc7730/linter/__init__.py @@ -7,13 +7,21 @@ from pydantic import BaseModel, FilePath -class Linter(ABC): +class ERC7730Linter(ABC): + """ + Linter for ERC-7730 descriptors, inspects a (structurally valid) descriptor and emits notes, warnings, or errors. + """ + @abstractmethod def lint(self, descriptor: ERC7730Descriptor, out: "OutputAdder") -> None: raise NotImplementedError() class Output(BaseModel): + """ERC7730Linter output notice/warning/error.""" + class Level(IntEnum): + """ERC7730Linter output level.""" + INFO = auto() WARNING = auto() ERROR = auto() @@ -25,3 +33,4 @@ class Level(IntEnum): level: Level = Level.ERROR OutputAdder = Callable[[Output], None] + """ERC7730Linter output sink.""" diff --git a/src/erc7730/linter/common/display_format_checker.py b/src/erc7730/linter/common/display_format_checker.py index 9d28930..dedd950 100644 --- a/src/erc7730/linter/common/display_format_checker.py +++ b/src/erc7730/linter/common/display_format_checker.py @@ -1,5 +1,5 @@ from erc7730.linter.classifier import TxClass -from erc7730.linter import Linter +from erc7730.linter import ERC7730Linter from erc7730.model.display import Display, Format @@ -26,22 +26,26 @@ def _get_all_displayed_fields(self, formats: dict[str, Format]) -> set[str]: fields.add(str(field)) return fields - def check(self) -> list[Linter.Output]: - res: list[Linter.Output] = [] + def check(self) -> list[ERC7730Linter.Output]: + res: list[ERC7730Linter.Output] = [] match self.c: case TxClass.PERMIT: formats = self.d.formats fields = self._get_all_displayed_fields(formats) if not _fields_contain("spender", fields): res.append( - Linter.Output( - title="Missing spender in displayed fields", message="", level=Linter.Output.Level.ERROR + ERC7730Linter.Output( + title="Missing spender in displayed fields", + message="", + level=ERC7730Linter.Output.Level.ERROR, ) ) if not _fields_contain("amount", fields): res.append( - Linter.Output( - title="Missing amount in displayed fields", message="", level=Linter.Output.Level.ERROR + ERC7730Linter.Output( + title="Missing amount in displayed fields", + message="", + level=ERC7730Linter.Output.Level.ERROR, ) ) if ( @@ -50,10 +54,10 @@ def check(self) -> list[Linter.Output]: and not _fields_contain("expiration", fields) ): res.append( - Linter.Output( + ERC7730Linter.Output( title="Field not displayed", message="Missing expiration date in displayed fields for permit", - level=Linter.Output.Level.ERROR, + level=ERC7730Linter.Output.Level.ERROR, ) ) return res diff --git a/src/erc7730/linter/lint.py b/src/erc7730/linter/lint.py index 5ddee98..2851a47 100644 --- a/src/erc7730/linter/lint.py +++ b/src/erc7730/linter/lint.py @@ -2,7 +2,7 @@ from erc7730 import ERC_7730_REGISTRY_CALLDATA_PREFIX, ERC_7730_REGISTRY_EIP712_PREFIX from erc7730.common.pydantic import model_from_json_file -from erc7730.linter import Linter +from erc7730.linter import ERC7730Linter from erc7730.linter.linter_base import MultiLinter from erc7730.linter.linter_transaction_type_classifier_ai import ClassifyTransactionTypeLinter from erc7730.linter.linter_validate_abi import ValidateABILinter @@ -13,7 +13,7 @@ from erc7730.model.utils import resolve_external_references -def lint_all(paths: list[Path]) -> list[Linter.Output]: +def lint_all(paths: list[Path]) -> list[ERC7730Linter.Output]: linter = MultiLinter( [ ValidateABILinter(), @@ -22,7 +22,7 @@ def lint_all(paths: list[Path]) -> list[Linter.Output]: ] ) - outputs: list[Linter.Output] = [] + outputs: list[ERC7730Linter.Output] = [] for path in paths: if path.is_file(): @@ -39,10 +39,10 @@ def lint_all(paths: list[Path]) -> list[Linter.Output]: return outputs -def lint_file(path: Path, linter: Linter, out: Linter.OutputAdder) -> None: +def lint_file(path: Path, linter: ERC7730Linter, out: ERC7730Linter.OutputAdder) -> None: print(f"checking {path}...") - def adder(output: Linter.Output) -> None: + def adder(output: ERC7730Linter.Output) -> None: out(output.model_copy(update={"file": path})) try: @@ -50,4 +50,8 @@ def adder(output: Linter.Output) -> None: descriptor = resolve_external_references(descriptor) linter.lint(descriptor, adder) except Exception as e: - out(Linter.Output(file=path, title="Failed to parse", message=str(e), level=Linter.Output.Level.ERROR)) + out( + ERC7730Linter.Output( + file=path, title="Failed to parse", message=str(e), level=ERC7730Linter.Output.Level.ERROR + ) + ) diff --git a/src/erc7730/linter/linter_base.py b/src/erc7730/linter/linter_base.py index 0e7269c..3923143 100644 --- a/src/erc7730/linter/linter_base.py +++ b/src/erc7730/linter/linter_base.py @@ -1,11 +1,11 @@ -from erc7730.linter import Linter +from erc7730.linter import ERC7730Linter from erc7730.model.erc7730_descriptor import ERC7730Descriptor -class MultiLinter(Linter): - def __init__(self, linters: list[Linter]): +class MultiLinter(ERC7730Linter): + def __init__(self, linters: list[ERC7730Linter]): self.linters = linters - def lint(self, descriptor: ERC7730Descriptor, out: Linter.OutputAdder) -> None: + def lint(self, descriptor: ERC7730Descriptor, out: ERC7730Linter.OutputAdder) -> None: for linter in self.linters: linter.lint(descriptor, out) diff --git a/src/erc7730/linter/linter_transaction_type_classifier_ai.py b/src/erc7730/linter/linter_transaction_type_classifier_ai.py index f3ccaf9..cd0cada 100644 --- a/src/erc7730/linter/linter_transaction_type_classifier_ai.py +++ b/src/erc7730/linter/linter_transaction_type_classifier_ai.py @@ -1,7 +1,7 @@ from erc7730.linter.classifier.abi_classifier import ABIClassifier from erc7730.linter.classifier.eip712_classifier import EIP712Classifier from erc7730.linter.common.display_format_checker import DisplayFormatChecker -from erc7730.linter import Linter +from erc7730.linter import ERC7730Linter from erc7730.model.context import EIP712Context, ContractContext, EIP712JsonSchema from erc7730.model.display import Display from erc7730.model.erc7730_descriptor import ERC7730Descriptor @@ -28,21 +28,21 @@ def determine_tx_class(descriptor: ERC7730Descriptor) -> TxClass | None: return None -class ClassifyTransactionTypeLinter(Linter): +class ClassifyTransactionTypeLinter(ERC7730Linter): """ - given schema/ABI, classify the transaction type - if class found, check descriptor display fields against predefined ruleset - possible class (swap, staking withdraw, staking deposit) """ - def lint(self, descriptor: ERC7730Descriptor, out: Linter.OutputAdder) -> None: + def lint(self, descriptor: ERC7730Descriptor, out: ERC7730Linter.OutputAdder) -> None: if descriptor.context is None: return None c = determine_tx_class(descriptor) if c is None: # could not determine transaction type return None - out(Linter.Output(title="Transaction type: ", message=str(c), level=Linter.Output.Level.INFO)) + out(ERC7730Linter.Output(title="Transaction type: ", message=str(c), level=ERC7730Linter.Output.Level.INFO)) d: Display | None = descriptor.display if d is None: return None diff --git a/src/erc7730/linter/linter_validate_abi.py b/src/erc7730/linter/linter_validate_abi.py index 703cebb..5804572 100644 --- a/src/erc7730/linter/linter_validate_abi.py +++ b/src/erc7730/linter/linter_validate_abi.py @@ -1,18 +1,18 @@ from erc7730.common.client.etherscan import get_contract_abis from erc7730.common.abi import get_functions, compute_signature -from erc7730.linter import Linter +from erc7730.linter import ERC7730Linter from erc7730.model.context import EIP712Context, ContractContext from erc7730.model.erc7730_descriptor import ERC7730Descriptor -class ValidateABILinter(Linter): +class ValidateABILinter(ERC7730Linter): """ - resolves the ABI from the descriptor (URL or provided) - resolves the ABI from *scan (given chainId and address of descriptor) - => compare the two ABIs """ - def lint(self, descriptor: ERC7730Descriptor, out: Linter.OutputAdder) -> None: + def lint(self, descriptor: ERC7730Descriptor, out: ERC7730Linter.OutputAdder) -> None: if isinstance(descriptor.context, EIP712Context): return self._validate_eip712_schemas(descriptor.context, out) if isinstance(descriptor.context, ContractContext): @@ -20,11 +20,11 @@ def lint(self, descriptor: ERC7730Descriptor, out: Linter.OutputAdder) -> None: raise ValueError("Invalid context type") @classmethod - def _validate_eip712_schemas(cls, context: EIP712Context, out: Linter.OutputAdder) -> None: + def _validate_eip712_schemas(cls, context: EIP712Context, out: ERC7730Linter.OutputAdder) -> None: pass # not implemented @classmethod - def _validate_contract_abis(cls, context: ContractContext, out: Linter.OutputAdder) -> None: + def _validate_contract_abis(cls, context: ContractContext, out: ERC7730Linter.OutputAdder) -> None: if not isinstance(context.contract.abi, list): raise ValueError("Contract ABIs should have been resolved") @@ -38,10 +38,10 @@ def _validate_contract_abis(cls, context: ContractContext, out: Linter.OutputAdd if etherscan_abis.proxy: out( - Linter.Output( + ERC7730Linter.Output( title="Proxy contract", message="Contract ABI on Etherscan is likely to be a proxy, validation skipped", - level=Linter.Output.Level.INFO, + level=ERC7730Linter.Output.Level.INFO, ) ) return @@ -49,18 +49,18 @@ def _validate_contract_abis(cls, context: ContractContext, out: Linter.OutputAdd for selector, abi in contract_abis.functions.items(): if selector not in etherscan_abis.functions: out( - Linter.Output( + ERC7730Linter.Output( title="Missing function", message=f"Function `{selector}/{compute_signature(abi)}` is not defined in Etherscan ABI", - level=Linter.Output.Level.ERROR, + level=ERC7730Linter.Output.Level.ERROR, ) ) else: if contract_abis.functions[selector] != etherscan_abis.functions[selector]: out( - Linter.Output( + ERC7730Linter.Output( title="Function mismatch", message=f"Function `{selector}/{compute_signature(abi)}` does not match Etherscan ABI", - level=Linter.Output.Level.WARNING, + level=ERC7730Linter.Output.Level.WARNING, ) ) diff --git a/src/erc7730/linter/linter_validate_display_fields.py b/src/erc7730/linter/linter_validate_display_fields.py index f183482..e2ffbc4 100644 --- a/src/erc7730/linter/linter_validate_display_fields.py +++ b/src/erc7730/linter/linter_validate_display_fields.py @@ -1,17 +1,17 @@ from erc7730.common.abi import compute_paths, compute_selector from erc7730.linter.common.paths import compute_eip712_paths, compute_format_paths -from erc7730.linter import Linter +from erc7730.linter import ERC7730Linter from erc7730.model.context import ContractContext, EIP712Context, EIP712JsonSchema from erc7730.model.erc7730_descriptor import ERC7730Descriptor -class ValidateDisplayFieldsLinter(Linter): +class ValidateDisplayFieldsLinter(ERC7730Linter): """ - for each field of schema/ABI, check that there is a display field - for each field, check that display configuration is relevant with field type """ - def _validate_eip712_paths(self, descriptor: ERC7730Descriptor, out: Linter.OutputAdder) -> None: + def _validate_eip712_paths(self, descriptor: ERC7730Descriptor, out: ERC7730Linter.OutputAdder) -> None: if ( descriptor.context is not None and descriptor.display is not None @@ -24,19 +24,19 @@ def _validate_eip712_paths(self, descriptor: ERC7730Descriptor, out: Linter.Outp primary_types.add(schema.primaryType) if schema.primaryType not in schema.types: out( - Linter.Output( + ERC7730Linter.Output( title="Invalid EIP712 Schema", message=f"Primary type `{schema.primaryType}` not found in types.", - level=Linter.Output.Level.ERROR, + level=ERC7730Linter.Output.Level.ERROR, ) ) continue if schema.primaryType not in descriptor.display.formats: out( - Linter.Output( + ERC7730Linter.Output( title="Missing Display field", message=f"Display field for primary type `{schema.primaryType}` is missing.", - level=Linter.Output.Level.WARNING, + level=ERC7730Linter.Output.Level.WARNING, ) ) continue @@ -45,41 +45,41 @@ def _validate_eip712_paths(self, descriptor: ERC7730Descriptor, out: Linter.Outp for path in eip712_paths - format_paths: out( - Linter.Output( + ERC7730Linter.Output( title="Missing Display field", message=f"Display field for path `{path}` is missing for message {schema.primaryType}.", - level=Linter.Output.Level.WARNING, + level=ERC7730Linter.Output.Level.WARNING, ) ) for path in format_paths - eip712_paths: out( - Linter.Output( + ERC7730Linter.Output( title="Extra Display field", message=f"Display field for path `{path}` is not in message {schema.primaryType}.", - level=Linter.Output.Level.ERROR, + level=ERC7730Linter.Output.Level.ERROR, ) ) else: out( - Linter.Output( + ERC7730Linter.Output( title="Missing EIP712 Schema", message=f"EIP712 Schema is missing (found {schema})", - level=Linter.Output.Level.ERROR, + level=ERC7730Linter.Output.Level.ERROR, ) ) for fmt in descriptor.display.formats.keys(): if fmt not in primary_types: out( - Linter.Output( + ERC7730Linter.Output( title="Invalid Display field", message=f"Format message `{fmt}` is not in EIP712 schemas.", - level=Linter.Output.Level.ERROR, + level=ERC7730Linter.Output.Level.ERROR, ) ) - def _validate_abi_paths(self, descriptor: ERC7730Descriptor, out: Linter.OutputAdder) -> None: + def _validate_abi_paths(self, descriptor: ERC7730Descriptor, out: ERC7730Linter.OutputAdder) -> None: if ( descriptor.context is not None and descriptor.display is not None @@ -94,10 +94,10 @@ def _validate_abi_paths(self, descriptor: ERC7730Descriptor, out: Linter.OutputA for selector, fmt in descriptor.display.formats.items(): if selector not in abi_paths_by_selector: out( - Linter.Output( + ERC7730Linter.Output( title="Invalid selector", message=f"Selector {selector} not found in ABI.", - level=Linter.Output.Level.ERROR, + level=ERC7730Linter.Output.Level.ERROR, ) ) continue @@ -106,21 +106,21 @@ def _validate_abi_paths(self, descriptor: ERC7730Descriptor, out: Linter.OutputA for path in abi_paths - format_paths: out( - Linter.Output( + ERC7730Linter.Output( title="Missing Display field", message=f"Display field for path `{path}` is missing for selector {selector}.", - level=Linter.Output.Level.WARNING, + level=ERC7730Linter.Output.Level.WARNING, ) ) for path in format_paths - abi_paths: out( - Linter.Output( + ERC7730Linter.Output( title="Invalid Display field", message=f"Display field for path `{path}` is not in selector {selector}.", - level=Linter.Output.Level.ERROR, + level=ERC7730Linter.Output.Level.ERROR, ) ) - def lint(self, descriptor: ERC7730Descriptor, out: Linter.OutputAdder) -> None: + def lint(self, descriptor: ERC7730Descriptor, out: ERC7730Linter.OutputAdder) -> None: self._validate_eip712_paths(descriptor, out) self._validate_abi_paths(descriptor, out) diff --git a/src/erc7730/main.py b/src/erc7730/main.py index fc12fa2..49c71aa 100644 --- a/src/erc7730/main.py +++ b/src/erc7730/main.py @@ -26,7 +26,7 @@ def lint( gha: Annotated[bool, typer.Option(help="Enable Github annotations output")] = False, ) -> None: from erc7730.linter.lint import lint_all - from erc7730.linter import Linter + from erc7730.linter import ERC7730Linter outputs = lint_all(paths) @@ -35,19 +35,19 @@ def lint( if gha: # msg = f"{output.title} {output.message}" match output.level: - case Linter.Output.Level.INFO: + case ERC7730Linter.Output.Level.INFO: print(f"::notice file={output.file},title={output.title}::{output.message}") - case Linter.Output.Level.WARNING: + case ERC7730Linter.Output.Level.WARNING: print(f"::warning file={output.file},title={output.title}::{output.message}") - case Linter.Output.Level.ERROR: + case ERC7730Linter.Output.Level.ERROR: print(f"::error file={output.file},title={output.title}::{output.message}") else: match output.level: - case Linter.Output.Level.INFO: + case ERC7730Linter.Output.Level.INFO: rprint(f"[blue]{p}: {output.level.name}: {output.title}[/blue]\n" f" {output.message}") - case Linter.Output.Level.WARNING: + case ERC7730Linter.Output.Level.WARNING: rprint(f"[yellow]{p}: {output.level.name}: {output.title}[/yellow]\n" f" {output.message}") - case Linter.Output.Level.ERROR: + case ERC7730Linter.Output.Level.ERROR: rprint(f"[red]{p}: {output.level.name}: {output.title}[/red]\n" f" {output.message}") if not outputs: