Changes based on PR comments

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
 	}

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)
+		}
 	}
 
 }