Skip to content

Commit 1ca8f85

Browse files
rjl493456442rjl493456442
committed
core/state: fix statedb.Copy
1 parent 77c4bbc commit 1ca8f85

File tree

5 files changed

+226
-45
lines changed

5 files changed

+226
-45
lines changed

‎core/state/journal.go

Lines changed: 131 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ type journalEntry interface {
3030

3131
// dirtied returns the Ethereum address modified by this journal entry.
3232
dirtied() *common.Address
33+
34+
// copy returns a deep-copied journal entry.
35+
copy() journalEntry
3336
}
3437

3538
// journal contains the list of state modifications applied since the last state
@@ -84,6 +87,22 @@ func (j *journal) length() int {
8487
return len(j.entries)
8588
}
8689

90+
// copy returns a deep-copied journal.
91+
func (j *journal) copy() *journal {
92+
entries := make([]journalEntry, 0, j.length())
93+
for i := 0; i < j.length(); i++ {
94+
entries = append(entries, j.entries[i].copy())
95+
}
96+
dirties := make(map[common.Address]int)
97+
for addr, count := range j.dirties {
98+
dirties[addr] = count
99+
}
100+
return &journal{
101+
entries: entries,
102+
dirties: dirties,
103+
}
104+
}
105+
87106
type (
88107
// Changes to the account trie.
89108
createObjectChange struct {
@@ -137,6 +156,7 @@ type (
137156
touchChange struct {
138157
account *common.Address
139158
}
159+
140160
// Changes to the access list
141161
accessListAddAccountChange struct {
142162
address *common.Address
@@ -146,6 +166,7 @@ type (
146166
slot *common.Hash
147167
}
148168

169+
// Changes to transient storage
149170
transientStorageChange struct {
150171
account *common.Address
151172
key, prevalue common.Hash
@@ -154,13 +175,18 @@ type (
154175

155176
func (ch createObjectChange) revert(s *StateDB) {
156177
delete(s.stateObjects, *ch.account)
157-
delete(s.stateObjectsDirty, *ch.account)
158178
}
159179

160180
func (ch createObjectChange) dirtied() *common.Address {
161181
return ch.account
162182
}
163183

184+
func (ch createObjectChange) copy() journalEntry {
185+
return createObjectChange{
186+
account: ch.account,
187+
}
188+
}
189+
164190
func (ch resetObjectChange) revert(s *StateDB) {
165191
s.setStateObject(ch.prev)
166192
if !ch.prevdestruct {
@@ -184,6 +210,27 @@ func (ch resetObjectChange) dirtied() *common.Address {
184210
return ch.account
185211
}
186212

213+
func (ch resetObjectChange) copy() journalEntry {
214+
prevStorage := make(map[common.Hash][]byte)
215+
for key, slot := range ch.prevStorage {
216+
prevStorage[key] = common.CopyBytes(slot)
217+
}
218+
prevStorageOrigin := make(map[common.Hash][]byte)
219+
for key, slot := range ch.prevStorageOrigin {
220+
prevStorageOrigin[key] = common.CopyBytes(slot)
221+
}
222+
return resetObjectChange{
223+
account: ch.account,
224+
prev: ch.prev.deepCopy(),
225+
prevdestruct: ch.prevdestruct,
226+
prevAccount: common.CopyBytes(ch.prevAccount),
227+
prevStorage: prevStorage,
228+
prevAccountOriginExist: ch.prevAccountOriginExist,
229+
prevAccountOrigin: common.CopyBytes(ch.prevAccountOrigin),
230+
prevStorageOrigin: prevStorageOrigin,
231+
}
232+
}
233+
187234
func (ch selfDestructChange) revert(s *StateDB) {
188235
obj := s.getStateObject(*ch.account)
189236
if obj != nil {
@@ -196,6 +243,14 @@ func (ch selfDestructChange) dirtied() *common.Address {
196243
return ch.account
197244
}
198245

246+
func (ch selfDestructChange) copy() journalEntry {
247+
return selfDestructChange{
248+
account: ch.account,
249+
prev: ch.prev,
250+
prevbalance: new(big.Int).Set(ch.prevbalance),
251+
}
252+
}
253+
199254
var ripemd = common.HexToAddress("0000000000000000000000000000000000000003")
200255

201256
func (ch touchChange) revert(s *StateDB) {
@@ -205,6 +260,12 @@ func (ch touchChange) dirtied() *common.Address {
205260
return ch.account
206261
}
207262

263+
func (ch touchChange) copy() journalEntry {
264+
return touchChange{
265+
account: ch.account,
266+
}
267+
}
268+
208269
func (ch balanceChange) revert(s *StateDB) {
209270
s.getStateObject(*ch.account).setBalance(ch.prev)
210271
}
@@ -213,6 +274,13 @@ func (ch balanceChange) dirtied() *common.Address {
213274
return ch.account
214275
}
215276

277+
func (ch balanceChange) copy() journalEntry {
278+
return balanceChange{
279+
account: ch.account,
280+
prev: new(big.Int).Set(ch.prev),
281+
}
282+
}
283+
216284
func (ch nonceChange) revert(s *StateDB) {
217285
s.getStateObject(*ch.account).setNonce(ch.prev)
218286
}
@@ -221,6 +289,13 @@ func (ch nonceChange) dirtied() *common.Address {
221289
return ch.account
222290
}
223291

292+
func (ch nonceChange) copy() journalEntry {
293+
return nonceChange{
294+
account: ch.account,
295+
prev: ch.prev,
296+
}
297+
}
298+
224299
func (ch codeChange) revert(s *StateDB) {
225300
s.getStateObject(*ch.account).setCode(common.BytesToHash(ch.prevhash), ch.prevcode)
226301
}
@@ -229,6 +304,14 @@ func (ch codeChange) dirtied() *common.Address {
229304
return ch.account
230305
}
231306

307+
func (ch codeChange) copy() journalEntry {
308+
return codeChange{
309+
account: ch.account,
310+
prevhash: common.CopyBytes(ch.prevhash),
311+
prevcode: common.CopyBytes(ch.prevcode),
312+
}
313+
}
314+
232315
func (ch storageChange) revert(s *StateDB) {
233316
s.getStateObject(*ch.account).setState(ch.key, ch.prevalue)
234317
}
@@ -237,6 +320,14 @@ func (ch storageChange) dirtied() *common.Address {
237320
return ch.account
238321
}
239322

323+
func (ch storageChange) copy() journalEntry {
324+
return storageChange{
325+
account: ch.account,
326+
key: ch.key,
327+
prevalue: ch.prevalue,
328+
}
329+
}
330+
240331
func (ch transientStorageChange) revert(s *StateDB) {
241332
s.setTransientState(*ch.account, ch.key, ch.prevalue)
242333
}
@@ -245,6 +336,14 @@ func (ch transientStorageChange) dirtied() *common.Address {
245336
return nil
246337
}
247338

339+
func (ch transientStorageChange) copy() journalEntry {
340+
return transientStorageChange{
341+
account: ch.account,
342+
key: ch.key,
343+
prevalue: ch.prevalue,
344+
}
345+
}
346+
248347
func (ch refundChange) revert(s *StateDB) {
249348
s.refund = ch.prev
250349
}
@@ -253,6 +352,12 @@ func (ch refundChange) dirtied() *common.Address {
253352
return nil
254353
}
255354

355+
func (ch refundChange) copy() journalEntry {
356+
return refundChange{
357+
prev: ch.prev,
358+
}
359+
}
360+
256361
func (ch addLogChange) revert(s *StateDB) {
257362
logs := s.logs[ch.txhash]
258363
if len(logs) == 1 {
@@ -267,6 +372,12 @@ func (ch addLogChange) dirtied() *common.Address {
267372
return nil
268373
}
269374

375+
func (ch addLogChange) copy() journalEntry {
376+
return addLogChange{
377+
txhash: ch.txhash,
378+
}
379+
}
380+
270381
func (ch addPreimageChange) revert(s *StateDB) {
271382
delete(s.preimages, ch.hash)
272383
}
@@ -275,6 +386,12 @@ func (ch addPreimageChange) dirtied() *common.Address {
275386
return nil
276387
}
277388

389+
func (ch addPreimageChange) copy() journalEntry {
390+
return addPreimageChange{
391+
hash: ch.hash,
392+
}
393+
}
394+
278395
func (ch accessListAddAccountChange) revert(s *StateDB) {
279396
/*
280397
One important invariant here, is that whenever a (addr, slot) is added, if the
@@ -292,10 +409,23 @@ func (ch accessListAddAccountChange) dirtied() *common.Address {
292409
return nil
293410
}
294411

412+
func (ch accessListAddAccountChange) copy() journalEntry {
413+
return accessListAddAccountChange{
414+
address: ch.address,
415+
}
416+
}
417+
295418
func (ch accessListAddSlotChange) revert(s *StateDB) {
296419
s.accessList.DeleteSlot(*ch.address, *ch.slot)
297420
}
298421

299422
func (ch accessListAddSlotChange) dirtied() *common.Address {
300423
return nil
301424
}
425+
426+
func (ch accessListAddSlotChange) copy() journalEntry {
427+
return accessListAddSlotChange{
428+
address: ch.address,
429+
slot: ch.slot,
430+
}
431+
}

‎core/state/state_object.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -438,16 +438,16 @@ func (s *stateObject) setBalance(amount *big.Int) {
438438
s.data.Balance = amount
439439
}
440440

441-
func (s *stateObject) deepCopy(db *StateDB) *stateObject {
441+
func (s *stateObject) deepCopy() *stateObject {
442442
obj := &stateObject{
443-
db: db,
443+
db: s.db,
444444
address: s.address,
445445
addrHash: s.addrHash,
446446
origin: s.origin,
447447
data: s.data,
448448
}
449449
if s.trie != nil {
450-
obj.trie = db.db.CopyTrie(s.trie)
450+
obj.trie = s.db.db.CopyTrie(s.trie)
451451
}
452452
obj.code = s.code
453453
obj.dirtyStorage = s.dirtyStorage.Copy()

‎core/state/state_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,3 +296,21 @@ func compareStateObjects(so0, so1 *stateObject, t *testing.T) {
296296
}
297297
}
298298
}
299+
300+
func TestCreateObjectRevert(t *testing.T) {
301+
state, _ := New(types.EmptyRootHash, NewDatabase(rawdb.NewMemoryDatabase()), nil)
302+
addr := common.BytesToAddress([]byte("so0"))
303+
snap := state.Snapshot()
304+
305+
state.CreateAccount(addr)
306+
so0 := state.getStateObject(addr)
307+
so0.SetBalance(big.NewInt(42))
308+
so0.SetNonce(43)
309+
so0.SetCode(crypto.Keccak256Hash([]byte{'c', 'a', 'f', 'e'}), []byte{'c', 'a', 'f', 'e'})
310+
state.setStateObject(so0)
311+
312+
state.RevertToSnapshot(snap)
313+
if state.Exist(addr) {
314+
t.Error("Unexpected account after revert")
315+
}
316+
}

‎core/state/statedb.go

Lines changed: 4 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,6 @@ func (s *StateDB) Selfdestruct6780(addr common.Address) {
461461
if stateObject == nil {
462462
return
463463
}
464-
465464
if stateObject.created {
466465
s.SelfDestruct(addr)
467466
}
@@ -702,7 +701,7 @@ func (s *StateDB) Copy() *StateDB {
702701
logs: make(map[common.Hash][]*types.Log, len(s.logs)),
703702
logSize: s.logSize,
704703
preimages: make(map[common.Hash][]byte, len(s.preimages)),
705-
journal: newJournal(),
704+
journal: s.journal.copy(),
706705
hasher: crypto.NewKeccakState(),
707706

708707
// In order for the block producer to be able to use and make additions
@@ -712,36 +711,14 @@ func (s *StateDB) Copy() *StateDB {
712711
snaps: s.snaps,
713712
snap: s.snap,
714713
}
715-
// Copy the dirty states, logs, and preimages
716-
for addr := range s.journal.dirties {
717-
// As documented [here](https://github.com/ethereum/go-ethereum/pull/16485#issuecomment-380438527),
718-
// and in the Finalise-method, there is a case where an object is in the journal but not
719-
// in the stateObjects: OOG after touch on ripeMD prior to Byzantium. Thus, we need to check for
720-
// nil
721-
if object, exist := s.stateObjects[addr]; exist {
722-
// Even though the original object is dirty, we are not copying the journal,
723-
// so we need to make sure that any side-effect the journal would have caused
724-
// during a commit (or similar op) is already applied to the copy.
725-
state.stateObjects[addr] = object.deepCopy(state)
726-
727-
state.stateObjectsDirty[addr] = struct{}{} // Mark the copy dirty to force internal (code/state) commits
728-
state.stateObjectsPending[addr] = struct{}{} // Mark the copy pending to force external (account) commits
729-
}
714+
// Deep copy cached state objects along with the pending and dirty markers.
715+
for addr, obj := range s.stateObjects {
716+
state.stateObjects[addr] = obj.deepCopy()
730717
}
731-
// Above, we don't copy the actual journal. This means that if the copy
732-
// is copied, the loop above will be a no-op, since the copy's journal
733-
// is empty. Thus, here we iterate over stateObjects, to enable copies
734-
// of copies.
735718
for addr := range s.stateObjectsPending {
736-
if _, exist := state.stateObjects[addr]; !exist {
737-
state.stateObjects[addr] = s.stateObjects[addr].deepCopy(state)
738-
}
739719
state.stateObjectsPending[addr] = struct{}{}
740720
}
741721
for addr := range s.stateObjectsDirty {
742-
if _, exist := state.stateObjects[addr]; !exist {
743-
state.stateObjects[addr] = s.stateObjects[addr].deepCopy(state)
744-
}
745722
state.stateObjectsDirty[addr] = struct{}{}
746723
}
747724
// Deep copy the destruction markers.

0 commit comments

Comments
 (0)