diff --git a/cmd/depositsign.go b/cmd/depositsign.go index 19a2ea489..e1a9a7916 100644 --- a/cmd/depositsign.go +++ b/cmd/depositsign.go @@ -27,8 +27,9 @@ import ( type depositSignConfig struct { depositConfig - WithdrawalAddresses []string - DepositAmounts []uint + WithdrawalAddresses []string + DepositAmounts []uint + AllowIncompleteKeystores bool } func newDepositSignCmd(runFunc func(context.Context, depositSignConfig) error) *cobra.Command { @@ -60,6 +61,7 @@ func newDepositSignCmd(runFunc func(context.Context, depositSignConfig) error) * func bindDepositSignFlags(cmd *cobra.Command, config *depositSignConfig) { cmd.Flags().StringSliceVar(&config.WithdrawalAddresses, "withdrawal-addresses", []string{}, "[REQUIRED] Withdrawal addresses for which the new deposits will be signed. Either a single address for all specified validator-public-keys or one address per key should be specified.") cmd.Flags().UintSliceVar(&config.DepositAmounts, "deposit-amounts", []uint{32}, "Comma separated list of partial deposit amounts (integers) in ETH.") + cmd.Flags().BoolVar(&config.AllowIncompleteKeystores, "allow-incomplete-keystores", false, "Allow the validator keys directory to contain key shares for only a subset of the cluster's validators. The command operates on the validators whose key shares are present.") } func runDepositSign(ctx context.Context, config depositSignConfig) error { @@ -91,19 +93,9 @@ func runDepositSign(ctx context.Context, config depositSignConfig) error { z.Int("validator_public_keys", len(config.ValidatorPublicKeys))) } - rawValKeys, err := keystore.LoadFilesUnordered(config.ValidatorKeysDir) + shares, err := loadValidatorShares(ctx, *cl, config.ValidatorKeysDir, config.AllowIncompleteKeystores) if err != nil { - return errors.Wrap(err, "load keystore, check if path exists", z.Str("validator_keys_dir", config.ValidatorKeysDir)) - } - - valKeys, err := rawValKeys.SequencedKeys() - if err != nil { - return errors.Wrap(err, "load keystore") - } - - shares, err := keystore.KeysharesToValidatorPubkey(*cl, valKeys) - if err != nil { - return errors.Wrap(err, "match local validator key shares with their counterparty in cluster lock") + return err } pubkeys := []eth2p0.BLSPubKey{} diff --git a/cmd/exit.go b/cmd/exit.go index 43544fc8a..e0e7bf4b8 100644 --- a/cmd/exit.go +++ b/cmd/exit.go @@ -19,28 +19,29 @@ import ( ) type exitConfig struct { - BeaconNodeEndpoints []string - ValidatorPubkey string - ValidatorIndex uint64 - ValidatorIndexPresent bool - SkipBeaconNodeCheck bool - PrivateKeyPath string - ValidatorKeysDir string - LockFilePath string - ExecutionEngineAddr string - PublishAddress string - PublishTimeout time.Duration - ExitEpoch uint64 - FetchedExitPath string - PlaintextOutput bool - BeaconNodeTimeout time.Duration - ExitFromFilePath string - ExitFromFileDir string - Log log.Config - All bool - testnetConfig eth2util.Network - BeaconNodeHeaders []string - FallbackBeaconNodeAddrs []string + BeaconNodeEndpoints []string + ValidatorPubkey string + ValidatorIndex uint64 + ValidatorIndexPresent bool + SkipBeaconNodeCheck bool + PrivateKeyPath string + ValidatorKeysDir string + LockFilePath string + ExecutionEngineAddr string + PublishAddress string + PublishTimeout time.Duration + ExitEpoch uint64 + FetchedExitPath string + PlaintextOutput bool + BeaconNodeTimeout time.Duration + ExitFromFilePath string + ExitFromFileDir string + Log log.Config + All bool + AllowIncompleteKeystores bool + testnetConfig eth2util.Network + BeaconNodeHeaders []string + FallbackBeaconNodeAddrs []string } func newExitCmd(cmds ...*cobra.Command) *cobra.Command { @@ -80,6 +81,7 @@ const ( beaconNodeHeaders fallbackBeaconNodeAddrs executionClientRPCEndpoint + allowIncompleteKeystores ) func (ef exitFlag) String() string { @@ -128,6 +130,8 @@ func (ef exitFlag) String() string { return "fallback-beacon-node-endpoints" case executionClientRPCEndpoint: return "execution-client-rpc-endpoint" + case allowIncompleteKeystores: + return "allow-incomplete-keystores" default: return "unknown" } @@ -196,6 +200,8 @@ func bindExitFlags(cmd *cobra.Command, config *exitConfig, flags []exitCLIFlag) cmd.Flags().StringSliceVar(&config.FallbackBeaconNodeAddrs, "fallback-beacon-node-endpoints", nil, "A list of beacon nodes to use if the primary list are offline or unhealthy.") case executionClientRPCEndpoint: cmd.Flags().StringVar(&config.ExecutionEngineAddr, executionClientRPCEndpoint.String(), "", maybeRequired("The address of the execution engine JSON-RPC API, used to verify ERC-1271 (e.g. Safe multisig) cluster signatures.")) + case allowIncompleteKeystores: + cmd.Flags().BoolVar(&config.AllowIncompleteKeystores, allowIncompleteKeystores.String(), false, "Allow the validator keys directory to contain key shares for only a subset of the cluster's validators. The command operates on the validators whose key shares are present.") } if f.required { diff --git a/cmd/exit_sign.go b/cmd/exit_sign.go index 083b570bf..f6b926da5 100644 --- a/cmd/exit_sign.go +++ b/cmd/exit_sign.go @@ -65,6 +65,7 @@ func newSignPartialExitCmd(runFunc func(context.Context, exitConfig) error) *cob {testnetCapellaHardFork, false}, {beaconNodeHeaders, false}, {fallbackBeaconNodeAddrs, false}, + {allowIncompleteKeystores, false}, }) bindLogFlags(cmd.Flags(), &config.Log) @@ -114,19 +115,9 @@ func runSignPartialExit(ctx context.Context, config exitConfig) error { return err } - rawValKeys, err := keystore.LoadFilesUnordered(config.ValidatorKeysDir) + shares, err := loadValidatorShares(ctx, *cl, config.ValidatorKeysDir, config.AllowIncompleteKeystores) if err != nil { - return errors.Wrap(err, "load keystore, check if path exists", z.Str("validator_keys_dir", config.ValidatorKeysDir)) - } - - valKeys, err := rawValKeys.SequencedKeys() - if err != nil { - return errors.Wrap(err, "load keystore") - } - - shares, err := keystore.KeysharesToValidatorPubkey(*cl, valKeys) - if err != nil { - return errors.Wrap(err, "match local validator key shares with their counterparty in cluster lock") + return err } shareIdx, err := keystore.ShareIdxForCluster(*cl, *identityKey.PubKey()) diff --git a/cmd/feerecipientsign.go b/cmd/feerecipientsign.go index 7ef3f8084..e769e48ba 100644 --- a/cmd/feerecipientsign.go +++ b/cmd/feerecipientsign.go @@ -39,10 +39,11 @@ type pubkeyToSign struct { type feerecipientSignConfig struct { feerecipientConfig - ValidatorKeysDir string - FeeRecipient string - GasLimit uint64 - Timestamp int64 + ValidatorKeysDir string + FeeRecipient string + GasLimit uint64 + Timestamp int64 + AllowIncompleteKeystores bool } func newFeeRecipientSignCmd(runFunc func(context.Context, feerecipientSignConfig) error) *cobra.Command { @@ -68,6 +69,7 @@ func newFeeRecipientSignCmd(runFunc func(context.Context, feerecipientSignConfig cmd.Flags().StringVar(&config.FeeRecipient, "fee-recipient", "", "[REQUIRED] New fee recipient address to be applied to all specified validators.") cmd.Flags().Uint64Var(&config.GasLimit, "gas-limit", 0, "Optional gas limit override for builder registrations. If not set, the most recent gas limit from the cluster lock, overrides file or remote API is used.") cmd.Flags().Int64Var(&config.Timestamp, "timestamp", 0, "Optional Unix timestamp for the builder registration message. When set, all operators can sign independently with the same timestamp. If not set, either the current time is used for new registrations or if another peer already submitted partial signature to the API, its timestamp is used.") + cmd.Flags().BoolVar(&config.AllowIncompleteKeystores, "allow-incomplete-keystores", false, "Allow the validator keys directory to contain key shares for only a subset of the cluster's validators. The command operates on the validators whose key shares are present.") wrapPreRunE(cmd, func(cmd *cobra.Command, _ []string) error { mustMarkFlagRequired(cmd, "validator-public-keys") @@ -175,19 +177,9 @@ func runFeeRecipientSign(ctx context.Context, config feerecipientSignConfig) err return err } - rawValKeys, err := keystore.LoadFilesUnordered(config.ValidatorKeysDir) + shares, err := loadValidatorShares(ctx, *cl, config.ValidatorKeysDir, config.AllowIncompleteKeystores) if err != nil { - return errors.Wrap(err, "load keystore, check if path exists", z.Str("validator_keys_dir", config.ValidatorKeysDir)) - } - - valKeys, err := rawValKeys.SequencedKeys() - if err != nil { - return errors.Wrap(err, "load keystore") - } - - shares, err := keystore.KeysharesToValidatorPubkey(*cl, valKeys) - if err != nil { - return errors.Wrap(err, "match local validator key shares with their counterparty in cluster lock") + return err } overrides, err := loadOverridesRegistrations(config.OverridesFilePath, cl.ForkVersion) diff --git a/cmd/loadvalidatorkeys.go b/cmd/loadvalidatorkeys.go new file mode 100644 index 000000000..5c11a9012 --- /dev/null +++ b/cmd/loadvalidatorkeys.go @@ -0,0 +1,56 @@ +// Copyright © 2022-2026 Obol Labs Inc. Licensed under the terms of a Business Source License 1.1 + +package cmd + +import ( + "context" + + "github.com/obolnetwork/charon/app/errors" + "github.com/obolnetwork/charon/app/log" + "github.com/obolnetwork/charon/app/z" + "github.com/obolnetwork/charon/cluster" + "github.com/obolnetwork/charon/eth2util/keystore" + "github.com/obolnetwork/charon/tbls" +) + +// loadValidatorShares loads the operator's local validator key shares from dir and maps them to +// their cluster validators. +// +// When allowIncomplete is false it requires shares for every validator in the lock. When true it +// accepts any subset (matching by public key, ignoring keystore filename-index gaps) and warns +// when the on-disk set covers fewer validators than the lock. +func loadValidatorShares(ctx context.Context, cl cluster.Lock, dir string, allowIncomplete bool) (keystore.ValidatorShares, error) { + rawValKeys, err := keystore.LoadFilesUnordered(dir) + if err != nil { + return nil, errors.Wrap(err, "load keystore, check if path exists", z.Str("validator_keys_dir", dir)) + } + + var valKeys []tbls.PrivateKey + if allowIncomplete { + // Match by derived public key; keystore filename indexes are meaningless for these + // commands, so a non-sequential subset (e.g. keystore-5.json alone) is valid. + valKeys = rawValKeys.Keys() + } else { + valKeys, err = rawValKeys.SequencedKeys() + if err != nil { + return nil, errors.Wrap(err, "load keystore") + } + } + + shares, err := keystore.KeysharesToValidatorPubkey(cl, valKeys) + if err != nil { + return nil, errors.Wrap(err, "match local validator key shares with their counterparty in cluster lock") + } + + if !allowIncomplete && len(shares) != len(cl.Validators) { + return nil, errors.New("validator_keys directory does not contain key shares for all cluster validators; use --allow-incomplete-keystores to operate on the available subset", + z.Int("found", len(shares)), z.Int("cluster_validators", len(cl.Validators)), z.Str("validator_keys_dir", dir)) + } + + if allowIncomplete && len(shares) < len(cl.Validators) { + log.Warn(ctx, "Operating on a subset of the cluster's validators", nil, + z.Int("found", len(shares)), z.Int("cluster_validators", len(cl.Validators)), z.Str("validator_keys_dir", dir)) + } + + return shares, nil +} diff --git a/cmd/loadvalidatorkeys_internal_test.go b/cmd/loadvalidatorkeys_internal_test.go new file mode 100644 index 000000000..ced7c3d9c --- /dev/null +++ b/cmd/loadvalidatorkeys_internal_test.go @@ -0,0 +1,104 @@ +// Copyright © 2022-2026 Obol Labs Inc. Licensed under the terms of a Business Source License 1.1 + +package cmd + +import ( + "math/rand" + "os" + "path/filepath" + "strconv" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/obolnetwork/charon/cluster" + "github.com/obolnetwork/charon/core" + "github.com/obolnetwork/charon/eth2util/keystore" + "github.com/obolnetwork/charon/tbls" +) + +// setupValidatorKeys builds a cluster lock and writes operator 0's key shares (one per validator, +// as keystore-insecure-.json) into a fresh validator_keys directory. It returns the +// lock and the directory. +func setupValidatorKeys(t *testing.T) (cluster.Lock, string) { + t.Helper() + + const ( + valAmt = 3 + operatorAmt = 4 + ) + + random := rand.New(rand.NewSource(0)) + + lock, _, keyShares := cluster.NewForT(t, valAmt, operatorAmt, operatorAmt, 0, random) + + var op0Shares []tbls.PrivateKey + for _, share := range keyShares { + op0Shares = append(op0Shares, share[0]) + } + + dir := filepath.Join(t.TempDir(), "validator_keys") + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.NoError(t, keystore.StoreKeysInsecure(op0Shares, dir, keystore.ConfirmInsecureKeys)) + + return lock, dir +} + +// removeKeystore deletes the keystore-insecure-.json/.txt pair from dir. +func removeKeystore(t *testing.T, dir string, idx int) { + t.Helper() + + for _, ext := range []string{".json", ".txt"} { + require.NoError(t, os.Remove(filepath.Join(dir, "keystore-insecure-"+strconv.Itoa(idx)+ext))) + } +} + +func TestLoadValidatorShares(t *testing.T) { + ctx := t.Context() + + t.Run("strict full set", func(t *testing.T) { + lock, dir := setupValidatorKeys(t) + + shares, err := loadValidatorShares(ctx, lock, dir, false) + require.NoError(t, err) + require.Len(t, shares, 3) + }) + + t.Run("strict rejects contiguous subset", func(t *testing.T) { + lock, dir := setupValidatorKeys(t) + removeKeystore(t, dir, 2) // leaves keystore-insecure-0 and -1 (SequencedKeys passes) + + _, err := loadValidatorShares(ctx, lock, dir, false) + require.ErrorContains(t, err, "allow-incomplete-keystores") + }) + + t.Run("strict rejects gapped subset", func(t *testing.T) { + lock, dir := setupValidatorKeys(t) + removeKeystore(t, dir, 0) + removeKeystore(t, dir, 1) // leaves only keystore-insecure-2 (out of sequence) + + _, err := loadValidatorShares(ctx, lock, dir, false) + require.ErrorContains(t, err, "out of sequence") + }) + + t.Run("lenient allows gapped subset", func(t *testing.T) { + lock, dir := setupValidatorKeys(t) + removeKeystore(t, dir, 0) + removeKeystore(t, dir, 1) // leaves only keystore-insecure-2 (validator index 2) + + shares, err := loadValidatorShares(ctx, lock, dir, true) + require.NoError(t, err) + require.Len(t, shares, 1) + + _, ok := shares[core.PubKey(lock.Validators[2].PublicKeyHex())] + require.True(t, ok, "expected validator 2 present in result") + }) + + t.Run("lenient allows full set", func(t *testing.T) { + lock, dir := setupValidatorKeys(t) + + shares, err := loadValidatorShares(ctx, lock, dir, true) + require.NoError(t, err) + require.Len(t, shares, 3) + }) +} diff --git a/eth2util/keystore/keystore.go b/eth2util/keystore/keystore.go index b2b17b1d9..3ce3b34d4 100644 --- a/eth2util/keystore/keystore.go +++ b/eth2util/keystore/keystore.go @@ -243,53 +243,48 @@ func checkDir(dir string) error { } // KeysharesToValidatorPubkey maps each provided private key share to the associated validator -// public key in the cluster lock. It returns an error if a provided private key share does not belong to any validator -// in the lock. +// public key in the cluster lock. It returns an error if a provided private key share does not +// belong to any validator in the lock, or if more than one provided share resolves to the same +// validator (an ambiguous, misconfigured key set). +// +// It accepts shares for any subset of the cluster's validators; callers that require shares for +// every validator must enforce that themselves (see cmd.loadValidatorShares). func KeysharesToValidatorPubkey(lock cluster.Lock, shares []tbls.PrivateKey) (ValidatorShares, error) { - ret := make(map[core.PubKey]IndexedKeyShare) + // Map each validator public key share to its validator public key. + shareToValidator := make(map[tbls.PublicKey]core.PubKey) - pubShares := make([]tbls.PublicKey, 0, len(shares)) + for _, validator := range lock.Validators { + valHex := core.PubKey(validator.PublicKeyHex()) + for _, valShare := range validator.PubShares { + pubShare := tbls.PublicKey(valShare) + if existing, ok := shareToValidator[pubShare]; ok && existing != valHex { + return nil, errors.New("public key share appears in more than one validator in the cluster lock", z.Str("validator", string(valHex)), z.Str("other_validator", string(existing))) + } - for _, share := range shares { - ps, err := tbls.SecretToPublicKey(share) - if err != nil { - return nil, errors.Wrap(err, "private share to public share") + shareToValidator[pubShare] = valHex } - - pubShares = append(pubShares, ps) } - // matched tracks which provided private key shares were found in the cluster lock. - matched := make([]bool, len(pubShares)) - - // this is sadly a O(n^2) search - for _, validator := range lock.Validators { - valHex := validator.PublicKeyHex() + ret := make(ValidatorShares) - valPubShares := make(map[tbls.PublicKey]struct{}) - for _, valShare := range validator.PubShares { - valPubShares[tbls.PublicKey(valShare)] = struct{}{} + for shareIdx, share := range shares { + pubShare, err := tbls.SecretToPublicKey(share) + if err != nil { + return nil, errors.Wrap(err, "private share to public share") } - for shareIdx, share := range pubShares { - if _, ok := valPubShares[share]; !ok { - continue - } - - ret[core.PubKey(valHex)] = IndexedKeyShare{ - Share: shares[shareIdx], - Index: shareIdx + 1, - } - matched[shareIdx] = true + valHex, ok := shareToValidator[pubShare] + if !ok { + return nil, errors.New("public key share from provided private key share not found in provided lock", z.Int("share_index", shareIdx)) + } - break + if _, ok := ret[valHex]; ok { + return nil, errors.New("multiple provided private key shares resolve to the same validator", z.Str("validator", string(valHex)), z.Int("share_index", shareIdx)) } - } - // Every provided private key share must belong to a validator in the cluster lock. - for shareIdx, ok := range matched { - if !ok { - return nil, errors.New("public key share from provided private key share not found in provided lock", z.Int("share_index", shareIdx)) + ret[valHex] = IndexedKeyShare{ + Share: share, + Index: shareIdx + 1, } } diff --git a/eth2util/keystore/keystore_test.go b/eth2util/keystore/keystore_test.go index 9b0e98e8e..11a4de934 100644 --- a/eth2util/keystore/keystore_test.go +++ b/eth2util/keystore/keystore_test.go @@ -512,6 +512,60 @@ func TestKeysharesToValidatorPubkeyForeignShare(t *testing.T) { require.ErrorContains(t, err, "not found in provided lock") } +// TestKeysharesToValidatorPubkeyDuplicateShares ensures that providing two distinct private key +// shares belonging to the same validator is rejected. Silently collapsing them (last-write-wins) +// could sign with the wrong operator's share, producing a partial signature that fails to +// aggregate, so the whole call must fail with a clear error. +func TestKeysharesToValidatorPubkeyDuplicateShares(t *testing.T) { + valPubk, err := tblsconv.PubkeyFromCore(testutil.RandomCorePubKey(t)) + require.NoError(t, err) + + validator := cluster.DistValidator{PubKey: valPubk[:]} + + var shares []tbls.PrivateKey + + for range 2 { + sharePriv, err := tbls.GenerateSecretKey() + require.NoError(t, err) + + sharePub, err := tbls.SecretToPublicKey(sharePriv) + require.NoError(t, err) + + validator.PubShares = append(validator.PubShares, sharePub[:]) + shares = append(shares, sharePriv) + } + + lock := cluster.Lock{Validators: []cluster.DistValidator{validator}} + + _, err = keystore.KeysharesToValidatorPubkey(lock, shares) + require.ErrorContains(t, err, "multiple provided private key shares resolve to the same validator") +} + +// TestKeysharesToValidatorPubkeyDuplicateLockShare ensures the matcher errors when the cluster +// lock contains the same public key share under more than one validator, which would otherwise +// mis-map a provided private share to the wrong validator. +func TestKeysharesToValidatorPubkeyDuplicateLockShare(t *testing.T) { + sharePriv, err := tbls.GenerateSecretKey() + require.NoError(t, err) + + sharePub, err := tbls.SecretToPublicKey(sharePriv) + require.NoError(t, err) + + newValidator := func(t *testing.T) cluster.DistValidator { + t.Helper() + + valPubk, err := tblsconv.PubkeyFromCore(testutil.RandomCorePubKey(t)) + require.NoError(t, err) + + return cluster.DistValidator{PubKey: valPubk[:], PubShares: [][]byte{sharePub[:]}} + } + + lock := cluster.Lock{Validators: []cluster.DistValidator{newValidator(t), newValidator(t)}} + + _, err = keystore.KeysharesToValidatorPubkey(lock, []tbls.PrivateKey{sharePriv}) + require.ErrorContains(t, err, "public key share appears in more than one validator") +} + func TestShareIdxForCluster(t *testing.T) { valAmt := 100 operatorAmt := 4