Skip to content
This repository has been archived by the owner on Oct 19, 2024. It is now read-only.

Refactor human readable abi parsing #225

Merged
merged 8 commits into from
Mar 12, 2021

Conversation

mattsse
Copy link
Collaborator

@mattsse mattsse commented Mar 11, 2021

This includes a more sophisticated human readable abi parser that should be on par with ethers-js's interface parser.

Motivation

The existing parser followed the grammar loosely;
False positives were possible, so that for example event SetValue_function_called() would be detected as Function.

Solution

All parser function are replaced with a handwritten parser that follows ethers-js abi parsing rules.
This also detects StateMutability and constructors.
All tests are successfully updated with the new parsers.

Up for discussion

  • mandatory returns statement: The new parser will fail if a function lacks a returns statement before its outputs. The contract_human_readable example did not succeed because a missing returns statement (function getValue() external view (string)) was detected and fixed in #
    f3bd156 . I'm not sure wether the preferential behavior would be to allow a missing returns or require it. ethers-js treats a missing returns before ouputs as an INVALID_ARGUMENT exception.
  • error handling: This introduces a lot more error checking. Right now there is not a lot feedback about what was the root cause of an error, most of the time a simple ParseError::ParseError(super::Error::InvalidData) is returned. I'm open to make this responsive by introducing a Message(String) error type or provide the ParseError::ParseError(super::Error::Other(anyow::Error)) with a message.
  • Side effects of constructor parsing: constructors are handled as Constructor now and asigned to Contract::constructor, I'm not sure about potential side effects of the constructor now missing from Contract::functions

Copy link
Owner

@gakonst gakonst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing, thank you.

@gakonst gakonst merged commit f599ae6 into gakonst:master Mar 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants