From b05150080949beca84cde77cb913801966144f89 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Fri, 3 Nov 2023 10:21:29 +0100 Subject: [PATCH 1/7] rpc: make subscription test faster reduces time for TestClientSubscriptionChannelClose from 25 sec to < 1 sec. --- rpc/client_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rpc/client_test.go b/rpc/client_test.go index 7c96b2d6667b..ac02ad33cf6a 100644 --- a/rpc/client_test.go +++ b/rpc/client_test.go @@ -595,7 +595,7 @@ func TestClientSubscriptionChannelClose(t *testing.T) { for i := 0; i < 100; i++ { ch := make(chan int, 100) - sub, err := client.Subscribe(context.Background(), "nftest", ch, "someSubscription", maxClientSubscriptionBuffer-1, 1) + sub, err := client.Subscribe(context.Background(), "nftest", ch, "someSubscription", 100, 1) if err != nil { t.Fatal(err) } From c413663418b7d87d8582ed835ebd4f22c65c6039 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Fri, 3 Nov 2023 11:14:37 +0100 Subject: [PATCH 2/7] trie: cache trie nodes for faster sanity check This reduces the time spent on TestIncompleteSyncHash from ~25s to ~16s. --- trie/sync_test.go | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/trie/sync_test.go b/trie/sync_test.go index 3b7986ef6792..4485c5293f13 100644 --- a/trie/sync_test.go +++ b/trie/sync_test.go @@ -73,7 +73,7 @@ func makeTestTrie(scheme string) (ethdb.Database, *Database, *StateTrie, map[str func checkTrieContents(t *testing.T, db ethdb.Database, scheme string, root []byte, content map[string][]byte, rawTrie bool) { // Check root availability and trie contents ndb := newTestDatabase(db, scheme) - if err := checkTrieConsistency(db, scheme, common.BytesToHash(root), rawTrie); err != nil { + if err := checkTrieConsistency(db, scheme, common.BytesToHash(root), rawTrie, nil); err != nil { t.Fatalf("inconsistent trie at %x: %v", root, err) } type reader interface { @@ -101,7 +101,7 @@ func checkTrieContents(t *testing.T, db ethdb.Database, scheme string, root []by } // checkTrieConsistency checks that all nodes in a trie are indeed present. -func checkTrieConsistency(db ethdb.Database, scheme string, root common.Hash, rawTrie bool) error { +func checkTrieConsistency(db ethdb.Database, scheme string, root common.Hash, rawTrie bool, resolver NodeResolver) error { ndb := newTestDatabase(db, scheme) var it NodeIterator if rawTrie { @@ -117,6 +117,9 @@ func checkTrieConsistency(db ethdb.Database, scheme string, root common.Hash, ra } it = trie.MustNodeIterator(nil) } + if resolver != nil { + it.AddResolver(resolver) + } for it.Next(true) { } return it.Error() @@ -512,8 +515,8 @@ func testDuplicateAvoidanceSync(t *testing.T, scheme string) { // Tests that at any point in time during a sync, only complete sub-tries are in // the database. func TestIncompleteSyncHash(t *testing.T) { - testIncompleteSync(t, rawdb.HashScheme) - testIncompleteSync(t, rawdb.PathScheme) + testIncompleteSync(t, rawdb.HashScheme) // 9.585s + testIncompleteSync(t, rawdb.PathScheme) // 19.522 } func testIncompleteSync(t *testing.T, scheme string) { @@ -585,16 +588,32 @@ func testIncompleteSync(t *testing.T, scheme string) { }) } } + + // Cache trie nodes for faster verification + nodesCache := make(map[string][]byte) + resolver := func(owner common.Hash, path []byte, hash common.Hash) []byte { + if scheme == rawdb.HashScheme { + return nodesCache[string(hash[:])] + } + return nodesCache[string(path)] + } + // Sanity check that removing any node from the database is detected for i, path := range addedKeys { owner, inner := ResolvePath([]byte(path)) nodeHash := addedHashes[i] value := rawdb.ReadTrieNode(diskdb, owner, inner, nodeHash, scheme) rawdb.DeleteTrieNode(diskdb, owner, inner, nodeHash, scheme) - if err := checkTrieConsistency(diskdb, srcDb.Scheme(), root, false); err == nil { + if err := checkTrieConsistency(diskdb, srcDb.Scheme(), root, false, resolver); err == nil { t.Fatalf("trie inconsistency not caught, missing: %x", path) } rawdb.WriteTrieNode(diskdb, owner, inner, nodeHash, value, scheme) + // We only add nodes to the cache that we already visited + if scheme == rawdb.HashScheme { + nodesCache[string(nodeHash[:])] = value + } else { + nodesCache[string(inner[:])] = value + } } } From cb349b0fa8ee0db2fe46032b1b8778f1f8e7718f Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Fri, 3 Nov 2023 11:49:55 +0100 Subject: [PATCH 3/7] core/forkid: speed up validation test This takes the validation test from > 5s to sub 1 sec --- core/forkid/forkid_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/forkid/forkid_test.go b/core/forkid/forkid_test.go index db634bc14b90..e311c0b43f89 100644 --- a/core/forkid/forkid_test.go +++ b/core/forkid/forkid_test.go @@ -366,8 +366,9 @@ func TestValidation(t *testing.T) { // TODO(karalabe): Enable this when Cancun is specced //{params.MainnetChainConfig, 20999999, 1677999999, ID{Hash: checksumToBytes(0x71147644), Next: 1678000000}, ErrLocalIncompatibleOrStale}, } + genesis := core.DefaultGenesisBlock().ToBlock() for i, tt := range tests { - filter := newFilter(tt.config, core.DefaultGenesisBlock().ToBlock(), func() (uint64, uint64) { return tt.head, tt.time }) + filter := newFilter(tt.config, genesis, func() (uint64, uint64) { return tt.head, tt.time }) if err := filter(tt.id); err != tt.err { t.Errorf("test %d: validation error mismatch: have %v, want %v", i, err, tt.err) } From be785658bfff30f340dc77c93226598f95401893 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Fri, 3 Nov 2023 12:34:14 +0100 Subject: [PATCH 4/7] core/state: improve snapshot test run brings the time for TestSnapshotRandom from 13s down to 6s --- core/state/statedb_test.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/core/state/statedb_test.go b/core/state/statedb_test.go index ad829a0c8f09..df1cd5547d3d 100644 --- a/core/state/statedb_test.go +++ b/core/state/statedb_test.go @@ -426,10 +426,12 @@ func (test *snapshotTest) run() bool { state, _ = New(types.EmptyRootHash, NewDatabase(rawdb.NewMemoryDatabase()), nil) snapshotRevs = make([]int, len(test.snapshots)) sindex = 0 + checkstates = make([]*StateDB, len(test.snapshots)) ) for i, action := range test.actions { if len(test.snapshots) > sindex && i == test.snapshots[sindex] { snapshotRevs[sindex] = state.Snapshot() + checkstates[sindex] = state.Copy() sindex++ } action.fn(action, state) @@ -437,12 +439,8 @@ func (test *snapshotTest) run() bool { // Revert all snapshots in reverse order. Each revert must yield a state // that is equivalent to fresh state with all actions up the snapshot applied. for sindex--; sindex >= 0; sindex-- { - checkstate, _ := New(types.EmptyRootHash, state.Database(), nil) - for _, action := range test.actions[:test.snapshots[sindex]] { - action.fn(action, checkstate) - } state.RevertToSnapshot(snapshotRevs[sindex]) - if err := test.checkEqual(state, checkstate); err != nil { + if err := test.checkEqual(state, checkstates[sindex]); err != nil { test.err = fmt.Errorf("state mismatch after revert to snapshot %d\n%v", sindex, err) return false } From 62a32916ee3915744ec5bfbe1e0db9a1f8764c54 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Fri, 3 Nov 2023 13:54:17 +0100 Subject: [PATCH 5/7] accounts/keystore: improve keyfile test This removes some unnecessary waits and reduces the runtime of TestUpdatedKeyfileContents from 5 to 3 seconds --- accounts/keystore/account_cache_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/accounts/keystore/account_cache_test.go b/accounts/keystore/account_cache_test.go index 3847e9daf6ae..371d274441e7 100644 --- a/accounts/keystore/account_cache_test.go +++ b/accounts/keystore/account_cache_test.go @@ -68,7 +68,7 @@ func waitWatcherStart(ks *KeyStore) bool { func waitForAccounts(wantAccounts []accounts.Account, ks *KeyStore) error { var list []accounts.Account - for t0 := time.Now(); time.Since(t0) < 5*time.Second; time.Sleep(200 * time.Millisecond) { + for t0 := time.Now(); time.Since(t0) < 5*time.Second; time.Sleep(100 * time.Millisecond) { list = ks.Accounts() if reflect.DeepEqual(list, wantAccounts) { // ks should have also received change notifications @@ -350,7 +350,7 @@ func TestUpdatedKeyfileContents(t *testing.T) { return } // needed so that modTime of `file` is different to its current value after forceCopyFile - time.Sleep(time.Second) + os.Chtimes(file, time.Now().Add(-time.Second), time.Now().Add(-time.Second)) // Now replace file contents if err := forceCopyFile(file, cachetestAccounts[1].URL.Path); err != nil { @@ -366,7 +366,7 @@ func TestUpdatedKeyfileContents(t *testing.T) { } // needed so that modTime of `file` is different to its current value after forceCopyFile - time.Sleep(time.Second) + os.Chtimes(file, time.Now().Add(-time.Second), time.Now().Add(-time.Second)) // Now replace file contents again if err := forceCopyFile(file, cachetestAccounts[2].URL.Path); err != nil { @@ -382,7 +382,7 @@ func TestUpdatedKeyfileContents(t *testing.T) { } // needed so that modTime of `file` is different to its current value after os.WriteFile - time.Sleep(time.Second) + os.Chtimes(file, time.Now().Add(-time.Second), time.Now().Add(-time.Second)) // Now replace file contents with crap if err := os.WriteFile(file, []byte("foo"), 0600); err != nil { From 08be7f116a6425b367ec1bbe6576c90dfb18be1a Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Wed, 15 Nov 2023 14:36:40 +0100 Subject: [PATCH 6/7] trie: remove resolver --- trie/sync_test.go | 28 +++++----------------------- 1 file changed, 5 insertions(+), 23 deletions(-) diff --git a/trie/sync_test.go b/trie/sync_test.go index 4485c5293f13..d9b4f93c1bb6 100644 --- a/trie/sync_test.go +++ b/trie/sync_test.go @@ -73,7 +73,7 @@ func makeTestTrie(scheme string) (ethdb.Database, *Database, *StateTrie, map[str func checkTrieContents(t *testing.T, db ethdb.Database, scheme string, root []byte, content map[string][]byte, rawTrie bool) { // Check root availability and trie contents ndb := newTestDatabase(db, scheme) - if err := checkTrieConsistency(db, scheme, common.BytesToHash(root), rawTrie, nil); err != nil { + if err := checkTrieConsistency(db, scheme, common.BytesToHash(root), rawTrie); err != nil { t.Fatalf("inconsistent trie at %x: %v", root, err) } type reader interface { @@ -101,7 +101,7 @@ func checkTrieContents(t *testing.T, db ethdb.Database, scheme string, root []by } // checkTrieConsistency checks that all nodes in a trie are indeed present. -func checkTrieConsistency(db ethdb.Database, scheme string, root common.Hash, rawTrie bool, resolver NodeResolver) error { +func checkTrieConsistency(db ethdb.Database, scheme string, root common.Hash, rawTrie bool) error { ndb := newTestDatabase(db, scheme) var it NodeIterator if rawTrie { @@ -117,9 +117,6 @@ func checkTrieConsistency(db ethdb.Database, scheme string, root common.Hash, ra } it = trie.MustNodeIterator(nil) } - if resolver != nil { - it.AddResolver(resolver) - } for it.Next(true) { } return it.Error() @@ -515,8 +512,8 @@ func testDuplicateAvoidanceSync(t *testing.T, scheme string) { // Tests that at any point in time during a sync, only complete sub-tries are in // the database. func TestIncompleteSyncHash(t *testing.T) { - testIncompleteSync(t, rawdb.HashScheme) // 9.585s - testIncompleteSync(t, rawdb.PathScheme) // 19.522 + testIncompleteSync(t, rawdb.HashScheme) + testIncompleteSync(t, rawdb.PathScheme) } func testIncompleteSync(t *testing.T, scheme string) { @@ -589,31 +586,16 @@ func testIncompleteSync(t *testing.T, scheme string) { } } - // Cache trie nodes for faster verification - nodesCache := make(map[string][]byte) - resolver := func(owner common.Hash, path []byte, hash common.Hash) []byte { - if scheme == rawdb.HashScheme { - return nodesCache[string(hash[:])] - } - return nodesCache[string(path)] - } - // Sanity check that removing any node from the database is detected for i, path := range addedKeys { owner, inner := ResolvePath([]byte(path)) nodeHash := addedHashes[i] value := rawdb.ReadTrieNode(diskdb, owner, inner, nodeHash, scheme) rawdb.DeleteTrieNode(diskdb, owner, inner, nodeHash, scheme) - if err := checkTrieConsistency(diskdb, srcDb.Scheme(), root, false, resolver); err == nil { + if err := checkTrieConsistency(diskdb, srcDb.Scheme(), root, false); err == nil { t.Fatalf("trie inconsistency not caught, missing: %x", path) } rawdb.WriteTrieNode(diskdb, owner, inner, nodeHash, value, scheme) - // We only add nodes to the cache that we already visited - if scheme == rawdb.HashScheme { - nodesCache[string(nodeHash[:])] = value - } else { - nodesCache[string(inner[:])] = value - } } } From 65bebf30ec41b4f349e00219a44305651ac05582 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Wed, 15 Nov 2023 14:40:15 +0100 Subject: [PATCH 7/7] trie: only check ~5% of all trie nodes --- trie/sync_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/trie/sync_test.go b/trie/sync_test.go index d9b4f93c1bb6..0e7a7a84f597 100644 --- a/trie/sync_test.go +++ b/trie/sync_test.go @@ -19,6 +19,7 @@ package trie import ( "bytes" "fmt" + "math/rand" "testing" "github.com/ethereum/go-ethereum/common" @@ -585,9 +586,12 @@ func testIncompleteSync(t *testing.T, scheme string) { }) } } - // Sanity check that removing any node from the database is detected for i, path := range addedKeys { + if rand.Int31n(100) > 5 { + // Only check 5 percent of added keys as a sanity check + continue + } owner, inner := ResolvePath([]byte(path)) nodeHash := addedHashes[i] value := rawdb.ReadTrieNode(diskdb, owner, inner, nodeHash, scheme)