From c57bbae650dce5545ea27064bd6f123fe36ce30b Mon Sep 17 00:00:00 2001 From: Aman Sanghi Date: Thu, 9 May 2024 20:06:11 +0530 Subject: [PATCH] Changes based on PR comments --- arbnode/node.go | 29 ++++++++++------------------- system_tests/snap_sync_test.go | 31 +++++++++++++++++++++++-------- 2 files changed, 33 insertions(+), 27 deletions(-) diff --git a/arbnode/node.go b/arbnode/node.go index 65a3bb2d7d..c346a38e14 100644 --- a/arbnode/node.go +++ b/arbnode/node.go @@ -93,7 +93,8 @@ type Config struct { TransactionStreamer TransactionStreamerConfig `koanf:"transaction-streamer" reload:"hot"` Maintenance MaintenanceConfig `koanf:"maintenance" reload:"hot"` ResourceMgmt resourcemanager.Config `koanf:"resource-mgmt" reload:"hot"` - SnapSync SnapSyncConfig `koanf:"snap-sync"` + // SnapSyncConfig is only used for testing purposes, these should not be configured in production. + SnapSyncTest SnapSyncConfig } func (c *Config) Validate() error { @@ -157,7 +158,6 @@ func ConfigAddOptions(prefix string, f *flag.FlagSet, feedInputEnable bool, feed DangerousConfigAddOptions(prefix+".dangerous", f) TransactionStreamerConfigAddOptions(prefix+".transaction-streamer", f) MaintenanceConfigAddOptions(prefix+".maintenance", f) - SnapSyncConfigAddOptions(prefix+".snap-sync", f) } var ConfigDefault = Config{ @@ -177,7 +177,7 @@ var ConfigDefault = Config{ TransactionStreamer: DefaultTransactionStreamerConfig, ResourceMgmt: resourcemanager.DefaultConfig, Maintenance: DefaultMaintenanceConfig, - SnapSync: DefaultSnapSyncConfig, + SnapSyncTest: DefaultSnapSyncConfig, } func ConfigDefaultL1Test() *Config { @@ -277,11 +277,11 @@ type Node struct { } type SnapSyncConfig struct { - Enabled bool `koanf:"enabled"` - PrevBatchMessageCount uint64 `koanf:"prev-batch-message-count"` - PrevDelayedRead uint64 `koanf:"prev-delayed-read"` - BatchCount uint64 `koanf:"batch-count"` - DelayedCount uint64 `koanf:"delayed-count"` + Enabled bool + PrevBatchMessageCount uint64 + PrevDelayedRead uint64 + BatchCount uint64 + DelayedCount uint64 } var DefaultSnapSyncConfig = SnapSyncConfig{ @@ -292,15 +292,6 @@ var DefaultSnapSyncConfig = SnapSyncConfig{ PrevDelayedRead: 0, } -func SnapSyncConfigAddOptions(prefix string, f *flag.FlagSet) { - f.Bool(prefix+".enabled", DefaultSnapSyncConfig.Enabled, "enable snap sync") - f.Uint64(prefix+".prev-batch-message-count", DefaultSnapSyncConfig.PrevBatchMessageCount, "previous batch message count") - f.Uint64(prefix+".batch-count", DefaultSnapSyncConfig.BatchCount, "batch count") - f.Uint64(prefix+".delayed-count", DefaultSnapSyncConfig.DelayedCount, "delayed count") - f.Uint64(prefix+".prev-delayed-read", DefaultSnapSyncConfig.PrevDelayedRead, "previous delayed read") - -} - type ConfigFetcher interface { Get() *Config Start(context.Context) @@ -438,7 +429,7 @@ func createNodeImpl( } transactionStreamerConfigFetcher := func() *TransactionStreamerConfig { return &configFetcher.Get().TransactionStreamer } - snapSyncConfigFetcher := func() *SnapSyncConfig { return &configFetcher.Get().SnapSync } + snapSyncConfigFetcher := func() *SnapSyncConfig { return &configFetcher.Get().SnapSyncTest } txStreamer, err := NewTransactionStreamer(arbDb, l2Config, exec, broadcastServer, fatalErrChan, transactionStreamerConfigFetcher, snapSyncConfigFetcher) if err != nil { return nil, err @@ -569,7 +560,7 @@ func createNodeImpl( if blobReader != nil { dapReaders = append(dapReaders, daprovider.NewReaderForBlobReader(blobReader)) } - inboxTracker, err := NewInboxTracker(arbDb, txStreamer, dapReaders, config.SnapSync) + inboxTracker, err := NewInboxTracker(arbDb, txStreamer, dapReaders, config.SnapSyncTest) if err != nil { return nil, err } diff --git a/system_tests/snap_sync_test.go b/system_tests/snap_sync_test.go index 3b052fda36..87bf09f6d7 100644 --- a/system_tests/snap_sync_test.go +++ b/system_tests/snap_sync_test.go @@ -55,11 +55,11 @@ func TestSnapSync(t *testing.T) { } // Wait for nodeB to sync up to the first node for { - count, err := builder.L2.ConsensusNode.InboxTracker.GetBatchCount() + header, err := builder.L2.Client.HeaderByNumber(ctx, nil) Require(t, err) - countNodeB, err := nodeB.ConsensusNode.InboxTracker.GetBatchCount() + headerNodeB, err := nodeB.Client.HeaderByNumber(ctx, nil) Require(t, err) - if count == countNodeB { + if header.Number.Cmp(headerNodeB.Number) == 0 { break } else { <-time.After(10 * time.Millisecond) @@ -77,11 +77,11 @@ func TestSnapSync(t *testing.T) { Require(t, err) // Create a config with snap sync enabled and same database directory as the 2nd node nodeConfig := builder.nodeConfig - nodeConfig.SnapSync.Enabled = true - nodeConfig.SnapSync.BatchCount = batchCount - nodeConfig.SnapSync.DelayedCount = delayedCount - nodeConfig.SnapSync.PrevDelayedRead = prevMessage.DelayedMessagesRead - nodeConfig.SnapSync.PrevBatchMessageCount = uint64(prevBatchMetaData.MessageCount) + nodeConfig.SnapSyncTest.Enabled = true + nodeConfig.SnapSyncTest.BatchCount = batchCount + nodeConfig.SnapSyncTest.DelayedCount = delayedCount + nodeConfig.SnapSyncTest.PrevDelayedRead = prevMessage.DelayedMessagesRead + nodeConfig.SnapSyncTest.PrevBatchMessageCount = uint64(prevBatchMetaData.MessageCount) // Cleanup the message data of 2nd node, but keep the block state data. // This is to simulate a snap sync environment where we’ve just gotten the block state but don’t have any messages. err = os.RemoveAll(nodeB.ConsensusNode.Stack.ResolvePath("arbitrumdata")) @@ -126,6 +126,21 @@ func TestSnapSync(t *testing.T) { } else { <-time.After(10 * time.Millisecond) } + + header, err := builder.L2.Client.HeaderByNumber(ctx, nil) + Require(t, err) + headerNodeB, err := nodeB.Client.HeaderByNumber(ctx, nil) + Require(t, err) + if header.Number.Cmp(headerNodeB.Number) == 0 { + // Once the node is synced up, check if the block hash is the same for the last block + // This is to ensure that the snap sync worked correctly + if header.Hash().Cmp(headerNodeB.Hash()) != 0 { + t.Error("Block hash mismatch") + } + break + } else { + <-time.After(10 * time.Millisecond) + } } }