cmd: fix create cluster regression

cmd/createcluster.go

@@ -132,20 +132,21 @@ func runCreateCluster(ctx context.Context, w io.Writer, conf clusterConfig) erro
 
 	var secrets []tbls.PrivateKey
 
+	// If we're splitting keys, read them from SplitKeysDir and set conf.NumDVs to the amount of
+	// secrets we read.
+	// If SplitKeys wasn't set, we wouldn't have reached this part of code because validateCreateConfig()
+	// would've already errored.
 	if conf.SplitKeys {
 		secrets, err = getKeys(conf.SplitKeysDir)
 		if err != nil {
 			return err
 		}
 
+		// Needed if --split-existing-keys is called without a definition file.
 		conf.NumDVs = len(secrets)
-	} else {
-		secrets, err = generateKeys(conf.NumDVs)
-		if err != nil {
-			return err
-		}
 	}
 
+	// Get a cluster definition, either from a definition file or from the config.
 	var def cluster.Definition
 	if conf.DefFile != "" { // Load definition from DefFile
 		def, err = loadDefinition(ctx, conf.DefFile)
@@ -159,15 +160,16 @@ func runCreateCluster(ctx context.Context, w io.Writer, conf clusterConfig) erro
 		}
 	}
 
-	if def.NumValidators != conf.NumDVs {
-		errTmpl := "provided cluster definition doesn't contain the same amount of validators"
-
-		err := errors.New(errTmpl + " as specified in --num-validators")
-		if conf.SplitKeys {
-			err = errors.New(errTmpl + " contained in --split-keys-dir")
+	if len(secrets) == 0 {
+		// this is the case in which split-keys is undefined and user passed validator amount on CLI
+		secrets, err = generateKeys(def.NumValidators)
+		if err != nil {
+			return err
 		}
+	}
 
-		return err
+	if len(secrets) != def.NumValidators {
+		return errors.New("amount of keys read from disk differs from cluster definition", z.Int("disk", len(secrets)), z.Int("definition", def.NumValidators))
 	}
 
 	numNodes := len(def.Operators)
@@ -189,8 +191,8 @@ func runCreateCluster(ctx context.Context, w io.Writer, conf clusterConfig) erro
 		return errors.Wrap(err, "mkdir")
 	}
 
-	// Create operators
-	ops, opsKeys, err := getOperators(numNodes, conf.ClusterDir)
+	// Create operators and their enr node keys
+	ops, nodeKeys, err := getOperators(numNodes, conf.ClusterDir)
 	if err != nil {
 		return err
 	}
@@ -246,18 +248,15 @@ func runCreateCluster(ctx context.Context, w io.Writer, conf clusterConfig) erro
 		return err
 	}
 
