diff --git a/triedb/pathdb/buffer.go b/triedb/pathdb/buffer.go index c7a3c041ac..68e136f193 100644 --- a/triedb/pathdb/buffer.go +++ b/triedb/pathdb/buffer.go @@ -79,10 +79,10 @@ func (b *buffer) commit(nodes *nodeSet, states *stateSet) *buffer { return b } -// revert is the reverse operation of commit. It also merges the provided states +// revertTo is the reverse operation of commit. It also merges the provided states // and trie nodes into the buffer. The key difference is that the provided state // set should reverse the changes made by the most recent state transition. -func (b *buffer) revert(db ethdb.KeyValueReader, nodes map[common.Hash]map[string]*trienode.Node, accounts map[common.Hash][]byte, storages map[common.Hash]map[common.Hash][]byte) error { +func (b *buffer) revertTo(db ethdb.KeyValueReader, nodes map[common.Hash]map[string]*trienode.Node, accounts map[common.Hash][]byte, storages map[common.Hash]map[common.Hash][]byte) error { // Short circuit if no embedded state transition to revert if b.layers == 0 { return errStateUnrecoverable @@ -94,8 +94,8 @@ func (b *buffer) revert(db ethdb.KeyValueReader, nodes map[common.Hash]map[strin b.reset() return nil } - b.nodes.revert(db, nodes) - b.states.revert(accounts, storages) + b.nodes.revertTo(db, nodes) + b.states.revertTo(accounts, storages) return nil } diff --git a/triedb/pathdb/difflayer.go b/triedb/pathdb/difflayer.go index 2c7407b6cc..c06026b6ca 100644 --- a/triedb/pathdb/difflayer.go +++ b/triedb/pathdb/difflayer.go @@ -126,7 +126,7 @@ func (dl *diffLayer) account(hash common.Hash, depth int) ([]byte, error) { // storage directly retrieves the storage data associated with a particular hash, // within a particular account. // -// Note the returned account is not a copy, please don't modify it. +// Note the returned storage slot is not a copy, please don't modify it. func (dl *diffLayer) storage(accountHash, storageHash common.Hash, depth int) ([]byte, error) { // Hold the lock, ensure the parent won't be changed during the // state accessing. diff --git a/triedb/pathdb/disklayer.go b/triedb/pathdb/disklayer.go index 206d85d256..003431b19b 100644 --- a/triedb/pathdb/disklayer.go +++ b/triedb/pathdb/disklayer.go @@ -332,7 +332,7 @@ func (dl *diskLayer) revert(h *history) (*diskLayer, error) { // needs to be reverted is not yet flushed and cached in node // buffer, otherwise, manipulate persistent state directly. if !dl.buffer.empty() { - err := dl.buffer.revert(dl.db.diskdb, nodes, accounts, storages) + err := dl.buffer.revertTo(dl.db.diskdb, nodes, accounts, storages) if err != nil { return nil, err } diff --git a/triedb/pathdb/nodes.go b/triedb/pathdb/nodes.go index ade669512e..dee8c872ac 100644 --- a/triedb/pathdb/nodes.go +++ b/triedb/pathdb/nodes.go @@ -131,9 +131,9 @@ func (s *nodeSet) merge(set *nodeSet) { s.updateSize(delta) } -// revert merges the provided trie nodes into the set. This should reverse the +// revertTo merges the provided trie nodes into the set. This should reverse the // changes made by the most recent state transition. -func (s *nodeSet) revert(db ethdb.KeyValueReader, nodes map[common.Hash]map[string]*trienode.Node) { +func (s *nodeSet) revertTo(db ethdb.KeyValueReader, nodes map[common.Hash]map[string]*trienode.Node) { var delta int64 for owner, subset := range nodes { current, ok := s.nodes[owner] diff --git a/triedb/pathdb/states.go b/triedb/pathdb/states.go index a33fe9eaa1..a569aad5b6 100644 --- a/triedb/pathdb/states.go +++ b/triedb/pathdb/states.go @@ -253,13 +253,13 @@ func (s *stateSet) merge(other *stateSet) { s.updateSize(delta) } -// revert takes the original value of accounts and storages as input and reverts +// revertTo takes the original value of accounts and storages as input and reverts // the latest state transition applied on the state set. // // Notably, this operation may result in the set containing more entries after a // revert. For example, if account x did not exist and was created during transition // w, reverting w will retain an x=nil entry in the set. -func (s *stateSet) revert(accountOrigin map[common.Hash][]byte, storageOrigin map[common.Hash]map[common.Hash][]byte) { +func (s *stateSet) revertTo(accountOrigin map[common.Hash][]byte, storageOrigin map[common.Hash]map[common.Hash][]byte) { var delta int // size tracking for addrHash, blob := range accountOrigin { data, ok := s.accountData[addrHash] diff --git a/triedb/pathdb/states_test.go b/triedb/pathdb/states_test.go index 1ac63325c0..4557fa958d 100644 --- a/triedb/pathdb/states_test.go +++ b/triedb/pathdb/states_test.go @@ -154,7 +154,7 @@ func TestStatesRevert(t *testing.T) { }, ) a.merge(b) - a.revert( + a.revertTo( map[common.Hash][]byte{ common.Hash{0xa}: {0xa0}, common.Hash{0xb}: {0xb0}, @@ -220,6 +220,69 @@ func TestStatesRevert(t *testing.T) { } } +// TestStateRevertAccountNullMarker tests the scenario that account x did not exist +// before and was created during transition w, reverting w will retain an x=nil +// entry in the set. +func TestStateRevertAccountNullMarker(t *testing.T) { + a := newStates(nil, nil) // empty initial state + b := newStates( + map[common.Hash][]byte{ + common.Hash{0xa}: {0xa}, + }, + nil, + ) + a.merge(b) // create account 0xa + a.revertTo( + map[common.Hash][]byte{ + common.Hash{0xa}: nil, + }, + nil, + ) // revert the transition b + + blob, exist := a.account(common.Hash{0xa}) + if !exist { + t.Fatal("null marker is not found") + } + if len(blob) != 0 { + t.Fatalf("Unexpected value for account, %v", blob) + } +} + +// TestStateRevertStorageNullMarker tests the scenario that slot x did not exist +// before and was created during transition w, reverting w will retain an x=nil +// entry in the set. +func TestStateRevertStorageNullMarker(t *testing.T) { + a := newStates(map[common.Hash][]byte{ + common.Hash{0xa}: {0xa}, + }, nil) // initial state with account 0xa + + b := newStates( + nil, + map[common.Hash]map[common.Hash][]byte{ + common.Hash{0xa}: { + common.Hash{0x1}: {0x1}, + }, + }, + ) + a.merge(b) // create slot 0x1 + a.revertTo( + nil, + map[common.Hash]map[common.Hash][]byte{ + common.Hash{0xa}: { + common.Hash{0x1}: nil, + }, + }, + ) // revert the transition b + + blob, exist := a.storage(common.Hash{0xa}, common.Hash{0x1}) + if !exist { + t.Fatal("null marker is not found") + } + if len(blob) != 0 { + t.Fatalf("Unexpected value for storage slot, %v", blob) + } +} + func TestStatesEncode(t *testing.T) { s := newStates( map[common.Hash][]byte{ @@ -359,7 +422,7 @@ func TestStateSizeTracking(t *testing.T) { } // Revert the set to original status - a.revert( + a.revertTo( map[common.Hash][]byte{ common.Hash{0xa}: {0xa0}, common.Hash{0xb}: {0xb0},