From ee2bca5f5cb7215fcadf89fbac736776e327ce58 Mon Sep 17 00:00:00 2001 From: rjl493456442 Date: Tue, 28 Apr 2020 19:12:48 +0800 Subject: [PATCH] core/state/snapshot: address peter and martin's comment --- core/state/snapshot/account.go | 44 ++++- core/state/snapshot/conversion.go | 241 +++++++++++++++++-------- core/state/snapshot/generate.go | 4 +- core/state/snapshot/iterator.go | 17 +- core/state/snapshot/iterator_binary.go | 8 +- core/state/snapshot/iterator_fast.go | 2 + core/state/snapshot/iterator_test.go | 15 +- core/state/statedb.go | 2 +- 8 files changed, 233 insertions(+), 100 deletions(-) diff --git a/core/state/snapshot/account.go b/core/state/snapshot/account.go index 1068dc2a01..b92e942950 100644 --- a/core/state/snapshot/account.go +++ b/core/state/snapshot/account.go @@ -24,8 +24,10 @@ import ( "github.com/ethereum/go-ethereum/rlp" ) -// Account is a slim version of a state.Account, where the root and code hash -// are replaced with a nil byte slice for empty accounts. +// Account is a modified version of a state.Account, where the root is replaced +// with a byte slice. This format can be used to represent full-consensus format +// or slim-snapshot format which replaces the empty root and code hash as nil +// byte slice. type Account struct { Nonce uint64 Balance *big.Int @@ -33,9 +35,8 @@ type Account struct { CodeHash []byte } -// AccountRLP converts a state.Account content into a slim snapshot version RLP -// encoded. -func AccountRLP(nonce uint64, balance *big.Int, root common.Hash, codehash []byte) []byte { +// SlimAccount converts a state.Account content into a slim snapshot account +func SlimAccount(nonce uint64, balance *big.Int, root common.Hash, codehash []byte) Account { slim := Account{ Nonce: nonce, Balance: balance, @@ -46,9 +47,40 @@ func AccountRLP(nonce uint64, balance *big.Int, root common.Hash, codehash []byt if !bytes.Equal(codehash, emptyCode[:]) { slim.CodeHash = codehash } - data, err := rlp.EncodeToBytes(slim) + return slim +} + +// SlimAccountRLP converts a state.Account content into a slim snapshot +// version RLP encoded. +func SlimAccountRLP(nonce uint64, balance *big.Int, root common.Hash, codehash []byte) []byte { + data, err := rlp.EncodeToBytes(SlimAccount(nonce, balance, root, codehash)) if err != nil { panic(err) } return data } + +// FullAccount decodes the data on the 'slim RLP' format and return +// the consensus format account. +func FullAccount(data []byte) (Account, error) { + var account Account + if err := rlp.DecodeBytes(data, &account); err != nil { + return Account{}, err + } + if len(account.Root) == 0 { + account.Root = emptyRoot[:] + } + if len(account.CodeHash) == 0 { + account.CodeHash = emptyCode[:] + } + return account, nil +} + +// FullAccountRLP converts data on the 'slim RLP' format into the full RLP-format. +func FullAccountRLP(data []byte) ([]byte, error) { + account, err := FullAccount(data) + if err != nil { + return nil, err + } + return rlp.EncodeToBytes(account) +} diff --git a/core/state/snapshot/conversion.go b/core/state/snapshot/conversion.go index f4eec0912c..622e1b4f0a 100644 --- a/core/state/snapshot/conversion.go +++ b/core/state/snapshot/conversion.go @@ -17,6 +17,7 @@ package snapshot import ( + "bytes" "fmt" "sync" "time" @@ -28,41 +29,6 @@ import ( "github.com/ethereum/go-ethereum/trie" ) -// conversionAccount is used for converting between full and slim format. When -// doing this, we can consider 'balance' as a byte array, as it has already -// been converted from big.Int into an rlp-byteslice. -type conversionAccount struct { - Nonce uint64 - Balance []byte - Root []byte - CodeHash []byte -} - -// SlimToFull converts data on the 'slim RLP' format into the full RLP-format. -// Besides, this function accepts another parameter "subRoot". If the root is -// not empty, apply it to account. Usually the subRoot is specified if we want -// to verify the whole state or re-generate state root with different trie algo. -func SlimToFull(data []byte, subRoot common.Hash) ([]byte, error) { - acc := &conversionAccount{} - if err := rlp.DecodeBytes(data, acc); err != nil { - return nil, err - } - if len(acc.Root) == 0 { - acc.Root = emptyRoot[:] - } - if subRoot != (common.Hash{}) { - acc.Root = subRoot.Bytes() - } - if len(acc.CodeHash) == 0 { - acc.CodeHash = emptyCode[:] - } - fullData, err := rlp.EncodeToBytes(acc) - if err != nil { - return nil, err - } - return fullData, nil -} - // trieKV represents a trie key-value pair type trieKV struct { key common.Hash @@ -76,17 +42,17 @@ type ( // leafCallbackFn is the callback invoked at the leaves of the trie, // returns the subtrie root with the specified subtrie identifier. - leafCallbackFn func(hash common.Hash) common.Hash + leafCallbackFn func(hash common.Hash, stat *generateStats) common.Hash ) // GenerateAccountTrieRoot takes an account iterator and reproduces the root hash. -func GenerateAccountTrieRoot(it AccountIterator) common.Hash { - return generateTrieRoot(it, true, stdGenerate, nil, true) +func GenerateAccountTrieRoot(it AccountIterator) (common.Hash, error) { + return generateTrieRoot(it, common.Hash{}, stdGenerate, nil, &generateStats{start: time.Now()}, true) } // GenerateStorageTrieRoot takes a storage iterator and reproduces the root hash. -func GenerateStorageTrieRoot(it StorageIterator) common.Hash { - return generateTrieRoot(it, false, stdGenerate, nil, true) +func GenerateStorageTrieRoot(account common.Hash, it StorageIterator) (common.Hash, error) { + return generateTrieRoot(it, account, stdGenerate, nil, &generateStats{start: time.Now()}, true) } // VerifyState takes the whole snapshot tree as the input, traverses all the accounts @@ -97,69 +63,198 @@ func VerifyState(snaptree *Tree, root common.Hash) error { if err != nil { return err } - got := generateTrieRoot(acctIt, true, stdGenerate, func(account common.Hash) common.Hash { + got, err := generateTrieRoot(acctIt, common.Hash{}, stdGenerate, func(account common.Hash, stat *generateStats) common.Hash { storageIt, err := snaptree.StorageIterator(root, account, common.Hash{}) if err != nil { return common.Hash{} } - return generateTrieRoot(storageIt, false, stdGenerate, nil, false) - }, true) + hash, err := generateTrieRoot(storageIt, account, stdGenerate, nil, stat, false) + if err != nil { + return common.Hash{} + } + return hash + }, &generateStats{start: time.Now()}, true) + if err != nil { + return err + } if got != root { return fmt.Errorf("State root hash mismatch, got %x, want %x", got, root) } return nil } -func generateTrieRoot(it Iterator, accountIterator bool, generatorFn trieGeneratorFn, leafCallback leafCallbackFn, report bool) common.Hash { +// generateStats is a collection of statistics gathered by the trie generator +// for logging purposes. +type generateStats struct { + accounts uint64 + slots uint64 + curAccount common.Hash + curSlot common.Hash + start time.Time + lock sync.RWMutex +} + +// progress records the progress trie generator made recently. +func (stat *generateStats) progress(accounts, slots uint64, curAccount common.Hash, curSlot common.Hash) { + stat.lock.Lock() + defer stat.lock.Unlock() + + stat.accounts += accounts + stat.slots += slots + if curAccount != (common.Hash{}) { + stat.curAccount = curAccount + } + if curSlot != (common.Hash{}) { + stat.curSlot = curSlot + } +} + +// report prints the cumulative progress statistic smartly. +func (stat *generateStats) report() { + stat.lock.RLock() + defer stat.lock.RUnlock() + + var ctx []interface{} + if stat.curSlot != (common.Hash{}) { + ctx = append(ctx, []interface{}{ + "in", stat.curAccount, + "at", stat.curSlot, + }...) + } else { + ctx = append(ctx, []interface{}{"at", stat.curAccount}...) + } + // Add the usual measurements + ctx = append(ctx, []interface{}{"accounts", stat.accounts}...) + if stat.slots != 0 { + ctx = append(ctx, []interface{}{"slots", stat.slots}...) + } + ctx = append(ctx, []interface{}{"elapsed", common.PrettyDuration(time.Since(stat.start))}...) + log.Info("Generating trie hash from snapshot", ctx) +} + +// reportDone prints the last log when the whole generation is finished. +func (stat *generateStats) reportDone() { + stat.lock.RLock() + defer stat.lock.RUnlock() + + var ctx []interface{} + ctx = append(ctx, []interface{}{"accounts", stat.accounts}...) + if stat.slots != 0 { + ctx = append(ctx, []interface{}{"slots", stat.slots}...) + } + ctx = append(ctx, []interface{}{"elapsed", common.PrettyDuration(time.Since(stat.start))}...) + log.Info("Generated trie hash from snapshot", ctx) +} + +// generateTrieRoot generates the trie hash based on the snapshot iterator. +// It can be used for generating account trie, storage trie or even the +// whole state which connects the accounts and the corresponding storages. +func generateTrieRoot(it Iterator, account common.Hash, generatorFn trieGeneratorFn, leafCallback leafCallbackFn, stats *generateStats, report bool) (common.Hash, error) { var ( - in = make(chan trieKV) // chan to pass leaves - out = make(chan common.Hash) // chan to collect result + in = make(chan trieKV) // chan to pass leaves + out = make(chan common.Hash, 1) // chan to collect result wg sync.WaitGroup ) + // Spin up a go-routine for trie hash re-generation wg.Add(1) go func() { + defer wg.Done() generatorFn(in, out) - wg.Done() }() + // Spin up a go-routine for progress logging + if report && stats != nil { + stopLogging := make(chan struct{}) + defer close(stopLogging) + + go func() { + defer wg.Done() + + timer := time.NewTimer(0) + defer timer.Stop() + <-timer.C // discard the initial tick + + for { + select { + case <-timer.C: + stats.report() + timer.Reset(time.Second * 8) + case <-stopLogging: + stats.reportDone() + return + } + } + }() + } var ( - start = time.Now() - logged = time.Now() - entries = 0 + logged = time.Now() + processed = uint64(0) + leaf trieKV + last common.Hash ) // Start to feed leaves for it.Next() { - // Apply the leaf callback first. Normally the callback is used - // to traverse the storage trie and re-generate the subtrie root. - // If the callback is specified, then replace the original storage - // root hash with new one. - var subRoot common.Hash - if leafCallback != nil { - subRoot = leafCallback(it.Hash()) - } - var l trieKV - if accountIterator { - fullData, _ := SlimToFull(it.(AccountIterator).Account(), subRoot) - l = trieKV{it.Hash(), fullData} + if account == (common.Hash{}) { + var ( + err error + fullData []byte + ) + if leafCallback == nil { + fullData, err = FullAccountRLP(it.(AccountIterator).Account()) + if err != nil { + close(in) + return common.Hash{}, err + } + } else { + account, err := FullAccount(it.(AccountIterator).Account()) + if err != nil { + close(in) + return common.Hash{}, err + } + // Apply the leaf callback. Normally the callback is used to traverse + // the storage trie and re-generate the subtrie root. + subroot := leafCallback(it.Hash(), stats) + if !bytes.Equal(account.Root, subroot.Bytes()) { + close(in) + return common.Hash{}, fmt.Errorf("invalid subroot(%x), want %x, got %x", it.Hash(), account.Root, subroot) + } + fullData, err = rlp.EncodeToBytes(account) + if err != nil { + close(in) + return common.Hash{}, err + } + } + leaf = trieKV{it.Hash(), fullData} } else { - l = trieKV{it.Hash(), it.(StorageIterator).Slot()} + leaf = trieKV{it.Hash(), it.(StorageIterator).Slot()} } - in <- l - if time.Since(logged) > 8*time.Second && report { - log.Info("Generating trie hash from snapshot", "at", l.key, "entries", entries, "elapsed", time.Since(start)) - logged = time.Now() + in <- leaf + + // Accumulate the generaation statistic if it's required. + processed++ + if time.Since(logged) > 3*time.Second && stats != nil { + if account == (common.Hash{}) { + stats.progress(processed, 0, it.Hash(), common.Hash{}) + } else { + stats.progress(0, processed, account, it.Hash()) + } + logged, processed = time.Now(), 0 + } + last = it.Hash() + } + // Commit the last part statistic. + if processed > 0 && stats != nil { + if account == (common.Hash{}) { + stats.progress(processed, 0, last, common.Hash{}) + } else { + stats.progress(0, processed, account, last) } - entries++ } close(in) result := <-out wg.Wait() - - if report { - log.Info("Generated trie hash from snapshot", "entries", entries, "elapsed", time.Since(start)) - } - return result + return result, nil } // stdGenerate is a very basic hexary trie builder which uses the same Trie diff --git a/core/state/snapshot/generate.go b/core/state/snapshot/generate.go index 4b017fe69b..c3a4a552ff 100644 --- a/core/state/snapshot/generate.go +++ b/core/state/snapshot/generate.go @@ -42,7 +42,7 @@ var ( ) // generatorStats is a collection of statistics gathered by the snapshot generator -// for logging purposes. +// for logging purposes. type generatorStats struct { wiping chan struct{} // Notification channel if wiping is in progress origin uint64 // Origin prefix where generation started @@ -167,7 +167,7 @@ func (dl *diskLayer) generate(stats *generatorStats) { if err := rlp.DecodeBytes(accIt.Value, &acc); err != nil { log.Crit("Invalid account encountered during snapshot creation", "err", err) } - data := AccountRLP(acc.Nonce, acc.Balance, acc.Root, acc.CodeHash) + data := SlimAccountRLP(acc.Nonce, acc.Balance, acc.Root, acc.CodeHash) // If the account is not yet in-progress, write it out if accMarker == nil || !bytes.Equal(accountHash[:], accMarker) { diff --git a/core/state/snapshot/iterator.go b/core/state/snapshot/iterator.go index f180402df8..adef367a3a 100644 --- a/core/state/snapshot/iterator.go +++ b/core/state/snapshot/iterator.go @@ -137,6 +137,8 @@ func (it *diffAccountIterator) Hash() common.Hash { // This method assumes that flattening does not delete elements from // the accountdata mapping (writing nil into it is fine though), and will panic // if elements have been deleted. +// +// Note the returned account is not a copy, please don't modify it. func (it *diffAccountIterator) Account() []byte { it.layer.lock.RLock() blob, ok := it.layer.accountData[it.curHash] @@ -151,7 +153,7 @@ func (it *diffAccountIterator) Account() []byte { if it.layer.Stale() { it.fail, it.keys = ErrSnapshotStale, nil } - return common.CopyBytes(blob) + return blob } // Release is a noop for diff account iterators as there are no held resources. @@ -181,7 +183,7 @@ func (it *diskAccountIterator) Next() bool { } // Try to advance the iterator and release it if we reached the end for { - if !it.it.Next() || !bytes.HasPrefix(it.it.Key(), rawdb.SnapshotAccountPrefix) { + if !it.it.Next() { it.it.Release() it.it = nil return false @@ -212,7 +214,7 @@ func (it *diskAccountIterator) Hash() common.Hash { // Account returns the RLP encoded slim account the iterator is currently at. func (it *diskAccountIterator) Account() []byte { - return common.CopyBytes(it.it.Value()) + return it.it.Value() } // Release releases the database snapshot held during iteration. @@ -302,6 +304,8 @@ func (it *diffStorageIterator) Hash() common.Hash { // This method assumes that flattening does not delete elements from // the storage mapping (writing nil into it is fine though), and will panic // if elements have been deleted. +// +// Note the returned slot is not a copy, please don't modify it. func (it *diffStorageIterator) Slot() []byte { it.layer.lock.RLock() storage, ok := it.layer.storageData[it.account] @@ -317,7 +321,7 @@ func (it *diffStorageIterator) Slot() []byte { if it.layer.Stale() { it.fail, it.keys = ErrSnapshotStale, nil } - return common.CopyBytes(blob) + return blob } // Release is a noop for diff account iterators as there are no held resources. @@ -351,9 +355,8 @@ func (it *diskStorageIterator) Next() bool { return false } // Try to advance the iterator and release it if we reached the end - prefix := append(rawdb.SnapshotStoragePrefix, it.account.Bytes()...) for { - if !it.it.Next() || !bytes.HasPrefix(it.it.Key(), prefix) { + if !it.it.Next() { it.it.Release() it.it = nil return false @@ -384,7 +387,7 @@ func (it *diskStorageIterator) Hash() common.Hash { // Slot returns the raw strorage slot content the iterator is currently at. func (it *diskStorageIterator) Slot() []byte { - return common.CopyBytes(it.it.Value()) + return it.it.Value() } // Release releases the database snapshot held during iteration. diff --git a/core/state/snapshot/iterator_binary.go b/core/state/snapshot/iterator_binary.go index 30ccff09dc..37a4b0b5c1 100644 --- a/core/state/snapshot/iterator_binary.go +++ b/core/state/snapshot/iterator_binary.go @@ -160,6 +160,8 @@ func (it *binaryIterator) Hash() common.Hash { // Account returns the RLP encoded slim account the iterator is currently at, or // nil if the iterated snapshot stack became stale (you can check Error after // to see if it failed or not). +// +// Note the returned account is not a copy, please don't modify it. func (it *binaryIterator) Account() []byte { if !it.accountIterator { return nil @@ -170,12 +172,14 @@ func (it *binaryIterator) Account() []byte { it.fail = err return nil } - return common.CopyBytes(blob) + return blob } // Slot returns the raw storage slot data the iterator is currently at, or // nil if the iterated snapshot stack became stale (you can check Error after // to see if it failed or not). +// +// Note the returned slot is not a copy, please don't modify it. func (it *binaryIterator) Slot() []byte { if it.accountIterator { return nil @@ -185,7 +189,7 @@ func (it *binaryIterator) Slot() []byte { it.fail = err return nil } - return common.CopyBytes(blob) + return blob } // Release recursively releases all the iterators in the stack. diff --git a/core/state/snapshot/iterator_fast.go b/core/state/snapshot/iterator_fast.go index ef3a27ac95..82c46f1fb6 100644 --- a/core/state/snapshot/iterator_fast.go +++ b/core/state/snapshot/iterator_fast.go @@ -307,11 +307,13 @@ func (fi *fastIterator) Hash() common.Hash { } // Account returns the current account blob. +// Note the returned account is not a copy, please don't modify it. func (fi *fastIterator) Account() []byte { return fi.curAccount } // Slot returns the current storage slot. +// Note the returned slot is not a copy, please don't modify it. func (fi *fastIterator) Slot() []byte { return fi.curSlot } diff --git a/core/state/snapshot/iterator_test.go b/core/state/snapshot/iterator_test.go index 504e8e672b..da38da7499 100644 --- a/core/state/snapshot/iterator_test.go +++ b/core/state/snapshot/iterator_test.go @@ -31,10 +31,9 @@ import ( // TestAccountIteratorBasics tests some simple single-layer(diff and disk) iteration func TestAccountIteratorBasics(t *testing.T) { var ( - nilAccount int - destructs = make(map[common.Hash]struct{}) - 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++ { @@ -44,8 +43,6 @@ func TestAccountIteratorBasics(t *testing.T) { accounts[h] = data if rand.Intn(4) == 0 { destructs[h] = struct{}{} - delete(accounts, h) - nilAccount += 1 } if rand.Intn(2) == 0 { accStorage := make(map[common.Hash][]byte) @@ -62,7 +59,7 @@ func TestAccountIteratorBasics(t *testing.T) { diskLayer := diffToDisk(diffLayer) it = diskLayer.AccountIterator(common.Hash{}) - verifyIterator(t, 100-nilAccount, it, verifyNothing) // Nil is allowed for single layer iterator + verifyIterator(t, 100, it, verifyNothing) // Nil is allowed for single layer iterator } // TestStorageIteratorBasics tests some simple single-layer(diff and disk) iteration for storage @@ -195,9 +192,9 @@ func verifyIterator(t *testing.T, expCount int, it Iterator, verify verifyConten t.Errorf("wrong order: %x >= %x", last, hash) } count++ - if verify == verifyAccount && it.(AccountIterator).Account() == nil { + if verify == verifyAccount && len(it.(AccountIterator).Account()) == 0 { t.Errorf("iterator returned nil-value for hash %x", hash) - } else if verify == verifyStorage && it.(StorageIterator).Slot() == nil { + } else if verify == verifyStorage && len(it.(StorageIterator).Slot()) == 0 { t.Errorf("iterator returned nil-value for hash %x", hash) } } diff --git a/core/state/statedb.go b/core/state/statedb.go index 4f5c1703ed..3f96e8707e 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -472,7 +472,7 @@ func (s *StateDB) updateStateObject(obj *stateObject) { // enough to track account updates at commit time, deletions need tracking // at transaction boundary level to ensure we capture state clearing. if s.snap != nil { - s.snapAccounts[obj.addrHash] = snapshot.AccountRLP(obj.data.Nonce, obj.data.Balance, obj.data.Root, obj.data.CodeHash) + s.snapAccounts[obj.addrHash] = snapshot.SlimAccountRLP(obj.data.Nonce, obj.data.Balance, obj.data.Root, obj.data.CodeHash) } }