feat: prevent incompatible alan version downgrade (#1762)

* draft

* get the alan network name from network config only

* clean

* lint

* fix testNetworkName

* if config is not found, mark the migration as completed

* make alanforkname const

cli/operator/node.go

@@ -168,7 +168,9 @@ var StartNodeCmd = &cobra.Command{
 
 		usingLocalEvents := len(cfg.LocalEventsPath) != 0
 
-		verifyConfig(logger, nodeStorage, networkConfig.Name, usingLocalEvents)
+		if err := validateConfig(nodeStorage, networkConfig.AlanForkNetworkName(), usingLocalEvents); err != nil {
+			logger.Fatal("failed to validate config", zap.Error(err))
+		}
 
 		ekmHashedKey, err := operatorPrivKey.EKMHash()
 		if err != nil {
@@ -420,10 +422,10 @@ var StartNodeCmd = &cobra.Command{
 	},
 }
 
-func verifyConfig(logger *zap.Logger, nodeStorage operatorstorage.Storage, networkName string, usingLocalEvents bool) {
+func validateConfig(nodeStorage operatorstorage.Storage, networkName string, usingLocalEvents bool) error {
 	storedConfig, foundConfig, err := nodeStorage.GetConfig(nil)
 	if err != nil {
-		logger.Fatal("could not check saved local events config", zap.Error(err))
+		return fmt.Errorf("failed to get stored config: %w", err)
 	}
 
 	currentConfig := &operatorstorage.ConfigLock{
@@ -432,16 +434,16 @@ func verifyConfig(logger *zap.Logger, nodeStorage operatorstorage.Storage, netwo
 	}
 
 	if foundConfig {
-		if err := storedConfig.EnsureSameWith(currentConfig); err != nil {
-			err = fmt.Errorf("incompatible config change: %w", err)
-			logger.Fatal(err.Error())
+		if err := storedConfig.ValidateCompatibility(currentConfig); err != nil {
+			return fmt.Errorf("incompatible config change: %w", err)
 		}
 	} else {
 		if err := nodeStorage.SaveConfig(nil, currentConfig); err != nil {
-			err = fmt.Errorf("failed to store config: %w", err)
-			logger.Fatal(err.Error())
+			return fmt.Errorf("failed to store config: %w", err)
 		}
 	}
+
+	return nil
 }
 
 func init() {

cli/operator/node_test.go

@@ -22,14 +22,14 @@ func Test_verifyConfig(t *testing.T) {
 	nodeStorage, err := operatorstorage.NewNodeStorage(logger, db)
 	require.NoError(t, err)
 
-	testNetworkName := networkconfig.TestNetwork.Name
+	testNetworkName := networkconfig.TestNetwork.AlanForkNetworkName()
 
 	t.Run("no config in DB", func(t *testing.T) {
 		c := &operatorstorage.ConfigLock{
 			NetworkName:      testNetworkName,
 			UsingLocalEvents: true,
 		}
-		verifyConfig(logger, nodeStorage, c.NetworkName, c.UsingLocalEvents)
+		require.NoError(t, validateConfig(nodeStorage, c.NetworkName, c.UsingLocalEvents))
 
 		storedConfig, found, err := nodeStorage.GetConfig(nil)
 		require.NoError(t, err)
@@ -45,8 +45,7 @@ func Test_verifyConfig(t *testing.T) {
 			UsingLocalEvents: true,
 		}
 		require.NoError(t, nodeStorage.SaveConfig(nil, c))
-
-		verifyConfig(logger, nodeStorage, testNetworkName, true)
+		require.NoError(t, validateConfig(nodeStorage, c.NetworkName, c.UsingLocalEvents))
 
 		storedConfig, found, err := nodeStorage.GetConfig(nil)
 		require.NoError(t, err)
@@ -62,10 +61,9 @@ func Test_verifyConfig(t *testing.T) {
 			UsingLocalEvents: false,
 		}
 		require.NoError(t, nodeStorage.SaveConfig(nil, c))
-
-		require.PanicsWithValue(t,
-			"incompatible config change: can't change network from \"testnet1\" to \"testnet\" in an existing database, it must be removed first",
-			func() { verifyConfig(logger, nodeStorage, testNetworkName, true) },
+		require.ErrorContains(t,
+			validateConfig(nodeStorage, testNetworkName, true),
+			"incompatible config change: network mismatch. Stored network testnet:alan1 does not match current network testnet:alan. The database must be removed or reinitialized",
 		)
 
 		storedConfig, found, err := nodeStorage.GetConfig(nil)
@@ -82,10 +80,9 @@ func Test_verifyConfig(t *testing.T) {
 			UsingLocalEvents: true,
 		}
 		require.NoError(t, nodeStorage.SaveConfig(nil, c))
-
-		require.PanicsWithValue(t,
-			"incompatible config change: can't change network from \"testnet1\" to \"testnet\" in an existing database, it must be removed first",
-			func() { verifyConfig(logger, nodeStorage, testNetworkName, true) },
+		require.ErrorContains(t,
+			validateConfig(nodeStorage, testNetworkName, c.UsingLocalEvents),
+			"incompatible config change: network mismatch. Stored network testnet:alan1 does not match current network testnet:alan. The database must be removed or reinitialized",
 		)
 
 		storedConfig, found, err := nodeStorage.GetConfig(nil)
@@ -102,10 +99,9 @@ func Test_verifyConfig(t *testing.T) {
 			UsingLocalEvents: false,
 		}
 		require.NoError(t, nodeStorage.SaveConfig(nil, c))
-
-		require.PanicsWithValue(t,
-			"incompatible config change: can't switch on localevents, database must be removed first",
-			func() { verifyConfig(logger, nodeStorage, testNetworkName, true) },
+		require.ErrorContains(t,
+			validateConfig(nodeStorage, c.NetworkName, true),
+			"incompatible config change: enabling local events is not allowed. The database must be removed or reinitialized",
 		)
 
 		storedConfig, found, err := nodeStorage.GetConfig(nil)
@@ -122,10 +118,9 @@ func Test_verifyConfig(t *testing.T) {
 			UsingLocalEvents: true,
 		}
 		require.NoError(t, nodeStorage.SaveConfig(nil, c))
-
-		require.PanicsWithValue(t,
-			"incompatible config change: can't switch off localevents, database must be removed first",
-			func() { verifyConfig(logger, nodeStorage, testNetworkName, false) },
+		require.ErrorContains(t,
+			validateConfig(nodeStorage, c.NetworkName, false),
+			"incompatible config change: disabling local events is not allowed. The database must be removed or reinitialized",
 		)
 
 		storedConfig, found, err := nodeStorage.GetConfig(nil)

migrations/migration_4_configlock_add_alan_fork_to_network_name.go

@@ -0,0 +1,44 @@
+package migrations
+
+import (
+	"context"
+	"fmt"
+
+	"go.uber.org/zap"
+
+	"github.com/ssvlabs/ssv/networkconfig"
+	"github.com/ssvlabs/ssv/storage/basedb"
+)
+
+// This migration adds the Alan fork name to the network name
+var migration_4_configlock_add_alan_fork_to_network_name = Migration{
+	Name: "migration_4_configlock_add_alan_fork_to_network_name",
+	Run: func(ctx context.Context, logger *zap.Logger, opt Options, key []byte, completed CompletedFunc) error {
+		return opt.Db.Update(func(txn basedb.Txn) error {
+			nodeStorage, err := opt.nodeStorage(logger)
+			if err != nil {
+				return fmt.Errorf("failed to get node storage: %w", err)
+			}
+
+			config, found, err := nodeStorage.GetConfig(txn)
+			if err != nil {
+				return fmt.Errorf("failed to get config: %w", err)
+			}
+
+			// If config is not found, it means the node is not initialized yet
+			if found {
+				networkConfig, err := networkconfig.GetNetworkConfigByName(config.NetworkName)
+				if err != nil {
+					return fmt.Errorf("failed to get network config by name: %w", err)
+				}
+
+				config.NetworkName = networkConfig.AlanForkNetworkName()
+				if err := nodeStorage.SaveConfig(txn, config); err != nil {
+					return fmt.Errorf("failed to save config: %w", err)
+				}
+			}
+
+			return completed(txn)
+		})
+	},
+}

migrations/migrations.go

@@ -26,6 +26,7 @@ var (
 		migration_1_example,
 		migration_2_encrypt_shares,
 		migration_3_drop_registry_data,
+		migration_4_configlock_add_alan_fork_to_network_name,
 	}
 )
 

networkconfig/config.go

@@ -9,6 +9,7 @@ import (
 	"github.com/attestantio/go-eth2-client/spec/phase0"
 
 	spectypes "github.com/ssvlabs/ssv-spec/types"
+
 	"github.com/ssvlabs/ssv/protocol/v2/blockchain/beacon"
 )
 
@@ -20,6 +21,8 @@ var SupportedConfigs = map[string]NetworkConfig{
 	HoleskyE2E.Name:   HoleskyE2E,
 }
 
+const alanForkName = "alan"
+
 func GetNetworkConfigByName(name string) (NetworkConfig, error) {
 	if network, ok := SupportedConfigs[name]; ok {
 		return network, nil
@@ -57,6 +60,10 @@ func (n NetworkConfig) String() string {
 	return string(b)
 }
 
+func (n NetworkConfig) AlanForkNetworkName() string {
+	return fmt.Sprintf("%s:%s", n.Name, alanForkName)
+}
+
 func (n NetworkConfig) PastAlanFork() bool {
 	return n.Beacon.EstimatedCurrentEpoch() >= n.AlanForkEpoch
 }

operator/storage/config_lock.go

@@ -9,18 +9,17 @@ type ConfigLock struct {
 	UsingLocalEvents bool   `json:"using_local_events"`
 }
 
-func (stored *ConfigLock) EnsureSameWith(current *ConfigLock) error {
+func (stored *ConfigLock) ValidateCompatibility(current *ConfigLock) error {
 	if stored.NetworkName != current.NetworkName {
-		return fmt.Errorf("can't change network from %q to %q in an existing database, it must be removed first",
-			stored.NetworkName, current.NetworkName)
+		return fmt.Errorf("network mismatch. Stored network %s does not match current network %s. The database must be removed or reinitialized", stored.NetworkName, current.NetworkName)
 	}
 
 	if stored.UsingLocalEvents && !current.UsingLocalEvents {
-		return fmt.Errorf("can't switch off localevents, database must be removed first")
+		return fmt.Errorf("disabling local events is not allowed. The database must be removed or reinitialized")
 	}
 
 	if !stored.UsingLocalEvents && current.UsingLocalEvents {
-		return fmt.Errorf("can't switch on localevents, database must be removed first")
+		return fmt.Errorf("enabling local events is not allowed. The database must be removed or reinitialized")
 	}
 
 	return nil

operator/storage/config_lock_test.go

@@ -18,7 +18,7 @@ func TestConfigLock(t *testing.T) {
 			UsingLocalEvents: true,
 		}
 
-		require.NoError(t, c1.EnsureSameWith(c2))
+		require.NoError(t, c1.ValidateCompatibility(c2))
 	})
 
 	t.Run("all fields are different", func(t *testing.T) {
@@ -32,7 +32,7 @@ func TestConfigLock(t *testing.T) {
 			UsingLocalEvents: false,
 		}
 
-		require.Error(t, c1.EnsureSameWith(c2))
+		require.Error(t, c1.ValidateCompatibility(c2))
 	})
 
 	t.Run("only network name is different", func(t *testing.T) {
@@ -46,7 +46,7 @@ func TestConfigLock(t *testing.T) {
 			UsingLocalEvents: true,
 		}
 
-		require.Error(t, c1.EnsureSameWith(c2))
+		require.Error(t, c1.ValidateCompatibility(c2))
 	})
 
 	t.Run("only local events usage is different", func(t *testing.T) {
@@ -60,6 +60,6 @@ func TestConfigLock(t *testing.T) {
 			UsingLocalEvents: false,
 		}
 
-		require.Error(t, c1.EnsureSameWith(c2))
+		require.Error(t, c1.ValidateCompatibility(c2))
 	})
 }