From 32ba9c3bd264c0d6d5db6116126a85a90e672008 Mon Sep 17 00:00:00 2001 From: andicrypt Date: Thu, 31 Oct 2024 16:09:39 +0700 Subject: [PATCH] eth/protocols/snap: fix snap sync failure on empty storage range commit https://github.com/ethereum/go-ethereum/commit/1cb3b6aee4a16ab8e684da63f3cfcd9b961c43af In PR https://github.com/ethereum/go-ethereum/commit/39fb82bcfb30a362aec259dd8a171e820f4d2123, we ensure that if accumulated storage response exceeds hard limit cap (and being cutdown in size), the proof will not be attached, ensuring that the prover will not mislead more entries to requested. However, we now mishandle the normal scenario when the particular range requested does not contain any slots. This PR aims to fix this. --- eth/protocols/snap/sync.go | 10 ++- eth/protocols/snap/sync_test.go | 113 ++++++++++++++++++++++++++------ internal/testrand/rand.go | 53 +++++++++++++++ 3 files changed, 154 insertions(+), 22 deletions(-) create mode 100644 internal/testrand/rand.go diff --git a/eth/protocols/snap/sync.go b/eth/protocols/snap/sync.go index 9e82682fb9..62816561fd 100644 --- a/eth/protocols/snap/sync.go +++ b/eth/protocols/snap/sync.go @@ -2575,7 +2575,7 @@ func (s *Syncer) OnStorage(peer SyncPeer, id uint64, hashes [][]common.Hash, slo // the requested data. For storage range queries that means the state being // retrieved was either already pruned remotely, or the peer is not yet // synced to our head. - if len(hashes) == 0 { + if len(hashes) == 0 && len(proof) == 0 { logger.Debug("Peer rejected storage request") s.statelessPeers[peer.ID()] = struct{}{} s.lock.Unlock() @@ -2587,6 +2587,14 @@ func (s *Syncer) OnStorage(peer SyncPeer, id uint64, hashes [][]common.Hash, slo // Reconstruct the partial tries from the response and verify them var cont bool + // If a proof was attached while the response is empty, it indicates that the + // requested range specified with 'origin' is empty. Construct an empty state + // response locally to finalize the range. + if len(hashes) == 0 && len(proof) > 0 { + hashes = append(hashes, []common.Hash{}) + slots = append(slots, [][]byte{}) + } + for i := 0; i < len(hashes); i++ { // Convert the keys and proofs into an internal format keys := make([][]byte, len(hashes[i])) diff --git a/eth/protocols/snap/sync_test.go b/eth/protocols/snap/sync_test.go index 4c7a7d88d0..a5fb298509 100644 --- a/eth/protocols/snap/sync_test.go +++ b/eth/protocols/snap/sync_test.go @@ -27,11 +27,14 @@ import ( "testing" "time" + mrand "math/rand" + "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/ethdb" + "github.com/ethereum/go-ethereum/internal/testrand" "github.com/ethereum/go-ethereum/light" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/rlp" @@ -729,7 +732,7 @@ func TestSyncWithStorage(t *testing.T) { }) } ) - sourceAccountTrie, elems, storageTries, storageElems := makeAccountTrieWithStorage(3, 3000, true, false) + sourceAccountTrie, elems, storageTries, storageElems := makeAccountTrieWithStorage(3, 3000, true, false, false) mkSource := func(name string) *testPeer { source := newTestPeer(name, t, term) @@ -761,7 +764,7 @@ func TestMultiSyncManyUseless(t *testing.T) { }) } ) - sourceAccountTrie, elems, storageTries, storageElems := makeAccountTrieWithStorage(100, 3000, true, false) + sourceAccountTrie, elems, storageTries, storageElems := makeAccountTrieWithStorage(100, 3000, true, false, false) mkSource := func(name string, noAccount, noStorage, noTrieNode bool) *testPeer { source := newTestPeer(name, t, term) @@ -807,7 +810,7 @@ func TestMultiSyncManyUselessWithLowTimeout(t *testing.T) { }) } ) - sourceAccountTrie, elems, storageTries, storageElems := makeAccountTrieWithStorage(100, 3000, true, false) + sourceAccountTrie, elems, storageTries, storageElems := makeAccountTrieWithStorage(100, 3000, true, false, false) mkSource := func(name string, noAccount, noStorage, noTrieNode bool) *testPeer { source := newTestPeer(name, t, term) @@ -858,7 +861,7 @@ func TestMultiSyncManyUnresponsive(t *testing.T) { }) } ) - sourceAccountTrie, elems, storageTries, storageElems := makeAccountTrieWithStorage(100, 3000, true, false) + sourceAccountTrie, elems, storageTries, storageElems := makeAccountTrieWithStorage(100, 3000, true, false, false) mkSource := func(name string, noAccount, noStorage, noTrieNode bool) *testPeer { source := newTestPeer(name, t, term) @@ -1125,7 +1128,7 @@ func TestSyncBoundaryStorageTrie(t *testing.T) { }) } ) - sourceAccountTrie, elems, storageTries, storageElems := makeAccountTrieWithStorage(10, 1000, false, true) + sourceAccountTrie, elems, storageTries, storageElems := makeAccountTrieWithStorage(10, 1000, false, true, false) mkSource := func(name string) *testPeer { source := newTestPeer(name, t, term) @@ -1161,7 +1164,7 @@ func TestSyncWithStorageAndOneCappedPeer(t *testing.T) { }) } ) - sourceAccountTrie, elems, storageTries, storageElems := makeAccountTrieWithStorage(300, 1000, false, false) + sourceAccountTrie, elems, storageTries, storageElems := makeAccountTrieWithStorage(300, 1000, false, false, false) mkSource := func(name string, slow bool) *testPeer { source := newTestPeer(name, t, term) @@ -1202,7 +1205,7 @@ func TestSyncWithStorageAndCorruptPeer(t *testing.T) { }) } ) - sourceAccountTrie, elems, storageTries, storageElems := makeAccountTrieWithStorage(100, 3000, true, false) + sourceAccountTrie, elems, storageTries, storageElems := makeAccountTrieWithStorage(100, 3000, true, false, false) mkSource := func(name string, handler storageHandlerFunc) *testPeer { source := newTestPeer(name, t, term) @@ -1240,7 +1243,7 @@ func TestSyncWithStorageAndNonProvingPeer(t *testing.T) { }) } ) - sourceAccountTrie, elems, storageTries, storageElems := makeAccountTrieWithStorage(100, 3000, true, false) + sourceAccountTrie, elems, storageTries, storageElems := makeAccountTrieWithStorage(100, 3000, true, false, false) mkSource := func(name string, handler storageHandlerFunc) *testPeer { source := newTestPeer(name, t, term) @@ -1298,6 +1301,37 @@ func TestSyncWithStorageMisbehavingProve(t *testing.T) { verifyTrie(syncer.db, sourceAccountTrie.Hash(), t) } +func TestSyncWithUnevenStorage(t *testing.T) { + t.Parallel() + var ( + once sync.Once + cancel = make(chan struct{}) + term = func() { + once.Do(func() { + close(cancel) + }) + } + ) + + accountTrie, accounts, storageTries, storageElems := makeAccountTrieWithStorage(3, 256, false, false, true) + mkSource := func(name string) *testPeer { + source := newTestPeer(name, t, term) + source.accountTrie = accountTrie + source.accountValues = accounts + source.storageTries = storageTries + source.storageValues = storageElems + source.storageRequestHandler = func(t *testPeer, reqId uint64, root common.Hash, accounts []common.Hash, origin, limit []byte, max uint64) error { + return defaultStorageRequestHandler(t, reqId, root, accounts, origin, limit, 128) // retrieve storage in large mode + } + return source + } + syncer := setupSyncer(mkSource("source")) + if err := syncer.Sync(accountTrie.Hash(), cancel); err != nil { + t.Fatalf("sync failed: %v", err) + } + verifyTrie(syncer.db, accountTrie.Hash(), t) +} + type kv struct { k, v []byte } @@ -1466,7 +1500,7 @@ func makeAccountTrieWithStorageWithUniqueStorage(accounts, slots int, code bool) } // makeAccountTrieWithStorage spits out a trie, along with the leafs -func makeAccountTrieWithStorage(accounts, slots int, code, boundary bool) (*trie.Trie, entrySlice, map[common.Hash]*trie.Trie, map[common.Hash]entrySlice) { +func makeAccountTrieWithStorage(accounts, slots int, code, boundary bool, uneven bool) (*trie.Trie, entrySlice, map[common.Hash]*trie.Trie, map[common.Hash]entrySlice) { var ( db = trie.NewDatabase(rawdb.NewMemoryDatabase()) accTrie, _ = trie.New(common.Hash{}, db) @@ -1474,25 +1508,29 @@ func makeAccountTrieWithStorage(accounts, slots int, code, boundary bool) (*trie storageTries = make(map[common.Hash]*trie.Trie) storageEntries = make(map[common.Hash]entrySlice) ) - // Make a storage trie which we reuse for the whole lot - var ( - stTrie *trie.Trie - stEntries entrySlice - ) - if boundary { - stTrie, stEntries = makeBoundaryStorageTrie(slots, db) - } else { - stTrie, stEntries = makeStorageTrieWithSeed(uint64(slots), 0, db) - } - stRoot := stTrie.Hash() // Create n accounts in the trie for i := uint64(1); i <= uint64(accounts); i++ { + var ( + stTrie *trie.Trie + stEntries entrySlice + ) + key := key32(i) codehash := emptyCode[:] if code { codehash = getCodeHash(i) } + + if boundary { + stTrie, stEntries = makeBoundaryStorageTrie(slots, db) + } else if uneven { + stTrie, stEntries = makeUnevenStorageTrie(slots, db) + } else { + stTrie, stEntries = makeStorageTrieWithSeed(uint64(slots), 0, db) + } + stRoot := stTrie.Hash() + value, _ := rlp.EncodeToBytes(types.StateAccount{ Nonce: i, Balance: big.NewInt(int64(i)), @@ -1507,8 +1545,8 @@ func makeAccountTrieWithStorage(accounts, slots int, code, boundary bool) (*trie storageEntries[common.BytesToHash(key)] = stEntries } sort.Sort(entries) - stTrie.Commit(nil) accTrie.Commit(nil) + return accTrie, entries, storageTries, storageEntries } @@ -1586,6 +1624,39 @@ func makeBoundaryStorageTrie(n int, db *trie.Database) (*trie.Trie, entrySlice) return trie, entries } +// makeUnevenStorageTrie constructs a storage tries will states distributed in +// different range unevenly. +func makeUnevenStorageTrie(slots int, db *trie.Database) (*trie.Trie, []*kv) { + var ( + entries entrySlice + // tr, _ = trie.New(trie.StorageTrieID(types.EmptyRootHash, owner, types.EmptyRootHash), db) + tr, _ = trie.New(common.Hash{}, db) + chosen = make(map[byte]struct{}) + ) + for i := 0; i < 3; i++ { + var n int + for { + n = mrand.Intn(15) // the last range is set empty deliberately + if _, ok := chosen[byte(n)]; ok { + continue + } + chosen[byte(n)] = struct{}{} + break + } + for j := 0; j < slots/3; j++ { + key := append([]byte{byte(n)}, testrand.Bytes(31)...) + val, _ := rlp.EncodeToBytes(testrand.Bytes(32)) + + elem := &kv{key, val} + tr.Update(elem.k, elem.v) + entries = append(entries, elem) + } + } + sort.Sort(entries) + tr.Commit(nil) + return tr, entries +} + func verifyTrie(db ethdb.KeyValueStore, root common.Hash, t *testing.T) { t.Helper() triedb := trie.NewDatabase(db) diff --git a/internal/testrand/rand.go b/internal/testrand/rand.go new file mode 100644 index 0000000000..690993de05 --- /dev/null +++ b/internal/testrand/rand.go @@ -0,0 +1,53 @@ +// Copyright 2023 The go-ethereum Authors +// This file is part of the go-ethereum library. +// +// The go-ethereum library is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// The go-ethereum library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with the go-ethereum library. If not, see . + +package testrand + +import ( + crand "crypto/rand" + "encoding/binary" + mrand "math/rand" + + "github.com/ethereum/go-ethereum/common" +) + +// prng is a pseudo random number generator seeded by strong randomness. +// The randomness is printed on startup in order to make failures reproducible. +var prng = initRand() + +func initRand() *mrand.Rand { + var seed [8]byte + crand.Read(seed[:]) + rnd := mrand.New(mrand.NewSource(int64(binary.LittleEndian.Uint64(seed[:])))) + return rnd +} + +// Bytes generates a random byte slice with specified length. +func Bytes(n int) []byte { + r := make([]byte, n) + prng.Read(r) + return r +} + +// Hash generates a random hash. +func Hash() common.Hash { + return common.BytesToHash(Bytes(common.HashLength)) +} + +// Address generates a random address. +func Address() common.Address { + return common.BytesToAddress(Bytes(common.AddressLength)) +}