-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Add EIP: BURN opcode #8914
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 EIP: BURN opcode #8914
Conversation
✅ All reviewers have approved. |
9b41bd1
to
835baf1
Compare
835baf1
to
9b039d9
Compare
The commit 0357fed (as a parent of 3b67b29) contains errors. |
8465830
to
7ec0652
Compare
7ec0652
to
062c6a0
Compare
12d3b9f
to
2994a19
Compare
|
||
The base gas cost for the `BURN` opcode is 100 gas. The dynamic gas cost is determined as follows: | ||
|
||
1. If the value to be burned is 0, the dynamic gas cost is 0. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0 is also too low, also for a no-op. A no-op JUMPDEST
costs 1 gas. Also note we have to pop one item from stack here. 3
would be reasonable here (same as the EQ
operator, which pops two items and compares those)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just the dynamic cost. The min cost of the opcode is always 100, should I add another 3 gas on this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jochem-brouwer thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The base cost is too low, see: #8914 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or wait, maybe dynamic should be made higher if the target is cold. The cost is too low for cold accounts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jochem-brouwer I think the dyn can be 0 if account is cold, since we can short circuit on balance. Updated to reflect. LMK what u think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All Reviewers Have Approved; Performing Automatic Merge...
ATTENTION: ERC-RELATED PULL REQUESTS NOW OCCUR IN ETHEREUM/ERCS
--
When opening a pull request to submit a new EIP, please use the suggested template: https://github.com/ethereum/EIPs/blob/master/eip-template.md
We have a GitHub bot that automatically merges some PRs. It will merge yours immediately if certain criteria are met: