From 0c7ff9e5f2869b9b1075604a4bf1365823b96920 Mon Sep 17 00:00:00 2001 From: Mario Rugiero Date: Mon, 25 Nov 2024 18:02:49 -0300 Subject: [PATCH 1/4] fix: remove unneeded uses of ECDSA private key Both the operator and aggregator were using the `BuildAll` helper that required a private key due to creation of signers, only to discard the signers and keep the reader and subscriber. Changing it to `BuildReadClients` allows removing some of it, in the case of the operator meaning only registration requires the ECDSA key to be present, reducing risks of losing funds. --- aggregator/pkg/aggregator.go | 6 ++---- core/chainio/avs_reader.go | 4 ++-- operator/pkg/operator.go | 4 +--- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/aggregator/pkg/aggregator.go b/aggregator/pkg/aggregator.go index b059c45148..af4d7886e2 100644 --- a/aggregator/pkg/aggregator.go +++ b/aggregator/pkg/aggregator.go @@ -105,7 +105,7 @@ func NewAggregator(aggregatorConfig config.AggregatorConfig) (*Aggregator, error // Telemetry aggregatorTelemetry := NewTelemetry(aggregatorConfig.Aggregator.TelemetryIpPortAddress, logger) - avsReader, err := chainio.NewAvsReaderFromConfig(aggregatorConfig.BaseConfig, aggregatorConfig.EcdsaConfig) + avsReader, err := chainio.NewAvsReaderFromConfig(aggregatorConfig.BaseConfig) if err != nil { return nil, err } @@ -134,9 +134,7 @@ func NewAggregator(aggregatorConfig config.AggregatorConfig) (*Aggregator, error PromMetricsIpPortAddress: ":9090", } - aggregatorPrivateKey := aggregatorConfig.EcdsaConfig.PrivateKey - - clients, err := sdkclients.BuildAll(chainioConfig, aggregatorPrivateKey, logger) + clients, err := sdkclients.BuildReadClients(chainioConfig, logger) if err != nil { logger.Errorf("Cannot create sdk clients", "err", err) return nil, err diff --git a/core/chainio/avs_reader.go b/core/chainio/avs_reader.go index 69b8f281b1..ae2ea0a9da 100644 --- a/core/chainio/avs_reader.go +++ b/core/chainio/avs_reader.go @@ -24,7 +24,7 @@ type AvsReader struct { logger logging.Logger } -func NewAvsReaderFromConfig(baseConfig *config.BaseConfig, ecdsaConfig *config.EcdsaConfig) (*AvsReader, error) { +func NewAvsReaderFromConfig(baseConfig *config.BaseConfig) (*AvsReader, error) { buildAllConfig := clients.BuildAllConfig{ EthHttpUrl: baseConfig.EthRpcUrl, @@ -35,7 +35,7 @@ func NewAvsReaderFromConfig(baseConfig *config.BaseConfig, ecdsaConfig *config.E PromMetricsIpPortAddress: baseConfig.EigenMetricsIpPortAddress, } - clients, err := clients.BuildAll(buildAllConfig, ecdsaConfig.PrivateKey, baseConfig.Logger) + clients, err := clients.BuildReadClients(buildAllConfig, baseConfig.Logger) if err != nil { return nil, err } diff --git a/operator/pkg/operator.go b/operator/pkg/operator.go index b5be4f9e0c..dd4cef9cad 100644 --- a/operator/pkg/operator.go +++ b/operator/pkg/operator.go @@ -3,7 +3,6 @@ package operator import ( "bytes" "context" - "crypto/ecdsa" "encoding/hex" "encoding/json" "fmt" @@ -48,7 +47,6 @@ type Operator struct { Address ethcommon.Address Socket string Timeout time.Duration - PrivKey *ecdsa.PrivateKey KeyPair *bls.KeyPair OperatorId eigentypes.OperatorId avsSubscriber chainio.AvsSubscriber @@ -75,7 +73,7 @@ const ( func NewOperatorFromConfig(configuration config.OperatorConfig) (*Operator, error) { logger := configuration.BaseConfig.Logger - avsReader, err := chainio.NewAvsReaderFromConfig(configuration.BaseConfig, configuration.EcdsaConfig) + avsReader, err := chainio.NewAvsReaderFromConfig(configuration.BaseConfig) if err != nil { log.Fatalf("Could not create AVS reader") } From 703ea89fe8596beb75fa2b11f4b608b7891ce10d Mon Sep 17 00:00:00 2001 From: Marcos Nicolau Date: Mon, 25 Nov 2024 17:05:56 -0300 Subject: [PATCH 2/4] fix(operator): replace ecdsa signature with bls for telemetry --- operator/pkg/operator.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/operator/pkg/operator.go b/operator/pkg/operator.go index dd4cef9cad..de446eb926 100644 --- a/operator/pkg/operator.go +++ b/operator/pkg/operator.go @@ -644,13 +644,12 @@ func (o *Operator) SendTelemetryData(ctx *cli.Context) error { hash.Write([]byte(ctx.App.Version)) // get hash - version := hash.Sum(nil) + var version [32]byte // All zeroed initially + copy(version[:], hash.Sum(nil)) // sign version - signature, err := crypto.Sign(version[:], o.Config.EcdsaConfig.PrivateKey) - if err != nil { - return err - } + signature := o.Config.BlsConfig.KeyPair.SignMessage(version) + ethRpcUrl, err := BaseUrlOnly(o.Config.BaseConfig.EthRpcUrl) if err != nil { return err From 6cfc5945a2da0f63957aa1b0e64f9b0f089268b1 Mon Sep 17 00:00:00 2001 From: Mario Rugiero Date: Mon, 25 Nov 2024 19:23:45 -0300 Subject: [PATCH 3/4] fix: actually remove dependence on ECDSA - Integrates commit from #1489 that migrates metrics' signing to BLS - Removes the EcdsaConfig field from OperatorConfig - Removes self-registration so `operator start` doesn't require the ECDSA key - Explicitly loads from the same file the config in the scenarios that actually require it, namely registration and deposit --- core/config/operator.go | 7 ------ operator/cmd/actions/deposit_into_strategy.go | 23 ++++++++++--------- operator/cmd/actions/register.go | 9 ++++---- operator/pkg/operator.go | 15 +----------- operator/pkg/register.go | 5 ++-- 5 files changed, 21 insertions(+), 38 deletions(-) diff --git a/core/config/operator.go b/core/config/operator.go index cac263f7a9..32d24b21e8 100644 --- a/core/config/operator.go +++ b/core/config/operator.go @@ -11,7 +11,6 @@ import ( type OperatorConfig struct { BaseConfig *BaseConfig - EcdsaConfig *EcdsaConfig BlsConfig *BlsConfig AlignedLayerDeploymentConfig *AlignedLayerDeploymentConfig @@ -60,11 +59,6 @@ func NewOperatorConfig(configFilePath string) *OperatorConfig { log.Fatal("Error reading base config: ") } - ecdsaConfig := NewEcdsaConfig(configFilePath, baseConfig.ChainId) - if ecdsaConfig == nil { - log.Fatal("Error reading ecdsa config: ") - } - blsConfig := NewBlsConfig(configFilePath) if blsConfig == nil { log.Fatal("Error reading bls config: ") @@ -79,7 +73,6 @@ func NewOperatorConfig(configFilePath string) *OperatorConfig { return &OperatorConfig{ BaseConfig: baseConfig, - EcdsaConfig: ecdsaConfig, BlsConfig: blsConfig, AlignedLayerDeploymentConfig: baseConfig.AlignedLayerDeploymentConfig, Operator: struct { diff --git a/operator/cmd/actions/deposit_into_strategy.go b/operator/cmd/actions/deposit_into_strategy.go index d80ba6ffc3..514cdf04d0 100644 --- a/operator/cmd/actions/deposit_into_strategy.go +++ b/operator/cmd/actions/deposit_into_strategy.go @@ -50,7 +50,8 @@ func depositIntoStrategyMain(ctx *cli.Context) error { return nil } - config := config.NewOperatorConfig(ctx.String(config.ConfigFileFlag.Name)) + opConfig := config.NewOperatorConfig(ctx.String(config.ConfigFileFlag.Name)) + ecdsaConfig := config.NewEcdsaConfig(ctx.String(config.ConfigFileFlag.Name), opConfig.BaseConfig.ChainId) strategyAddressStr := ctx.String(StrategyAddressFlag.Name) if strategyAddressStr == "" { log.Println("Strategy address is required") @@ -59,35 +60,35 @@ func depositIntoStrategyMain(ctx *cli.Context) error { log.Println("Depositing into strategy", strategyAddressStr) strategyAddr := common.HexToAddress(strategyAddressStr) - delegationManagerAddr := config.BaseConfig.EigenLayerDeploymentConfig.DelegationManagerAddr - avsDirectoryAddr := config.BaseConfig.EigenLayerDeploymentConfig.AVSDirectoryAddr + delegationManagerAddr := opConfig.BaseConfig.EigenLayerDeploymentConfig.DelegationManagerAddr + avsDirectoryAddr := opConfig.BaseConfig.EigenLayerDeploymentConfig.AVSDirectoryAddr signerConfig := signerv2.Config{ - PrivateKey: config.EcdsaConfig.PrivateKey, + PrivateKey: ecdsaConfig.PrivateKey, } - signerFn, _, err := signerv2.SignerFromConfig(signerConfig, config.BaseConfig.ChainId) + signerFn, _, err := signerv2.SignerFromConfig(signerConfig, opConfig.BaseConfig.ChainId) if err != nil { return err } - w, err := wallet.NewPrivateKeyWallet(&config.BaseConfig.EthRpcClient, signerFn, - config.Operator.Address, config.BaseConfig.Logger) + w, err := wallet.NewPrivateKeyWallet(&opConfig.BaseConfig.EthRpcClient, signerFn, + opConfig.Operator.Address, opConfig.BaseConfig.Logger) if err != nil { return err } - txMgr := txmgr.NewSimpleTxManager(w, &config.BaseConfig.EthRpcClient, config.BaseConfig.Logger, - config.Operator.Address) + txMgr := txmgr.NewSimpleTxManager(w, &opConfig.BaseConfig.EthRpcClient, opConfig.BaseConfig.Logger, + opConfig.Operator.Address) eigenMetrics := metrics.NewNoopMetrics() eigenLayerWriter, err := elcontracts.BuildELChainWriter(delegationManagerAddr, avsDirectoryAddr, - &config.BaseConfig.EthRpcClient, config.BaseConfig.Logger, eigenMetrics, txMgr) + &opConfig.BaseConfig.EthRpcClient, opConfig.BaseConfig.Logger, eigenMetrics, txMgr) if err != nil { return err } _, err = eigenLayerWriter.DepositERC20IntoStrategy(context.Background(), strategyAddr, amount, true) if err != nil { - config.BaseConfig.Logger.Errorf("Error depositing into strategy") + opConfig.BaseConfig.Logger.Errorf("Error depositing into strategy") return err } return nil diff --git a/operator/cmd/actions/register.go b/operator/cmd/actions/register.go index bfc9ad87a0..5559c2a6eb 100644 --- a/operator/cmd/actions/register.go +++ b/operator/cmd/actions/register.go @@ -23,19 +23,20 @@ var RegisterCommand = &cli.Command{ } func registerOperatorMain(ctx *cli.Context) error { - config := config.NewOperatorConfig(ctx.String(config.ConfigFileFlag.Name)) + operatorConfig := config.NewOperatorConfig(ctx.String(config.ConfigFileFlag.Name)) + ecdsaConfig := config.NewEcdsaConfig(ctx.String(config.ConfigFileFlag.Name), operatorConfig.BaseConfig.ChainId) quorumNumbers := []byte{0} // Generate salt and expiry - privateKeyBytes := []byte(config.BlsConfig.KeyPair.PrivKey.String()) + privateKeyBytes := []byte(operatorConfig.BlsConfig.KeyPair.PrivKey.String()) salt := [32]byte{} copy(salt[:], crypto.Keccak256([]byte("churn"), []byte(time.Now().String()), quorumNumbers, privateKeyBytes)) - err := operator.RegisterOperator(context.Background(), config, salt) + err := operator.RegisterOperator(context.Background(), operatorConfig, ecdsaConfig, salt) if err != nil { - config.BaseConfig.Logger.Error("Failed to register operator", "err", err) + operatorConfig.BaseConfig.Logger.Error("Failed to register operator", "err", err) return err } diff --git a/operator/pkg/operator.go b/operator/pkg/operator.go index de446eb926..f33caa33ca 100644 --- a/operator/pkg/operator.go +++ b/operator/pkg/operator.go @@ -82,21 +82,8 @@ func NewOperatorFromConfig(configuration config.OperatorConfig) (*Operator, erro if err != nil { log.Fatalf("Could not check if operator is registered") } - if !registered { - log.Println("Operator is not registered with AlignedLayer AVS, registering...") - quorumNumbers := []byte{0} - - // Generate salt and expiry - privateKeyBytes := []byte(configuration.BlsConfig.KeyPair.PrivKey.String()) - salt := [32]byte{} - - copy(salt[:], crypto.Keccak256([]byte("churn"), []byte(time.Now().String()), quorumNumbers, privateKeyBytes)) - - err = RegisterOperator(context.Background(), &configuration, salt) - if err != nil { - log.Fatalf("Could not register operator") - } + log.Fatal("Operator not registered") } avsSubscriber, err := chainio.NewAvsSubscriberFromConfig(configuration.BaseConfig) diff --git a/operator/pkg/register.go b/operator/pkg/register.go index dc186c54e9..263bad4af8 100644 --- a/operator/pkg/register.go +++ b/operator/pkg/register.go @@ -15,9 +15,10 @@ import ( func RegisterOperator( ctx context.Context, configuration *config.OperatorConfig, + ecdsaConfig *config.EcdsaConfig, operatorToAvsRegistrationSigSalt [32]byte, ) error { - writer, err := chainio.NewAvsWriterFromConfig(configuration.BaseConfig, configuration.EcdsaConfig, nil) + writer, err := chainio.NewAvsWriterFromConfig(configuration.BaseConfig, ecdsaConfig, nil) if err != nil { configuration.BaseConfig.Logger.Error("Failed to create AVS writer", "err", err) return err @@ -27,7 +28,7 @@ func RegisterOperator( quorumNumbers := types.QuorumNums{0} - _, err = writer.RegisterOperator(ctx, configuration.EcdsaConfig.PrivateKey, + _, err = writer.RegisterOperator(ctx, ecdsaConfig.PrivateKey, configuration.BlsConfig.KeyPair, quorumNumbers, socket, true) From 8dbf26139ce376e7c593d58095f0b931e51e6ac2 Mon Sep 17 00:00:00 2001 From: Urix <43704209+uri-99@users.noreply.github.com> Date: Tue, 26 Nov 2024 17:32:36 -0300 Subject: [PATCH 4/4] refactor: remove EcdsaConfigFromYaml from OperatorConfigFromYaml --- core/config/operator.go | 1 - 1 file changed, 1 deletion(-) diff --git a/core/config/operator.go b/core/config/operator.go index 32d24b21e8..bd38a53a4a 100644 --- a/core/config/operator.go +++ b/core/config/operator.go @@ -45,7 +45,6 @@ type OperatorConfigFromYaml struct { MaxBatchSize int64 `yaml:"max_batch_size"` LastProcessedBatchFilePath string `yaml:"last_processed_batch_filepath"` } `yaml:"operator"` - EcdsaConfigFromYaml EcdsaConfigFromYaml `yaml:"ecdsa"` BlsConfigFromYaml BlsConfigFromYaml `yaml:"bls"` }