Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add support for function input/output encoding/decoding #227

Merged
merged 3 commits into from
Aug 23, 2023

Conversation

davebryson
Copy link
Contributor

Motivation

Added support for Function input encoding and output decoding. This is in reference to issue #210

Solution

Added trait FunctionExt that defines 2 methods: encode_input(...) and decode_output(...) and an implementation of the trait for json_abi::Function. Currently this resides in resolve.rs.

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

@davebryson davebryson changed the title added support for function input/output encoding/decoding feat: Add support for function input/output encoding/decoding Aug 7, 2023
@prestwich
Copy link
Member

discussing this approach internally and will loop back

@davebryson
Copy link
Contributor Author

As I dig in a bit more, to grok all of core, I'm wondering if SolType is intended as a (better) replacement for ethabi Token. If that's the case, should encode_input() take a SolType as arguments vs DynSolValue?

@DaniPopes
Copy link
Member

ethabi::ParamType -> DynSolType
ethabi::Token -> DynSolValue

@prestwich
Copy link
Member

As I dig in a bit more, to grok all of core, I'm wondering if SolType is intended as a (better) replacement for ethabi Token. If that's the case, should encode_input() take a SolType as arguments vs DynSolValue?

The problem with doing this is that SolType types must be known at compile time. This means that they can't be used with runtime supplied schema. So if you have a Json ABI Function at runtime, you need DynSolValue and DynSolType, and cannot use SolType

@davebryson
Copy link
Contributor Author

ethabi::ParamType -> DynSolType ethabi::Token -> DynSolValue

Ok. I'm on the right track then. However, at a higher level, I can still envision the need for something like ethers::Tokenizable to go to and from DynSolValue.

@prestwich
Copy link
Member

implementing Into<DynSolValue> seems to be sufficient? 🤔

Copy link
Member

@DaniPopes DaniPopes left a comment

Choose a reason for hiding this comment

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

Thanks, will touch up this PR in a follow-up. (#243)

@DaniPopes DaniPopes merged commit c21e47e into alloy-rs:main Aug 23, 2023
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants