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-2200: Structured Definitions for Net Gas Metering #2200

Merged
merged 17 commits into from Oct 25, 2019

Conversation

sorpaas
Copy link
Contributor

@sorpaas sorpaas commented Jul 18, 2019

A repricing efforts for several trie-size-dependent opcodes are being carried out at EIP-1884. This EIP also reprices net-metered SSTORE opcode for EIP-1283.

EIPS/eip-2200.md Outdated Show resolved Hide resolved
EIPS/eip-2200.md Outdated Show resolved Hide resolved
EIPS/eip-2200.md Outdated Show resolved Hide resolved
EIPS/eip-2200.md Outdated Show resolved Hide resolved
EIPS/eip-2200.md Outdated Show resolved Hide resolved
EIPS/eip-2200.md Outdated Show resolved Hide resolved
@axic
Copy link
Member

axic commented Jul 26, 2019

@sorpaas do you plan to make the clarifications @nicksavers requested?

@sorpaas
Copy link
Contributor Author

sorpaas commented Jul 26, 2019

Will do. Sorry for the delay.

@gumb0
Copy link
Member

gumb0 commented Aug 1, 2019

I re-ran test cases from 1283 with these new prices, here are the results sorpaas#1 (comment)

EIPS/eip-2200.md Outdated Show resolved Hide resolved
EIPS/eip-2200.md Outdated Show resolved Hide resolved
@sorpaas
Copy link
Contributor Author

sorpaas commented Aug 1, 2019

I haven't got time to update this. I'm on vacation and got caught on some other stuff I'm working on. If people feel it's not ready, I'm okay to postpone this. However, personally I think there's only documentation work remain and everything else is already finished.

@chfast
Copy link
Contributor

chfast commented Aug 8, 2019

@AlexeyAkhunov's proposal

https://gitter.im/ethereum/AllCoreDevs?at=5d31988ae2d1aa6688d09f39

Instead of doing the check "gasleft < 2300" at the end of the operation, we could instead make a mandatory (unconditional) charge of 2300 in the beginning, and adjust other cases (by issuing more refunds or reducing refunds) to accommodate that. In that way, "gasleft" is not explicitly accessed, but the semantics is the same

@sorpaas @AlexeyAkhunov I did quick analysis of @AlexeyAkhunov alternative proposal to charge min 2300 for SSTORE but add 1500 refund more. None of the EIPs list use cases for it, so I took a simple lock/mutex which is

SLOAD  # check the lock status
SSTORE 0 -> 1 # lock
... # do something, e.g. make a call
SSTORE 1 -> 0 # unlock

@AlexeyAkhunov's proposal (let's call it variant 2) looks pretty solid because SSTORE / SLOAD will cost 800 now (I still have 200 in my mind).
In simplest operation of a CALL costing 700 the variant 2 is less than 7% more expensive in the end.
For the case where in variant 1 we get full refund and the protected operation cost ~17k the difference is less than 4%.
In the final case where variant 2 also get full refund the protected operation cost ~20k (and the end costs are the same obviously).

https://docs.google.com/spreadsheets/d/1l6HHVAEcmJyb76J-trNQQFD1lipjkBkl5-QYx3ME2mc/edit?usp=sharing

This was previously sent to official discussion place for this EIP: sorpaas#1 (comment), but has received no feedback.

@holiman
Copy link
Contributor

holiman commented Aug 8, 2019

I personally think that @AlexeyAkhunov 's proposal is the most elegant. So the variants are

  • Go into some kind of semi-readonly-mode when entry-gas is below 2300. Increases complexity needlessly.
  • Perform a check that we're above 2300 on every SSTORE. Also very quirky -- require N gas but does not use N gas, for some obscure reason that people will forget in a year or two.
  • Increase both SSTORE cost and refund size, thus requiring a larger runtime gas margin which is returned post-facto. Only downside that I can see is that we cannot reuse Constantinople gas formula, but need to have one formula for Rinkeby and another for Mainnet.

All in all, I prefer the latter one, mainly since it's self-contained and does not cause new abstractions or quirks to be added in other places.

@chfast
Copy link
Contributor

chfast commented Aug 8, 2019

