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.

Sign up for GitHub

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

Jump to bottom

Require "payable" modifier if function wants to receive ether. #962

Closed
wants to merge 1 commit into from
Closed

Require "payable" modifier if function wants to receive ether. #962

wants to merge 1 commit into from

Conversation

chriseth
Copy link
Contributor

@chriseth chriseth commented Aug 26, 2016 •
edited
Loading

Work in progress

Replaces #665

Partially implements #500 and #563. Comments welcome.

Fixes #602.

todo:

  • library function should not be payable
  • contract is payable should create payable fallback function (and error if there already is one)
  • calling a non-payable function with value set should report an error
  • accessor functions should not be able to receive ether (-> callvalue check should probably move to dispatcher)
  • internal functions should not allow payable modifier
  • overwriting a base contract function should not change payable

@Denton-L
Copy link
Contributor

What's the difference between this and #958?

@chriseth chriseth closed this Aug 30, 2016
@chriseth chriseth mentioned this pull request Sep 1, 2016
Closed
@axic axic deleted the payable branch September 6, 2016 10:49
axic pushed a commit that referenced this pull request Nov 20, 2018
@davesag @Arachnid
[Optimisation, ERC-884] spelling, grammar and style adjustments (#962)
4b1d9aa 
* ERC-884 spelling, grammar and style adjustments

* ERC-884 other minor style alts

* ERC-884 minor alt in response to feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@chriseth @Denton-L @axic