Skip to content

Commit ee92bc5

Browse files
authored
Merge pull request #17383 from holiman/eip1283
Eip1283
2 parents 81080bf + 360a72d commit ee92bc5

File tree

11 files changed

+195
-146
lines changed

11 files changed

+195
-146
lines changed

core/state/state_object.go

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ type stateObject struct {
7777
trie Trie // storage trie, which becomes non-nil on first access
7878
code Code // contract bytecode, which gets set when code is loaded
7979

80-
cachedStorage Storage // Storage entry cache to avoid duplicate reads
80+
originStorage Storage // Storage cache of original entries to dedup rewrites
8181
dirtyStorage Storage // Storage entries that need to be flushed to disk
8282

8383
// Cache flags.
@@ -115,7 +115,7 @@ func newObject(db *StateDB, address common.Address, data Account) *stateObject {
115115
address: address,
116116
addrHash: crypto.Keccak256Hash(address[:]),
117117
data: data,
118-
cachedStorage: make(Storage),
118+
originStorage: make(Storage),
119119
dirtyStorage: make(Storage),
120120
}
121121
}
@@ -159,13 +159,25 @@ func (c *stateObject) getTrie(db Database) Trie {
159159
return c.trie
160160
}
161161

162-
// GetState returns a value in account storage.
162+
// GetState retrieves a value from the account storage trie.
163163
func (self *stateObject) GetState(db Database, key common.Hash) common.Hash {
164-
value, exists := self.cachedStorage[key]
165-
if exists {
164+
// If we have a dirty value for this state entry, return it
165+
value, dirty := self.dirtyStorage[key]
166+
if dirty {
166167
return value
167168
}
168-
// Load from DB in case it is missing.
169+
// Otherwise return the entry's original value
170+
return self.GetCommittedState(db, key)
171+
}
172+
173+
// GetCommittedState retrieves a value from the committed account storage trie.
174+
func (self *stateObject) GetCommittedState(db Database, key common.Hash) common.Hash {
175+
// If we have the original value cached, return that
176+
value, cached := self.originStorage[key]
177+
if cached {
178+
return value
179+
}
180+
// Otherwise load the value from the database
169181
enc, err := self.getTrie(db).TryGet(key[:])
170182
if err != nil {
171183
self.setError(err)
@@ -178,22 +190,27 @@ func (self *stateObject) GetState(db Database, key common.Hash) common.Hash {
178190
}
179191
value.SetBytes(content)
180192
}
181-
self.cachedStorage[key] = value
193+
self.originStorage[key] = value
182194
return value
183195
}
184196

185197
// SetState updates a value in account storage.
186198
func (self *stateObject) SetState(db Database, key, value common.Hash) {
199+
// If the new value is the same as old, don't set
200+
prev := self.GetState(db, key)
201+
if prev == value {
202+
return
203+
}
204+
// New value is different, update and journal the change
187205
self.db.journal.append(storageChange{
188206
account: &self.address,
189207
key: key,
190-
prevalue: self.GetState(db, key),
208+
prevalue: prev,
191209
})
192210
self.setState(key, value)
193211
}
194212

195213
func (self *stateObject) setState(key, value common.Hash) {
196-
self.cachedStorage[key] = value
197214
self.dirtyStorage[key] = value
198215
}
199216

@@ -202,6 +219,13 @@ func (self *stateObject) updateTrie(db Database) Trie {
202219
tr := self.getTrie(db)
203220
for key, value := range self.dirtyStorage {
204221
delete(self.dirtyStorage, key)
222+
223+
// Skip noop changes, persist actual changes
224+
if value == self.originStorage[key] {
225+
continue
226+
}
227+
self.originStorage[key] = value
228+
205229
if (value == common.Hash{}) {
206230
self.setError(tr.TryDelete(key[:]))
207231
continue
@@ -279,7 +303,7 @@ func (self *stateObject) deepCopy(db *StateDB) *stateObject {
279303
}
280304
stateObject.code = self.code
281305
stateObject.dirtyStorage = self.dirtyStorage.Copy()
282-
stateObject.cachedStorage = self.dirtyStorage.Copy()
306+
stateObject.originStorage = self.originStorage.Copy()
283307
stateObject.suicided = self.suicided
284308
stateObject.dirtyCode = self.dirtyCode
285309
stateObject.deleted = self.deleted

core/state/state_test.go

Lines changed: 36 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,15 @@ func (s *StateSuite) TestNull(c *checker.C) {
9696
s.state.CreateAccount(address)
9797
//value := common.FromHex("0x823140710bf13990e4500136726d8b55")
9898
var value common.Hash
99+
99100
s.state.SetState(address, common.Hash{}, value)
100101
s.state.Commit(false)
101-
value = s.state.GetState(address, common.Hash{})
102-
if value != (common.Hash{}) {
103-
c.Errorf("expected empty hash. got %x", value)
102+
103+
if value := s.state.GetState(address, common.Hash{}); value != (common.Hash{}) {
104+
c.Errorf("expected empty current value, got %x", value)
105+
}
106+
if value := s.state.GetCommittedState(address, common.Hash{}); value != (common.Hash{}) {
107+
c.Errorf("expected empty committed value, got %x", value)
104108
}
105109
}
106110

@@ -110,20 +114,24 @@ func (s *StateSuite) TestSnapshot(c *checker.C) {
110114
data1 := common.BytesToHash([]byte{42})
111115
data2 := common.BytesToHash([]byte{43})
112116

117+
// snapshot the genesis state
118+
genesis := s.state.Snapshot()
119+
113120
// set initial state object value
114121
s.state.SetState(stateobjaddr, storageaddr, data1)
115-
// get snapshot of current state
116122
snapshot := s.state.Snapshot()
117123

118-
// set new state object value
124+
// set a new state object value, revert it and ensure correct content
119125
s.state.SetState(stateobjaddr, storageaddr, data2)
120-
// restore snapshot
121126
s.state.RevertToSnapshot(snapshot)
122127

123-
// get state storage value
124-
res := s.state.GetState(stateobjaddr, storageaddr)
128+
c.Assert(s.state.GetState(stateobjaddr, storageaddr), checker.DeepEquals, data1)
129+
c.Assert(s.state.GetCommittedState(stateobjaddr, storageaddr), checker.DeepEquals, common.Hash{})
125130

126-
c.Assert(data1, checker.DeepEquals, res)
131+
// revert up to the genesis state and ensure correct content
132+
s.state.RevertToSnapshot(genesis)
133+
c.Assert(s.state.GetState(stateobjaddr, storageaddr), checker.DeepEquals, common.Hash{})
134+
c.Assert(s.state.GetCommittedState(stateobjaddr, storageaddr), checker.DeepEquals, common.Hash{})
127135
}
128136

129137
func (s *StateSuite) TestSnapshotEmpty(c *checker.C) {
@@ -208,24 +216,30 @@ func compareStateObjects(so0, so1 *stateObject, t *testing.T) {
208216
t.Fatalf("Code mismatch: have %v, want %v", so0.code, so1.code)
209217
}
210218

211-
if len(so1.cachedStorage) != len(so0.cachedStorage) {
212-
t.Errorf("Storage size mismatch: have %d, want %d", len(so1.cachedStorage), len(so0.cachedStorage))
219+
if len(so1.dirtyStorage) != len(so0.dirtyStorage) {
220+
t.Errorf("Dirty storage size mismatch: have %d, want %d", len(so1.dirtyStorage), len(so0.dirtyStorage))
213221
}
214-
for k, v := range so1.cachedStorage {
215-
if so0.cachedStorage[k] != v {
216-
t.Errorf("Storage key %x mismatch: have %v, want %v", k, so0.cachedStorage[k], v)
222+
for k, v := range so1.dirtyStorage {
223+
if so0.dirtyStorage[k] != v {
224+
t.Errorf("Dirty storage key %x mismatch: have %v, want %v", k, so0.dirtyStorage[k], v)
217225
}
218226
}
219-
for k, v := range so0.cachedStorage {
220-
if so1.cachedStorage[k] != v {
221-
t.Errorf("Storage key %x mismatch: have %v, want none.", k, v)
227+
for k, v := range so0.dirtyStorage {
228+
if so1.dirtyStorage[k] != v {
229+
t.Errorf("Dirty storage key %x mismatch: have %v, want none.", k, v)
222230
}
223231
}
224-
225-
if so0.suicided != so1.suicided {
226-
t.Fatalf("suicided mismatch: have %v, want %v", so0.suicided, so1.suicided)
232+
if len(so1.originStorage) != len(so0.originStorage) {
233+
t.Errorf("Origin storage size mismatch: have %d, want %d", len(so1.originStorage), len(so0.originStorage))
234+
}
235+
for k, v := range so1.originStorage {
236+
if so0.originStorage[k] != v {
237+
t.Errorf("Origin storage key %x mismatch: have %v, want %v", k, so0.originStorage[k], v)
238+
}
227239
}
228-
if so0.deleted != so1.deleted {
229-
t.Fatalf("Deleted mismatch: have %v, want %v", so0.deleted, so1.deleted)
240+
for k, v := range so0.originStorage {
241+
if so1.originStorage[k] != v {
242+
t.Errorf("Origin storage key %x mismatch: have %v, want none.", k, v)
243+
}
230244
}
231245
}

core/state/statedb.go

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -169,11 +169,22 @@ func (self *StateDB) Preimages() map[common.Hash][]byte {
169169
return self.preimages
170170
}
171171

172+
// AddRefund adds gas to the refund counter
172173
func (self *StateDB) AddRefund(gas uint64) {
173174
self.journal.append(refundChange{prev: self.refund})
174175
self.refund += gas
175176
}
176177

178+
// SubRefund removes gas from the refund counter.
179+
// This method will panic if the refund counter goes below zero
180+
func (self *StateDB) SubRefund(gas uint64) {
181+
self.journal.append(refundChange{prev: self.refund})
182+
if gas > self.refund {
183+
panic("Refund counter below zero")
184+
}
185+
self.refund -= gas
186+
}
187+
177188
// Exist reports whether the given account address exists in the state.
178189
// Notably this also returns true for suicided accounts.
179190
func (self *StateDB) Exist(addr common.Address) bool {
@@ -236,10 +247,20 @@ func (self *StateDB) GetCodeHash(addr common.Address) common.Hash {
236247
return common.BytesToHash(stateObject.CodeHash())
237248
}
238249

239-
func (self *StateDB) GetState(addr common.Address, bhash common.Hash) common.Hash {
250+
// GetState retrieves a value from the given account's storage trie.
251+
func (self *StateDB) GetState(addr common.Address, hash common.Hash) common.Hash {
252+
stateObject := self.getStateObject(addr)
253+
if stateObject != nil {
254+
return stateObject.GetState(self.db, hash)
255+
}
256+
return common.Hash{}
257+
}
258+
259+
// GetCommittedState retrieves a value from the given account's committed storage trie.
260+
func (self *StateDB) GetCommittedState(addr common.Address, hash common.Hash) common.Hash {
240261
stateObject := self.getStateObject(addr)
241262
if stateObject != nil {
242-
return stateObject.GetState(self.db, bhash)
263+
return stateObject.GetCommittedState(self.db, hash)
243264
}
244265
return common.Hash{}
245266
}
@@ -435,19 +456,14 @@ func (db *StateDB) ForEachStorage(addr common.Address, cb func(key, value common
435456
if so == nil {
436457
return
437458
}
438-
439-
// When iterating over the storage check the cache first
440-
for h, value := range so.cachedStorage {
441-
cb(h, value)
442-
}
443-
444459
it := trie.NewIterator(so.getTrie(db.db).NodeIterator(nil))
445460
for it.Next() {
446-
// ignore cached values
447461
key := common.BytesToHash(db.trie.GetKey(it.Key))
448-
if _, ok := so.cachedStorage[key]; !ok {
449-
cb(key, common.BytesToHash(it.Value))
462+
if value, dirty := so.dirtyStorage[key]; dirty {
463+
cb(key, value)
464+
continue
450465
}
466+
cb(key, common.BytesToHash(it.Value))
451467
}
452468
}
453469

core/state/statedb_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -381,11 +381,11 @@ func (test *snapshotTest) checkEqual(state, checkstate *StateDB) error {
381381
checkeq("GetCodeSize", state.GetCodeSize(addr), checkstate.GetCodeSize(addr))
382382
// Check storage.
383383
if obj := state.getStateObject(addr); obj != nil {
384-
state.ForEachStorage(addr, func(key, val common.Hash) bool {
385-
return checkeq("GetState("+key.Hex()+")", val, checkstate.GetState(addr, key))
384+
state.ForEachStorage(addr, func(key, value common.Hash) bool {
385+
return checkeq("GetState("+key.Hex()+")", checkstate.GetState(addr, key), value)
386386
})
387-
checkstate.ForEachStorage(addr, func(key, checkval common.Hash) bool {
388-
return checkeq("GetState("+key.Hex()+")", state.GetState(addr, key), checkval)
387+
checkstate.ForEachStorage(addr, func(key, value common.Hash) bool {
388+
return checkeq("GetState("+key.Hex()+")", checkstate.GetState(addr, key), value)
389389
})
390390
}
391391
if err != nil {

core/vm/gas_table.go

Lines changed: 61 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -117,24 +117,69 @@ func gasReturnDataCopy(gt params.GasTable, evm *EVM, contract *Contract, stack *
117117

118118
func gasSStore(gt params.GasTable, evm *EVM, contract *Contract, stack *Stack, mem *Memory, memorySize uint64) (uint64, error) {
119119
var (
120-
y, x = stack.Back(1), stack.Back(0)
121-
val = evm.StateDB.GetState(contract.Address(), common.BigToHash(x))
120+
y, x = stack.Back(1), stack.Back(0)
121+
current = evm.StateDB.GetState(contract.Address(), common.BigToHash(x))
122122
)
123-
// This checks for 3 scenario's and calculates gas accordingly
124-
// 1. From a zero-value address to a non-zero value (NEW VALUE)
125-
// 2. From a non-zero value address to a zero-value address (DELETE)
126-
// 3. From a non-zero to a non-zero (CHANGE)
127-
if val == (common.Hash{}) && y.Sign() != 0 {
128-
// 0 => non 0
129-
return params.SstoreSetGas, nil
130-
} else if val != (common.Hash{}) && y.Sign() == 0 {
131-
// non 0 => 0
132-
evm.StateDB.AddRefund(params.SstoreRefundGas)
133-
return params.SstoreClearGas, nil
134-
} else {
135-
// non 0 => non 0 (or 0 => 0)
136-
return params.SstoreResetGas, nil
123+
// The legacy gas metering only takes into consideration the current state
124+
if !evm.chainRules.IsConstantinople {
125+
// This checks for 3 scenario's and calculates gas accordingly:
126+
//
127+
// 1. From a zero-value address to a non-zero value (NEW VALUE)
128+
// 2. From a non-zero value address to a zero-value address (DELETE)
129+
// 3. From a non-zero to a non-zero (CHANGE)
130+
switch {
131+
case current == (common.Hash{}) && y.Sign() != 0: // 0 => non 0
132+
return params.SstoreSetGas, nil
133+
case current != (common.Hash{}) && y.Sign() == 0: // non 0 => 0
134+
evm.StateDB.AddRefund(params.SstoreRefundGas)
135+
return params.SstoreClearGas, nil
136+
default: // non 0 => non 0 (or 0 => 0)
137+
return params.SstoreResetGas, nil
138+
}
139+
}
140+
// The new gas metering is based on net gas costs (EIP-1283):
141+
//
142+
// 1. If current value equals new value (this is a no-op), 200 gas is deducted.
143+
// 2. If current value does not equal new value
144+
// 2.1. If original value equals current value (this storage slot has not been changed by the current execution context)
145+
// 2.1.1. If original value is 0, 20000 gas is deducted.
146+
// 2.1.2. Otherwise, 5000 gas is deducted. If new value is 0, add 15000 gas to refund counter.
147+
// 2.2. If original value does not equal current value (this storage slot is dirty), 200 gas is deducted. Apply both of the following clauses.
148+
// 2.2.1. If original value is not 0
149+
// 2.2.1.1. If current value is 0 (also means that new value is not 0), remove 15000 gas from refund counter. We can prove that refund counter will never go below 0.
150+
// 2.2.1.2. If new value is 0 (also means that current value is not 0), add 15000 gas to refund counter.
151+
// 2.2.2. If original value equals new value (this storage slot is reset)
152+
// 2.2.2.1. If original value is 0, add 19800 gas to refund counter.
153+
// 2.2.2.2. Otherwise, add 4800 gas to refund counter.
154+
value := common.BigToHash(y)
155+
if current == value { // noop (1)
156+
return params.NetSstoreNoopGas, nil
157+
}
158+
original := evm.StateDB.GetCommittedState(contract.Address(), common.BigToHash(x))
159+
if original == current {
160+
if original == (common.Hash{}) { // create slot (2.1.1)
161+
return params.NetSstoreInitGas, nil
162+
}
163+
if value == (common.Hash{}) { // delete slot (2.1.2b)
164+
evm.StateDB.AddRefund(params.NetSstoreClearRefund)
165+
}
166+
return params.NetSstoreCleanGas, nil // write existing slot (2.1.2)
167+
}
168+
if original != (common.Hash{}) {
169+
if current == (common.Hash{}) { // recreate slot (2.2.1.1)
170+
evm.StateDB.SubRefund(params.NetSstoreClearRefund)
171+
} else if value == (common.Hash{}) { // delete slot (2.2.1.2)
172+
evm.StateDB.AddRefund(params.NetSstoreClearRefund)
173+
}
174+
}
175+
if original == value {
176+
if original == (common.Hash{}) { // reset to original inexistent slot (2.2.2.1)
177+
evm.StateDB.AddRefund(params.NetSstoreResetClearRefund)
178+
} else { // reset to original existing slot (2.2.2.2)
179+
evm.StateDB.AddRefund(params.NetSstoreResetRefund)
180+
}
137181
}
182+
return params.NetSstoreDirtyGas, nil
138183
}
139184

140185
func makeGasLog(n uint64) gasFunc {

core/vm/interface.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,10 @@ type StateDB interface {
4040
GetCodeSize(common.Address) int
4141

4242
AddRefund(uint64)
43+
SubRefund(uint64)
4344
GetRefund() uint64
4445

46+
GetCommittedState(common.Address, common.Hash) common.Hash
4547
GetState(common.Address, common.Hash) common.Hash
4648
SetState(common.Address, common.Hash, common.Hash)
4749

core/vm/logger_test.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,6 @@ func (d *dummyContractRef) SetBalance(*big.Int) {}
4141
func (d *dummyContractRef) SetNonce(uint64) {}
4242
func (d *dummyContractRef) Balance() *big.Int { return new(big.Int) }
4343

44-
type dummyStateDB struct {
45-
NoopStateDB
46-
ref *dummyContractRef
47-
}
48-
4944
func TestStoreCapture(t *testing.T) {
5045
var (
5146
env = NewEVM(Context{}, nil, params.TestChainConfig, Config{})

0 commit comments

Comments
 (0)