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-1283: Net gas metering for SSTORE without dirty maps #1283

Merged
merged 32 commits into from Aug 7, 2018

Conversation

Projects
None yet
6 participants
@sorpaas
Copy link
Contributor

sorpaas commented Aug 1, 2018

eip: 1283
title: Net gas metering for SSTORE without dirty maps
author: Wei Tang (@sorpaas)
discussions-to: https://github.com/sorpaas/EIPs/issues/1
status: Draft
type: Standards Track
category: Core
created: 2018-08-01

Abstract: This EIP proposes net gas metering changes for SSTORE opcode, as an
alternative for EIP-1087. It tries to be friendlier to implementations
that uses different opetimiazation strategies for storage change
caches.

Rendered

sorpaas added some commits Aug 1, 2018

@sorpaas sorpaas changed the title EIP-x: Net gas metering for SSTORE without dirty maps EIP-1283: Net gas metering for SSTORE without dirty maps Aug 1, 2018

@sorpaas sorpaas referenced this pull request Aug 1, 2018

Closed

Constantinople Release #8427

5 of 6 tasks complete
Replace SSTORE opcode gas cost calculation (including refunds) with
the following logic:

* If *current value* equals *new value* (this is a no-op), 200 gas is

This comment has been minimized.

@Arachnid

Arachnid Aug 1, 2018

Collaborator

Following this process, starting with original = 1 and current = 1:

  1. new = 0; deducts 5k gas and adds 15k gas to the refund counter; current = 0.
  2. new = 1; adds 4800 gas to the refund counter
  3. Goto 1.

Thus for every 5k gas deducted from the gas meter, we can add 19800 gas to the refund counter.

This comment has been minimized.

@sorpaas

sorpaas Aug 1, 2018

Author Contributor

Fixed! Added an additional clause when storage slot is dirty. If original value is not 0, if current value is 0 (also means that new value is not 0, because of the parent clause "If current value does not equal new value"), deduct 15000 gas.

  1. new = 0, original = 1, current = 1; deducts 5k gas and adds 15k gas to refund counter.
  2. new = 1, original = 1, current = 0; deducts 200 + 15k gas, adds 4800 gas to the refund counter.
  3. original = 1, current = 1; goto 1; for every round 200 + 20k gas is deducted, and 19800 gas in the refund counter.

This comment has been minimized.

@Arachnid

Arachnid Aug 2, 2018

Collaborator

This seems reasonable. A couple of critiques:

  • Unlike the existing proposal, this can result in consuming a lot of gas and adding it to the refund counter. Since refunds are limited to 1/2 the total gas used, you could easily end up paying more than expected if you set a value to and from its original value a number of times.
  • It's hard to follow the logic here. Some diagramming, or breaking down the cases more would help.

This comment has been minimized.

@sorpaas

sorpaas Aug 2, 2018

Author Contributor

Thanks! I think (1) is a really good point. If there're cases where it can cost a lot most gases compared with the current scheme, then we may break backward compatibility for existing contracts. So in 23ebc94 I changed the clause from deducting 15k gas to removing 15k gas from refund counter. In this way, we will never consume more gases compared with current scheme. Indeed, this adds a new semantics for gas refunds (we never removed gas from refund counter before), so I'm definitely open to better ideas on this. The current rationale/assumption for this is:

  • Backward compatibility is important.
  • We can prove that refund counter will never go below 0.
  • This new semantic of removing gas from refund counter is trivial to implement in most clients.

Talking about other cases where it may cost more gases compared with EIP-1087 -- it happens with sub-call frames to the same contract where we cannot track the dirtiness. But I still think it may be worth the trade off because:

  • We don't suffer from the optimization limitation of EIP-1087. After EIP-658, an efficient storage cache implementation would probably use an in-memory trie (without RLP encoding/decoding) or other immutable data structures to keep track of storage changes, and only commit changes at the end of a block. For those implementations, we cannot efficiently iterate over a transaction's storage change slots without doing a full diff of the trie.
  • It never costs more gases compared with current scheme.
  • It covers commons usages like reentry locks, same-contract multi-send, etc.

Still working on (2). Let me try to see whether there are clearer ways to describe the spec.

This comment has been minimized.

@sorpaas

sorpaas Aug 3, 2018

