eth/protocols/snap: fix the flaws in the snap sync

[rjl493456442]

No description provided.

[holiman]

One nitpick

[holiman]

One thing I've been testing with (no issues found so far) is adding a verifyTrie(syncer.db, sourceAccountTrie.Hash(), t) in the cases where sync should pass. And add this:

func verifyTrie(db ethdb.KeyValueStore, root common.Hash, t *testing.T) {
	triedb := trie.NewDatabase(db)
	accTrie, err := trie.New(root, triedb)
	if err != nil {
		t.Fatal(err)
	}
	accounts, slots := 0, 0
	accIt := trie.NewIterator(accTrie.NodeIterator(nil))
	for accIt.Next() {
		var acc struct {
			Nonce    uint64
			Balance  *big.Int
			Root     common.Hash
			CodeHash []byte
		}
		if err := rlp.DecodeBytes(accIt.Value, &acc); err != nil {
			log.Crit("Invalid account encountered during snapshot creation", "err", err)
		}
		accounts++
		if acc.Root != emptyRoot {
			storeTrie, err := trie.NewSecure(acc.Root, triedb)
			if err != nil {
				t.Fatal(err)
			}
			storeIt := trie.NewIterator(storeTrie.NodeIterator(nil))
			for storeIt.Next() {
				slots++
			}
			if err := storeIt.Err; err != nil {
				t.Fatal(err)

			}
		}
	}
	if err := accIt.Err; err != nil {
		t.Fatal(err)
	}
	t.Logf("accounts: %d, slots: %d", accounts, slots)
}

[holiman]

Should we fix the sender/part too?

diff --git a/eth/protocols/snap/handler.go b/eth/protocols/snap/handler.go
index b9515b8a39..0b4f83a75e 100644
--- a/eth/protocols/snap/handler.go
+++ b/eth/protocols/snap/handler.go
@@ -256,8 +256,13 @@ func handleMessage(backend Backend, peer *Peer) error {
 			var (
 				storage []*StorageData
 				last    common.Hash
+				aborted = false
 			)
-			for it.Next() && size < hardLimit {
+			for it.Next() {
+				if size > hardLimit {
+					aborted = true
+					break
+				}
 				hash, slot := it.Hash(), common.CopyBytes(it.Slot())
 
 				// Track the returned interval for the Merkle proofs
@@ -280,7 +285,7 @@ func handleMessage(backend Backend, peer *Peer) error {
 			// Generate the Merkle proofs for the first and last storage slot, but
 			// only if the response was capped. If the entire storage trie included
 			// in the response, no need for any proofs.
-			if origin != (common.Hash{}) || size >= hardLimit {
+			if origin != (common.Hash{}) || aborted {
 				// Request started at a non-zero hash or was capped prematurely, add
 				// the endpoint Merkle proofs
 				accTrie, err := trie.New(req.Root, backend.Chain().StateCache().TrieDB())

[rjl493456442]

@holiman I think if the last slot exceeds the limit, we should also generate the proof. So I would say in the current code, the last packet proof of the range is always missing? No, in this case the origin is not equal to common.Hash{}

Please check if this version works for you

diff --git a/eth/protocols/snap/handler.go b/eth/protocols/snap/handler.go
index b9515b8a3..bd2964e91 100644
--- a/eth/protocols/snap/handler.go
+++ b/eth/protocols/snap/handler.go
@@ -256,8 +256,13 @@ func handleMessage(backend Backend, peer *Peer) error {
 			var (
 				storage []*StorageData
 				last    common.Hash
+				abort   bool
 			)
-			for it.Next() && size < hardLimit {
+			for it.Next() {
+				if size >= hardLimit {
+					abort = true
+					break
+				}
 				hash, slot := it.Hash(), common.CopyBytes(it.Slot())
 
 				// Track the returned interval for the Merkle proofs
@@ -271,6 +276,7 @@ func handleMessage(backend Backend, peer *Peer) error {
 				})
 				// If we've exceeded the request threshold, abort
 				if bytes.Compare(hash[:], limit[:]) >= 0 {
+					abort = true
 					break
 				}
 			}
@@ -280,7 +286,7 @@ func handleMessage(backend Backend, peer *Peer) error {
 			// Generate the Merkle proofs for the first and last storage slot, but
 			// only if the response was capped. If the entire storage trie included
 			// in the response, no need for any proofs.
-			if origin != (common.Hash{}) || size >= hardLimit {
+			if abort {
 				// Request started at a non-zero hash or was capped prematurely, add
 				// the endpoint Merkle proofs
 				accTrie, err := trie.New(req.Root, backend.Chain().StateCache().TrieDB())

[holiman]

J'approuf

[karalabe]

LGTM