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

Support the stateMutability field of the ABI JSON #1931

Closed
axic opened this issue Apr 21, 2020 · 11 comments · Fixed by #2054
Closed

Support the stateMutability field of the ABI JSON #1931

axic opened this issue Apr 21, 2020 · 11 comments · Fixed by #2054
Labels
bug Bug that shouldn't change language semantics when fixed. Easy Pickings Used to denote issues that should be easy to implement

Comments

@axic
Copy link
Contributor

axic commented Apr 21, 2020

This has been briefly discussed in #1247 too.

Vyper still only outputs the constant and payable fields. The stateMutability field was introduced a while ago (probably 2 years ago?) to supersede both constant and payable as well as to support more granularity:

  • pure
  • view
  • nonpayable
  • payable

pure means the function doesn't execute state modifying and neither state reading instructions; view allows for state reading instructions; nonpayable allows modifying the state, but no incoming ether transfer; and payable allows modifying the state and incoming ether transfers.

It would be nice add support to Vyper to also output this field.

Are there any reasons against it?

@fubuloubu
Copy link
Member

No, it looks this was missed in #1244. We can make this happen.

@fubuloubu fubuloubu added bug Bug that shouldn't change language semantics when fixed. Easy Pickings Used to denote issues that should be easy to implement labels Apr 21, 2020
@gakonst
Copy link

gakonst commented Jun 16, 2020

@fubuloubu are there any updates on this?

@fubuloubu
Copy link
Member

Not started yet, but it should be fairly easy to add if you want to give it a shot. We're planning on a release, hopefully in the next week or two.

Appears there might be some work to determine pure if you think that would be important to add, but the new type system @iamdefinitelyahuman should make it easy enough to query if self is used inside a method.

@iamdefinitelyahuman
Copy link
Contributor

Yes, shouldn't be hard to search for self. We can and should implement this in the upcoming release.

gakonst added a commit to gakonst/ethabi that referenced this issue Jun 20, 2020
Previously stateMutability would not be interpreted

The abi generation tests have been removed because they relied on ABIs
generated with the deprecated ABI spec.

Note: Vyper still hasn't updated vyperlang/vyper#1931
@fubuloubu
Copy link
Member

As of #2049 we can handle ABI inputs with either stateMutability or constant/payable set

@axic
Copy link
Contributor Author

axic commented Jun 26, 2020

Do you support all the various states? And does the generated ABI JSON contain the stateMutability field?

@fubuloubu
Copy link
Member

fubuloubu commented Jun 26, 2020

Not yet, #2041 and #2042 will complete support of all 4 states in both contract definitions and interface definitions, as well as output the field

@fubuloubu
Copy link
Member

@axic just to confirm, once we add the stateMutability field, we should drop constant/payable? (but retain support for processing both)

@axic
Copy link
Contributor Author

axic commented Jun 26, 2020

once we add the stateMutability field, we should drop constant/payable? (but retain support for processing both)

It is up to you. Solidity stopped outputting constant/payable with 0.6.x because the stateMutability feature was introduced 2 years ago. This showed that two projects did not made the update: ethers-js (fixed since) and geth (they use some ancient web3.js for the console -- not fixed yet).

It seems it won't be detrimental if you stop outputting it, but I'm not entirely sure if there are frameworks specifically made for Vyper and if they depend on it (truffle, buidler, waffle should be fine).

@fubuloubu
Copy link
Member

@iamdefinitelyahuman Brownie is okay with this, right? I think web3py handles this well

@iamdefinitelyahuman
Copy link
Contributor

Yeah, Brownie understands both formats.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug that shouldn't change language semantics when fixed. Easy Pickings Used to denote issues that should be easy to implement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants