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

Update EIP-6780: Add clarifications to EIP-6780 #7308

Merged
merged 9 commits into from
Sep 10, 2023
Merged

Update EIP-6780: Add clarifications to EIP-6780 #7308

merged 9 commits into from
Sep 10, 2023

Conversation

shemnon
Copy link
Contributor

@shemnon shemnon commented Jul 11, 2023 •
edited
Loading

  • Contract creation and account creation can occur in separate transactions
  • If a contract is not destroyed, any SELFDESTRUCT to self burning will not occur.
  • SELFDESTRUCT always halts the current frame, wether in "sweep" mode or legacy mode

@shemnon
Add clarifications to EIP-6780
6907a88 
* Contract creation and account creation can occur in separate transactions
* If a contract is not destroyed, any SELFDESTRUCT to self burning will not occur.
@shemnon shemnon requested a review from eth-bot as a code owner July 11, 2023 01:55
@github-actions github-actions bot added c-update Modifies an existing proposal s-draft This EIP is a Draft t-core labels Jul 11, 2023
@eth-bot
Copy link
Collaborator

eth-bot commented Jul 11, 2023 •
edited
Loading

✅ All reviewers have approved.

@eth-bot eth-bot changed the title Add clarifications to EIP-6780 Update EIP-6780: Add clarifications to EIP-6780 Jul 11, 2023
@eth-bot eth-bot added the a-review Waiting on author to review label Jul 11, 2023
@shemnon shemnon mentioned this pull request Jul 11, 2023
Merged
@shemnon shemnon mentioned this pull request Jul 11, 2023
Merged
@shemnon shemnon mentioned this pull request Jul 18, 2023
Closed
@gfukushima gfukushima mentioned this pull request Jul 19, 2023
Closed
invocamanman
invocamanman reviewed Aug 2, 2023
View reviewed changes
EIPS/eip-6780.md Outdated Show resolved Hide resolved
@shemnon shemnon mentioned this pull request Aug 2, 2023
Closed
smartprogrammer93
smartprogrammer93 reviewed Aug 4, 2023
View reviewed changes
EIPS/eip-6780.md Outdated Show resolved Hide resolved
@invocamanman
Copy link

invocamanman commented Aug 4, 2023 •
edited
Loading

So, IMHO think that after this EIP self destruct basically will be useful for:

  • Burning/injecting ether
  • Creating contracts on different transactions on the same address that finally get destroyed.

As per the second point, if an account wants to execute certain logic, isn't cheaper to just create a contract and then halt the execution with an STOP for example? It won't be on the same address every time, but it's way cheaper than Selfdestruct, anyway there are no refunds attached.
My point is that i think there are a very few use cases except for burning/injecting ether currently.

dankrad
dankrad reviewed Aug 6, 2023
View reviewed changes
EIPS/eip-6780.md Outdated Show resolved Hide resolved
dankrad
dankrad reviewed Aug 6, 2023
View reviewed changes
EIPS/eip-6780.md Outdated Show resolved Hide resolved
dankrad
dankrad reviewed Aug 6, 2023
View reviewed changes
EIPS/eip-6780.md Outdated Show resolved Hide resolved
dankrad
dankrad reviewed Aug 6, 2023
View reviewed changes
EIPS/eip-6780.md Outdated Show resolved Hide resolved
dankrad
dankrad reviewed Aug 6, 2023
View reviewed changes
EIPS/eip-6780.md Outdated Show resolved Hide resolved
@smartprogrammer93
Copy link
Contributor

smartprogrammer93 commented Aug 16, 2023 •
edited
Loading

Why are we not merging this already? clients' implementations match this now
I guess just need to apply @dankrad's suggestions and we are done.

@smartprogrammer93 smartprogrammer93 mentioned this pull request Aug 16, 2023
Merged
16 tasks
shemnon and others added 2 commits August 16, 2023 08:30
@shemnon @dankrad
Apply suggestions from code review
9efec05 
Co-authored-by: dankrad <mail@dankradfeist.de>
@shemnon
review changes
b9e19d8
@shemnon
Copy link
Contributor Author

shemnon commented Aug 16, 2023

suggestions merged. @dankrad @gballet @vbuterin please merge pr into master.

jochem-brouwer
jochem-brouwer reviewed Aug 16, 2023
View reviewed changes
Copy link
Member

@jochem-brouwer jochem-brouwer left a comment

Choose a reason for hiding this comment

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

Two style comments.

Need to await thoughts about destroying pre-existing accounts (i.e. with balance) regarding Verkle.

EIPS/eip-6780.md Show resolved Hide resolved
EIPS/eip-6780.md Outdated Show resolved Hide resolved
@shemnon shemnon requested a review from dankrad August 22, 2023 14:51
@eth-bot eth-bot enabled auto-merge (squash) September 10, 2023 03:35
eth-bot
eth-bot approved these changes Sep 10, 2023
View reviewed changes
Copy link
Collaborator

@eth-bot eth-bot left a 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...

@eth-bot eth-bot merged commit 22c0ef2 into ethereum:master Sep 10, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smartprogrammer93 smartprogrammer93 smartprogrammer93 left review comments

@invocamanman invocamanman invocamanman left review comments

@jochem-brouwer jochem-brouwer jochem-brouwer left review comments

@maurelian maurelian maurelian left review comments

@eth-bot eth-bot eth-bot approved these changes

@dankrad dankrad dankrad approved these changes

Assignees
No one assigned
Labels
a-review Waiting on author to review c-update Modifies an existing proposal s-draft This EIP is a Draft t-core
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@shemnon @eth-bot @invocamanman @smartprogrammer93 @dankrad @maurelian @jochem-brouwer