* Go into some kind of semi-readonly-mode when entry-gas is below `2300`. Increases complexity needlessly.

I don't think anyone want to go with this one any more. As the original author I propose to cross it out for clarity. Others can always object and bring it back.

EIPS/eip-2200.md Outdated Show resolved Hide resolved
@karalabe
Copy link
Member

Where is this proposal from @AlexeyAkhunov? Everyone's mentioning it, but I have no clue what it is.

@chfast
Copy link
Contributor

chfast commented Aug 14, 2019

Where is this proposal from @AlexeyAkhunov? Everyone's mentioning it, but I have no clue what it is.

There is no formal spec. But summary is this:

proposal to charge min 2300 for SSTORE but add 1500 refund more

I dug up the original message from gitter and put it in #2200 (comment).

@karalabe
Copy link
Member

@chfast The original text of the EIP is If gasleft is less than or equal to 2300, fail the current call frame with 'out of gas' exception. (i.e. it fails on equal too), whereas the suggestion from @AlexeyAkhunov allows equal. Please elaborate

@chfast
Copy link
Contributor

chfast commented Aug 14, 2019

@chfast The original text of the EIP is If gasleft is less than or equal to 2300, fail the current call frame with 'out of gas' exception. (i.e. it fails on equal too), whereas the suggestion from @AlexeyAkhunov allows equal. Please elaborate

Because you also need other instructions to provide arguments for SSTORE, you still will not be able to squeeze SSTORE in a call providing only 2300 gas. Just to keep the set of magic numbers in EVM small, I believe the value 2300 is ok this case.

@karalabe
Copy link
Member

karalabe commented Aug 14, 2019

That proposal makes things quite funky. I've tried to integrate it into the EIP, someone please double check everything. We'll end up duplicating quite a few constants because now we need to take that +1500 gas into consideration on all code paths and compensate.

My attempt(!) at updating the spec

Definitions of constants are as below:

  • SLOAD_GAS: 800
  • SSTORE_GAS: 2300
  • SSTORE_SET_GAS: 20000
  • SSTORE_SET_REFUND: 20700
  • SSTORE_RESET_GAS: 5000
  • SSTORE_RESET_REFUND: 5700
  • SSTORE_CLEAR_REFUND: 15000
  • SSTORE_CLEAR_REFUND_REVERT: 13500

Replace SSTORE opcode gas cost calculation (including refunds) with the following logic:

  • If current value equals new value (this is a no-op), SLOAD_GAS gas is deducted.
  • If current value does not equal new value:
    • If original value equals current value (this storage slot has not been changed by the current execution context):
      • If original value is 0, SSTORE_SET_GAS gas is deducted.
      • Otherwise, SSTORE_RESET_GAS gas is deducted. If new value is 0, add SSTORE_CLEAR_REFUND to refund counter.
    • If original value does not equal current value (this storage slot is dirty), SSTORE_GAS gas is deducted. Apply both of the following clauses:
      • If original value is not 0:
        • If current value is 0 (also means that new value is not 0), subtract SSTORE_CLEAR_REFUND_REVERT gas from refund counter. We can prove that refund counter will never go below 0.
        • If new value is 0 (also means that current value is not 0), add SSTORE_CLEAR_REFUND gas to refund counter.
      • If original value equals new value (this storage slot is reset):
        • If original value is 0, add SSTORE_SET_REFUND to refund counter.
        • Otherwise, add SSTORE_RESET_REFUND gas to refund counter.

@karalabe
Copy link
Member

karalabe commented Aug 14, 2019

Someone would really need to reproduce the entire test matrix for all the variations, otherwise we might end up with some path not being in line with the others. The +1500 addition to SSTORE means we need to compensate all the refunds and refund revertals properly.

A nasty side effect of raising the gas for SSTORE and refunding it later is that whilst the gas paid by end users might be the same, a significant portion of the block gas space might end up wasted, since the transaction still "consumes" that gas out of the block, just is not charged for. Hmm, I guess only the gas used is charged against the block space too.

@karalabe
Copy link
Member

cc @Arachnid Some feedback? ^

@holiman
Copy link
Contributor

holiman commented Aug 14, 2019

since the transaction still "consumes" that gas out of the block, just is not charged fo

Hmm... Are you sure? I don't think so

