combine: explicit output directory, new output keystore directory format

cmd/combine.go

@@ -11,9 +11,10 @@ import (
 	"github.com/obolnetwork/charon/combine"
 )
 
-func newCombineCmd(runFunc func(ctx context.Context, clusterDir string, force bool) error) *cobra.Command {
+func newCombineCmd(runFunc func(ctx context.Context, clusterDir, outputDir string, force bool) error) *cobra.Command {
 	var (
 		clusterDir string
+		outputDir  string
 		force      bool
 	)
 
@@ -23,24 +24,26 @@ func newCombineCmd(runFunc func(ctx context.Context, clusterDir string, force bo
 		Long:  "Combines the private key shares from a threshold of operators in a distributed validator cluster into a set of validator private keys that can be imported into a standard Ethereum validator client.\n\nWarning: running the resulting private keys in a validator alongside the original distributed validator cluster *will* result in slashing.",
 		Args:  cobra.NoArgs,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return runFunc(cmd.Context(), clusterDir, force)
+			return runFunc(cmd.Context(), clusterDir, outputDir, force)
 		},
 	}
 
 	bindCombineFlags(
 		cmd.Flags(),
 		&clusterDir,
+		&outputDir,
 		&force,
 	)
 
 	return cmd
 }
 
-func newCombineFunc(ctx context.Context, clusterDir string, force bool) error {
-	return combine.Combine(ctx, clusterDir, force)
+func newCombineFunc(ctx context.Context, clusterDir, outputDir string, force bool) error {
+	return combine.Combine(ctx, clusterDir, outputDir, force)
 }
 
-func bindCombineFlags(flags *pflag.FlagSet, clusterDir *string, force *bool) {
-	flags.StringVar(clusterDir, "cluster-dir", ".charon/", `Parent directory containing a number of .charon subdirectories from each node in the cluster.`)
+func bindCombineFlags(flags *pflag.FlagSet, clusterDir, outputDir *string, force *bool) {
+	flags.StringVar(clusterDir, "cluster-dir", ".charon/cluster", `Parent directory containing a number of .charon subdirectories from the required threshold of nodes in the cluster.`)
+	flags.StringVar(outputDir, "output-dir", "./validator_keys", "Directory to output the combined private keys to.")
 	flags.BoolVar(force, "force", false, "Overwrites private keys with the same name if present.")
 }

combine/combine.go

@@ -24,11 +24,29 @@ import (
 // To do so, the user must prepare inputDir as follows:
 //   - place the ".charon" directory in inputDir, renamed to another name
 //
-// Combine will create a new directory named after the public key of each validator key reconstructed, containing each
-// keystore under the "validator_keys" subdirectory.
-func Combine(ctx context.Context, inputDir string, force bool) error {
+// Combine will create a new directory named after "outputDir", which will contain Keystore files.
+func Combine(ctx context.Context, inputDir, outputDir string, force bool) error {
+	if !filepath.IsAbs(outputDir) {
+		fp, err := filepath.Abs(outputDir)
+		if err != nil {
+			return errors.Wrap(err, "cannot make full path from relative output path")
+		}
+
+		outputDir = fp
+	}
+
+	if !filepath.IsAbs(inputDir) {
+		fp, err := filepath.Abs(inputDir)
+		if err != nil {
+			return errors.Wrap(err, "cannot make full path from relative input path")
+		}
+
+		inputDir = fp
+	}
+
 	log.Info(ctx, "Recombining key shares",
 		z.Str("input_dir", inputDir),
+		z.Str("output_dir", outputDir),
 	)
 
 	lock, possibleKeyPaths, err := loadLockfile(inputDir)
@@ -49,6 +67,8 @@ func Combine(ctx context.Context, inputDir string, force bool) error {
 		}
 	}
 
+	var combinedKeys []tblsv2.PrivateKey
+
 	for idx, pkSet := range privkeys {
 		log.Info(ctx, "Recombining key share", z.Int("validator_number", idx))
 		shares, err := secretsToShares(lock, pkSet)
@@ -82,20 +102,17 @@ func Combine(ctx context.Context, inputDir string, force bool) error {
 			return errors.New("generated and lockfile public key for validator DO NOT match", z.Int("validator_number", idx))
 		}
 
-		outPath := filepath.Join(inputDir, val.PublicKeyHex(), "validator_keys")
-		if err := os.MkdirAll(outPath, 0o755); err != nil {
-			return errors.Wrap(err, "output directory creation", z.Int("validator_number", idx))
-		}
+		combinedKeys = append(combinedKeys, secret)
+	}
 
-		ksPath := filepath.Join(outPath, "keystore-0.json")
-		_, err = os.Stat(ksPath)
-		if err == nil && !force {
-			return errors.New("refusing to overwrite existing private key", z.Int("validator_number", idx), z.Str("path", ksPath))
-		}
+	ksPath := filepath.Join(outputDir, "keystore-0.json")
+	_, err = os.Stat(ksPath)
+	if err == nil && !force {
+		return errors.New("refusing to overwrite existing private key", z.Str("path", ksPath))
+	}
 
-		if err := keystore.StoreKeys([]tblsv2.PrivateKey{secret}, outPath); err != nil {
-			return errors.Wrap(err, "cannot store keystore", z.Int("validator_number", idx))
-		}
+	if err := keystore.StoreKeys(combinedKeys, outputDir); err != nil {
+		return errors.Wrap(err, "cannot store keystore")
 	}
 
 	return nil

combine/combine_test.go

@@ -25,7 +25,8 @@ func TestMain(m *testing.M) {
 
 func TestCombineNoLockfile(t *testing.T) {
 	td := t.TempDir()
-	err := combine.Combine(context.Background(), td, false)
+	od := t.TempDir()
+	err := combine.Combine(context.Background(), td, od, false)
 	require.ErrorContains(t, err, "lock file not found")
 }
 
@@ -43,6 +44,7 @@ func TestCombineCannotLoadKeystore(t *testing.T) {
 	}
 
 	dir := t.TempDir()
+	od := t.TempDir()
 
 	// flatten secrets, each validator slice is unpacked in a flat structure
 	var rawSecrets []tblsv2.PrivateKey
@@ -80,7 +82,7 @@ func TestCombineCannotLoadKeystore(t *testing.T) {
 
 	require.NoError(t, os.RemoveAll(filepath.Join(dir, "node0")))
 
-	err := combine.Combine(context.Background(), dir, false)
+	err := combine.Combine(context.Background(), dir, od, false)
 	require.Error(t, err)
 }
 
@@ -117,6 +119,7 @@ func TestCombine(t *testing.T) {
 	}
 
 	dir := t.TempDir()
+	od := t.TempDir()
 
 	// flatten secrets, each validator slice is unpacked in a flat structure
 	var rawSecrets []tblsv2.PrivateKey
@@ -152,14 +155,26 @@ func TestCombine(t *testing.T) {
 		require.NoError(t, json.NewEncoder(lf).Encode(lock))
 	}
 
-	err := combine.Combine(context.Background(), dir, true)
+	err := combine.Combine(context.Background(), dir, od, true)
 	require.NoError(t, err)
 
-	for _, exp := range expectedData {
-		keys, err := keystore.LoadKeys(filepath.Join(dir, exp.pubkey, "validator_keys"))
+	keys, err := keystore.LoadKeys(od)
+	require.NoError(t, err)
+
+	keysMap := make(map[string]string)
+	for _, key := range keys {
+		pk, err := tblsv2.SecretToPublicKey(key)
 		require.NoError(t, err)
-		require.Equal(t, exp.secret, fmt.Sprintf("%#x", keys[0]))
+
+		keysMap[fmt.Sprintf("%#x", pk)] = fmt.Sprintf("%#x", key)
 	}
+
+	for _, exp := range expectedData {
+		require.Contains(t, keysMap, exp.pubkey)
+		require.Equal(t, exp.secret, keysMap[exp.pubkey])
+	}
+
+	require.Len(t, keysMap, len(expectedData))
 }
 
 func TestCombineTwiceWithoutForceFails(t *testing.T) {
@@ -195,6 +210,7 @@ func TestCombineTwiceWithoutForceFails(t *testing.T) {
 	}
 
 	dir := t.TempDir()
+	od := t.TempDir()
 
 	// flatten secrets, each validator slice is unpacked in a flat structure
 	var rawSecrets []tblsv2.PrivateKey
@@ -230,15 +246,27 @@ func TestCombineTwiceWithoutForceFails(t *testing.T) {
 		require.NoError(t, json.NewEncoder(lf).Encode(lock))
 	}
 
-	err := combine.Combine(context.Background(), dir, false)
+	err := combine.Combine(context.Background(), dir, od, false)
 	require.NoError(t, err)
 
-	err = combine.Combine(context.Background(), dir, false)
+	err = combine.Combine(context.Background(), dir, od, false)
 	require.Error(t, err)
 
-	for _, exp := range expectedData {
-		keys, err := keystore.LoadKeys(filepath.Join(dir, exp.pubkey, "validator_keys"))
+	keys, err := keystore.LoadKeys(od)
+	require.NoError(t, err)
+
+	keysMap := make(map[string]string)
+	for _, key := range keys {
+		pk, err := tblsv2.SecretToPublicKey(key)
 		require.NoError(t, err)
-		require.Equal(t, exp.secret, fmt.Sprintf("%#x", keys[0]))
+
+		keysMap[fmt.Sprintf("%#x", pk)] = fmt.Sprintf("%#x", key)
 	}
+
+	for _, exp := range expectedData {
+		require.Contains(t, keysMap, exp.pubkey)
+		require.Equal(t, exp.secret, keysMap[exp.pubkey])
+	}
+
+	require.Len(t, keysMap, len(expectedData))
 }