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: support legacy json abis #350

Closed
wants to merge 14 commits into from

Conversation

Will-Smith11
Copy link
Contributor

@Will-Smith11 Will-Smith11 commented Oct 8, 2023

currently there is no support for legacy abis. I quickly added it, ignore the extra commits, from my fork where i also quickly patched rust keyword impl by using r#

@Will-Smith11 Will-Smith11 marked this pull request as ready for review October 8, 2023 23:57
$(#[$fattr:meta])*
$fvis:vis $field:ident : $type:ty,
)*}
)*) => {
trait FlattenStateMutability {
Copy link
Member

@DaniPopes DaniPopes Oct 9, 2023

Choose a reason for hiding this comment

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

I think we can simplify this and flatten at serde-time in StateMutability with a custom impl. There's also constant: bool alongside payable: bool

Copy link
Contributor Author

Choose a reason for hiding this comment

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

does constant: bool effect the generation of the bindings at all? I totally agree with flattening it but do you think we can just ignore the constant field?

Copy link
Member

@DaniPopes DaniPopes Oct 9, 2023

Choose a reason for hiding this comment

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

I assume it's converted like this

match (constant, payable) {
    (false, false) => StateMutability::NonPayable,
    (false, true) => StateMutability::Payable,
    (true, _) => StateMutability::View,
}

Copy link
Member

Choose a reason for hiding this comment

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

(true, true) should be an error, no? as constant is a contract not to read or write state, both being set would be an invalid ABI?

@Will-Smith11
Copy link
Contributor Author

hey sorry was super busy with work. made the changes yall wanted

@gakonst
Copy link
Member

gakonst commented Mar 15, 2024

@DaniPopes @prestwich what do we want to do with this PR? totally defer to you guys

@DaniPopes
Copy link
Member

DaniPopes commented Mar 30, 2024

I don't think we want this

Edit: nevermind, I found an easier way to implement this: #596

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.

4 participants