EIPS/eip-2200.md Show resolved Hide resolved
EIPS/eip-2200.md Outdated Show resolved Hide resolved
@sarawut8128
Copy link

ช่วยตรวจสอบและแก้ไขในส่วนนี้ให้ผมที

EIPS/eip-2200.md Show resolved Hide resolved
EIPS/eip-2200.md Outdated Show resolved Hide resolved
@gumb0
Copy link
Member

gumb0 commented Sep 23, 2019

ping @nicksavers Are your comments addressed?

@fubuloubu
Copy link
Contributor

fubuloubu commented Oct 24, 2019

This needs to be merged ASAP for Istanbul (already in progress)

Edit: without this being merged, the EIP is not available at eips.ethereum.org

@sorpaas
Copy link
Contributor Author

sorpaas commented Oct 24, 2019

Someone needs to tell me what I still need to fix at this moment. Otherwise I believe it's good to merge!

@sorpaas
Copy link
Contributor Author

sorpaas commented Oct 25, 2019

@karalabe I don't think your version at #2200 (comment) is the same as the current spec. The current spec is basically EIP-1283 where all SLOAD_GAS is changed from 200 to 800, which should have been the agreed version.

Please make sure Parity and Geth are implementing the same thing.

@sorpaas
Copy link
Contributor Author

sorpaas commented Oct 25, 2019

@karalabe Looks like I misred. That comment you gave was based on another proposal and not related to the current spec.

@gcolvin gcolvin merged commit ac1a509 into ethereum:master Oct 25, 2019
ilanolkies pushed a commit to ilanolkies/EIPs that referenced this pull request Nov 12, 2019
* Rebalance net-metered SSTORE gas cost with consideration of SLOAD gas cost change

* Rename to EIP-2200

* Update eip-2200.md

* Update eip-2200.md

* Update eip-2200.md

* Copy sections from EIP-1283 and EIP-1706 to EIP-2200

* Fix spelling

* Escape the "SSTORE" word

* Make EIP1283/1706/1884 a link (as customary)

* Update eip-2200.md

* Update eip-2200.md

* Update eip-2200.md

* Update eip-2200.md

* Update eip-2200.md

* Update eip-2200.md
MadeofTin pushed a commit to MadeofTin/EIPs that referenced this pull request Nov 13, 2019
* Rebalance net-metered SSTORE gas cost with consideration of SLOAD gas cost change

* Rename to EIP-2200

* Update eip-2200.md

* Update eip-2200.md

* Update eip-2200.md

* Copy sections from EIP-1283 and EIP-1706 to EIP-2200

* Fix spelling

* Escape the "SSTORE" word

* Make EIP1283/1706/1884 a link (as customary)

* Update eip-2200.md

* Update eip-2200.md

* Update eip-2200.md

* Update eip-2200.md

* Update eip-2200.md

* Update eip-2200.md
tkstanczak pushed a commit to tkstanczak/EIPs that referenced this pull request Nov 7, 2020
* Rebalance net-metered SSTORE gas cost with consideration of SLOAD gas cost change

* Rename to EIP-2200

* Update eip-2200.md

* Update eip-2200.md

* Update eip-2200.md

* Copy sections from EIP-1283 and EIP-1706 to EIP-2200

* Fix spelling

* Escape the "SSTORE" word

* Make EIP1283/1706/1884 a link (as customary)

* Update eip-2200.md

* Update eip-2200.md

* Update eip-2200.md

* Update eip-2200.md

* Update eip-2200.md

* Update eip-2200.md
Arachnid pushed a commit to Arachnid/EIPs that referenced this pull request Mar 6, 2021
* Rebalance net-metered SSTORE gas cost with consideration of SLOAD gas cost change

* Rename to EIP-2200

* Update eip-2200.md

* Update eip-2200.md

* Update eip-2200.md

* Copy sections from EIP-1283 and EIP-1706 to EIP-2200

* Fix spelling

* Escape the "SSTORE" word

* Make EIP1283/1706/1884 a link (as customary)

* Update eip-2200.md

* Update eip-2200.md

* Update eip-2200.md

* Update eip-2200.md

* Update eip-2200.md

* Update eip-2200.md
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.

None yet