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

Add statemutability field to ABI #2732

Merged
merged 2 commits into from
Aug 15, 2017
Merged

Add statemutability field to ABI #2732

merged 2 commits into from
Aug 15, 2017

Conversation

axic
Copy link
Member

@axic axic commented Aug 11, 2017

Depends on #2722. Part of #992.

Changelog.md Outdated
@@ -1,6 +1,7 @@
### 0.4.16 (unreleased)

Features:
* ABI: Include new field ``statemutability`` with values ``view``, ``nonpayable`` and ``payable``.
Copy link
Contributor

Choose a reason for hiding this comment

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

More specifically: json-ABI. Also, why is pure not a valid option? Should we name it stateaccess? Or stateaccessrestrictions?

Copy link
Contributor

Choose a reason for hiding this comment

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

Argh, sorry, of course because pure is not yet part of this PR...

Copy link
Member Author

Choose a reason for hiding this comment

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

Going to add pure in a separate PR.

// How a function can mutate the EVM state.
enum class StateMutability { View, NonPayable, Payable };

inline std::string stateMutabilityToString(StateMutability const& _stateMutability)
Copy link
Contributor

Choose a reason for hiding this comment

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

C++ has nicely-usable overloads, we can call this just toString.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this part of the review belongs to #2722.

@@ -985,6 +982,7 @@ class FunctionType: public Type
/// @returns true if the ABI is used for this call (only meaningful for external calls)
bool isBareCall() const;
Kind const& kind() const { return m_kind; }
StateMutability const& stateMutability() const { return m_stateMutability; }
Copy link
Contributor

Choose a reason for hiding this comment

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

It is probably better to return by value.

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed this in the real PR.

Copy link
Contributor

@chriseth chriseth left a comment

Choose a reason for hiding this comment

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

Only minor requests, can also be merged as is, if needed.

@axic axic changed the title Add statemutability field to ABI [WIP] Add statemutability field to ABI Aug 14, 2017
@axic axic force-pushed the statemutability-abi branch from b9db2ef to db10c9f Compare August 14, 2017 14:38
@axic
Copy link
Member Author

axic commented Aug 14, 2017

Need to update tests.

@axic axic force-pushed the statemutability-abi branch 2 times, most recently from c0baac5 to 3cbf446 Compare August 15, 2017 00:41
@axic axic changed the title [WIP] Add statemutability field to ABI [WIP] Aug 15, 2017
@axic axic changed the title [WIP] Add statemutability field to ABI Aug 15, 2017
@@ -548,12 +561,44 @@ BOOST_AUTO_TEST_CASE(constructor_abi)
}
],
"payable": false,
"statemutability" :"nonpayable",
Copy link
Member Author

Choose a reason for hiding this comment

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

Wrong space around colon.

}
],
"payable": true,
"statemutability" :"payable",
Copy link
Member Author

Choose a reason for hiding this comment

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

Wrong space around colon.

@axic axic force-pushed the statemutability-abi branch from 3cbf446 to 557e379 Compare August 15, 2017 10:28
@axic
Copy link
Member Author

axic commented Aug 15, 2017

@chriseth ready to merge

@axic axic force-pushed the statemutability-abi branch 2 times, most recently from 8b1efdd to 56a7881 Compare August 15, 2017 10:32
@axic axic force-pushed the statemutability-abi branch from 56a7881 to 1f5ab60 Compare August 15, 2017 10:36
@axic
Copy link
Member Author

axic commented Aug 15, 2017

@chriseth check this? :)

@chriseth chriseth merged commit 29cf3d9 into develop Aug 15, 2017
@axic axic deleted the statemutability-abi branch August 15, 2017 18:22
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.

2 participants