From a4cf279494f53276cf3576ae89b14b58efe644fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Tue, 3 Mar 2020 15:52:00 +0200 Subject: [PATCH] core/state: extend snapshotter to handle account resurrections --- core/blockchain.go | 1 + core/blockchain_test.go | 10 +- core/state/snapshot/difflayer.go | 142 +++++++++++++++-------- core/state/snapshot/difflayer_test.go | 157 ++++++++++++++++---------- core/state/snapshot/disklayer.go | 4 +- core/state/snapshot/disklayer_test.go | 30 ++--- core/state/snapshot/iterator_test.go | 101 +++++++++-------- core/state/snapshot/journal.go | 22 +++- core/state/snapshot/snapshot.go | 63 ++++++----- core/state/snapshot/snapshot_test.go | 28 ++--- core/state/statedb.go | 45 ++++---- core/vm/opcodes.go | 13 +-- 12 files changed, 365 insertions(+), 251 deletions(-) diff --git a/core/blockchain.go b/core/blockchain.go index b0309ef70..de0d4f399 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -198,6 +198,7 @@ func NewBlockChain(db ethdb.Database, cacheConfig *CacheConfig, chainConfig *par TrieDirtyLimit: 256, TrieTimeLimit: 5 * time.Minute, SnapshotLimit: 256, + SnapshotWait: true, } } bodyCache, _ := lru.New(bodyCacheLimit) diff --git a/core/blockchain_test.go b/core/blockchain_test.go index 69a245322..5e2a21023 100644 --- a/core/blockchain_test.go +++ b/core/blockchain_test.go @@ -2315,7 +2315,7 @@ func TestDeleteCreateRevert(t *testing.T) { // The address 0xAAAAA selfdestructs if called aa: { // Code needs to just selfdestruct - Code: []byte{byte(vm.PC), 0xFF}, + Code: []byte{byte(vm.PC), byte(vm.SELFDESTRUCT)}, Nonce: 1, Balance: big.NewInt(0), }, @@ -2382,8 +2382,8 @@ func TestDeleteRecreateSlots(t *testing.T) { aa = common.HexToAddress("0x7217d81b76bdd8707601e959454e3d776aee5f43") bb = common.HexToAddress("0x000000000000000000000000000000000000bbbb") - aaStorage = make(map[common.Hash]common.Hash) // Initial storage in AA - aaCode = []byte{byte(vm.PC), 0xFF} // Code for AA (simple selfdestruct) + aaStorage = make(map[common.Hash]common.Hash) // Initial storage in AA + aaCode = []byte{byte(vm.PC), byte(vm.SELFDESTRUCT)} // Code for AA (simple selfdestruct) ) // Populate two slots aaStorage[common.HexToHash("01")] = common.HexToHash("01") @@ -2506,8 +2506,8 @@ func TestDeleteRecreateAccount(t *testing.T) { funds = big.NewInt(1000000000) aa = common.HexToAddress("0x7217d81b76bdd8707601e959454e3d776aee5f43") - aaStorage = make(map[common.Hash]common.Hash) // Initial storage in AA - aaCode = []byte{byte(vm.PC), 0xFF} // Code for AA (simple selfdestruct) + aaStorage = make(map[common.Hash]common.Hash) // Initial storage in AA + aaCode = []byte{byte(vm.PC), byte(vm.SELFDESTRUCT)} // Code for AA (simple selfdestruct) ) // Populate two slots aaStorage[common.HexToHash("01")] = common.HexToHash("01") diff --git a/core/state/snapshot/difflayer.go b/core/state/snapshot/difflayer.go index 3c1bea421..0915fb6bc 100644 --- a/core/state/snapshot/difflayer.go +++ b/core/state/snapshot/difflayer.go @@ -27,7 +27,6 @@ import ( "time" "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/rlp" "github.com/steakknife/bloomfilter" ) @@ -68,17 +67,28 @@ var ( // entry count). bloomFuncs = math.Round((bloomSize / float64(aggregatorItemLimit)) * math.Log(2)) - // bloomHashesOffset is a runtime constant which determines which part of the + // the bloom offsets are runtime constants which determines which part of the // the account/storage hash the hasher functions looks at, to determine the // bloom key for an account/slot. This is randomized at init(), so that the // global population of nodes do not all display the exact same behaviour with // regards to bloom content - bloomHasherOffset = 0 + bloomDestructHasherOffset = 0 + bloomAccountHasherOffset = 0 + bloomStorageHasherOffset = 0 ) func init() { - // Init bloomHasherOffset in the range [0:24] (requires 8 bytes) - bloomHasherOffset = rand.Intn(25) + // Init the bloom offsets in the range [0:24] (requires 8 bytes) + bloomDestructHasherOffset = rand.Intn(25) + bloomAccountHasherOffset = rand.Intn(25) + bloomStorageHasherOffset = rand.Intn(25) + + // The destruct and account blooms must be different, as the storage slots + // will check for destruction too for every bloom miss. It should not collide + // with modified accounts. + for bloomAccountHasherOffset == bloomDestructHasherOffset { + bloomAccountHasherOffset = rand.Intn(25) + } } // diffLayer represents a collection of modifications made to a state snapshot @@ -95,6 +105,7 @@ type diffLayer struct { root common.Hash // Root hash to which this snapshot diff belongs to stale uint32 // Signals that the layer became stale (state progressed) + destructSet map[common.Hash]struct{} // Keyed markers for deleted (and potentially) recreated accounts accountList []common.Hash // List of account for iteration. If it exists, it's sorted, otherwise it's nil accountData map[common.Hash][]byte // Keyed accounts for direct retrival (nil means deleted) storageList map[common.Hash][]common.Hash // List of storage slots for iterated retrievals, one per account. Any existing lists are sorted if non-nil @@ -105,6 +116,20 @@ type diffLayer struct { lock sync.RWMutex } +// destructBloomHasher is a wrapper around a common.Hash to satisfy the interface +// API requirements of the bloom library used. It's used to convert a destruct +// event into a 64 bit mini hash. +type destructBloomHasher common.Hash + +func (h destructBloomHasher) Write(p []byte) (n int, err error) { panic("not implemented") } +func (h destructBloomHasher) Sum(b []byte) []byte { panic("not implemented") } +func (h destructBloomHasher) Reset() { panic("not implemented") } +func (h destructBloomHasher) BlockSize() int { panic("not implemented") } +func (h destructBloomHasher) Size() int { return 8 } +func (h destructBloomHasher) Sum64() uint64 { + return binary.BigEndian.Uint64(h[bloomDestructHasherOffset : bloomDestructHasherOffset+8]) +} + // accountBloomHasher is a wrapper around a common.Hash to satisfy the interface // API requirements of the bloom library used. It's used to convert an account // hash into a 64 bit mini hash. @@ -116,7 +141,7 @@ func (h accountBloomHasher) Reset() { panic("not impl func (h accountBloomHasher) BlockSize() int { panic("not implemented") } func (h accountBloomHasher) Size() int { return 8 } func (h accountBloomHasher) Sum64() uint64 { - return binary.BigEndian.Uint64(h[bloomHasherOffset : bloomHasherOffset+8]) + return binary.BigEndian.Uint64(h[bloomAccountHasherOffset : bloomAccountHasherOffset+8]) } // storageBloomHasher is a wrapper around a [2]common.Hash to satisfy the interface @@ -130,17 +155,18 @@ func (h storageBloomHasher) Reset() { panic("not impl func (h storageBloomHasher) BlockSize() int { panic("not implemented") } func (h storageBloomHasher) Size() int { return 8 } func (h storageBloomHasher) Sum64() uint64 { - return binary.BigEndian.Uint64(h[0][bloomHasherOffset:bloomHasherOffset+8]) ^ - binary.BigEndian.Uint64(h[1][bloomHasherOffset:bloomHasherOffset+8]) + return binary.BigEndian.Uint64(h[0][bloomStorageHasherOffset:bloomStorageHasherOffset+8]) ^ + binary.BigEndian.Uint64(h[1][bloomStorageHasherOffset:bloomStorageHasherOffset+8]) } // newDiffLayer creates a new diff on top of an existing snapshot, whether that's a low // level persistent database or a hierarchical diff already. -func newDiffLayer(parent snapshot, root common.Hash, accounts map[common.Hash][]byte, storage map[common.Hash]map[common.Hash][]byte) *diffLayer { +func newDiffLayer(parent snapshot, root common.Hash, destructs map[common.Hash]struct{}, accounts map[common.Hash][]byte, storage map[common.Hash]map[common.Hash][]byte) *diffLayer { // Create the new layer with some pre-allocated data segments dl := &diffLayer{ parent: parent, root: root, + destructSet: destructs, accountData: accounts, storageData: storage, } @@ -152,6 +178,17 @@ func newDiffLayer(parent snapshot, root common.Hash, accounts map[common.Hash][] default: panic("unknown parent type") } + // Sanity check that accounts or storage slots are never nil + for accountHash, blob := range accounts { + if blob == nil { + panic(fmt.Sprintf("account %#x nil", accountHash)) + } + } + for accountHash, slots := range storage { + if slots == nil { + panic(fmt.Sprintf("storage %#x nil", accountHash)) + } + } // Determine memory size and track the dirty writes for _, data := range accounts { dl.memory += uint64(common.HashLength + len(data)) @@ -159,24 +196,11 @@ func newDiffLayer(parent snapshot, root common.Hash, accounts map[common.Hash][] } // Fill the storage hashes and sort them for the iterator dl.storageList = make(map[common.Hash][]common.Hash) - - for accountHash, slots := range storage { - // If the slots are nil, sanity check that it's a deleted account - if slots == nil { - // Ensure that the account was just marked as deleted - if account, ok := accounts[accountHash]; account != nil || !ok { - panic(fmt.Sprintf("storage in %#x nil, but account conflicts (%#x, exists: %v)", accountHash, account, ok)) - } - // Everything ok, store the deletion mark and continue - dl.storageList[accountHash] = nil - continue - } - // Storage slots are not nil so entire contract was not deleted, ensure the - // account was just updated. - if account, ok := accounts[accountHash]; account == nil || !ok { - log.Error(fmt.Sprintf("storage in %#x exists, but account nil (exists: %v)", accountHash, ok)) - } - // Determine memory size and track the dirty writes + for accountHash := range destructs { + dl.storageList[accountHash] = nil + } + // Determine memory size and track the dirty writes + for _, slots := range storage { for _, data := range slots { dl.memory += uint64(common.HashLength + len(data)) snapshotDirtyStorageWriteMeter.Mark(int64(len(data))) @@ -208,6 +232,9 @@ func (dl *diffLayer) rebloom(origin *diskLayer) { dl.diffed, _ = bloomfilter.New(uint64(bloomSize), uint64(bloomFuncs)) } // Iterate over all the accounts and storage slots and index them + for hash := range dl.destructSet { + dl.diffed.Add(destructBloomHasher(hash)) + } for hash := range dl.accountData { dl.diffed.Add(accountBloomHasher(hash)) } @@ -265,6 +292,9 @@ func (dl *diffLayer) AccountRLP(hash common.Hash) ([]byte, error) { // all the maps in all the layers below dl.lock.RLock() hit := dl.diffed.Contains(accountBloomHasher(hash)) + if !hit { + hit = dl.diffed.Contains(destructBloomHasher(hash)) + } dl.lock.RUnlock() // If the bloom filter misses, don't even bother with traversing the memory @@ -289,19 +319,22 @@ func (dl *diffLayer) accountRLP(hash common.Hash, depth int) ([]byte, error) { if dl.Stale() { return nil, ErrSnapshotStale } - // If the account is known locally, return it. Note, a nil account means it was - // deleted, and is a different notion than an unknown account! + // If the account is known locally, return it if data, ok := dl.accountData[hash]; ok { snapshotDirtyAccountHitMeter.Mark(1) snapshotDirtyAccountHitDepthHist.Update(int64(depth)) - if n := len(data); n > 0 { - snapshotDirtyAccountReadMeter.Mark(int64(n)) - } else { - snapshotDirtyAccountInexMeter.Mark(1) - } + snapshotDirtyAccountReadMeter.Mark(int64(len(data))) snapshotBloomAccountTrueHitMeter.Mark(1) return data, nil } + // If the account is known locally, but deleted, return it + if _, ok := dl.destructSet[hash]; ok { + snapshotDirtyAccountHitMeter.Mark(1) + snapshotDirtyAccountHitDepthHist.Update(int64(depth)) + snapshotDirtyAccountInexMeter.Mark(1) + snapshotBloomAccountTrueHitMeter.Mark(1) + return nil, nil + } // Account unknown to this diff, resolve from parent if diff, ok := dl.parent.(*diffLayer); ok { return diff.accountRLP(hash, depth+1) @@ -319,6 +352,9 @@ func (dl *diffLayer) Storage(accountHash, storageHash common.Hash) ([]byte, erro // all the maps in all the layers below dl.lock.RLock() hit := dl.diffed.Contains(storageBloomHasher{accountHash, storageHash}) + if !hit { + hit = dl.diffed.Contains(destructBloomHasher(accountHash)) + } dl.lock.RUnlock() // If the bloom filter misses, don't even bother with traversing the memory @@ -343,16 +379,8 @@ func (dl *diffLayer) storage(accountHash, storageHash common.Hash, depth int) ([ if dl.Stale() { return nil, ErrSnapshotStale } - // If the account is known locally, try to resolve the slot locally. Note, a nil - // account means it was deleted, and is a different notion than an unknown account! + // If the account is known locally, try to resolve the slot locally if storage, ok := dl.storageData[accountHash]; ok { - if storage == nil { - snapshotDirtyStorageHitMeter.Mark(1) - snapshotDirtyStorageHitDepthHist.Update(int64(depth)) - snapshotDirtyStorageInexMeter.Mark(1) - snapshotBloomStorageTrueHitMeter.Mark(1) - return nil, nil - } if data, ok := storage[storageHash]; ok { snapshotDirtyStorageHitMeter.Mark(1) snapshotDirtyStorageHitDepthHist.Update(int64(depth)) @@ -365,6 +393,14 @@ func (dl *diffLayer) storage(accountHash, storageHash common.Hash, depth int) ([ return data, nil } } + // If the account is known locally, but deleted, return an empty slot + if _, ok := dl.destructSet[accountHash]; ok { + snapshotDirtyStorageHitMeter.Mark(1) + snapshotDirtyStorageHitDepthHist.Update(int64(depth)) + snapshotDirtyStorageInexMeter.Mark(1) + snapshotBloomStorageTrueHitMeter.Mark(1) + return nil, nil + } // Storage slot unknown to this diff, resolve from parent if diff, ok := dl.parent.(*diffLayer); ok { return diff.storage(accountHash, storageHash, depth+1) @@ -376,8 +412,8 @@ func (dl *diffLayer) storage(accountHash, storageHash common.Hash, depth int) ([ // Update creates a new layer on top of the existing snapshot diff tree with // the specified data items. -func (dl *diffLayer) Update(blockRoot common.Hash, accounts map[common.Hash][]byte, storage map[common.Hash]map[common.Hash][]byte) *diffLayer { - return newDiffLayer(dl, blockRoot, accounts, storage) +func (dl *diffLayer) Update(blockRoot common.Hash, destructs map[common.Hash]struct{}, accounts map[common.Hash][]byte, storage map[common.Hash]map[common.Hash][]byte) *diffLayer { + return newDiffLayer(dl, blockRoot, destructs, accounts, storage) } // flatten pushes all data from this point downwards, flattening everything into @@ -403,14 +439,18 @@ func (dl *diffLayer) flatten() snapshot { panic("parent diff layer is stale") // we've flattened into the same parent from two children, boo } // Overwrite all the updated accounts blindly, merge the sorted list + for hash := range dl.destructSet { + parent.destructSet[hash] = struct{}{} + delete(parent.accountData, hash) + delete(parent.storageData, hash) + } for hash, data := range dl.accountData { parent.accountData[hash] = data } // Overwrite all the updated storage slots (individually) for accountHash, storage := range dl.storageData { - // If storage didn't exist (or was deleted) in the parent; or if the storage - // was freshly deleted in the child, overwrite blindly - if parent.storageData[accountHash] == nil || storage == nil { + // If storage didn't exist (or was deleted) in the parent, overwrite blindly + if _, ok := parent.storageData[accountHash]; !ok { parent.storageData[accountHash] = storage continue } @@ -426,6 +466,7 @@ func (dl *diffLayer) flatten() snapshot { parent: parent.parent, origin: parent.origin, root: dl.root, + destructSet: parent.destructSet, accountData: parent.accountData, storageData: parent.storageData, storageList: make(map[common.Hash][]common.Hash), @@ -451,7 +492,10 @@ func (dl *diffLayer) AccountList() []common.Hash { dl.lock.Lock() defer dl.lock.Unlock() - dl.accountList = make([]common.Hash, 0, len(dl.accountData)) + dl.accountList = make([]common.Hash, 0, len(dl.destructSet)+len(dl.accountData)) + for hash := range dl.destructSet { + dl.accountList = append(dl.accountList, hash) + } for hash := range dl.accountData { dl.accountList = append(dl.accountList, hash) } diff --git a/core/state/snapshot/difflayer_test.go b/core/state/snapshot/difflayer_test.go index d8212d317..61d2ed9c0 100644 --- a/core/state/snapshot/difflayer_test.go +++ b/core/state/snapshot/difflayer_test.go @@ -30,8 +30,9 @@ import ( // TestMergeBasics tests some simple merges func TestMergeBasics(t *testing.T) { var ( - accounts = make(map[common.Hash][]byte) - storage = make(map[common.Hash]map[common.Hash][]byte) + destructs = make(map[common.Hash]struct{}) + accounts = make(map[common.Hash][]byte) + storage = make(map[common.Hash]map[common.Hash][]byte) ) // Fill up a parent for i := 0; i < 100; i++ { @@ -39,7 +40,10 @@ func TestMergeBasics(t *testing.T) { data := randomAccount() accounts[h] = data - if rand.Intn(20) < 10 { + if rand.Intn(4) == 0 { + destructs[h] = struct{}{} + } + if rand.Intn(2) == 0 { accStorage := make(map[common.Hash][]byte) value := make([]byte, 32) rand.Read(value) @@ -48,20 +52,18 @@ func TestMergeBasics(t *testing.T) { } } // Add some (identical) layers on top - parent := newDiffLayer(emptyLayer(), common.Hash{}, accounts, storage) - child := newDiffLayer(parent, common.Hash{}, accounts, storage) - child = newDiffLayer(child, common.Hash{}, accounts, storage) - child = newDiffLayer(child, common.Hash{}, accounts, storage) - child = newDiffLayer(child, common.Hash{}, accounts, storage) + parent := newDiffLayer(emptyLayer(), common.Hash{}, destructs, accounts, storage) + child := newDiffLayer(parent, common.Hash{}, destructs, accounts, storage) + child = newDiffLayer(child, common.Hash{}, destructs, accounts, storage) + child = newDiffLayer(child, common.Hash{}, destructs, accounts, storage) + child = newDiffLayer(child, common.Hash{}, destructs, accounts, storage) // And flatten merged := (child.flatten()).(*diffLayer) { // Check account lists - // Should be zero/nil first if got, exp := len(merged.accountList), 0; got != exp { t.Errorf("accountList wrong, got %v exp %v", got, exp) } - // Then set when we call AccountList if got, exp := len(merged.AccountList()), len(accounts); got != exp { t.Errorf("AccountList() wrong, got %v exp %v", got, exp) } @@ -69,6 +71,11 @@ func TestMergeBasics(t *testing.T) { t.Errorf("accountList [2] wrong, got %v exp %v", got, exp) } } + { // Check account drops + if got, exp := len(merged.destructSet), len(destructs); got != exp { + t.Errorf("accountDrop wrong, got %v exp %v", got, exp) + } + } { // Check storage lists i := 0 for aHash, sMap := range storage { @@ -95,42 +102,61 @@ func TestMergeDelete(t *testing.T) { h1 := common.HexToHash("0x01") h2 := common.HexToHash("0x02") - flip := func() map[common.Hash][]byte { - accs := make(map[common.Hash][]byte) - accs[h1] = randomAccount() - accs[h2] = nil - return accs + flipDrops := func() map[common.Hash]struct{} { + return map[common.Hash]struct{}{ + h2: struct{}{}, + } } - flop := func() map[common.Hash][]byte { - accs := make(map[common.Hash][]byte) - accs[h1] = nil - accs[h2] = randomAccount() - return accs + flipAccs := func() map[common.Hash][]byte { + return map[common.Hash][]byte{ + h1: randomAccount(), + } } - - // Add some flip-flopping layers on top - parent := newDiffLayer(emptyLayer(), common.Hash{}, flip(), storage) - child := parent.Update(common.Hash{}, flop(), storage) - child = child.Update(common.Hash{}, flip(), storage) - child = child.Update(common.Hash{}, flop(), storage) - child = child.Update(common.Hash{}, flip(), storage) - child = child.Update(common.Hash{}, flop(), storage) - child = child.Update(common.Hash{}, flip(), storage) + flopDrops := func() map[common.Hash]struct{} { + return map[common.Hash]struct{}{ + h1: struct{}{}, + } + } + flopAccs := func() map[common.Hash][]byte { + return map[common.Hash][]byte{ + h2: randomAccount(), + } + } + // Add some flipAccs-flopping layers on top + parent := newDiffLayer(emptyLayer(), common.Hash{}, flipDrops(), flipAccs(), storage) + child := parent.Update(common.Hash{}, flopDrops(), flopAccs(), storage) + child = child.Update(common.Hash{}, flipDrops(), flipAccs(), storage) + child = child.Update(common.Hash{}, flopDrops(), flopAccs(), storage) + child = child.Update(common.Hash{}, flipDrops(), flipAccs(), storage) + child = child.Update(common.Hash{}, flopDrops(), flopAccs(), storage) + child = child.Update(common.Hash{}, flipDrops(), flipAccs(), storage) if data, _ := child.Account(h1); data == nil { - t.Errorf("last diff layer: expected %x to be non-nil", h1) + t.Errorf("last diff layer: expected %x account to be non-nil", h1) } if data, _ := child.Account(h2); data != nil { - t.Errorf("last diff layer: expected %x to be nil", h2) + t.Errorf("last diff layer: expected %x account to be nil", h2) + } + if _, ok := child.destructSet[h1]; ok { + t.Errorf("last diff layer: expected %x drop to be missing", h1) + } + if _, ok := child.destructSet[h2]; !ok { + t.Errorf("last diff layer: expected %x drop to be present", h1) } // And flatten merged := (child.flatten()).(*diffLayer) if data, _ := merged.Account(h1); data == nil { - t.Errorf("merged layer: expected %x to be non-nil", h1) + t.Errorf("merged layer: expected %x account to be non-nil", h1) } if data, _ := merged.Account(h2); data != nil { - t.Errorf("merged layer: expected %x to be nil", h2) + t.Errorf("merged layer: expected %x account to be nil", h2) + } + if _, ok := merged.destructSet[h1]; !ok { // Note, drops stay alive until persisted to disk! + t.Errorf("merged diff layer: expected %x drop to be present", h1) + } + if _, ok := merged.destructSet[h2]; !ok { // Note, drops stay alive until persisted to disk! + t.Errorf("merged diff layer: expected %x drop to be present", h1) } // If we add more granular metering of memory, we can enable this again, // but it's not implemented for now @@ -150,18 +176,23 @@ func TestInsertAndMerge(t *testing.T) { child *diffLayer ) { - var accounts = make(map[common.Hash][]byte) - var storage = make(map[common.Hash]map[common.Hash][]byte) - parent = newDiffLayer(emptyLayer(), common.Hash{}, accounts, storage) + var ( + destructs = make(map[common.Hash]struct{}) + accounts = make(map[common.Hash][]byte) + storage = make(map[common.Hash]map[common.Hash][]byte) + ) + parent = newDiffLayer(emptyLayer(), common.Hash{}, destructs, accounts, storage) } { - var accounts = make(map[common.Hash][]byte) - var storage = make(map[common.Hash]map[common.Hash][]byte) + var ( + destructs = make(map[common.Hash]struct{}) + accounts = make(map[common.Hash][]byte) + storage = make(map[common.Hash]map[common.Hash][]byte) + ) accounts[acc] = randomAccount() - accstorage := make(map[common.Hash][]byte) - storage[acc] = accstorage + storage[acc] = make(map[common.Hash][]byte) storage[acc][slot] = []byte{0x01} - child = newDiffLayer(parent, common.Hash{}, accounts, storage) + child = newDiffLayer(parent, common.Hash{}, destructs, accounts, storage) } // And flatten merged := (child.flatten()).(*diffLayer) @@ -189,20 +220,21 @@ func emptyLayer() *diskLayer { func BenchmarkSearch(b *testing.B) { // First, we set up 128 diff layers, with 1K items each fill := func(parent snapshot) *diffLayer { - accounts := make(map[common.Hash][]byte) - storage := make(map[common.Hash]map[common.Hash][]byte) - + var ( + destructs = make(map[common.Hash]struct{}) + accounts = make(map[common.Hash][]byte) + storage = make(map[common.Hash]map[common.Hash][]byte) + ) for i := 0; i < 10000; i++ { accounts[randomHash()] = randomAccount() } - return newDiffLayer(parent, common.Hash{}, accounts, storage) + return newDiffLayer(parent, common.Hash{}, destructs, accounts, storage) } var layer snapshot layer = emptyLayer() for i := 0; i < 128; i++ { layer = fill(layer) } - key := crypto.Keccak256Hash([]byte{0x13, 0x38}) b.ResetTimer() for i := 0; i < b.N; i++ { @@ -224,9 +256,12 @@ func BenchmarkSearchSlot(b *testing.B) { storageKey := crypto.Keccak256Hash([]byte{0x13, 0x37}) accountRLP := randomAccount() fill := func(parent snapshot) *diffLayer { - accounts := make(map[common.Hash][]byte) + var ( + destructs = make(map[common.Hash]struct{}) + accounts = make(map[common.Hash][]byte) + storage = make(map[common.Hash]map[common.Hash][]byte) + ) accounts[accountKey] = accountRLP - storage := make(map[common.Hash]map[common.Hash][]byte) accStorage := make(map[common.Hash][]byte) for i := 0; i < 5; i++ { @@ -235,7 +270,7 @@ func BenchmarkSearchSlot(b *testing.B) { accStorage[randomHash()] = value storage[accountKey] = accStorage } - return newDiffLayer(parent, common.Hash{}, accounts, storage) + return newDiffLayer(parent, common.Hash{}, destructs, accounts, storage) } var layer snapshot layer = emptyLayer() @@ -249,15 +284,17 @@ func BenchmarkSearchSlot(b *testing.B) { } // With accountList and sorting -//BenchmarkFlatten-6 50 29890856 ns/op +// BenchmarkFlatten-6 50 29890856 ns/op // // Without sorting and tracking accountlist // BenchmarkFlatten-6 300 5511511 ns/op func BenchmarkFlatten(b *testing.B) { fill := func(parent snapshot) *diffLayer { - accounts := make(map[common.Hash][]byte) - storage := make(map[common.Hash]map[common.Hash][]byte) - + var ( + destructs = make(map[common.Hash]struct{}) + accounts = make(map[common.Hash][]byte) + storage = make(map[common.Hash]map[common.Hash][]byte) + ) for i := 0; i < 100; i++ { accountKey := randomHash() accounts[accountKey] = randomAccount() @@ -271,11 +308,9 @@ func BenchmarkFlatten(b *testing.B) { } storage[accountKey] = accStorage } - return newDiffLayer(parent, common.Hash{}, accounts, storage) + return newDiffLayer(parent, common.Hash{}, destructs, accounts, storage) } - b.ResetTimer() - for i := 0; i < b.N; i++ { b.StopTimer() var layer snapshot @@ -305,9 +340,11 @@ func BenchmarkFlatten(b *testing.B) { // BenchmarkJournal-6 1 1208083335 ns/op // bufio writer func BenchmarkJournal(b *testing.B) { fill := func(parent snapshot) *diffLayer { - accounts := make(map[common.Hash][]byte) - storage := make(map[common.Hash]map[common.Hash][]byte) - + var ( + destructs = make(map[common.Hash]struct{}) + accounts = make(map[common.Hash][]byte) + storage = make(map[common.Hash]map[common.Hash][]byte) + ) for i := 0; i < 200; i++ { accountKey := randomHash() accounts[accountKey] = randomAccount() @@ -321,7 +358,7 @@ func BenchmarkJournal(b *testing.B) { } storage[accountKey] = accStorage } - return newDiffLayer(parent, common.Hash{}, accounts, storage) + return newDiffLayer(parent, common.Hash{}, destructs, accounts, storage) } layer := snapshot(new(diskLayer)) for i := 1; i < 128; i++ { diff --git a/core/state/snapshot/disklayer.go b/core/state/snapshot/disklayer.go index 3266424a8..e8f2bc853 100644 --- a/core/state/snapshot/disklayer.go +++ b/core/state/snapshot/disklayer.go @@ -161,6 +161,6 @@ func (dl *diskLayer) Storage(accountHash, storageHash common.Hash) ([]byte, erro // Update creates a new layer on top of the existing snapshot diff tree with // the specified data items. Note, the maps are retained by the method to avoid // copying everything. -func (dl *diskLayer) Update(blockHash common.Hash, accounts map[common.Hash][]byte, storage map[common.Hash]map[common.Hash][]byte) *diffLayer { - return newDiffLayer(dl, blockHash, accounts, storage) +func (dl *diskLayer) Update(blockHash common.Hash, destructs map[common.Hash]struct{}, accounts map[common.Hash][]byte, storage map[common.Hash]map[common.Hash][]byte) *diffLayer { + return newDiffLayer(dl, blockHash, destructs, accounts, storage) } diff --git a/core/state/snapshot/disklayer_test.go b/core/state/snapshot/disklayer_test.go index b8dded0d8..aae2aa6b5 100644 --- a/core/state/snapshot/disklayer_test.go +++ b/core/state/snapshot/disklayer_test.go @@ -116,13 +116,14 @@ func TestDiskMerge(t *testing.T) { base.Storage(conNukeCache, conNukeCacheSlot) // Modify or delete some accounts, flatten everything onto disk - if err := snaps.Update(diffRoot, baseRoot, map[common.Hash][]byte{ - accModNoCache: reverse(accModNoCache[:]), - accModCache: reverse(accModCache[:]), - accDelNoCache: nil, - accDelCache: nil, - conNukeNoCache: nil, - conNukeCache: nil, + if err := snaps.Update(diffRoot, baseRoot, map[common.Hash]struct{}{ + accDelNoCache: struct{}{}, + accDelCache: struct{}{}, + conNukeNoCache: struct{}{}, + conNukeCache: struct{}{}, + }, map[common.Hash][]byte{ + accModNoCache: reverse(accModNoCache[:]), + accModCache: reverse(accModCache[:]), }, map[common.Hash]map[common.Hash][]byte{ conModNoCache: {conModNoCacheSlot: reverse(conModNoCacheSlot[:])}, conModCache: {conModCacheSlot: reverse(conModCacheSlot[:])}, @@ -338,13 +339,14 @@ func TestDiskPartialMerge(t *testing.T) { assertStorage(conNukeCache, conNukeCacheSlot, conNukeCacheSlot[:]) // Modify or delete some accounts, flatten everything onto disk - if err := snaps.Update(diffRoot, baseRoot, map[common.Hash][]byte{ - accModNoCache: reverse(accModNoCache[:]), - accModCache: reverse(accModCache[:]), - accDelNoCache: nil, - accDelCache: nil, - conNukeNoCache: nil, - conNukeCache: nil, + if err := snaps.Update(diffRoot, baseRoot, map[common.Hash]struct{}{ + accDelNoCache: struct{}{}, + accDelCache: struct{}{}, + conNukeNoCache: struct{}{}, + conNukeCache: struct{}{}, + }, map[common.Hash][]byte{ + accModNoCache: reverse(accModNoCache[:]), + accModCache: reverse(accModCache[:]), }, map[common.Hash]map[common.Hash][]byte{ conModNoCache: {conModNoCacheSlot: reverse(conModNoCacheSlot[:])}, conModCache: {conModCacheSlot: reverse(conModCacheSlot[:])}, diff --git a/core/state/snapshot/iterator_test.go b/core/state/snapshot/iterator_test.go index dbfafd73d..832be10a4 100644 --- a/core/state/snapshot/iterator_test.go +++ b/core/state/snapshot/iterator_test.go @@ -28,18 +28,23 @@ import ( "github.com/ethereum/go-ethereum/core/rawdb" ) -// TestIteratorBasics tests some simple single-layer iteration -func TestIteratorBasics(t *testing.T) { +// TestAccountIteratorBasics tests some simple single-layer iteration +func TestAccountIteratorBasics(t *testing.T) { var ( - accounts = make(map[common.Hash][]byte) - storage = make(map[common.Hash]map[common.Hash][]byte) + destructs = make(map[common.Hash]struct{}) + accounts = make(map[common.Hash][]byte) + storage = make(map[common.Hash]map[common.Hash][]byte) ) // Fill up a parent for i := 0; i < 100; i++ { h := randomHash() data := randomAccount() + accounts[h] = data - if rand.Intn(20) < 10 { + if rand.Intn(4) == 0 { + destructs[h] = struct{}{} + } + if rand.Intn(2) == 0 { accStorage := make(map[common.Hash][]byte) value := make([]byte, 32) rand.Read(value) @@ -48,7 +53,7 @@ func TestIteratorBasics(t *testing.T) { } } // Add some (identical) layers on top - parent := newDiffLayer(emptyLayer(), common.Hash{}, accounts, storage) + parent := newDiffLayer(emptyLayer(), common.Hash{}, destructs, accounts, storage) it := parent.AccountIterator(common.Hash{}) verifyIterator(t, 100, it) } @@ -138,8 +143,8 @@ func verifyIterator(t *testing.T, expCount int, it AccountIterator) { } } -// TestIteratorTraversal tests some simple multi-layer iteration. -func TestIteratorTraversal(t *testing.T) { +// TestAccountIteratorTraversal tests some simple multi-layer iteration. +func TestAccountIteratorTraversal(t *testing.T) { // Create an empty base layer and a snapshot tree out of it base := &diskLayer{ diskdb: rawdb.NewMemoryDatabase(), @@ -152,13 +157,13 @@ func TestIteratorTraversal(t *testing.T) { }, } // Stack three diff layers on top with various overlaps - snaps.Update(common.HexToHash("0x02"), common.HexToHash("0x01"), + snaps.Update(common.HexToHash("0x02"), common.HexToHash("0x01"), nil, randomAccountSet("0xaa", "0xee", "0xff", "0xf0"), nil) - snaps.Update(common.HexToHash("0x03"), common.HexToHash("0x02"), + snaps.Update(common.HexToHash("0x03"), common.HexToHash("0x02"), nil, randomAccountSet("0xbb", "0xdd", "0xf0"), nil) - snaps.Update(common.HexToHash("0x04"), common.HexToHash("0x03"), + snaps.Update(common.HexToHash("0x04"), common.HexToHash("0x03"), nil, randomAccountSet("0xcc", "0xf0", "0xff"), nil) // Verify the single and multi-layer iterators @@ -173,9 +178,9 @@ func TestIteratorTraversal(t *testing.T) { verifyIterator(t, 7, it) } -// TestIteratorTraversalValues tests some multi-layer iteration, where we +// TestAccountIteratorTraversalValues tests some multi-layer iteration, where we // also expect the correct values to show up. -func TestIteratorTraversalValues(t *testing.T) { +func TestAccountIteratorTraversalValues(t *testing.T) { // Create an empty base layer and a snapshot tree out of it base := &diskLayer{ diskdb: rawdb.NewMemoryDatabase(), @@ -223,14 +228,14 @@ func TestIteratorTraversalValues(t *testing.T) { } } // Assemble a stack of snapshots from the account layers - snaps.Update(common.HexToHash("0x02"), common.HexToHash("0x01"), a, nil) - snaps.Update(common.HexToHash("0x03"), common.HexToHash("0x02"), b, nil) - snaps.Update(common.HexToHash("0x04"), common.HexToHash("0x03"), c, nil) - snaps.Update(common.HexToHash("0x05"), common.HexToHash("0x04"), d, nil) - snaps.Update(common.HexToHash("0x06"), common.HexToHash("0x05"), e, nil) - snaps.Update(common.HexToHash("0x07"), common.HexToHash("0x06"), f, nil) - snaps.Update(common.HexToHash("0x08"), common.HexToHash("0x07"), g, nil) - snaps.Update(common.HexToHash("0x09"), common.HexToHash("0x08"), h, nil) + snaps.Update(common.HexToHash("0x02"), common.HexToHash("0x01"), nil, a, nil) + snaps.Update(common.HexToHash("0x03"), common.HexToHash("0x02"), nil, b, nil) + snaps.Update(common.HexToHash("0x04"), common.HexToHash("0x03"), nil, c, nil) + snaps.Update(common.HexToHash("0x05"), common.HexToHash("0x04"), nil, d, nil) + snaps.Update(common.HexToHash("0x06"), common.HexToHash("0x05"), nil, e, nil) + snaps.Update(common.HexToHash("0x07"), common.HexToHash("0x06"), nil, f, nil) + snaps.Update(common.HexToHash("0x08"), common.HexToHash("0x07"), nil, g, nil) + snaps.Update(common.HexToHash("0x09"), common.HexToHash("0x08"), nil, h, nil) it, _ := snaps.AccountIterator(common.HexToHash("0x09"), common.Hash{}) defer it.Release() @@ -249,7 +254,7 @@ func TestIteratorTraversalValues(t *testing.T) { } // This testcase is notorious, all layers contain the exact same 200 accounts. -func TestIteratorLargeTraversal(t *testing.T) { +func TestAccountIteratorLargeTraversal(t *testing.T) { // Create a custom account factory to recreate the same addresses makeAccounts := func(num int) map[common.Hash][]byte { accounts := make(map[common.Hash][]byte) @@ -272,7 +277,7 @@ func TestIteratorLargeTraversal(t *testing.T) { }, } for i := 1; i < 128; i++ { - snaps.Update(common.HexToHash(fmt.Sprintf("0x%02x", i+1)), common.HexToHash(fmt.Sprintf("0x%02x", i)), makeAccounts(200), nil) + snaps.Update(common.HexToHash(fmt.Sprintf("0x%02x", i+1)), common.HexToHash(fmt.Sprintf("0x%02x", i)), nil, makeAccounts(200), nil) } // Iterate the entire stack and ensure everything is hit only once head := snaps.Snapshot(common.HexToHash("0x80")) @@ -285,11 +290,11 @@ func TestIteratorLargeTraversal(t *testing.T) { verifyIterator(t, 200, it) } -// TestIteratorFlattening tests what happens when we +// TestAccountIteratorFlattening tests what happens when we // - have a live iterator on child C (parent C1 -> C2 .. CN) // - flattens C2 all the way into CN // - continues iterating -func TestIteratorFlattening(t *testing.T) { +func TestAccountIteratorFlattening(t *testing.T) { // Create an empty base layer and a snapshot tree out of it base := &diskLayer{ diskdb: rawdb.NewMemoryDatabase(), @@ -302,13 +307,13 @@ func TestIteratorFlattening(t *testing.T) { }, } // Create a stack of diffs on top - snaps.Update(common.HexToHash("0x02"), common.HexToHash("0x01"), + snaps.Update(common.HexToHash("0x02"), common.HexToHash("0x01"), nil, randomAccountSet("0xaa", "0xee", "0xff", "0xf0"), nil) - snaps.Update(common.HexToHash("0x03"), common.HexToHash("0x02"), + snaps.Update(common.HexToHash("0x03"), common.HexToHash("0x02"), nil, randomAccountSet("0xbb", "0xdd", "0xf0"), nil) - snaps.Update(common.HexToHash("0x04"), common.HexToHash("0x03"), + snaps.Update(common.HexToHash("0x04"), common.HexToHash("0x03"), nil, randomAccountSet("0xcc", "0xf0", "0xff"), nil) // Create an iterator and flatten the data from underneath it @@ -321,7 +326,7 @@ func TestIteratorFlattening(t *testing.T) { //verifyIterator(t, 7, it) } -func TestIteratorSeek(t *testing.T) { +func TestAccountIteratorSeek(t *testing.T) { // Create a snapshot stack with some initial data base := &diskLayer{ diskdb: rawdb.NewMemoryDatabase(), @@ -333,13 +338,13 @@ func TestIteratorSeek(t *testing.T) { base.root: base, }, } - snaps.Update(common.HexToHash("0x02"), common.HexToHash("0x01"), + snaps.Update(common.HexToHash("0x02"), common.HexToHash("0x01"), nil, randomAccountSet("0xaa", "0xee", "0xff", "0xf0"), nil) - snaps.Update(common.HexToHash("0x03"), common.HexToHash("0x02"), + snaps.Update(common.HexToHash("0x03"), common.HexToHash("0x02"), nil, randomAccountSet("0xbb", "0xdd", "0xf0"), nil) - snaps.Update(common.HexToHash("0x04"), common.HexToHash("0x03"), + snaps.Update(common.HexToHash("0x04"), common.HexToHash("0x03"), nil, randomAccountSet("0xcc", "0xf0", "0xff"), nil) // Construct various iterators and ensure their tranversal is correct @@ -372,18 +377,18 @@ func TestIteratorSeek(t *testing.T) { verifyIterator(t, 0, it) // expected: nothing } -// BenchmarkIteratorTraversal is a bit a bit notorious -- all layers contain the +// BenchmarkAccountIteratorTraversal is a bit a bit notorious -- all layers contain the // exact same 200 accounts. That means that we need to process 2000 items, but // only spit out 200 values eventually. // // The value-fetching benchmark is easy on the binary iterator, since it never has to reach // down at any depth for retrieving the values -- all are on the toppmost layer // -// BenchmarkIteratorTraversal/binary_iterator_keys-6 2239 483674 ns/op -// BenchmarkIteratorTraversal/binary_iterator_values-6 2403 501810 ns/op -// BenchmarkIteratorTraversal/fast_iterator_keys-6 1923 677966 ns/op -// BenchmarkIteratorTraversal/fast_iterator_values-6 1741 649967 ns/op -func BenchmarkIteratorTraversal(b *testing.B) { +// BenchmarkAccountIteratorTraversal/binary_iterator_keys-6 2239 483674 ns/op +// BenchmarkAccountIteratorTraversal/binary_iterator_values-6 2403 501810 ns/op +// BenchmarkAccountIteratorTraversal/fast_iterator_keys-6 1923 677966 ns/op +// BenchmarkAccountIteratorTraversal/fast_iterator_values-6 1741 649967 ns/op +func BenchmarkAccountIteratorTraversal(b *testing.B) { // Create a custom account factory to recreate the same addresses makeAccounts := func(num int) map[common.Hash][]byte { accounts := make(map[common.Hash][]byte) @@ -406,7 +411,7 @@ func BenchmarkIteratorTraversal(b *testing.B) { }, } for i := 1; i <= 100; i++ { - snaps.Update(common.HexToHash(fmt.Sprintf("0x%02x", i+1)), common.HexToHash(fmt.Sprintf("0x%02x", i)), makeAccounts(200), nil) + snaps.Update(common.HexToHash(fmt.Sprintf("0x%02x", i+1)), common.HexToHash(fmt.Sprintf("0x%02x", i)), nil, makeAccounts(200), nil) } // We call this once before the benchmark, so the creation of // sorted accountlists are not included in the results. @@ -469,17 +474,17 @@ func BenchmarkIteratorTraversal(b *testing.B) { }) } -// BenchmarkIteratorLargeBaselayer is a pretty realistic benchmark, where +// BenchmarkAccountIteratorLargeBaselayer is a pretty realistic benchmark, where // the baselayer is a lot larger than the upper layer. // // This is heavy on the binary iterator, which in most cases will have to // call recursively 100 times for the majority of the values // -// BenchmarkIteratorLargeBaselayer/binary_iterator_(keys)-6 514 1971999 ns/op -// BenchmarkIteratorLargeBaselayer/binary_iterator_(values)-6 61 18997492 ns/op -// BenchmarkIteratorLargeBaselayer/fast_iterator_(keys)-6 10000 114385 ns/op -// BenchmarkIteratorLargeBaselayer/fast_iterator_(values)-6 4047 296823 ns/op -func BenchmarkIteratorLargeBaselayer(b *testing.B) { +// BenchmarkAccountIteratorLargeBaselayer/binary_iterator_(keys)-6 514 1971999 ns/op +// BenchmarkAccountIteratorLargeBaselayer/binary_iterator_(values)-6 61 18997492 ns/op +// BenchmarkAccountIteratorLargeBaselayer/fast_iterator_(keys)-6 10000 114385 ns/op +// BenchmarkAccountIteratorLargeBaselayer/fast_iterator_(values)-6 4047 296823 ns/op +func BenchmarkAccountIteratorLargeBaselayer(b *testing.B) { // Create a custom account factory to recreate the same addresses makeAccounts := func(num int) map[common.Hash][]byte { accounts := make(map[common.Hash][]byte) @@ -501,9 +506,9 @@ func BenchmarkIteratorLargeBaselayer(b *testing.B) { base.root: base, }, } - snaps.Update(common.HexToHash("0x02"), common.HexToHash("0x01"), makeAccounts(2000), nil) + snaps.Update(common.HexToHash("0x02"), common.HexToHash("0x01"), nil, makeAccounts(2000), nil) for i := 2; i <= 100; i++ { - snaps.Update(common.HexToHash(fmt.Sprintf("0x%02x", i+1)), common.HexToHash(fmt.Sprintf("0x%02x", i)), makeAccounts(20), nil) + snaps.Update(common.HexToHash(fmt.Sprintf("0x%02x", i+1)), common.HexToHash(fmt.Sprintf("0x%02x", i)), nil, makeAccounts(20), nil) } // We call this once before the benchmark, so the creation of // sorted accountlists are not included in the results. @@ -590,7 +595,7 @@ func benchmarkAccountIteration(b *testing.B, iterator func(snap snapshot) Accoun } stack := snapshot(emptyLayer()) for _, layer := range layers { - stack = stack.Update(common.Hash{}, layer, nil) + stack = stack.Update(common.Hash{}, layer, nil, nil) } // Reset the timers and report all the stats it := iterator(stack) diff --git a/core/state/snapshot/journal.go b/core/state/snapshot/journal.go index c42a26d21..66c7aee0a 100644 --- a/core/state/snapshot/journal.go +++ b/core/state/snapshot/journal.go @@ -43,6 +43,11 @@ type journalGenerator struct { Storage uint64 } +// journalDestruct is an account deletion entry in a diffLayer's disk journal. +type journalDestruct struct { + Hash common.Hash +} + // journalAccount is an account entry in a diffLayer's disk journal. type journalAccount struct { Hash common.Hash @@ -139,6 +144,14 @@ func loadDiffLayer(parent snapshot, r *rlp.Stream) (snapshot, error) { } return nil, fmt.Errorf("load diff root: %v", err) } + var destructs []journalDestruct + if err := r.Decode(&destructs); err != nil { + return nil, fmt.Errorf("load diff destructs: %v", err) + } + destructSet := make(map[common.Hash]struct{}) + for _, entry := range destructs { + destructSet[entry.Hash] = struct{}{} + } var accounts []journalAccount if err := r.Decode(&accounts); err != nil { return nil, fmt.Errorf("load diff accounts: %v", err) @@ -159,7 +172,7 @@ func loadDiffLayer(parent snapshot, r *rlp.Stream) (snapshot, error) { } storageData[entry.Hash] = slots } - return loadDiffLayer(newDiffLayer(parent, root, accountData, storageData), r) + return loadDiffLayer(newDiffLayer(parent, root, destructSet, accountData, storageData), r) } // Journal writes the persistent layer generator stats into a buffer to be stored @@ -218,6 +231,13 @@ func (dl *diffLayer) Journal(buffer *bytes.Buffer) (common.Hash, error) { if err := rlp.Encode(buffer, dl.root); err != nil { return common.Hash{}, err } + destructs := make([]journalDestruct, 0, len(dl.destructSet)) + for hash := range dl.destructSet { + destructs = append(destructs, journalDestruct{Hash: hash}) + } + if err := rlp.Encode(buffer, destructs); err != nil { + return common.Hash{}, err + } accounts := make([]journalAccount, 0, len(dl.accountData)) for hash, blob := range dl.accountData { accounts = append(accounts, journalAccount{Hash: hash, Blob: blob}) diff --git a/core/state/snapshot/snapshot.go b/core/state/snapshot/snapshot.go index d031dd2c1..27a8c7f0b 100644 --- a/core/state/snapshot/snapshot.go +++ b/core/state/snapshot/snapshot.go @@ -125,7 +125,7 @@ type snapshot interface { // the specified data items. // // Note, the maps are retained by the method to avoid copying everything. - Update(blockRoot common.Hash, accounts map[common.Hash][]byte, storage map[common.Hash]map[common.Hash][]byte) *diffLayer + Update(blockRoot common.Hash, destructs map[common.Hash]struct{}, accounts map[common.Hash][]byte, storage map[common.Hash]map[common.Hash][]byte) *diffLayer // Journal commits an entire diff hierarchy to disk into a single journal entry. // This is meant to be used during shutdown to persist the snapshot without @@ -222,7 +222,7 @@ func (t *Tree) Snapshot(blockRoot common.Hash) Snapshot { // Update adds a new snapshot into the tree, if that can be linked to an existing // old parent. It is disallowed to insert a disk layer (the origin of all). -func (t *Tree) Update(blockRoot common.Hash, parentRoot common.Hash, accounts map[common.Hash][]byte, storage map[common.Hash]map[common.Hash][]byte) error { +func (t *Tree) Update(blockRoot common.Hash, parentRoot common.Hash, destructs map[common.Hash]struct{}, accounts map[common.Hash][]byte, storage map[common.Hash]map[common.Hash][]byte) error { // Reject noop updates to avoid self-loops in the snapshot tree. This is a // special case that can only happen for Clique networks where empty blocks // don't modify the state (0 block subsidy). @@ -237,7 +237,7 @@ func (t *Tree) Update(blockRoot common.Hash, parentRoot common.Hash, accounts ma if parent == nil { return fmt.Errorf("parent [%#x] snapshot missing", parentRoot) } - snap := parent.Update(blockRoot, accounts, storage) + snap := parent.Update(blockRoot, destructs, accounts, storage) // Save the new snapshot for later t.lock.Lock() @@ -425,40 +425,43 @@ func diffToDisk(bottom *diffLayer) *diskLayer { base.stale = true base.lock.Unlock() - // Push all the accounts into the database + // Destroy all the destructed accounts from the database + for hash := range bottom.destructSet { + // Skip any account not covered yet by the snapshot + if base.genMarker != nil && bytes.Compare(hash[:], base.genMarker) > 0 { + continue + } + // Remove all storage slots + rawdb.DeleteAccountSnapshot(batch, hash) + base.cache.Set(hash[:], nil) + + it := rawdb.IterateStorageSnapshots(base.diskdb, hash) + for it.Next() { + if key := it.Key(); len(key) == 65 { // TODO(karalabe): Yuck, we should move this into the iterator + batch.Delete(key) + base.cache.Del(key[1:]) + + snapshotFlushStorageItemMeter.Mark(1) + } + } + it.Release() + } + // Push all updated accounts into the database for hash, data := range bottom.accountData { // Skip any account not covered yet by the snapshot if base.genMarker != nil && bytes.Compare(hash[:], base.genMarker) > 0 { continue } - if len(data) > 0 { - // Account was updated, push to disk - rawdb.WriteAccountSnapshot(batch, hash, data) - base.cache.Set(hash[:], data) - snapshotCleanAccountWriteMeter.Mark(int64(len(data))) + // Push the account to disk + rawdb.WriteAccountSnapshot(batch, hash, data) + base.cache.Set(hash[:], data) + snapshotCleanAccountWriteMeter.Mark(int64(len(data))) - if batch.ValueSize() > ethdb.IdealBatchSize { - if err := batch.Write(); err != nil { - log.Crit("Failed to write account snapshot", "err", err) - } - batch.Reset() + if batch.ValueSize() > ethdb.IdealBatchSize { + if err := batch.Write(); err != nil { + log.Crit("Failed to write account snapshot", "err", err) } - } else { - // Account was deleted, remove all storage slots too - rawdb.DeleteAccountSnapshot(batch, hash) - base.cache.Set(hash[:], nil) - - it := rawdb.IterateStorageSnapshots(base.diskdb, hash) - for it.Next() { - if key := it.Key(); len(key) == 65 { // TODO(karalabe): Yuck, we should move this into the iterator - batch.Delete(key) - base.cache.Del(key[1:]) - - snapshotFlushStorageItemMeter.Mark(1) - snapshotFlushStorageSizeMeter.Mark(int64(len(data))) - } - } - it.Release() + batch.Reset() } snapshotFlushAccountItemMeter.Mark(1) snapshotFlushAccountSizeMeter.Mark(int64(len(data))) diff --git a/core/state/snapshot/snapshot_test.go b/core/state/snapshot/snapshot_test.go index 2b1482817..910923841 100644 --- a/core/state/snapshot/snapshot_test.go +++ b/core/state/snapshot/snapshot_test.go @@ -81,7 +81,7 @@ func TestDiskLayerExternalInvalidationFullFlatten(t *testing.T) { accounts := map[common.Hash][]byte{ common.HexToHash("0xa1"): randomAccount(), } - if err := snaps.Update(common.HexToHash("0x02"), common.HexToHash("0x01"), accounts, nil); err != nil { + if err := snaps.Update(common.HexToHash("0x02"), common.HexToHash("0x01"), nil, accounts, nil); err != nil { t.Fatalf("failed to create a diff layer: %v", err) } if n := len(snaps.layers); n != 2 { @@ -91,7 +91,7 @@ func TestDiskLayerExternalInvalidationFullFlatten(t *testing.T) { if err := snaps.Cap(common.HexToHash("0x02"), 0); err != nil { t.Fatalf("failed to merge diff layer onto disk: %v", err) } - // Since the base layer was modified, ensure that data retrievald on the external reference fail + // Since the base layer was modified, ensure that data retrieval on the external reference fail if acc, err := ref.Account(common.HexToHash("0x01")); err != ErrSnapshotStale { t.Errorf("stale reference returned account: %#x (err: %v)", acc, err) } @@ -125,10 +125,10 @@ func TestDiskLayerExternalInvalidationPartialFlatten(t *testing.T) { accounts := map[common.Hash][]byte{ common.HexToHash("0xa1"): randomAccount(), } - if err := snaps.Update(common.HexToHash("0x02"), common.HexToHash("0x01"), accounts, nil); err != nil { + if err := snaps.Update(common.HexToHash("0x02"), common.HexToHash("0x01"), nil, accounts, nil); err != nil { t.Fatalf("failed to create a diff layer: %v", err) } - if err := snaps.Update(common.HexToHash("0x03"), common.HexToHash("0x02"), accounts, nil); err != nil { + if err := snaps.Update(common.HexToHash("0x03"), common.HexToHash("0x02"), nil, accounts, nil); err != nil { t.Fatalf("failed to create a diff layer: %v", err) } if n := len(snaps.layers); n != 3 { @@ -173,10 +173,10 @@ func TestDiffLayerExternalInvalidationFullFlatten(t *testing.T) { accounts := map[common.Hash][]byte{ common.HexToHash("0xa1"): randomAccount(), } - if err := snaps.Update(common.HexToHash("0x02"), common.HexToHash("0x01"), accounts, nil); err != nil { + if err := snaps.Update(common.HexToHash("0x02"), common.HexToHash("0x01"), nil, accounts, nil); err != nil { t.Fatalf("failed to create a diff layer: %v", err) } - if err := snaps.Update(common.HexToHash("0x03"), common.HexToHash("0x02"), accounts, nil); err != nil { + if err := snaps.Update(common.HexToHash("0x03"), common.HexToHash("0x02"), nil, accounts, nil); err != nil { t.Fatalf("failed to create a diff layer: %v", err) } if n := len(snaps.layers); n != 3 { @@ -220,13 +220,13 @@ func TestDiffLayerExternalInvalidationPartialFlatten(t *testing.T) { accounts := map[common.Hash][]byte{ common.HexToHash("0xa1"): randomAccount(), } - if err := snaps.Update(common.HexToHash("0x02"), common.HexToHash("0x01"), accounts, nil); err != nil { + if err := snaps.Update(common.HexToHash("0x02"), common.HexToHash("0x01"), nil, accounts, nil); err != nil { t.Fatalf("failed to create a diff layer: %v", err) } - if err := snaps.Update(common.HexToHash("0x03"), common.HexToHash("0x02"), accounts, nil); err != nil { + if err := snaps.Update(common.HexToHash("0x03"), common.HexToHash("0x02"), nil, accounts, nil); err != nil { t.Fatalf("failed to create a diff layer: %v", err) } - if err := snaps.Update(common.HexToHash("0x04"), common.HexToHash("0x03"), accounts, nil); err != nil { + if err := snaps.Update(common.HexToHash("0x04"), common.HexToHash("0x03"), nil, accounts, nil); err != nil { t.Fatalf("failed to create a diff layer: %v", err) } if n := len(snaps.layers); n != 4 { @@ -280,12 +280,12 @@ func TestPostCapBasicDataAccess(t *testing.T) { }, } // The lowest difflayer - snaps.Update(common.HexToHash("0xa1"), common.HexToHash("0x01"), setAccount("0xa1"), nil) - snaps.Update(common.HexToHash("0xa2"), common.HexToHash("0xa1"), setAccount("0xa2"), nil) - snaps.Update(common.HexToHash("0xb2"), common.HexToHash("0xa1"), setAccount("0xb2"), nil) + snaps.Update(common.HexToHash("0xa1"), common.HexToHash("0x01"), nil, setAccount("0xa1"), nil) + snaps.Update(common.HexToHash("0xa2"), common.HexToHash("0xa1"), nil, setAccount("0xa2"), nil) + snaps.Update(common.HexToHash("0xb2"), common.HexToHash("0xa1"), nil, setAccount("0xb2"), nil) - snaps.Update(common.HexToHash("0xa3"), common.HexToHash("0xa2"), setAccount("0xa3"), nil) - snaps.Update(common.HexToHash("0xb3"), common.HexToHash("0xb2"), setAccount("0xb3"), nil) + snaps.Update(common.HexToHash("0xa3"), common.HexToHash("0xa2"), nil, setAccount("0xa3"), nil) + snaps.Update(common.HexToHash("0xb3"), common.HexToHash("0xb2"), nil, setAccount("0xb3"), nil) // checkExist verifies if an account exiss in a snapshot checkExist := func(layer *diffLayer, key string) error { diff --git a/core/state/statedb.go b/core/state/statedb.go index d4a91ee71..55e9752fa 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -67,10 +67,11 @@ type StateDB struct { db Database trie Trie - snaps *snapshot.Tree - snap snapshot.Snapshot - snapAccounts map[common.Hash][]byte - snapStorage map[common.Hash]map[common.Hash][]byte + snaps *snapshot.Tree + snap snapshot.Snapshot + snapDestructs map[common.Hash]struct{} + snapAccounts map[common.Hash][]byte + snapStorage map[common.Hash]map[common.Hash][]byte // This map holds 'live' objects, which will get modified while processing a state transition. stateObjects map[common.Address]*stateObject @@ -133,6 +134,7 @@ func New(root common.Hash, db Database, snaps *snapshot.Tree) (*StateDB, error) } if sdb.snaps != nil { if sdb.snap = sdb.snaps.Snapshot(root); sdb.snap != nil { + sdb.snapDestructs = make(map[common.Hash]struct{}) sdb.snapAccounts = make(map[common.Hash][]byte) sdb.snapStorage = make(map[common.Hash]map[common.Hash][]byte) } @@ -171,8 +173,9 @@ func (s *StateDB) Reset(root common.Hash) error { s.clearJournalAndRefund() if s.snaps != nil { - s.snapAccounts, s.snapStorage = nil, nil + s.snapAccounts, s.snapDestructs, s.snapStorage = nil, nil, nil if s.snap = s.snaps.Snapshot(root); s.snap != nil { + s.snapDestructs = make(map[common.Hash]struct{}) s.snapAccounts = make(map[common.Hash][]byte) s.snapStorage = make(map[common.Hash]map[common.Hash][]byte) } @@ -463,15 +466,6 @@ func (s *StateDB) updateStateObject(obj *stateObject) { panic(fmt.Errorf("can't encode object at %x: %v", addr[:], err)) } s.setError(s.trie.TryUpdate(addr[:], data)) - - // If state snapshotting is active, cache the data til commit - if s.snap != nil { - // If the account is an empty resurrection, unmark the storage nil-ness - if storage, ok := s.snapStorage[obj.addrHash]; storage == nil && ok { - delete(s.snapStorage, obj.addrHash) - } - s.snapAccounts[obj.addrHash] = snapshot.AccountRLP(obj.data.Nonce, obj.data.Balance, obj.data.Root, obj.data.CodeHash) - } } // deleteStateObject removes the given object from the state trie. @@ -483,12 +477,6 @@ func (s *StateDB) deleteStateObject(obj *stateObject) { // Delete the account from the trie addr := obj.Address() s.setError(s.trie.TryDelete(addr[:])) - - // If state snapshotting is active, cache the data til commit - if s.snap != nil { - s.snapAccounts[obj.addrHash] = nil // We need to maintain account deletions explicitly - s.snapStorage[obj.addrHash] = nil // We need to maintain storage deletions explicitly - } } // getStateObject retrieves a state object given by the address, returning nil if @@ -737,8 +725,23 @@ func (s *StateDB) Finalise(deleteEmptyObjects bool) { } if obj.suicided || (deleteEmptyObjects && obj.empty()) { obj.deleted = true + + // If state snapshotting is active, also mark the destruction there. + // Note, we can't do this only at the end of a block because multiple + // transactions within the same block might self destruct and then + // ressurrect an account and the snapshotter needs both events. + if s.snap != nil { + s.snapDestructs[obj.addrHash] = struct{}{} // We need to maintain account deletions explicitly (will remain set indefinitely) + delete(s.snapAccounts, obj.addrHash) // Clear out any previously updated account data (may be recreated via a ressurrect) + delete(s.snapStorage, obj.addrHash) // Clear out any previously updated storage data (may be recreated via a ressurrect) + } } else { obj.finalise() + + // If state snapshotting is active, cache the data til commit + if s.snap != nil { + s.snapAccounts[obj.addrHash] = snapshot.AccountRLP(obj.data.Nonce, obj.data.Balance, obj.data.Root, obj.data.CodeHash) + } } s.stateObjectsPending[addr] = struct{}{} s.stateObjectsDirty[addr] = struct{}{} @@ -842,7 +845,7 @@ func (s *StateDB) Commit(deleteEmptyObjects bool) (common.Hash, error) { } // Only update if there's a state transition (skip empty Clique blocks) if parent := s.snap.Root(); parent != root { - if err := s.snaps.Update(root, parent, s.snapAccounts, s.snapStorage); err != nil { + if err := s.snaps.Update(root, parent, s.snapDestructs, s.snapAccounts, s.snapStorage); err != nil { log.Warn("Failed to update snapshot tree", "from", parent, "to", root, "err", err) } if err := s.snaps.Cap(root, 127); err != nil { // Persistent layer is 128th, the last available trie diff --git a/core/vm/opcodes.go b/core/vm/opcodes.go index ba0ba01b8..71ef0724a 100644 --- a/core/vm/opcodes.go +++ b/core/vm/opcodes.go @@ -70,7 +70,7 @@ const ( SHR SAR - SHA3 = 0x20 + SHA3 OpCode = 0x20 ) // 0x30 range - closure state. @@ -101,8 +101,8 @@ const ( NUMBER DIFFICULTY GASLIMIT - CHAINID = 0x46 - SELFBALANCE = 0x47 + CHAINID OpCode = 0x46 + SELFBALANCE OpCode = 0x47 ) // 0x50 range - 'storage' and execution. @@ -213,10 +213,9 @@ const ( RETURN DELEGATECALL CREATE2 - STATICCALL = 0xfa - - REVERT = 0xfd - SELFDESTRUCT = 0xff + STATICCALL OpCode = 0xfa + REVERT OpCode = 0xfd + SELFDESTRUCT OpCode = 0xff ) // Since the opcodes aren't all in order we can't use a regular slice.