-
Notifications
You must be signed in to change notification settings - Fork 6k
Update EIP-7708: Move to Draft #9003
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
Merged
Merged
Changes from 9 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
2d0af4a
Update EIP-7708: Provide more details and add fee payments
etan-status 1939890
Fix lint
etan-status 68deb0c
Update wording on open question
etan-status 700face
Merge branch 'master' into fl-ethlogs
etan-status 15abb8e
Drop logs that can be reconstructed from tx/receipt data
etan-status bb546d6
Cleanup
etan-status 7b25326
Nonzero-value-transferring
etan-status f14d40a
Change system address to one that is used in JSON-RPC
etan-status 370938e
Extend write-access list
etan-status 4b19eb7
Change address to 0xfff..e
etan-status 48758bc
Clarifying comments
etan-status 5459529
Add Selfdestruct event
etan-status File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Some edge cases to think about here:
For the sake of simplicity, I suggest we treat both cases as a transfer of value to self. There is not really a proper way to indicate that the balance is being burned in the case of 1.
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.
Is it possible from solely the information in the logs (whatever is proposed to be emitted here), the transactions (amount, max prio fee), the receipts (gas used), the block header (base block fee, withdrawals), and the genesis balance (or, whatever the balance was at the time of the fork when this activates) to reconstruct the account balance accurately, in all cases?
Or do we need a second log for burning ETH in these situations?
Uh oh!
There was an error while loading. Please reload this page.
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.
I think we would need a special log to indicate burn in the case of 1.
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.
for the case of 2. we can just mark this as a transfer of zero value to self. It's the only case where a value transfer log of this type would be allowed.
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 special log could either be a LOG2
Burn(from, amount), or reuse the Transfer log (with a destination of system address). TheBurnlog would probably be cleaner, as it doesn't actually increase the balance of the system address. TheTransferlog would be a bit more consistent with ERC20 (no need to combine the result from multiple log topics); but for ERC20 it actually gets credited to zero/burnaddress so it's not the same semantics.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,
Selfdestruct(from, amount)? To indicate the special operation? Then, (1) and (2) would be the same?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.
Yeah, I like that
This could work.
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.
Added
Selfdestruct(from, amount).Uh oh!
There was an error while loading. Please reload this page.
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.
Isn't burn for ETH a transfer to 0x0? (to null) like emission is a transfer from 0x0
Symbolic burns transfer to
0xdEaDbut that's not what we are talking about hereThere 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.
In a
Transfer, the value is actually credited to the destination.This Selfdestruct doesn't credit anything. One couldn't reconstruct the actual value of the zero / 0xdead address if the special case isn't denoted as such.