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

EIP 3529: alternative refund reduction #3529

Merged
merged 9 commits into from
Apr 26, 2021
Merged

EIP 3529: alternative refund reduction #3529

merged 9 commits into from
Apr 26, 2021

Conversation

vbuterin
Copy link
Contributor

An alternative way to reduce refunds to deal with the issues addressed in EIPs 3298 and 3403.

An alternative way to reduce refunds to deal with the issues addressed in EIPs 3298 and 3403.
@alita-moore
Copy link
Contributor

Hi! I'm a bot, and I wanted to automerge your PR, but couldn't because of the following issue(s):

 - File with name EIPS/eip-3529.md is new and new files must be reviewed

Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

LGTM

@alita-moore alita-moore reopened this Apr 25, 2021
Copy link
Contributor

@MicahZoltu MicahZoltu left a comment

Choose a reason for hiding this comment

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

Only blockers are the external EIP links and the non-standard section. The other points of feedback are not required for Draft.

EIPS/eip-3529.md Outdated
eip: 3529
title: Reduction in refunds
author: Vitalik Buterin (@vbuterin), Martin Swende (@holiman)
discussions-to: https://ethereum-magicians.org/t/eip-3298-removal-of-refunds/5430
Copy link
Contributor

Choose a reason for hiding this comment

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

While not strictly required, consider opening up a separate discussion thread for this EIP so it is clear throughout the discussion what people are talking about. Having multiple EIPs (especially competing ones) that all use the same discussion issue can lead to confusion among participants as people end up not all discussing the same thing.

EIPS/eip-3529.md Outdated Show resolved Hide resolved
EIPS/eip-3529.md Outdated
For blocks where `block.number >= FORK_BLOCK`, the following changes apply.

1. Remove the `SELFDESTRUCT` refund.
2. Replace `SSTORE_CLEARS_SCHEDULE` with `SSTORE_RESET_GAS + ACCESS_LIST_STORAGE_KEY_COST` (4,800 gas as of EIP 2929 + 2930)
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be good to define what SSTORE_CLEARS_SCHEDULE is referring to more specifically. Maybe something like the following (note: I legitimately don't know what this is referring to, so the following is a guess/sample):

Suggested change
2. Replace `SSTORE_CLEARS_SCHEDULE` with `SSTORE_RESET_GAS + ACCESS_LIST_STORAGE_KEY_COST` (4,800 gas as of EIP 2929 + 2930)
2. When a storage value starts a transaction with a non-zero value and ends a transaction with a zero value, subtract `SSTORE_RESET_GAS + ACCESS_LIST_STORAGE_KEY_COST` (4800 at the time of this writing) from the transaction gas used accumulator.

EIPS/eip-3529.md Outdated Show resolved Hide resolved
EIPS/eip-3529.md Outdated Show resolved Hide resolved
EIPS/eip-3529.md Outdated Show resolved Hide resolved
EIPS/eip-3529.md Outdated Show resolved Hide resolved
Copy link
Contributor

@alita-moore alita-moore left a comment

Choose a reason for hiding this comment

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

testing bot

vbuterin and others added 3 commits April 25, 2021 09:18
EIPS/eip-3529.md Outdated Show resolved Hide resolved
EIPS/eip-3529.md Outdated Show resolved Hide resolved
Copy link
Contributor

@alita-moore alita-moore left a comment

Choose a reason for hiding this comment

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

test bot

@alita-moore alita-moore reopened this Apr 26, 2021
EIPS/eip-3529.md Outdated Show resolved Hide resolved
EIPS/eip-3529.md Outdated Show resolved Hide resolved
Copy link
Contributor

@MicahZoltu MicahZoltu left a comment

Choose a reason for hiding this comment

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

One more suggestion, but not a blocker.


## Simple Summary

Remove gas refunds for SELFDESTRUCT, and reduce gas refunds for SSTORE to a lower level where the refunds are still substantial, but they are no longer high enough for current "exploits" of the refund mechanism to be viable.
Copy link
Contributor

Choose a reason for hiding this comment

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

Recommend shortening this up. Think email subject line or forum thread title or GitHub PR title.

Suggested change
Remove gas refunds for SELFDESTRUCT, and reduce gas refunds for SSTORE to a lower level where the refunds are still substantial, but they are no longer high enough for current "exploits" of the refund mechanism to be viable.
Remove gas refunds for SELFDESTRUCT, and reduce gas refunds for SSTORE.

The latter part of this sentence should be left for the motivation section.

@MicahZoltu MicahZoltu merged commit 6079eba into master Apr 26, 2021
@MicahZoltu MicahZoltu deleted the vbuterin-patch-1 branch April 26, 2021 08:10
phi-line pushed a commit to phi-line/EIPs that referenced this pull request Apr 29, 2021
The latter part of this sentence should be left for the motivation section.
PhABC pushed a commit to PhABC/EIPs that referenced this pull request Jan 25, 2022
The latter part of this sentence should be left for the motivation section.
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.

4 participants