Skip to content

Commit 6280e59

Browse files
austinabellaustinabell
authored
State Trie clearing journal edge case fix (#14)
* Fixed dirty object journal edge case * Removed duplicate logic * Fixed mutex locks for tests and fixed variable for dirty objects * Removed redundant code * Readded bug fix and added related test
1 parent c248a8a commit 6280e59

9 files changed

Lines changed: 203 additions & 102 deletions

File tree

‎core/state/dump.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"sync"
2727

2828
"fmt"
29+
2930
"github.com/eth-classic/go-ethereum/common"
3031
"github.com/eth-classic/go-ethereum/rlp"
3132
"github.com/eth-classic/go-ethereum/trie"
@@ -80,7 +81,7 @@ func (self *StateDB) RawDump(addresses []common.Address) Dump {
8081
panic(err)
8182
}
8283

83-
obj := newObject(nil, addrA, data, nil)
84+
obj := newObject(nil, addrA, data)
8485
account := DumpAccount{
8586
Balance: data.Balance.String(),
8687
Nonce: data.Nonce,
@@ -178,7 +179,7 @@ func iterator(sdb *StateDB, addresses []common.Address, c chan *AddressedRawAcco
178179
panic(err)
179180
}
180181

181-
obj := newObject(nil, addrA, data, nil)
182+
obj := newObject(nil, addrA, data)
182183
account := AddressedRawAccount{
183184
DumpAccount: DumpAccount{
184185
Balance: data.Balance.String(),

‎core/state/journal.go‎

Lines changed: 113 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,67 @@ import (
2222
"github.com/eth-classic/go-ethereum/common"
2323
)
2424

25+
// journalEntry is a modification entry in the state change journal that can be
26+
// reverted on demand.
2527
type journalEntry interface {
26-
undo(*StateDB)
28+
// revert undoes the changes introduced by this journal entry.
29+
revert(*StateDB)
30+
31+
// dirtied returns the Ethereum address modified by this journal entry.
32+
dirtied() *common.Address
33+
}
34+
35+
// journal contains the list of state modifications applied since the last state
36+
// commit. These are tracked to be able to be reverted in case of an execution
37+
// exception or revertal request.
38+
type journal struct {
39+
entries []journalEntry // Current changes tracked by the journal
40+
dirties map[common.Address]int // Dirty accounts and the number of changes
41+
}
42+
43+
// newJournal create a new initialized journal.
44+
func newJournal() *journal {
45+
return &journal{
46+
dirties: make(map[common.Address]int),
47+
}
48+
}
49+
50+
// append inserts a new modification entry to the end of the change journal.
51+
func (j *journal) append(entry journalEntry) {
52+
j.entries = append(j.entries, entry)
53+
if addr := entry.dirtied(); addr != nil {
54+
j.dirties[*addr]++
55+
}
56+
}
57+
58+
// revert undoes a batch of journalled modifications along with any reverted
59+
// dirty handling too.
60+
func (j *journal) revert(statedb *StateDB, snapshot int) {
61+
for i := len(j.entries) - 1; i >= snapshot; i-- {
62+
// Undo the changes made by the operation
63+
j.entries[i].revert(statedb)
64+
65+
// Drop any dirty tracking induced by the change
66+
if addr := j.entries[i].dirtied(); addr != nil {
67+
if j.dirties[*addr]--; j.dirties[*addr] == 0 {
68+
delete(j.dirties, *addr)
69+
}
70+
}
71+
}
72+
j.entries = j.entries[:snapshot]
73+
}
74+
75+
// dirty explicitly sets an address to dirty, even if the change entries would
76+
// otherwise suggest it as clean. This method is an ugly hack to handle the RIPEMD
77+
// precompile consensus exception.
78+
func (j *journal) dirty(addr common.Address) {
79+
j.dirties[addr]++
2780
}
2881

29-
type journal []journalEntry
82+
// length returns the current number of entries in the journal.
83+
func (j *journal) length() int {
84+
return len(j.entries)
85+
}
3086

3187
type (
3288
// Changes to the account trie.
@@ -77,55 +133,85 @@ type (
77133
}
78134
)
79135

80-
func (ch createObjectChange) undo(s *StateDB) {
136+
func (ch createObjectChange) revert(s *StateDB) {
81137
delete(s.stateObjects, *ch.account)
82138
delete(s.stateObjectsDirty, *ch.account)
83139
}
84140

85-
func (ch resetObjectChange) undo(s *StateDB) {
141+
func (ch createObjectChange) dirtied() *common.Address {
142+
return ch.account
143+
}
144+
145+
func (ch resetObjectChange) revert(s *StateDB) {
86146
s.setStateObject(ch.prev)
87147
}
88148

89-
func (ch suicideChange) undo(s *StateDB) {
149+
func (ch resetObjectChange) dirtied() *common.Address {
150+
return nil
151+
}
152+
153+
func (ch suicideChange) revert(s *StateDB) {
90154
obj := s.getStateObject(*ch.account)
91155
if obj != nil {
92156
obj.suicided = ch.prev
93157
obj.setBalance(ch.prevbalance)
94158
}
95159
}
96160

161+
func (ch suicideChange) dirtied() *common.Address {
162+
return ch.account
163+
}
164+
97165
var ripemd = common.HexToAddress("0000000000000000000000000000000000000003")
98166

99-
func (ch touchChange) undo(s *StateDB) {
100-
if !ch.prev && *ch.account != ripemd {
101-
s.getStateObject(*ch.account).touched = ch.prev
102-
if !ch.prevDirty {
103-
delete(s.stateObjectsDirty, *ch.account)
104-
}
105-
}
167+
func (ch touchChange) revert(s *StateDB) {
168+
}
169+
170+
func (ch touchChange) dirtied() *common.Address {
171+
return ch.account
106172
}
107173

108-
func (ch balanceChange) undo(s *StateDB) {
174+
func (ch balanceChange) revert(s *StateDB) {
109175
s.getStateObject(*ch.account).setBalance(ch.prev)
110176
}
111177

112-
func (ch nonceChange) undo(s *StateDB) {
178+
func (ch balanceChange) dirtied() *common.Address {
179+
return ch.account
180+
}
181+
182+
func (ch nonceChange) revert(s *StateDB) {
113183
s.getStateObject(*ch.account).setNonce(ch.prev)
114184
}
115185

116-
func (ch codeChange) undo(s *StateDB) {
186+
func (ch nonceChange) dirtied() *common.Address {
187+
return ch.account
188+
}
189+
190+
func (ch codeChange) revert(s *StateDB) {
117191
s.getStateObject(*ch.account).setCode(common.BytesToHash(ch.prevhash), ch.prevcode)
118192
}
119193

120-
func (ch storageChange) undo(s *StateDB) {
194+
func (ch codeChange) dirtied() *common.Address {
195+
return ch.account
196+
}
197+
198+
func (ch storageChange) revert(s *StateDB) {
121199
s.getStateObject(*ch.account).setState(ch.key, ch.prevalue)
122200
}
123201

124-
func (ch refundChange) undo(s *StateDB) {
202+
func (ch storageChange) dirtied() *common.Address {
203+
return ch.account
204+
}
205+
206+
func (ch refundChange) revert(s *StateDB) {
125207
s.refund = ch.prev
126208
}
127209

128-
func (ch addLogChange) undo(s *StateDB) {
210+
func (ch refundChange) dirtied() *common.Address {
211+
return nil
212+
}
213+
214+
func (ch addLogChange) revert(s *StateDB) {
129215
logs := s.logs[ch.txhash]
130216
if len(logs) == 1 {
131217
delete(s.logs, ch.txhash)
@@ -135,6 +221,14 @@ func (ch addLogChange) undo(s *StateDB) {
135221
s.logSize--
136222
}
137223

138-
func (ch addPreimageChange) undo(s *StateDB) {
224+
func (ch addLogChange) dirtied() *common.Address {
225+
return nil
226+
}
227+
228+
func (ch addPreimageChange) revert(s *StateDB) {
139229
delete(s.preimages, ch.hash)
140230
}
231+
232+
func (ch addPreimageChange) dirtied() *common.Address {
233+
return nil
234+
}

‎core/state/managed_state.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,8 @@ func (ms *ManagedState) NewNonce(addr common.Address) uint64 {
8484

8585
// GetNonce returns the canonical nonce for the managed or unmanaged account
8686
func (ms *ManagedState) GetNonce(addr common.Address) uint64 {
87-
ms.mu.RLock()
88-
defer ms.mu.RUnlock()
87+
ms.mu.Lock()
88+
defer ms.mu.Unlock()
8989

9090
if ms.hasAccount(addr) {
9191
account := ms.getAccount(addr)

‎core/state/state_object.go‎

Lines changed: 13 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,7 @@ type StateObject struct {
8888
// during the "update" phase of the state transition.
8989
dirtyCode bool // true if the code was updated
9090
suicided bool
91-
touched bool
9291
deleted bool
93-
onDirty func(addr common.Address) // Callback method to mark a state object newly dirty
9492
}
9593

9694
// empty returns whether the account is considered empty.
@@ -108,7 +106,7 @@ type Account struct {
108106
}
109107

110108
// newObject creates a state object.
111-
func newObject(db *StateDB, address common.Address, data Account, onDirty func(addr common.Address)) *StateObject {
109+
func newObject(db *StateDB, address common.Address, data Account) *StateObject {
112110
if data.Balance == nil {
113111
data.Balance = new(big.Int)
114112
}
@@ -122,7 +120,6 @@ func newObject(db *StateDB, address common.Address, data Account, onDirty func(a
122120
data: data,
123121
cachedStorage: make(Storage),
124122
dirtyStorage: make(Storage),
125-
onDirty: onDirty,
126123
}
127124
}
128125

@@ -140,23 +137,17 @@ func (self *StateObject) setError(err error) {
140137

141138
func (self *StateObject) markSuicided() {
142139
self.suicided = true
143-
if self.onDirty != nil {
144-
self.onDirty(self.Address())
145-
self.onDirty = nil
146-
}
147140
}
148141

149142
func (c *StateObject) touch() {
150-
c.db.journal = append(c.db.journal, touchChange{
151-
account: &c.address,
152-
prev: c.touched,
153-
prevDirty: c.onDirty == nil,
143+
c.db.journal.append(touchChange{
144+
account: &c.address,
154145
})
155-
if c.onDirty != nil {
156-
c.onDirty(c.Address())
157-
c.onDirty = nil
146+
if c.address == ripemd {
147+
// Explicitly put it in the dirty-cache, which is otherwise generated from
148+
// flattened journals.
149+
c.db.journal.dirty(c.address)
158150
}
159-
c.touched = true
160151
}
161152

162153
func (c *StateObject) getTrie(db Database) Trie {
@@ -198,7 +189,7 @@ func (self *StateObject) GetState(db Database, key common.Hash) common.Hash {
198189

199190
// SetState updates a value in account storage.
200191
func (self *StateObject) SetState(db Database, key, value common.Hash) {
201-
self.db.journal = append(self.db.journal, storageChange{
192+
self.db.journal.append(storageChange{
202193
account: &self.address,
203194
key: key,
204195
prevalue: self.GetState(db, key),
@@ -209,11 +200,6 @@ func (self *StateObject) SetState(db Database, key, value common.Hash) {
209200
func (self *StateObject) setState(key, value common.Hash) {
210201
self.cachedStorage[key] = value
211202
self.dirtyStorage[key] = value
212-
213-
if self.onDirty != nil {
214-
self.onDirty(self.Address())
215-
self.onDirty = nil
216-
}
217203
}
218204

219205
// updateTrie writes cached storage modifications into the object's storage trie.
@@ -299,7 +285,7 @@ func (c *StateObject) SubBalance(amount *big.Int) {
299285
}
300286

301287
func (self *StateObject) SetBalance(amount *big.Int) {
302-
self.db.journal = append(self.db.journal, balanceChange{
288+
self.db.journal.append(balanceChange{
303289
account: &self.address,
304290
prev: new(big.Int).Set(self.data.Balance),
305291
})
@@ -308,17 +294,13 @@ func (self *StateObject) SetBalance(amount *big.Int) {
308294

309295
func (self *StateObject) setBalance(amount *big.Int) {
310296
self.data.Balance = amount
311-
if self.onDirty != nil {
312-
self.onDirty(self.Address())
313-
self.onDirty = nil
314-
}
315297
}
316298

317299
// Return the gas back to the origin. Used by the Virtual machine or Closures
318300
func (c *StateObject) ReturnGas(*big.Int, *big.Int) {}
319301

320-
func (self *StateObject) deepCopy(db *StateDB, onDirty func(addr common.Address)) *StateObject {
321-
stateObject := newObject(db, self.address, self.data, onDirty)
302+
func (self *StateObject) deepCopy(db *StateDB) *StateObject {
303+
stateObject := newObject(db, self.address, self.data)
322304
if self.trie != nil {
323305
stateObject.trie = db.db.CopyTrie(self.trie)
324306
}
@@ -358,7 +340,7 @@ func (self *StateObject) Code(db Database) []byte {
358340

359341
func (self *StateObject) SetCode(codeHash common.Hash, code []byte) {
360342
prevcode := self.Code(self.db.db)
361-
self.db.journal = append(self.db.journal, codeChange{
343+
self.db.journal.append(codeChange{
362344
account: &self.address,
363345
prevhash: self.CodeHash(),
364346
prevcode: prevcode,
@@ -370,14 +352,10 @@ func (self *StateObject) setCode(codeHash common.Hash, code []byte) {
370352
self.code = code
371353
self.data.CodeHash = codeHash[:]
372354
self.dirtyCode = true
373-
if self.onDirty != nil {
374-
self.onDirty(self.Address())
375-
self.onDirty = nil
376-
}
377355
}
378356

379357
func (self *StateObject) SetNonce(nonce uint64) {
380-
self.db.journal = append(self.db.journal, nonceChange{
358+
self.db.journal.append(nonceChange{
381359
account: &self.address,
382360
prev: self.data.Nonce,
383361
})
@@ -386,10 +364,6 @@ func (self *StateObject) SetNonce(nonce uint64) {
386364

387365
func (self *StateObject) setNonce(nonce uint64) {
388366
self.data.Nonce = nonce
389-
if self.onDirty != nil {
390-
self.onDirty(self.Address())
391-
self.onDirty = nil
392-
}
393367
}
394368

395369
func (self *StateObject) CodeHash() []byte {

0 commit comments

Comments
 (0)