-	var opsLockSigs [][]byte
-	for _, opKey := range opsKeys {
-		sig, err := k1util.Sign(opKey, lock.LockHash)
+	for _, opKey := range nodeKeys {
+		nodeSig, err := k1util.Sign(opKey, lock.LockHash)
 		if err != nil {
 			return err
 		}
 
-		opsLockSigs = append(opsLockSigs, sig)
+		lock.NodeSignatures = append(lock.NodeSignatures, nodeSig)
 	}
 
-	lock.NodeSignatures = opsLockSigs
-
 	// Write cluster-lock file
 	if conf.Publish {
 		if err = writeLockToAPI(ctx, conf.PublishAddr, lock); err != nil {
@@ -278,7 +277,7 @@ func runCreateCluster(ctx context.Context, w io.Writer, conf clusterConfig) erro
 
 // validateCreateConfig returns an error if any of the provided config parameters are invalid.
 func validateCreateConfig(conf clusterConfig) error {
-	if conf.NumNodes == 0 {
+	if conf.NumNodes == 0 && conf.DefFile == "" { // if there's a definition file, infer this value from it later
 		return errors.New("missing --nodes flag")
 	}
 
@@ -300,7 +299,7 @@ func validateCreateConfig(conf clusterConfig) error {
 			return errors.New("can't specify --num-validators with --split-existing-keys. Please fix configuration flags")
 		}
 	} else {
-		if conf.NumDVs == 0 {
+		if conf.NumDVs == 0 && conf.DefFile == "" { // if there's a definition file, infer this value from it later
 			return errors.New("missing --num-validators flag")
 		}
 	}

cmd/createcluster_internal_test.go

@@ -48,6 +48,7 @@ func TestCreateCluster(t *testing.T) {
 				NumNodes:  4,
 				Threshold: 3,
 				NumDVs:    1,
+				Network:   eth2util.Goerli.Name,
 			},
 		},
 		{
@@ -56,6 +57,7 @@ func TestCreateCluster(t *testing.T) {
 				NumNodes:  4,
 				Threshold: 3,
 				SplitKeys: true,
+				Network:   eth2util.Goerli.Name,
 			},
 			Prep: func(t *testing.T, config clusterConfig) clusterConfig {
 				t.Helper()
@@ -76,8 +78,59 @@ func TestCreateCluster(t *testing.T) {
 			},
 		},
 		{
-			Name:        "missing nodes amount flag",
-			Config:      clusterConfig{},
+			Name: "splitkeys with cluster definition",
+			Config: clusterConfig{
+				DefFile:   defPath,
+				SplitKeys: true,
+				Network:   defaultNetwork,
+			},
+			Prep: func(t *testing.T, config clusterConfig) clusterConfig {
+				t.Helper()
+
+				keyDir := t.TempDir()
+
+				secret1, err := tbls.GenerateSecretKey()
+				require.NoError(t, err)
+				secret2, err := tbls.GenerateSecretKey()
+				require.NoError(t, err)
+
+				err = keystore.StoreKeysInsecure([]tbls.PrivateKey{secret1, secret2}, keyDir, keystore.ConfirmInsecureKeys)
+				require.NoError(t, err)
+
+				config.SplitKeysDir = keyDir
+
+				return config
+			},
+		},
+		{
+			Name: "splitkeys with cluster definition, but amount of keys read from disk differ",
+			Config: clusterConfig{
+				DefFile:   defPath,
+				SplitKeys: true,
+				Network:   defaultNetwork,
+			},
+			Prep: func(t *testing.T, config clusterConfig) clusterConfig {
+				t.Helper()
+
+				keyDir := t.TempDir()
+
+				secret1, err := tbls.GenerateSecretKey()
+				require.NoError(t, err)
+
+				err = keystore.StoreKeysInsecure([]tbls.PrivateKey{secret1}, keyDir, keystore.ConfirmInsecureKeys)
+				require.NoError(t, err)
+
+				config.SplitKeysDir = keyDir
+
+				return config
+			},
+			expectedErr: "amount of keys read from disk differs from cluster definition",
+		},
+		{
+			Name: "missing nodes amount flag",
+			Config: clusterConfig{
+				Network: defaultNetwork,
+			},
 			expectedErr: "missing --nodes flag",
 		},
 		{
@@ -120,16 +173,14 @@ func TestCreateCluster(t *testing.T) {
 		{
 			Name: "solo flow definition from disk",
 			Config: clusterConfig{
-				DefFile:  defPath,
-				NumDVs:   2,
-				NumNodes: 4,
+				DefFile: defPath,
+				Network: eth2util.Goerli.Name,
 			},
 		},
 		{
 			Name: "solo flow definition from network",
 			Config: clusterConfig{
-				NumDVs:   2,
-				NumNodes: 4,
+				Network: eth2util.Goerli.Name,
 			},
 			defFileProvider: func() []byte {
 				data, err := json.Marshal(def)
@@ -138,61 +189,6 @@ func TestCreateCluster(t *testing.T) {
 				return data
 			},
 		},
-		{
-			Name: "number of validators in def file differs from --num-validators",
-			Config: clusterConfig{
-				NumDVs:   2,
-				NumNodes: 4,
-				Network:  defaultNetwork,
-			},
-			defFileProvider: func() []byte {
-				def := def
-
-				def.NumValidators = 3
-				def.ValidatorAddresses = make([]cluster.ValidatorAddresses, 3)
-				data, err := json.Marshal(def)
-				require.NoError(t, err)
-
-				return data
-			},
-			expectedErr: "provided cluster definition doesn't contain the same amount of validators as specified in --num-validators",
-		},
-		{
-			Name: "number of validators in def file differs from amount of keys in splitkeys",
-			Config: clusterConfig{
-				SplitKeys: true,
-				NumNodes:  4,
-				Network:   defaultNetwork,
-			},
-			defFileProvider: func() []byte {
-				def := def
-
-				def.NumValidators = 3
-				def.ValidatorAddresses = make([]cluster.ValidatorAddresses, 3)
-				data, err := json.Marshal(def)
-				require.NoError(t, err)
-
-				return data
-			},
-			expectedErr: "provided cluster definition doesn't contain the same amount of validators contained in --split-keys-dir",
-			Prep: func(t *testing.T, config clusterConfig) clusterConfig {
-				t.Helper()
-
-				keyDir := t.TempDir()
-
-				secret1, err := tbls.GenerateSecretKey()
-				require.NoError(t, err)
-				secret2, err := tbls.GenerateSecretKey()
-				require.NoError(t, err)
-
-				err = keystore.StoreKeysInsecure([]tbls.PrivateKey{secret1, secret2}, keyDir, keystore.ConfirmInsecureKeys)
-				require.NoError(t, err)
-
-				config.SplitKeysDir = keyDir
-
-				return config
-			},
-		},
 	}
 	for _, test := range tests {
 		t.Run(test.Name, func(t *testing.T) {
@@ -215,10 +211,6 @@ func TestCreateCluster(t *testing.T) {
 			test.Config.WithdrawalAddrs = []string{zeroAddress}
 			test.Config.FeeRecipientAddrs = []string{zeroAddress}
 
-			if test.Config.Network == "" && test.expectedErr == "" {
-				test.Config.Network = eth2util.Goerli.Name
-			}
-
 			testCreateCluster(t, test.Config, def, test.expectedErr)
 		})
 	}
@@ -420,17 +412,7 @@ func TestSplitKeys(t *testing.T) {
 		expectedErrMsg string
 	}{
 		{
-			name:         "split keys from local definition with mismatch NumValidators",
-			numSplitKeys: 2,
-			conf: clusterConfig{
-				DefFile:  "../cluster/examples/cluster-definition-002.json",
-				NumNodes: 4,
-				Network:  defaultNetwork,
-			},
-			expectedErrMsg: "provided cluster definition doesn't contain the same amount of validators contained in --split-keys-dir",
-		},
-		{
-			name:         "split keys from local definition with same NumValidators",
+			name:         "split keys from local definition",
 			numSplitKeys: 1,
 			conf: clusterConfig{
 				DefFile:    "../cluster/examples/cluster-definition-002.json",

cmd/testdata/TestCreateCluster_splitkeys_with_cluster_definition_files.golden

@@ -0,0 +1,22 @@
+[
+ "node0",
+ "node1",
+ "node2",
+ "node3",
+ "node0/charon-enr-private-key",
+ "node0/cluster-lock.json",
+ "node0/deposit-data.json",
+ "node0/validator_keys",
+ "node1/charon-enr-private-key",
+ "node1/cluster-lock.json",
+ "node1/deposit-data.json",
+ "node1/validator_keys",
+ "node2/charon-enr-private-key",
+ "node2/cluster-lock.json",
+ "node2/deposit-data.json",
+ "node2/validator_keys",
+ "node3/charon-enr-private-key",
+ "node3/cluster-lock.json",
+ "node3/deposit-data.json",
+ "node3/validator_keys"
+]

cmd/testdata/TestCreateCluster_splitkeys_with_cluster_definition_output.golden

@@ -0,0 +1,18 @@
+
+***************** WARNING: Splitting keys **********************
+ Please make sure any existing validator has been shut down for
+ at least 2 finalised epochs before starting the charon cluster,
+ otherwise slashing could occur.                               
+****************************************************************
+
+Created charon cluster:
+ --split-existing-keys=true
+
+charon/
+├─ node[0-3]/			Directory for each node
+│  ├─ charon-enr-private-key	Charon networking private key for node authentication
+│  ├─ cluster-lock.json		Cluster lock defines the cluster lock file which is signed by all nodes
+│  ├─ deposit-data.json		Deposit data file is used to activate a Distributed Validator on DV Launchpad
+│  ├─ validator_keys		Validator keystores and password
+│  │  ├─ keystore-*.json	Validator private share key for duty signing
+│  │  ├─ keystore-*.txt		Keystore password files for keystore-*.json