Author Contributor

Added an explanation section to make the logic more clearer!

@Arachnid

This comment has been minimized.

Copy link
Collaborator

Arachnid commented Aug 3, 2018

So, here's a state transition diagram as best I can establish. This diagram ignores no-op writes, which are trivial. The graph has two parts: one for where the original value is 0, and one for where the original value is nonzero.

I agree that there doesn't seem to be a way to "pump" the refund counter. There are a couple of situations where a series of writes can result in moving gas in 5k chunks from gas meter to refund counter still, however - such as when the value is nonzero to start, and it's repeatedly set to another value and then back to its original value.

I think a table version of this would probably be useful, too. Each of the two graphs can likely be summarised in a table fairly intuitively.

screen shot 2018-08-03 at 12 42 25

Here's the graphviz source for the diagram:

digraph G {
  00 -> 01 [label="~0; 20k gas"];
  01 -> 01 [label="~0; 200 gas"];
  01 -> 00 [label="0; 200 gas, 19.8k refund"];

  11 -> 12 [label="~orig, ~0; 5k gas"];
  12 -> 12 [label="~orig; 200 gas"];
  12 -> 11 [label="orig; 200 gas, 4.8k refund"];
  12 -> 10 [label="0; 200 gas, 15k refund"];
  11 -> 10 [label="0; 5k gas, 15k refund"];
  10 -> 11 [label="orig; 200 gas, -10.2k refund"];
  10 -> 12 [label="~orig; ~0; 200 gas, -15k refund"];

  00 [label="current=orig=0"];
  01 [label="current!=orig"];
  11 [label="current=orig!=0"];
  12 [label="current!=orig"];
  10 [label="current=0"];
}

sorpaas added some commits Aug 3, 2018

@sorpaas

This comment has been minimized.

Copy link
Contributor Author

sorpaas commented Aug 3, 2018

Thanks so much! I added the diagram and the table version to the spec Explanation section (if that's okay for you)!

I agree that there doesn't seem to be a way to "pump" the refund counter. There are a couple of situations where a series of writes can result in moving gas in 5k chunks from gas meter to refund counter still, however - such as when the value is nonzero to start, and it's repeatedly set to another value and then back to its original value.

Yes indeed. That's the case for current scheme as well (repeatedly call SSTORE to set a slot from 0 to non-zero, and then back to zero, each cycle would cost 25k and gets 15k refund). I can't think of a way to use the methodology in this EIP to fix that, though.

Below are table version of the above diagram. Horizontal shows the
next state, and vertical shows the current state.

| | `current=orig=0` | `current!=orig` |

This comment has been minimized.

@Arachnid

Arachnid Aug 6, 2018

Collaborator

Wouldn't this be clearer if one axis had (current, original) and the other axis had new?


![State Transition](../assets/eip-1283/state.png)

Below are table version of the above diagram. Horizontal shows the

This comment has been minimized.

@Arachnid

Arachnid Aug 6, 2018

Collaborator

I think you have 'horizontal' and 'vertical' reversed here.

This comment has been minimized.

@sorpaas

sorpaas Aug 6, 2018

Author Contributor

Oops! Fixed!

sorpaas and others added some commits Aug 6, 2018

sorpaas added some commits Aug 7, 2018

@Arachnid Arachnid merged commit d087ea8 into ethereum:master Aug 7, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@chfast

This comment has been minimized.

Copy link
Contributor

chfast commented Aug 8, 2018

Some test cases: ethereum/tests#483.

@0828567702

This comment has been minimized.

Copy link

0828567702 commented on EIPS/eip-sstore-no-dirty-map.md in 6e0338f Dec 27, 2018

76210001132487.bivd

@0828567702

This comment has been minimized.

Copy link

0828567702 commented on EIPS/eip-sstore-no-dirty-map.md in 6e0338f Dec 27, 2018

Sstore

@k06a

This comment has been minimized.

Copy link

k06a commented Jan 15, 2019

@banteg

This comment has been minimized.

Copy link

banteg commented Jan 15, 2019

charge 5000, refund 4800?

@k06a

This comment has been minimized.

Copy link

k06a commented Jan 16, 2019

charge 5000, refund 4800?

Charge 2300, refund 2100? :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment