From dd5176eaa9abaa9e1d671c18f9a3853fdbcf67c2 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Mon, 17 Feb 2025 12:52:29 -0500 Subject: [PATCH 01/37] Add comments to signer-config setup --- config/config.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/config/config.go b/config/config.go index fda0780e4036..fe39fa97be42 100644 --- a/config/config.go +++ b/config/config.go @@ -676,10 +676,16 @@ func getStakingSigner(v *viper.Viper) (bls.Signer, error) { return key, nil } + // If the key is set, but a user-file isn't provided, we don't create one. + // The siging key is only stored to the default file-location if it's created + // and saved by the current application run. + if v.IsSet(StakingSignerKeyPathKey) { return nil, errMissingStakingSigningKeyFile } + // no config values explicitly set, + key, err := localsigner.New() if err != nil { return nil, fmt.Errorf("couldn't generate new signing key: %w", err) From ed0c5f8d72bb2bf2b4e2a2fd61dfa1d1d0971b75 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Fri, 14 Feb 2025 18:01:52 -0500 Subject: [PATCH 02/37] Extend siging configuration to include RPC --- config/config.go | 95 ++++++++++++++++++++++++----------- config/config_test.go | 112 ++++++++++++++++++++++++++++++++++++++++++ config/flags.go | 1 + config/keys.go | 1 + config/node/config.go | 1 + main/main.go | 3 +- 6 files changed, 184 insertions(+), 29 deletions(-) diff --git a/config/config.go b/config/config.go index fe39fa97be42..910be46349b2 100644 --- a/config/config.go +++ b/config/config.go @@ -4,6 +4,7 @@ package config import ( + "context" "crypto/tls" "encoding/base64" "encoding/json" @@ -17,6 +18,8 @@ import ( "time" "github.com/spf13/viper" + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" "github.com/ava-labs/avalanchego/api/server" "github.com/ava-labs/avalanchego/chains" @@ -38,6 +41,7 @@ import ( "github.com/ava-labs/avalanchego/utils/constants" "github.com/ava-labs/avalanchego/utils/crypto/bls" "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/localsigner" + "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/rpcsigner" "github.com/ava-labs/avalanchego/utils/ips" "github.com/ava-labs/avalanchego/utils/logging" "github.com/ava-labs/avalanchego/utils/perms" @@ -84,6 +88,7 @@ var ( errCannotReadDirectory = errors.New("cannot read directory") errUnmarshalling = errors.New("unmarshalling failed") errFileDoesNotExist = errors.New("file does not exist") + errInvalidSignerConfig = fmt.Errorf("only one of the following flags can be set: %s, %s, %s, %s", StakingEphemeralSignerEnabledKey, StakingSignerKeyContentKey, StakingSignerKeyPathKey, StakingRPCSignerKey) ) func getConsensusConfig(v *viper.Viper) snowball.Parameters { @@ -640,53 +645,84 @@ func getStakingTLSCert(v *viper.Viper) (tls.Certificate, error) { } } -func getStakingSigner(v *viper.Viper) (bls.Signer, error) { - if v.GetBool(StakingEphemeralSignerEnabledKey) { - key, err := localsigner.New() +func getStakingSigner(ctx context.Context, v *viper.Viper) (bls.Signer, error) { + ephemeralSignerEnabled := v.GetBool(StakingEphemeralSignerEnabledKey) + contentKeyIsSet := v.IsSet(StakingSignerKeyContentKey) + keyPathIsSet := v.IsSet(StakingSignerKeyPathKey) + rpcSignerURLIsSet := v.IsSet(StakingRPCSignerKey) + + signingKeyPath := getExpandedArg(v, StakingSignerKeyPathKey) + + switch { + case ephemeralSignerEnabled && !contentKeyIsSet && !keyPathIsSet && !rpcSignerURLIsSet: + signer, err := localsigner.New() if err != nil { - return nil, fmt.Errorf("couldn't generate ephemeral signing key: %w", err) + return nil, fmt.Errorf("couldn't generate ephemeral signing signer: %w", err) } - return key, nil - } - if v.IsSet(StakingSignerKeyContentKey) { + return signer, nil + + case !ephemeralSignerEnabled && contentKeyIsSet && !keyPathIsSet && !rpcSignerURLIsSet: signerKeyRawContent := v.GetString(StakingSignerKeyContentKey) signerKeyContent, err := base64.StdEncoding.DecodeString(signerKeyRawContent) if err != nil { return nil, fmt.Errorf("unable to decode base64 content: %w", err) } - key, err := localsigner.FromBytes(signerKeyContent) + + signer, err := localsigner.FromBytes(signerKeyContent) if err != nil { return nil, fmt.Errorf("couldn't parse signing key: %w", err) } - return key, nil - } - signingKeyPath := getExpandedArg(v, StakingSignerKeyPathKey) - _, err := os.Stat(signingKeyPath) - if !errors.Is(err, fs.ErrNotExist) { + return signer, nil + + case !ephemeralSignerEnabled && !contentKeyIsSet && keyPathIsSet && !rpcSignerURLIsSet: + // If the key is set, but a user-file isn't provided, we don't create one. + // The siging key is only stored to the default file-location if it's created + // and saved by the current application run. + _, err := os.Stat(signingKeyPath) + + if errors.Is(err, fs.ErrNotExist) { + return nil, errMissingStakingSigningKeyFile + } + signingKeyBytes, err := os.ReadFile(signingKeyPath) if err != nil { return nil, err } - key, err := localsigner.FromBytes(signingKeyBytes) + + signer, err := localsigner.FromBytes(signingKeyBytes) if err != nil { return nil, fmt.Errorf("couldn't parse signing key: %w", err) } - return key, nil - } - // If the key is set, but a user-file isn't provided, we don't create one. - // The siging key is only stored to the default file-location if it's created - // and saved by the current application run. + return signer, nil + + case !ephemeralSignerEnabled && !contentKeyIsSet && !keyPathIsSet && rpcSignerURLIsSet: + rpcSignerURL := v.GetString(StakingRPCSignerKey) + + // the rpc-signer client should call a proxy server (on the same machine) that forwards + // the request to the actual signer instead of relying on tls-credentials + conn, err := grpc.NewClient(rpcSignerURL, grpc.WithTransportCredentials(insecure.NewCredentials())) + if err != nil { + return nil, fmt.Errorf("couldn't create rpc signer client: %w", err) + } + + signer, err := rpcsigner.NewClient(ctx, conn) + if err != nil { + conn.Close() + return nil, fmt.Errorf("couldn't create rpc signer client: %w", err) + } + + return signer, nil - if v.IsSet(StakingSignerKeyPathKey) { - return nil, errMissingStakingSigningKeyFile + case ephemeralSignerEnabled || contentKeyIsSet || keyPathIsSet || rpcSignerURLIsSet: + return nil, errInvalidSignerConfig } // no config values explicitly set, - key, err := localsigner.New() + signer, err := localsigner.New() if err != nil { return nil, fmt.Errorf("couldn't generate new signing key: %w", err) } @@ -695,17 +731,19 @@ func getStakingSigner(v *viper.Viper) (bls.Signer, error) { return nil, fmt.Errorf("couldn't create path for signing key at %s: %w", signingKeyPath, err) } - keyBytes := key.ToBytes() + keyBytes := signer.ToBytes() if err := os.WriteFile(signingKeyPath, keyBytes, perms.ReadWrite); err != nil { return nil, fmt.Errorf("couldn't write new signing key to %s: %w", signingKeyPath, err) } + if err := os.Chmod(signingKeyPath, perms.ReadOnly); err != nil { return nil, fmt.Errorf("couldn't restrict permissions on new signing key at %s: %w", signingKeyPath, err) } - return key, nil + + return signer, nil } -func getStakingConfig(v *viper.Viper, networkID uint32) (node.StakingConfig, error) { +func getStakingConfig(ctx context.Context, v *viper.Viper, networkID uint32) (node.StakingConfig, error) { config := node.StakingConfig{ SybilProtectionEnabled: v.GetBool(SybilProtectionEnabledKey), SybilProtectionDisabledWeight: v.GetUint64(SybilProtectionDisabledWeightKey), @@ -713,6 +751,7 @@ func getStakingConfig(v *viper.Viper, networkID uint32) (node.StakingConfig, err StakingKeyPath: getExpandedArg(v, StakingTLSKeyPathKey), StakingCertPath: getExpandedArg(v, StakingCertPathKey), StakingSignerPath: getExpandedArg(v, StakingSignerKeyPathKey), + StakingSignerRpc: getExpandedArg(v, StakingRPCSignerKey), } if !config.SybilProtectionEnabled && config.SybilProtectionDisabledWeight == 0 { return node.StakingConfig{}, errSybilProtectionDisabledStakerWeights @@ -727,7 +766,7 @@ func getStakingConfig(v *viper.Viper, networkID uint32) (node.StakingConfig, err if err != nil { return node.StakingConfig{}, err } - config.StakingSigningKey, err = getStakingSigner(v) + config.StakingSigningKey, err = getStakingSigner(ctx, v) if err != nil { return node.StakingConfig{}, err } @@ -1257,7 +1296,7 @@ func getPluginDir(v *viper.Viper) (string, error) { return pluginDir, nil } -func GetNodeConfig(v *viper.Viper) (node.Config, error) { +func GetNodeConfig(ctx context.Context, v *viper.Viper) (node.Config, error) { var ( nodeConfig node.Config err error @@ -1312,7 +1351,7 @@ func GetNodeConfig(v *viper.Viper) (node.Config, error) { } // Staking - nodeConfig.StakingConfig, err = getStakingConfig(v, nodeConfig.NetworkID) + nodeConfig.StakingConfig, err = getStakingConfig(ctx, v, nodeConfig.NetworkID) if err != nil { return node.Config{}, err } diff --git a/config/config_test.go b/config/config_test.go index 0aed70c79f44..a868e8fc24ff 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -4,22 +4,31 @@ package config import ( + "context" "encoding/base64" "encoding/json" "fmt" "log" + "net" "os" "path/filepath" + "reflect" "testing" + "google.golang.org/grpc" + "github.com/spf13/pflag" "github.com/spf13/viper" "github.com/stretchr/testify/require" "github.com/ava-labs/avalanchego/chains" "github.com/ava-labs/avalanchego/ids" + "github.com/ava-labs/avalanchego/proto/pb/signer" "github.com/ava-labs/avalanchego/snow/consensus/snowball" "github.com/ava-labs/avalanchego/subnets" + "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/localsigner" + "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/rpcsigner" + "github.com/ava-labs/avalanchego/utils/perms" ) const chainConfigFilenameExtension = ".ex" @@ -549,6 +558,109 @@ func TestGetSubnetConfigsFromFlags(t *testing.T) { } } +type signerServer struct { + signer.UnimplementedSignerServer +} + +func (*signerServer) PublicKey(context.Context, *signer.PublicKeyRequest) (*signer.PublicKeyResponse, error) { + // for tests to pass, this must be the base64 encoding of a 32 byte public key + // but it does not need to be associated with any private key + bytes, err := base64.StdEncoding.DecodeString("j8Ndzc1I6EYWYUWAdhcwpQ1I2xX/i4fdwgJIaxbHlf9yQKMT0jlReiiLYsydgaS1") + if err != nil { + return nil, err + } + + return &signer.PublicKeyResponse{ + PublicKey: bytes, + }, nil +} + +func TestGetStakingSigner(t *testing.T) { + testKey := "HLimS3vRibTMk9lZD4b+Z+GLuSBShvgbsu0WTLt2Kd4=" + rpcServer := grpc.NewServer() + defer rpcServer.GracefulStop() + + signer.RegisterSignerServer(rpcServer, &signerServer{}) + + listener, err := net.Listen("tcp", "[::1]:0") + require.NoError(t, err) + + go func() { + err := rpcServer.Serve(listener) + require.NoError(t, err) + }() + + type config map[string]any + + tests := []struct { + name string + viperKeys string + config config + expectedSignerType reflect.Type + expectedErr error + }{ + { + name: "default-signer", + expectedSignerType: reflect.TypeOf(&localsigner.LocalSigner{}), + }, + { + name: "ephemeral-signer", + config: config{StakingEphemeralSignerEnabledKey: true}, + expectedSignerType: reflect.TypeOf(&localsigner.LocalSigner{}), + }, + { + name: "content-key", + config: config{StakingSignerKeyContentKey: testKey}, + expectedSignerType: reflect.TypeOf(&localsigner.LocalSigner{}), + }, + { + name: "file-key", + config: config{ + StakingSignerKeyPathKey: func() string { + filePath := filepath.Join(t.TempDir(), "signer.key") + bytes, err := base64.StdEncoding.DecodeString(testKey) + require.NoError(t, err) + require.NoError(t, os.WriteFile(filePath, bytes, perms.ReadWrite)) + return filePath + }(), + }, + expectedSignerType: reflect.TypeOf(&localsigner.LocalSigner{}), + }, + { + name: "rpc-signer", + config: config{StakingRPCSignerKey: listener.Addr().String()}, + expectedSignerType: reflect.TypeOf(&rpcsigner.Client{}), + }, + { + name: "multiple-configurations-set", + config: config{ + StakingEphemeralSignerEnabledKey: true, + StakingSignerKeyContentKey: testKey, + }, + expectedErr: errInvalidSignerConfig, + }, + } + + // required for proper write permissions for the default signer-key location + t.Setenv("HOME", t.TempDir()) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require := require.New(t) + v := setupViperFlags() + + for key, value := range tt.config { + v.Set(key, value) + } + + signer, err := getStakingSigner(context.Background(), v) + + require.ErrorIs(err, tt.expectedErr) + require.Equal(tt.expectedSignerType, reflect.TypeOf(signer)) + }) + } +} + // setups config json file and writes content func setupConfigJSON(t *testing.T, rootPath string, value string) string { configFilePath := filepath.Join(rootPath, "config.json") diff --git a/config/flags.go b/config/flags.go index d906f18d93c6..c415a7c44942 100644 --- a/config/flags.go +++ b/config/flags.go @@ -264,6 +264,7 @@ func addNodeFlags(fs *pflag.FlagSet) { fs.Bool(StakingEphemeralSignerEnabledKey, false, "If true, the node uses an ephemeral staking signer key") fs.String(StakingSignerKeyPathKey, defaultStakingSignerKeyPath, fmt.Sprintf("Path to the signer private key for staking. Ignored if %s is specified", StakingSignerKeyContentKey)) fs.String(StakingSignerKeyContentKey, "", "Specifies base64 encoded signer private key for staking") + fs.String(StakingRPCSignerKey, "", "Specifies the RPC endpoint of the staking signer") fs.Bool(SybilProtectionEnabledKey, true, "Enables sybil protection. If enabled, Network TLS is required") fs.Uint64(SybilProtectionDisabledWeightKey, 100, "Weight to provide to each peer when sybil protection is disabled") fs.Bool(PartialSyncPrimaryNetworkKey, false, "Only sync the P-chain on the Primary Network. If the node is a Primary Network validator, it will report unhealthy") diff --git a/config/keys.go b/config/keys.go index 764faee4c428..2938baf3ec7f 100644 --- a/config/keys.go +++ b/config/keys.go @@ -85,6 +85,7 @@ const ( StakingEphemeralSignerEnabledKey = "staking-ephemeral-signer-enabled" StakingSignerKeyPathKey = "staking-signer-key-file" StakingSignerKeyContentKey = "staking-signer-key-file-content" + StakingRPCSignerKey = "staking-rpc-signer" SybilProtectionEnabledKey = "sybil-protection-enabled" SybilProtectionDisabledWeightKey = "sybil-protection-disabled-weight" NetworkInitialTimeoutKey = "network-initial-timeout" diff --git a/config/node/config.go b/config/node/config.go index a97c3b84df2e..4c8e20c9f566 100644 --- a/config/node/config.go +++ b/config/node/config.go @@ -82,6 +82,7 @@ type StakingConfig struct { StakingKeyPath string `json:"stakingKeyPath"` StakingCertPath string `json:"stakingCertPath"` StakingSignerPath string `json:"stakingSignerPath"` + StakingSignerRpc string `json:"stakingSignerRpc"` } type StateSyncConfig struct { diff --git a/main/main.go b/main/main.go index 88de72dadbbe..47f9ba245f3e 100644 --- a/main/main.go +++ b/main/main.go @@ -4,6 +4,7 @@ package main import ( + "context" "encoding/json" "errors" "fmt" @@ -51,7 +52,7 @@ func main() { os.Exit(0) } - nodeConfig, err := config.GetNodeConfig(v) + nodeConfig, err := config.GetNodeConfig(context.Background(), v) if err != nil { fmt.Printf("couldn't load node config: %s\n", err) os.Exit(1) From dd99c36b5e9fee05126eb966d4ec1f5c802a4c66 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Wed, 19 Feb 2025 16:01:38 -0500 Subject: [PATCH 03/37] Add default behaviour to switch --- config/config.go | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/config/config.go b/config/config.go index 910be46349b2..e6f328f6fc17 100644 --- a/config/config.go +++ b/config/config.go @@ -718,29 +718,29 @@ func getStakingSigner(ctx context.Context, v *viper.Viper) (bls.Signer, error) { case ephemeralSignerEnabled || contentKeyIsSet || keyPathIsSet || rpcSignerURLIsSet: return nil, errInvalidSignerConfig - } + default: - // no config values explicitly set, + signer, err := localsigner.New() + if err != nil { + return nil, fmt.Errorf("couldn't generate new signing key: %w", err) + } - signer, err := localsigner.New() - if err != nil { - return nil, fmt.Errorf("couldn't generate new signing key: %w", err) - } + if err := os.MkdirAll(filepath.Dir(signingKeyPath), perms.ReadWriteExecute); err != nil { + return nil, fmt.Errorf("couldn't create path for signing key at %s: %w", signingKeyPath, err) + } - if err := os.MkdirAll(filepath.Dir(signingKeyPath), perms.ReadWriteExecute); err != nil { - return nil, fmt.Errorf("couldn't create path for signing key at %s: %w", signingKeyPath, err) - } + keyBytes := signer.ToBytes() + if err := os.WriteFile(signingKeyPath, keyBytes, perms.ReadWrite); err != nil { + return nil, fmt.Errorf("couldn't write new signing key to %s: %w", signingKeyPath, err) + } - keyBytes := signer.ToBytes() - if err := os.WriteFile(signingKeyPath, keyBytes, perms.ReadWrite); err != nil { - return nil, fmt.Errorf("couldn't write new signing key to %s: %w", signingKeyPath, err) - } + if err := os.Chmod(signingKeyPath, perms.ReadOnly); err != nil { + return nil, fmt.Errorf("couldn't restrict permissions on new signing key at %s: %w", signingKeyPath, err) + } - if err := os.Chmod(signingKeyPath, perms.ReadOnly); err != nil { - return nil, fmt.Errorf("couldn't restrict permissions on new signing key at %s: %w", signingKeyPath, err) + return signer, nil } - return signer, nil } func getStakingConfig(ctx context.Context, v *viper.Viper, networkID uint32) (node.StakingConfig, error) { From 9f61b1cef500edba2fe65bd051e88b408fe357c4 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Wed, 19 Feb 2025 16:27:42 -0500 Subject: [PATCH 04/37] Add timeout to signer instantiation --- config/config.go | 2 -- main/main.go | 5 ++++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/config/config.go b/config/config.go index e6f328f6fc17..21bfc878d25a 100644 --- a/config/config.go +++ b/config/config.go @@ -719,7 +719,6 @@ func getStakingSigner(ctx context.Context, v *viper.Viper) (bls.Signer, error) { case ephemeralSignerEnabled || contentKeyIsSet || keyPathIsSet || rpcSignerURLIsSet: return nil, errInvalidSignerConfig default: - signer, err := localsigner.New() if err != nil { return nil, fmt.Errorf("couldn't generate new signing key: %w", err) @@ -740,7 +739,6 @@ func getStakingSigner(ctx context.Context, v *viper.Viper) (bls.Signer, error) { return signer, nil } - } func getStakingConfig(ctx context.Context, v *viper.Viper, networkID uint32) (node.StakingConfig, error) { diff --git a/main/main.go b/main/main.go index 47f9ba245f3e..9d5e55c0bb5d 100644 --- a/main/main.go +++ b/main/main.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "os" + "time" "github.com/spf13/pflag" "golang.org/x/term" @@ -52,7 +53,9 @@ func main() { os.Exit(0) } - nodeConfig, err := config.GetNodeConfig(context.Background(), v) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + nodeConfig, err := config.GetNodeConfig(ctx, v) + cancel() if err != nil { fmt.Printf("couldn't load node config: %s\n", err) os.Exit(1) From f106e92209fb8a0d9f7f72d92fce92dc8a3911fa Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Thu, 20 Feb 2025 17:41:02 -0500 Subject: [PATCH 05/37] Make rpc-signer client handle the connection --- config/config.go | 14 ++---------- config/config_test.go | 3 +-- config/node/config.go | 2 +- utils/crypto/bls/signer/rpcsigner/client.go | 24 ++++++++++++++++++++- 4 files changed, 27 insertions(+), 16 deletions(-) diff --git a/config/config.go b/config/config.go index 21bfc878d25a..9ba1cc16bbd9 100644 --- a/config/config.go +++ b/config/config.go @@ -18,8 +18,6 @@ import ( "time" "github.com/spf13/viper" - "google.golang.org/grpc" - "google.golang.org/grpc/credentials/insecure" "github.com/ava-labs/avalanchego/api/server" "github.com/ava-labs/avalanchego/chains" @@ -701,19 +699,11 @@ func getStakingSigner(ctx context.Context, v *viper.Viper) (bls.Signer, error) { case !ephemeralSignerEnabled && !contentKeyIsSet && !keyPathIsSet && rpcSignerURLIsSet: rpcSignerURL := v.GetString(StakingRPCSignerKey) - // the rpc-signer client should call a proxy server (on the same machine) that forwards - // the request to the actual signer instead of relying on tls-credentials - conn, err := grpc.NewClient(rpcSignerURL, grpc.WithTransportCredentials(insecure.NewCredentials())) + signer, err := rpcsigner.NewClient(ctx, rpcSignerURL) if err != nil { return nil, fmt.Errorf("couldn't create rpc signer client: %w", err) } - signer, err := rpcsigner.NewClient(ctx, conn) - if err != nil { - conn.Close() - return nil, fmt.Errorf("couldn't create rpc signer client: %w", err) - } - return signer, nil case ephemeralSignerEnabled || contentKeyIsSet || keyPathIsSet || rpcSignerURLIsSet: @@ -749,7 +739,7 @@ func getStakingConfig(ctx context.Context, v *viper.Viper, networkID uint32) (no StakingKeyPath: getExpandedArg(v, StakingTLSKeyPathKey), StakingCertPath: getExpandedArg(v, StakingCertPathKey), StakingSignerPath: getExpandedArg(v, StakingSignerKeyPathKey), - StakingSignerRpc: getExpandedArg(v, StakingRPCSignerKey), + StakingSignerRPC: getExpandedArg(v, StakingRPCSignerKey), } if !config.SybilProtectionEnabled && config.SybilProtectionDisabledWeight == 0 { return node.StakingConfig{}, errSybilProtectionDisabledStakerWeights diff --git a/config/config_test.go b/config/config_test.go index a868e8fc24ff..599ef8df5d90 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -15,11 +15,10 @@ import ( "reflect" "testing" - "google.golang.org/grpc" - "github.com/spf13/pflag" "github.com/spf13/viper" "github.com/stretchr/testify/require" + "google.golang.org/grpc" "github.com/ava-labs/avalanchego/chains" "github.com/ava-labs/avalanchego/ids" diff --git a/config/node/config.go b/config/node/config.go index 4c8e20c9f566..8abef8328988 100644 --- a/config/node/config.go +++ b/config/node/config.go @@ -82,7 +82,7 @@ type StakingConfig struct { StakingKeyPath string `json:"stakingKeyPath"` StakingCertPath string `json:"stakingCertPath"` StakingSignerPath string `json:"stakingSignerPath"` - StakingSignerRpc string `json:"stakingSignerRpc"` + StakingSignerRPC string `json:"stakingSignerRpc"` } type StateSyncConfig struct { diff --git a/utils/crypto/bls/signer/rpcsigner/client.go b/utils/crypto/bls/signer/rpcsigner/client.go index 294bb1829d81..d7c2bd5c94aa 100644 --- a/utils/crypto/bls/signer/rpcsigner/client.go +++ b/utils/crypto/bls/signer/rpcsigner/client.go @@ -5,8 +5,11 @@ package rpcsigner import ( "context" + "fmt" "google.golang.org/grpc" + "google.golang.org/grpc/backoff" + "google.golang.org/grpc/credentials/insecure" "github.com/ava-labs/avalanchego/utils/crypto/bls" @@ -21,17 +24,30 @@ type Client struct { pk *bls.PublicKey } -func NewClient(ctx context.Context, conn *grpc.ClientConn) (*Client, error) { +func NewClient(ctx context.Context, rpcSignerURL string) (*Client, error) { + // TODO: figure out the best parameters here given the target block-time + opts := grpc.WithConnectParams(grpc.ConnectParams{ + Backoff: backoff.DefaultConfig, + }) + + // the rpc-signer client should call a proxy server (on the same machine) that forwards + // the request to the actual signer instead of relying on tls-credentials + conn, err := grpc.NewClient(rpcSignerURL, opts, grpc.WithTransportCredentials(insecure.NewCredentials())) + if err != nil { + return nil, fmt.Errorf("couldn't create rpc signer client: %w", err) + } client := pb.NewSignerClient(conn) pubkeyResponse, err := client.PublicKey(ctx, &pb.PublicKeyRequest{}) if err != nil { + conn.Close() return nil, err } pkBytes := pubkeyResponse.GetPublicKey() pk, err := bls.PublicKeyFromCompressedBytes(pkBytes) if err != nil { + conn.Close() return nil, err } @@ -46,9 +62,12 @@ func (c *Client) PublicKey() *bls.PublicKey { return c.pk } +// Sign a message. The [Client] already handles transient connection errors. If this method fails, it will +// render the client in an unusable state and the client should be discarded. func (c *Client) Sign(message []byte) (*bls.Signature, error) { resp, err := c.client.Sign(context.TODO(), &pb.SignRequest{Message: message}) if err != nil { + c.conn.Close() return nil, err } signature := resp.GetSignature() @@ -56,9 +75,12 @@ func (c *Client) Sign(message []byte) (*bls.Signature, error) { return bls.SignatureFromBytes(signature) } +// [SignProofOfPossession] has the same behavior as [Sign] but will product a different signature. +// See BLS spec for more details. func (c *Client) SignProofOfPossession(message []byte) (*bls.Signature, error) { resp, err := c.client.SignProofOfPossession(context.TODO(), &pb.SignProofOfPossessionRequest{Message: message}) if err != nil { + c.conn.Close() return nil, err } signature := resp.GetSignature() From de574a432355231b913610d547f8861a1190ad39 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Thu, 20 Feb 2025 23:51:21 -0500 Subject: [PATCH 06/37] Fix linter error --- config/config_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/config/config_test.go b/config/config_test.go index 599ef8df5d90..4819a5ccaf16 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -585,8 +585,7 @@ func TestGetStakingSigner(t *testing.T) { require.NoError(t, err) go func() { - err := rpcServer.Serve(listener) - require.NoError(t, err) + require.NoError(t, rpcServer.Serve(listener)) }() type config map[string]any From a7e895a25f15cffd5fcd5b5dcc582d90e913ca1d Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Fri, 21 Feb 2025 00:24:03 -0500 Subject: [PATCH 07/37] Close the connection on any error --- utils/crypto/bls/signer/rpcsigner/client.go | 50 +++++++++++++++++---- 1 file changed, 42 insertions(+), 8 deletions(-) diff --git a/utils/crypto/bls/signer/rpcsigner/client.go b/utils/crypto/bls/signer/rpcsigner/client.go index d7c2bd5c94aa..af0a5eda150c 100644 --- a/utils/crypto/bls/signer/rpcsigner/client.go +++ b/utils/crypto/bls/signer/rpcsigner/client.go @@ -36,18 +36,22 @@ func NewClient(ctx context.Context, rpcSignerURL string) (*Client, error) { if err != nil { return nil, fmt.Errorf("couldn't create rpc signer client: %w", err) } + defer func() { + if err != nil { + conn.Close() + } + }() + client := pb.NewSignerClient(conn) pubkeyResponse, err := client.PublicKey(ctx, &pb.PublicKeyRequest{}) if err != nil { - conn.Close() return nil, err } pkBytes := pubkeyResponse.GetPublicKey() pk, err := bls.PublicKeyFromCompressedBytes(pkBytes) if err != nil { - conn.Close() return nil, err } @@ -65,25 +69,55 @@ func (c *Client) PublicKey() *bls.PublicKey { // Sign a message. The [Client] already handles transient connection errors. If this method fails, it will // render the client in an unusable state and the client should be discarded. func (c *Client) Sign(message []byte) (*bls.Signature, error) { + var err error + defer func() { + if err != nil { + c.Close() + } + }() + resp, err := c.client.Sign(context.TODO(), &pb.SignRequest{Message: message}) if err != nil { - c.conn.Close() return nil, err } - signature := resp.GetSignature() - return bls.SignatureFromBytes(signature) + sigBytes := resp.GetSignature() + sig, err := bls.SignatureFromBytes(sigBytes) + if err != nil { + return nil, err + } + + return sig, nil } // [SignProofOfPossession] has the same behavior as [Sign] but will product a different signature. // See BLS spec for more details. func (c *Client) SignProofOfPossession(message []byte) (*bls.Signature, error) { + var err error + defer func() { + if err != nil { + c.Close() + } + }() + resp, err := c.client.SignProofOfPossession(context.TODO(), &pb.SignProofOfPossessionRequest{Message: message}) if err != nil { - c.conn.Close() return nil, err } - signature := resp.GetSignature() - return bls.SignatureFromBytes(signature) + sigBytes := resp.GetSignature() + sig, err := bls.SignatureFromBytes(sigBytes) + if err != nil { + return nil, err + } + + return sig, nil +} + +func (c *Client) Close() error { + if c.conn == nil { + return nil + } + + return c.conn.Close() } From 15a53f04ace2242048d553ae9ce33cf55951461a Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Wed, 12 Mar 2025 15:00:07 -0400 Subject: [PATCH 08/37] Remove special character from error message --- utils/crypto/bls/signer/rpcsigner/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/crypto/bls/signer/rpcsigner/client.go b/utils/crypto/bls/signer/rpcsigner/client.go index af0a5eda150c..2645d0e52cd3 100644 --- a/utils/crypto/bls/signer/rpcsigner/client.go +++ b/utils/crypto/bls/signer/rpcsigner/client.go @@ -34,7 +34,7 @@ func NewClient(ctx context.Context, rpcSignerURL string) (*Client, error) { // the request to the actual signer instead of relying on tls-credentials conn, err := grpc.NewClient(rpcSignerURL, opts, grpc.WithTransportCredentials(insecure.NewCredentials())) if err != nil { - return nil, fmt.Errorf("couldn't create rpc signer client: %w", err) + return nil, fmt.Errorf("failed to create rpc signer client: %w", err) } defer func() { if err != nil { From cc11cac1adc5f74a3db5f57551525019ad5d6b65 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Wed, 12 Mar 2025 18:04:19 -0400 Subject: [PATCH 09/37] Use Go casing conventions in json config --- config/node/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/node/config.go b/config/node/config.go index 8abef8328988..ad8945413a8c 100644 --- a/config/node/config.go +++ b/config/node/config.go @@ -82,7 +82,7 @@ type StakingConfig struct { StakingKeyPath string `json:"stakingKeyPath"` StakingCertPath string `json:"stakingCertPath"` StakingSignerPath string `json:"stakingSignerPath"` - StakingSignerRPC string `json:"stakingSignerRpc"` + StakingSignerRPC string `json:"stakingSignerRPC"` } type StateSyncConfig struct { From eeb3644cbffbfc085a4f5b7fe63e5b2e8aedfd7d Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Wed, 12 Mar 2025 19:29:38 -0400 Subject: [PATCH 10/37] Fix default signing config behaviour --- config/config.go | 49 +++++++++++++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/config/config.go b/config/config.go index 9ba1cc16bbd9..4587d732ac57 100644 --- a/config/config.go +++ b/config/config.go @@ -35,6 +35,7 @@ import ( "github.com/ava-labs/avalanchego/subnets" "github.com/ava-labs/avalanchego/trace" "github.com/ava-labs/avalanchego/upgrade" + "github.com/ava-labs/avalanchego/utils/bag" "github.com/ava-labs/avalanchego/utils/compression" "github.com/ava-labs/avalanchego/utils/constants" "github.com/ava-labs/avalanchego/utils/crypto/bls" @@ -651,8 +652,13 @@ func getStakingSigner(ctx context.Context, v *viper.Viper) (bls.Signer, error) { signingKeyPath := getExpandedArg(v, StakingSignerKeyPathKey) + bools := bag.Of(ephemeralSignerEnabled, contentKeyIsSet, keyPathIsSet, rpcSignerURLIsSet) + if bools.Count(true) > 1 { + return nil, errInvalidSignerConfig + } + switch { - case ephemeralSignerEnabled && !contentKeyIsSet && !keyPathIsSet && !rpcSignerURLIsSet: + case ephemeralSignerEnabled: signer, err := localsigner.New() if err != nil { return nil, fmt.Errorf("couldn't generate ephemeral signing signer: %w", err) @@ -660,7 +666,7 @@ func getStakingSigner(ctx context.Context, v *viper.Viper) (bls.Signer, error) { return signer, nil - case !ephemeralSignerEnabled && contentKeyIsSet && !keyPathIsSet && !rpcSignerURLIsSet: + case contentKeyIsSet: signerKeyRawContent := v.GetString(StakingSignerKeyContentKey) signerKeyContent, err := base64.StdEncoding.DecodeString(signerKeyRawContent) if err != nil { @@ -674,7 +680,7 @@ func getStakingSigner(ctx context.Context, v *viper.Viper) (bls.Signer, error) { return signer, nil - case !ephemeralSignerEnabled && !contentKeyIsSet && keyPathIsSet && !rpcSignerURLIsSet: + case keyPathIsSet: // If the key is set, but a user-file isn't provided, we don't create one. // The siging key is only stored to the default file-location if it's created // and saved by the current application run. @@ -684,19 +690,9 @@ func getStakingSigner(ctx context.Context, v *viper.Viper) (bls.Signer, error) { return nil, errMissingStakingSigningKeyFile } - signingKeyBytes, err := os.ReadFile(signingKeyPath) - if err != nil { - return nil, err - } + return createSignerFromFile(signingKeyPath) - signer, err := localsigner.FromBytes(signingKeyBytes) - if err != nil { - return nil, fmt.Errorf("couldn't parse signing key: %w", err) - } - - return signer, nil - - case !ephemeralSignerEnabled && !contentKeyIsSet && !keyPathIsSet && rpcSignerURLIsSet: + case rpcSignerURLIsSet: rpcSignerURL := v.GetString(StakingRPCSignerKey) signer, err := rpcsigner.NewClient(ctx, rpcSignerURL) @@ -706,9 +702,14 @@ func getStakingSigner(ctx context.Context, v *viper.Viper) (bls.Signer, error) { return signer, nil - case ephemeralSignerEnabled || contentKeyIsSet || keyPathIsSet || rpcSignerURLIsSet: - return nil, errInvalidSignerConfig default: + _, err := os.Stat(signingKeyPath) + + // if the file exists, try to load the key from the file + if !errors.Is(err, fs.ErrNotExist) { + return createSignerFromFile(signingKeyPath) + } + signer, err := localsigner.New() if err != nil { return nil, fmt.Errorf("couldn't generate new signing key: %w", err) @@ -731,6 +732,20 @@ func getStakingSigner(ctx context.Context, v *viper.Viper) (bls.Signer, error) { } } +func createSignerFromFile(signingKeyPath string) (bls.Signer, error) { + signingKeyBytes, err := os.ReadFile(signingKeyPath) + if err != nil { + return nil, err + } + + signer, err := localsigner.FromBytes(signingKeyBytes) + if err != nil { + return nil, fmt.Errorf("couldn't parse signing key: %w", err) + } + + return signer, nil +} + func getStakingConfig(ctx context.Context, v *viper.Viper, networkID uint32) (node.StakingConfig, error) { config := node.StakingConfig{ SybilProtectionEnabled: v.GetBool(SybilProtectionEnabledKey), From 47e37fa210c2f281bbe9beb0c574c0edf7fd2ee0 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Wed, 12 Mar 2025 20:05:40 -0400 Subject: [PATCH 11/37] Cleanup type-check in config-test --- config/config_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/config/config_test.go b/config/config_test.go index 4819a5ccaf16..4496ffdb5e6b 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -12,7 +12,6 @@ import ( "net" "os" "path/filepath" - "reflect" "testing" "github.com/spf13/pflag" @@ -25,6 +24,7 @@ import ( "github.com/ava-labs/avalanchego/proto/pb/signer" "github.com/ava-labs/avalanchego/snow/consensus/snowball" "github.com/ava-labs/avalanchego/subnets" + "github.com/ava-labs/avalanchego/utils/crypto/bls" "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/localsigner" "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/rpcsigner" "github.com/ava-labs/avalanchego/utils/perms" @@ -594,22 +594,22 @@ func TestGetStakingSigner(t *testing.T) { name string viperKeys string config config - expectedSignerType reflect.Type + expectedSignerType bls.Signer expectedErr error }{ { name: "default-signer", - expectedSignerType: reflect.TypeOf(&localsigner.LocalSigner{}), + expectedSignerType: &localsigner.LocalSigner{}, }, { name: "ephemeral-signer", config: config{StakingEphemeralSignerEnabledKey: true}, - expectedSignerType: reflect.TypeOf(&localsigner.LocalSigner{}), + expectedSignerType: &localsigner.LocalSigner{}, }, { name: "content-key", config: config{StakingSignerKeyContentKey: testKey}, - expectedSignerType: reflect.TypeOf(&localsigner.LocalSigner{}), + expectedSignerType: &localsigner.LocalSigner{}, }, { name: "file-key", @@ -622,12 +622,12 @@ func TestGetStakingSigner(t *testing.T) { return filePath }(), }, - expectedSignerType: reflect.TypeOf(&localsigner.LocalSigner{}), + expectedSignerType: &localsigner.LocalSigner{}, }, { name: "rpc-signer", config: config{StakingRPCSignerKey: listener.Addr().String()}, - expectedSignerType: reflect.TypeOf(&rpcsigner.Client{}), + expectedSignerType: &rpcsigner.Client{}, }, { name: "multiple-configurations-set", @@ -654,7 +654,7 @@ func TestGetStakingSigner(t *testing.T) { signer, err := getStakingSigner(context.Background(), v) require.ErrorIs(err, tt.expectedErr) - require.Equal(tt.expectedSignerType, reflect.TypeOf(signer)) + require.IsType(tt.expectedSignerType, signer) }) } } From 505c7fb85178223e43a5aac5807b6aa67ba30124 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Wed, 12 Mar 2025 20:07:33 -0400 Subject: [PATCH 12/37] Simply variable name for url --- utils/crypto/bls/signer/rpcsigner/client.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/utils/crypto/bls/signer/rpcsigner/client.go b/utils/crypto/bls/signer/rpcsigner/client.go index 2645d0e52cd3..7731b152189e 100644 --- a/utils/crypto/bls/signer/rpcsigner/client.go +++ b/utils/crypto/bls/signer/rpcsigner/client.go @@ -24,7 +24,7 @@ type Client struct { pk *bls.PublicKey } -func NewClient(ctx context.Context, rpcSignerURL string) (*Client, error) { +func NewClient(ctx context.Context, url string) (*Client, error) { // TODO: figure out the best parameters here given the target block-time opts := grpc.WithConnectParams(grpc.ConnectParams{ Backoff: backoff.DefaultConfig, @@ -32,7 +32,7 @@ func NewClient(ctx context.Context, rpcSignerURL string) (*Client, error) { // the rpc-signer client should call a proxy server (on the same machine) that forwards // the request to the actual signer instead of relying on tls-credentials - conn, err := grpc.NewClient(rpcSignerURL, opts, grpc.WithTransportCredentials(insecure.NewCredentials())) + conn, err := grpc.NewClient(url, opts, grpc.WithTransportCredentials(insecure.NewCredentials())) if err != nil { return nil, fmt.Errorf("failed to create rpc signer client: %w", err) } From 0d66c3ae970a83974e33ee64ba6a93fdc98ac37c Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Wed, 12 Mar 2025 20:24:05 -0400 Subject: [PATCH 13/37] Only use public config api in test --- config/config_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/config_test.go b/config/config_test.go index 4496ffdb5e6b..862d91472e9c 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -651,10 +651,10 @@ func TestGetStakingSigner(t *testing.T) { v.Set(key, value) } - signer, err := getStakingSigner(context.Background(), v) + config, err := GetNodeConfig(context.Background(), v) require.ErrorIs(err, tt.expectedErr) - require.IsType(tt.expectedSignerType, signer) + require.IsType(tt.expectedSignerType, config.StakingSigningKey) }) } } From f8837e8e6cf93b4b5ef04732f0f22636226bf6bc Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Wed, 12 Mar 2025 20:33:35 -0400 Subject: [PATCH 14/37] Add test for default config signer with multiple inits --- config/config_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/config/config_test.go b/config/config_test.go index 862d91472e9c..46eb189f6650 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -659,6 +659,21 @@ func TestGetStakingSigner(t *testing.T) { } } +func TestDefaultConfigInitializtionUsesExistingDefaultKey(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + require := require.New(t) + v := setupViperFlags() + + config1, err := GetNodeConfig(context.Background(), v) + require.NoError(err) + + config2, err := GetNodeConfig(context.Background(), v) + require.NoError(err) + + require.Equal(config1.StakingSigningKey.PublicKey(), config2.StakingSigningKey.PublicKey()) +} + // setups config json file and writes content func setupConfigJSON(t *testing.T, rootPath string, value string) string { configFilePath := filepath.Join(rootPath, "config.json") From bb56fd56f3e9a79f61463563e0752b50d751d846 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Mon, 17 Mar 2025 22:27:53 -0400 Subject: [PATCH 15/37] Bubble up cleanup function --- config/config.go | 164 ++++++++++++-------- config/config_test.go | 13 +- main/main.go | 8 +- utils/crypto/bls/signer/rpcsigner/client.go | 58 ++----- 4 files changed, 126 insertions(+), 117 deletions(-) diff --git a/config/config.go b/config/config.go index 4587d732ac57..47da58e753c6 100644 --- a/config/config.go +++ b/config/config.go @@ -644,7 +644,7 @@ func getStakingTLSCert(v *viper.Viper) (tls.Certificate, error) { } } -func getStakingSigner(ctx context.Context, v *viper.Viper) (bls.Signer, error) { +func getStakingSigner(ctx context.Context, v *viper.Viper) (bls.Signer, func() error, error) { ephemeralSignerEnabled := v.GetBool(StakingEphemeralSignerEnabledKey) contentKeyIsSet := v.IsSet(StakingSignerKeyContentKey) keyPathIsSet := v.IsSet(StakingSignerKeyPathKey) @@ -654,31 +654,33 @@ func getStakingSigner(ctx context.Context, v *viper.Viper) (bls.Signer, error) { bools := bag.Of(ephemeralSignerEnabled, contentKeyIsSet, keyPathIsSet, rpcSignerURLIsSet) if bools.Count(true) > 1 { - return nil, errInvalidSignerConfig + return nil, nil, errInvalidSignerConfig } + cleanup := func() error { return nil } + switch { case ephemeralSignerEnabled: signer, err := localsigner.New() if err != nil { - return nil, fmt.Errorf("couldn't generate ephemeral signing signer: %w", err) + return nil, nil, fmt.Errorf("couldn't generate ephemeral signing signer: %w", err) } - return signer, nil + return signer, cleanup, nil case contentKeyIsSet: signerKeyRawContent := v.GetString(StakingSignerKeyContentKey) signerKeyContent, err := base64.StdEncoding.DecodeString(signerKeyRawContent) if err != nil { - return nil, fmt.Errorf("unable to decode base64 content: %w", err) + return nil, nil, fmt.Errorf("unable to decode base64 content: %w", err) } signer, err := localsigner.FromBytes(signerKeyContent) if err != nil { - return nil, fmt.Errorf("couldn't parse signing key: %w", err) + return nil, nil, fmt.Errorf("couldn't parse signing key: %w", err) } - return signer, nil + return signer, cleanup, nil case keyPathIsSet: // If the key is set, but a user-file isn't provided, we don't create one. @@ -687,48 +689,57 @@ func getStakingSigner(ctx context.Context, v *viper.Viper) (bls.Signer, error) { _, err := os.Stat(signingKeyPath) if errors.Is(err, fs.ErrNotExist) { - return nil, errMissingStakingSigningKeyFile + return nil, nil, errMissingStakingSigningKeyFile + } + + signer, err := createSignerFromFile(signingKeyPath) + if err != nil { + return nil, nil, fmt.Errorf("couldn't parse signing key: %w", err) } - return createSignerFromFile(signingKeyPath) + return signer, cleanup, nil case rpcSignerURLIsSet: rpcSignerURL := v.GetString(StakingRPCSignerKey) - signer, err := rpcsigner.NewClient(ctx, rpcSignerURL) + signer, cleanup, err := rpcsigner.NewClient(ctx, rpcSignerURL) if err != nil { - return nil, fmt.Errorf("couldn't create rpc signer client: %w", err) + return nil, nil, fmt.Errorf("couldn't create rpc signer client: %w", err) } - return signer, nil + return signer, cleanup, nil default: _, err := os.Stat(signingKeyPath) // if the file exists, try to load the key from the file if !errors.Is(err, fs.ErrNotExist) { - return createSignerFromFile(signingKeyPath) + signer, err := createSignerFromFile(signingKeyPath) + if err != nil { + return nil, nil, errors.Join(err, cleanup()) + } + return signer, cleanup, nil } signer, err := localsigner.New() if err != nil { - return nil, fmt.Errorf("couldn't generate new signing key: %w", err) + return nil, nil, fmt.Errorf("couldn't generate new signing key: %w", err) } if err := os.MkdirAll(filepath.Dir(signingKeyPath), perms.ReadWriteExecute); err != nil { - return nil, fmt.Errorf("couldn't create path for signing key at %s: %w", signingKeyPath, err) + return nil, nil, fmt.Errorf("couldn't create path for signing key at %s: %w", signingKeyPath, err) } keyBytes := signer.ToBytes() if err := os.WriteFile(signingKeyPath, keyBytes, perms.ReadWrite); err != nil { - return nil, fmt.Errorf("couldn't write new signing key to %s: %w", signingKeyPath, err) + return nil, nil, fmt.Errorf("couldn't write new signing key to %s: %w", signingKeyPath, err) } if err := os.Chmod(signingKeyPath, perms.ReadOnly); err != nil { - return nil, fmt.Errorf("couldn't restrict permissions on new signing key at %s: %w", signingKeyPath, err) + return nil, nil, fmt.Errorf("couldn't restrict permissions on new signing key at %s: %w", signingKeyPath, err) } - return signer, nil + return signer, cleanup, nil } } @@ -746,7 +757,7 @@ func createSignerFromFile(signingKeyPath string) (bls.Signer, error) { return signer, nil } -func getStakingConfig(ctx context.Context, v *viper.Viper, networkID uint32) (node.StakingConfig, error) { +func getStakingConfig(ctx context.Context, v *viper.Viper, networkID uint32) (node.StakingConfig, func() error, error) { config := node.StakingConfig{ SybilProtectionEnabled: v.GetBool(SybilProtectionEnabledKey), SybilProtectionDisabledWeight: v.GetUint64(SybilProtectionDisabledWeightKey), @@ -756,23 +767,27 @@ func getStakingConfig(ctx context.Context, v *viper.Viper, networkID uint32) (no StakingSignerPath: getExpandedArg(v, StakingSignerKeyPathKey), StakingSignerRPC: getExpandedArg(v, StakingRPCSignerKey), } + if !config.SybilProtectionEnabled && config.SybilProtectionDisabledWeight == 0 { - return node.StakingConfig{}, errSybilProtectionDisabledStakerWeights + return node.StakingConfig{}, nil, errSybilProtectionDisabledStakerWeights } if !config.SybilProtectionEnabled && (networkID == constants.MainnetID || networkID == constants.FujiID) { - return node.StakingConfig{}, errSybilProtectionDisabledOnPublicNetwork + return node.StakingConfig{}, nil, errSybilProtectionDisabledOnPublicNetwork } var err error config.StakingTLSCert, err = getStakingTLSCert(v) if err != nil { - return node.StakingConfig{}, err + return node.StakingConfig{}, nil, err } - config.StakingSigningKey, err = getStakingSigner(ctx, v) + + var cleanup func() error + config.StakingSigningKey, cleanup, err = getStakingSigner(ctx, v) if err != nil { - return node.StakingConfig{}, err + return node.StakingConfig{}, nil, err } + if networkID != constants.MainnetID && networkID != constants.FujiID { config.UptimeRequirement = v.GetFloat64(UptimeRequirementKey) config.MinValidatorStake = v.GetUint64(MinValidatorStakeKey) @@ -785,28 +800,35 @@ func getStakingConfig(ctx context.Context, v *viper.Viper, networkID uint32) (no config.RewardConfig.MintingPeriod = v.GetDuration(StakeMintingPeriodKey) config.RewardConfig.SupplyCap = v.GetUint64(StakeSupplyCapKey) config.MinDelegationFee = v.GetUint32(MinDelegatorFeeKey) + + var err error switch { case config.UptimeRequirement < 0 || config.UptimeRequirement > 1: - return node.StakingConfig{}, errInvalidUptimeRequirement + err = errInvalidUptimeRequirement case config.MinValidatorStake > config.MaxValidatorStake: - return node.StakingConfig{}, errMinValidatorStakeAboveMax + err = errMinValidatorStakeAboveMax case config.MinDelegationFee > 1_000_000: - return node.StakingConfig{}, errInvalidDelegationFee + err = errInvalidDelegationFee case config.MinStakeDuration <= 0: - return node.StakingConfig{}, errInvalidMinStakeDuration + err = errInvalidMinStakeDuration case config.MaxStakeDuration < config.MinStakeDuration: - return node.StakingConfig{}, errMinStakeDurationAboveMax + err = errMinStakeDurationAboveMax case config.RewardConfig.MaxConsumptionRate > reward.PercentDenominator: - return node.StakingConfig{}, errStakeMaxConsumptionTooLarge + err = errStakeMaxConsumptionTooLarge case config.RewardConfig.MaxConsumptionRate < config.RewardConfig.MinConsumptionRate: - return node.StakingConfig{}, errStakeMaxConsumptionBelowMin + err = errStakeMaxConsumptionBelowMin case config.RewardConfig.MintingPeriod < config.MaxStakeDuration: - return node.StakingConfig{}, errStakeMintingPeriodBelowMin + err = errStakeMintingPeriodBelowMin + } + + if err != nil { + return node.StakingConfig{}, nil, errors.Join(err, cleanup()) } } else { config.StakingConfig = genesis.GetStakingConfig(networkID) } - return config, nil + + return config, cleanup, nil } func getTxFeeConfig(v *viper.Viper, networkID uint32) genesis.TxFeeConfig { @@ -1299,7 +1321,7 @@ func getPluginDir(v *viper.Viper) (string, error) { return pluginDir, nil } -func GetNodeConfig(ctx context.Context, v *viper.Viper) (node.Config, error) { +func GetNodeConfig(ctx context.Context, v *viper.Viper) (node.Config, func() error, error) { var ( nodeConfig node.Config err error @@ -1307,24 +1329,24 @@ func GetNodeConfig(ctx context.Context, v *viper.Viper) (node.Config, error) { nodeConfig.PluginDir, err = getPluginDir(v) if err != nil { - return node.Config{}, err + return node.Config{}, nil, err } nodeConfig.ConsensusShutdownTimeout = v.GetDuration(ConsensusShutdownTimeoutKey) if nodeConfig.ConsensusShutdownTimeout < 0 { - return node.Config{}, fmt.Errorf("%q must be >= 0", ConsensusShutdownTimeoutKey) + return node.Config{}, nil, fmt.Errorf("%q must be >= 0", ConsensusShutdownTimeoutKey) } // Gossiping nodeConfig.FrontierPollFrequency = v.GetDuration(ConsensusFrontierPollFrequencyKey) if nodeConfig.FrontierPollFrequency < 0 { - return node.Config{}, fmt.Errorf("%s must be >= 0", ConsensusFrontierPollFrequencyKey) + return node.Config{}, nil, fmt.Errorf("%s must be >= 0", ConsensusFrontierPollFrequencyKey) } // App handling nodeConfig.ConsensusAppConcurrency = int(v.GetUint(ConsensusAppConcurrencyKey)) if nodeConfig.ConsensusAppConcurrency <= 0 { - return node.Config{}, fmt.Errorf("%s must be > 0", ConsensusAppConcurrencyKey) + return node.Config{}, nil, fmt.Errorf("%s must be > 0", ConsensusAppConcurrencyKey) } nodeConfig.UseCurrentHeight = v.GetBool(ProposerVMUseCurrentHeightKey) @@ -1332,60 +1354,63 @@ func GetNodeConfig(ctx context.Context, v *viper.Viper) (node.Config, error) { // Logging nodeConfig.LoggingConfig, err = getLoggingConfig(v) if err != nil { - return node.Config{}, err + return node.Config{}, nil, err } // Network ID nodeConfig.NetworkID, err = constants.NetworkID(v.GetString(NetworkNameKey)) if err != nil { - return node.Config{}, err + return node.Config{}, nil, err } // Database nodeConfig.DatabaseConfig, err = getDatabaseConfig(v, nodeConfig.NetworkID) if err != nil { - return node.Config{}, err + return node.Config{}, nil, err } // IP configuration nodeConfig.IPConfig, err = getIPConfig(v) if err != nil { - return node.Config{}, err + return node.Config{}, nil, err } // Staking - nodeConfig.StakingConfig, err = getStakingConfig(ctx, v, nodeConfig.NetworkID) + var cleanup func() error + nodeConfig.StakingConfig, cleanup, err = getStakingConfig(ctx, v, nodeConfig.NetworkID) if err != nil { - return node.Config{}, err + return node.Config{}, nil, err } // Tracked Subnets nodeConfig.TrackedSubnets, err = getTrackedSubnets(v) if err != nil { - return node.Config{}, err + return node.Config{}, nil, errors.Join(err, cleanup()) } // HTTP APIs nodeConfig.HTTPConfig, err = getHTTPConfig(v) if err != nil { - return node.Config{}, err + return node.Config{}, nil, errors.Join(err, cleanup()) } // Health nodeConfig.HealthCheckFreq = v.GetDuration(HealthCheckFreqKey) if nodeConfig.HealthCheckFreq < 0 { - return node.Config{}, fmt.Errorf("%s must be positive", HealthCheckFreqKey) + err = fmt.Errorf("%s must be positive", HealthCheckFreqKey) + return node.Config{}, nil, errors.Join(err, cleanup()) } // Halflife of continuous averager used in health checks healthCheckAveragerHalflife := v.GetDuration(HealthCheckAveragerHalflifeKey) if healthCheckAveragerHalflife <= 0 { - return node.Config{}, fmt.Errorf("%s must be positive", HealthCheckAveragerHalflifeKey) + err = fmt.Errorf("%s must be positive", HealthCheckAveragerHalflifeKey) + return node.Config{}, nil, errors.Join(err, cleanup()) } // Router nodeConfig.RouterHealthConfig, err = getRouterHealthConfig(v, healthCheckAveragerHalflife) if err != nil { - return node.Config{}, err + return node.Config{}, nil, errors.Join(err, cleanup()) } // Metrics @@ -1394,13 +1419,13 @@ func GetNodeConfig(ctx context.Context, v *viper.Viper) (node.Config, error) { // Adaptive Timeout Config nodeConfig.AdaptiveTimeoutConfig, err = getAdaptiveTimeoutConfig(v) if err != nil { - return node.Config{}, err + return node.Config{}, nil, errors.Join(err, cleanup()) } // Upgrade config nodeConfig.UpgradeConfig, err = getUpgradeConfig(v, nodeConfig.NetworkID) if err != nil { - return node.Config{}, err + return node.Config{}, nil, errors.Join(err, cleanup()) } // Network Config @@ -1411,18 +1436,20 @@ func GetNodeConfig(ctx context.Context, v *viper.Viper) (node.Config, error) { healthCheckAveragerHalflife, ) if err != nil { - return node.Config{}, err + return node.Config{}, nil, errors.Join(err, cleanup()) } // Subnet Configs subnetConfigs, err := getSubnetConfigs(v, nodeConfig.TrackedSubnets.List()) if err != nil { - return node.Config{}, fmt.Errorf("couldn't read subnet configs: %w", err) + err = fmt.Errorf("couldn't read subnet configs: %w", err) + return node.Config{}, nil, errors.Join(err, cleanup()) } primaryNetworkConfig := getDefaultSubnetConfig(v) if err := primaryNetworkConfig.Valid(); err != nil { - return node.Config{}, fmt.Errorf("invalid consensus parameters: %w", err) + err = fmt.Errorf("invalid consensus parameters: %w", err) + return node.Config{}, nil, errors.Join(err, cleanup()) } subnetConfigs[constants.PrimaryNetworkID] = primaryNetworkConfig @@ -1431,7 +1458,7 @@ func GetNodeConfig(ctx context.Context, v *viper.Viper) (node.Config, error) { // Benchlist nodeConfig.BenchlistConfig, err = getBenchlistConfig(v, primaryNetworkConfig.ConsensusParameters) if err != nil { - return node.Config{}, err + return node.Config{}, nil, errors.Join(err, cleanup()) } // File Descriptor Limit @@ -1444,42 +1471,44 @@ func GetNodeConfig(ctx context.Context, v *viper.Viper) (node.Config, error) { genesisStakingCfg := nodeConfig.StakingConfig.StakingConfig nodeConfig.GenesisBytes, nodeConfig.AvaxAssetID, err = getGenesisData(v, nodeConfig.NetworkID, &genesisStakingCfg) if err != nil { - return node.Config{}, fmt.Errorf("unable to load genesis file: %w", err) + err = fmt.Errorf("unable to load genesis file: %w", err) + return node.Config{}, nil, errors.Join(err, cleanup()) } // StateSync Configs nodeConfig.StateSyncConfig, err = getStateSyncConfig(v) if err != nil { - return node.Config{}, err + return node.Config{}, nil, errors.Join(err, cleanup()) } // Bootstrap Configs nodeConfig.BootstrapConfig, err = getBootstrapConfig(v, nodeConfig.NetworkID) if err != nil { - return node.Config{}, err + return node.Config{}, nil, errors.Join(err, cleanup()) } // Chain Configs nodeConfig.ChainConfigs, err = getChainConfigs(v) if err != nil { - return node.Config{}, fmt.Errorf("couldn't read chain configs: %w", err) + err = fmt.Errorf("couldn't read chain configs: %w", err) + return node.Config{}, nil, errors.Join(err, cleanup()) } // Profiler nodeConfig.ProfilerConfig, err = getProfilerConfig(v) if err != nil { - return node.Config{}, err + return node.Config{}, nil, errors.Join(err, cleanup()) } // VM Aliases nodeConfig.VMAliases, err = getVMAliases(v) if err != nil { - return node.Config{}, err + return node.Config{}, nil, errors.Join(err, cleanup()) } // Chain aliases nodeConfig.ChainAliases, err = getChainAliases(v) if err != nil { - return node.Config{}, err + return node.Config{}, nil, errors.Join(err, cleanup()) } nodeConfig.SystemTrackerFrequency = v.GetDuration(SystemTrackerFrequencyKey) @@ -1489,22 +1518,22 @@ func GetNodeConfig(ctx context.Context, v *viper.Viper) (node.Config, error) { nodeConfig.RequiredAvailableDiskSpace, nodeConfig.WarningThresholdAvailableDiskSpace, err = getDiskSpaceConfig(v) if err != nil { - return node.Config{}, err + return node.Config{}, nil, errors.Join(err, cleanup()) } nodeConfig.CPUTargeterConfig, err = getCPUTargeterConfig(v) if err != nil { - return node.Config{}, err + return node.Config{}, nil, errors.Join(err, cleanup()) } nodeConfig.DiskTargeterConfig, err = getDiskTargeterConfig(v) if err != nil { - return node.Config{}, err + return node.Config{}, nil, errors.Join(err, cleanup()) } nodeConfig.TraceConfig, err = getTraceConfig(v) if err != nil { - return node.Config{}, err + return node.Config{}, nil, errors.Join(err, cleanup()) } nodeConfig.ChainDataDir = getExpandedArg(v, ChainDataDirKey) @@ -1512,7 +1541,8 @@ func GetNodeConfig(ctx context.Context, v *viper.Viper) (node.Config, error) { nodeConfig.ProcessContextFilePath = getExpandedArg(v, ProcessContextFileKey) nodeConfig.ProvidedFlags = providedFlags(v) - return nodeConfig, nil + + return nodeConfig, cleanup, nil } func providedFlags(v *viper.Viper) map[string]interface{} { diff --git a/config/config_test.go b/config/config_test.go index 46eb189f6650..05c3c7e47571 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -651,7 +651,12 @@ func TestGetStakingSigner(t *testing.T) { v.Set(key, value) } - config, err := GetNodeConfig(context.Background(), v) + config, cleanup, err := GetNodeConfig(context.Background(), v) + defer func() { + if err == nil { + _ = cleanup() + } + }() require.ErrorIs(err, tt.expectedErr) require.IsType(tt.expectedSignerType, config.StakingSigningKey) @@ -665,10 +670,12 @@ func TestDefaultConfigInitializtionUsesExistingDefaultKey(t *testing.T) { require := require.New(t) v := setupViperFlags() - config1, err := GetNodeConfig(context.Background(), v) + config1, cleanup1, err := GetNodeConfig(context.Background(), v) + defer func() { _ = cleanup1() }() require.NoError(err) - config2, err := GetNodeConfig(context.Background(), v) + config2, cleanup2, err := GetNodeConfig(context.Background(), v) + defer func() { _ = cleanup2() }() require.NoError(err) require.Equal(config1.StakingSigningKey.PublicKey(), config2.StakingSigningKey.PublicKey()) diff --git a/main/main.go b/main/main.go index 9d5e55c0bb5d..72fa23778740 100644 --- a/main/main.go +++ b/main/main.go @@ -54,7 +54,7 @@ func main() { } ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - nodeConfig, err := config.GetNodeConfig(ctx, v) + nodeConfig, cleanup, err := config.GetNodeConfig(ctx, v) cancel() if err != nil { fmt.Printf("couldn't load node config: %s\n", err) @@ -68,9 +68,15 @@ func main() { nodeApp, err := app.New(nodeConfig) if err != nil { fmt.Printf("couldn't start node: %s\n", err) + if err := cleanup(); err != nil { + fmt.Printf("error cleaning up: %s\n", err) + } os.Exit(1) } exitCode := app.Run(nodeApp) + if err := cleanup(); err != nil { + fmt.Printf("error cleaning up: %s\n", err) + } os.Exit(exitCode) } diff --git a/utils/crypto/bls/signer/rpcsigner/client.go b/utils/crypto/bls/signer/rpcsigner/client.go index 7731b152189e..34b8c66c3391 100644 --- a/utils/crypto/bls/signer/rpcsigner/client.go +++ b/utils/crypto/bls/signer/rpcsigner/client.go @@ -5,6 +5,7 @@ package rpcsigner import ( "context" + "errors" "fmt" "google.golang.org/grpc" @@ -20,11 +21,10 @@ var _ bls.Signer = (*Client)(nil) type Client struct { client pb.SignerClient - conn *grpc.ClientConn pk *bls.PublicKey } -func NewClient(ctx context.Context, url string) (*Client, error) { +func NewClient(ctx context.Context, url string) (*Client, func() error, error) { // TODO: figure out the best parameters here given the target block-time opts := grpc.WithConnectParams(grpc.ConnectParams{ Backoff: backoff.DefaultConfig, @@ -34,32 +34,30 @@ func NewClient(ctx context.Context, url string) (*Client, error) { // the request to the actual signer instead of relying on tls-credentials conn, err := grpc.NewClient(url, opts, grpc.WithTransportCredentials(insecure.NewCredentials())) if err != nil { - return nil, fmt.Errorf("failed to create rpc signer client: %w", err) + return nil, func() error { return nil }, fmt.Errorf("failed to create rpc signer client: %w", err) + } + + cleanup := func() error { + return conn.Close() } - defer func() { - if err != nil { - conn.Close() - } - }() client := pb.NewSignerClient(conn) pubkeyResponse, err := client.PublicKey(ctx, &pb.PublicKeyRequest{}) if err != nil { - return nil, err + return nil, nil, errors.Join(err, cleanup()) } pkBytes := pubkeyResponse.GetPublicKey() pk, err := bls.PublicKeyFromCompressedBytes(pkBytes) if err != nil { - return nil, err + return nil, nil, errors.Join(err, cleanup()) } return &Client{ client: client, - conn: conn, pk: pk, - }, nil + }, cleanup, nil } func (c *Client) PublicKey() *bls.PublicKey { @@ -69,55 +67,23 @@ func (c *Client) PublicKey() *bls.PublicKey { // Sign a message. The [Client] already handles transient connection errors. If this method fails, it will // render the client in an unusable state and the client should be discarded. func (c *Client) Sign(message []byte) (*bls.Signature, error) { - var err error - defer func() { - if err != nil { - c.Close() - } - }() - resp, err := c.client.Sign(context.TODO(), &pb.SignRequest{Message: message}) if err != nil { return nil, err } sigBytes := resp.GetSignature() - sig, err := bls.SignatureFromBytes(sigBytes) - if err != nil { - return nil, err - } - - return sig, nil + return bls.SignatureFromBytes(sigBytes) } // [SignProofOfPossession] has the same behavior as [Sign] but will product a different signature. // See BLS spec for more details. func (c *Client) SignProofOfPossession(message []byte) (*bls.Signature, error) { - var err error - defer func() { - if err != nil { - c.Close() - } - }() - resp, err := c.client.SignProofOfPossession(context.TODO(), &pb.SignProofOfPossessionRequest{Message: message}) if err != nil { return nil, err } sigBytes := resp.GetSignature() - sig, err := bls.SignatureFromBytes(sigBytes) - if err != nil { - return nil, err - } - - return sig, nil -} - -func (c *Client) Close() error { - if c.conn == nil { - return nil - } - - return c.conn.Close() + return bls.SignatureFromBytes(sigBytes) } From bd77f88af84d10d226497e291524fb734180b7d7 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Fri, 28 Mar 2025 13:34:51 -0400 Subject: [PATCH 16/37] Simplify default signer creation code The code was changed without changing the behaviour. Instead of `os.Stat`ing the file, we just try to read the file (which returned the same error type). --- config/config.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/config/config.go b/config/config.go index 47da58e753c6..1eeb3038aa9e 100644 --- a/config/config.go +++ b/config/config.go @@ -710,15 +710,13 @@ func getStakingSigner(ctx context.Context, v *viper.Viper) (bls.Signer, func() e return signer, cleanup, nil default: - _, err := os.Stat(signingKeyPath) + signerFromFile, err := createSignerFromFile(signingKeyPath) + if err != nil && !errors.Is(err, fs.ErrNotExist) { + return nil, nil, err + } - // if the file exists, try to load the key from the file - if !errors.Is(err, fs.ErrNotExist) { - signer, err := createSignerFromFile(signingKeyPath) - if err != nil { - return nil, nil, errors.Join(err, cleanup()) - } - return signer, cleanup, nil + if signerFromFile != nil { + return signerFromFile, cleanup, nil } signer, err := localsigner.New() From 5bc1bc0d21a82efd1574fbb8607127da40c507f3 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Fri, 28 Mar 2025 13:50:56 -0400 Subject: [PATCH 17/37] Cleanup signer creation when key path is set Prior to this, we were checking if the file existed before attempting to readd it. It was unnecessary and the logic has been removed. --- config/config.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/config/config.go b/config/config.go index 1eeb3038aa9e..9a35c810a424 100644 --- a/config/config.go +++ b/config/config.go @@ -686,13 +686,11 @@ func getStakingSigner(ctx context.Context, v *viper.Viper) (bls.Signer, func() e // If the key is set, but a user-file isn't provided, we don't create one. // The siging key is only stored to the default file-location if it's created // and saved by the current application run. - _, err := os.Stat(signingKeyPath) - + signer, err := createSignerFromFile(signingKeyPath) if errors.Is(err, fs.ErrNotExist) { return nil, nil, errMissingStakingSigningKeyFile } - signer, err := createSignerFromFile(signingKeyPath) if err != nil { return nil, nil, fmt.Errorf("couldn't parse signing key: %w", err) } From 64f1a1d057e71351be93635ca1dff72cd98ef610 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Fri, 28 Mar 2025 13:54:29 -0400 Subject: [PATCH 18/37] Remove redundant word in error message --- config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/config.go b/config/config.go index 9a35c810a424..2df2544e86a3 100644 --- a/config/config.go +++ b/config/config.go @@ -663,7 +663,7 @@ func getStakingSigner(ctx context.Context, v *viper.Viper) (bls.Signer, func() e case ephemeralSignerEnabled: signer, err := localsigner.New() if err != nil { - return nil, nil, fmt.Errorf("couldn't generate ephemeral signing signer: %w", err) + return nil, nil, fmt.Errorf("couldn't generate ephemeral signer: %w", err) } return signer, cleanup, nil From b55f05d6c07d77c017a3ef93eacddf7161cb60c3 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Fri, 28 Mar 2025 13:56:19 -0400 Subject: [PATCH 19/37] Fix config test name --- config/config_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/config_test.go b/config/config_test.go index 05c3c7e47571..ab97eaba8682 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -664,7 +664,7 @@ func TestGetStakingSigner(t *testing.T) { } } -func TestDefaultConfigInitializtionUsesExistingDefaultKey(t *testing.T) { +func TestDefaultConfigInitializationUsesExistingDefaultKey(t *testing.T) { t.Setenv("HOME", t.TempDir()) require := require.New(t) From e85e815fed022400928fd848477638cf7507454e Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Fri, 28 Mar 2025 14:00:15 -0400 Subject: [PATCH 20/37] Small rpc-signer-client cleanup refactor --- utils/crypto/bls/signer/rpcsigner/client.go | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/utils/crypto/bls/signer/rpcsigner/client.go b/utils/crypto/bls/signer/rpcsigner/client.go index 34b8c66c3391..9ff7888ef00a 100644 --- a/utils/crypto/bls/signer/rpcsigner/client.go +++ b/utils/crypto/bls/signer/rpcsigner/client.go @@ -34,38 +34,33 @@ func NewClient(ctx context.Context, url string) (*Client, func() error, error) { // the request to the actual signer instead of relying on tls-credentials conn, err := grpc.NewClient(url, opts, grpc.WithTransportCredentials(insecure.NewCredentials())) if err != nil { - return nil, func() error { return nil }, fmt.Errorf("failed to create rpc signer client: %w", err) - } - - cleanup := func() error { - return conn.Close() + return nil, nil, fmt.Errorf("failed to create rpc signer client: %w", err) } client := pb.NewSignerClient(conn) pubkeyResponse, err := client.PublicKey(ctx, &pb.PublicKeyRequest{}) if err != nil { - return nil, nil, errors.Join(err, cleanup()) + return nil, nil, errors.Join(err, conn.Close()) } pkBytes := pubkeyResponse.GetPublicKey() pk, err := bls.PublicKeyFromCompressedBytes(pkBytes) if err != nil { - return nil, nil, errors.Join(err, cleanup()) + return nil, nil, errors.Join(err, conn.Close()) } return &Client{ client: client, pk: pk, - }, cleanup, nil + }, conn.Close, nil } func (c *Client) PublicKey() *bls.PublicKey { return c.pk } -// Sign a message. The [Client] already handles transient connection errors. If this method fails, it will -// render the client in an unusable state and the client should be discarded. +// Sign a message. The [Client] already handles transient connection errors. func (c *Client) Sign(message []byte) (*bls.Signature, error) { resp, err := c.client.Sign(context.TODO(), &pb.SignRequest{Message: message}) if err != nil { From c8b3714acee7ad4d4e7e3d14efd5667049459b3d Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Fri, 28 Mar 2025 14:20:52 -0400 Subject: [PATCH 21/37] Fix the min-connect-timeout for grpc signer --- utils/crypto/bls/signer/rpcsigner/client.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/utils/crypto/bls/signer/rpcsigner/client.go b/utils/crypto/bls/signer/rpcsigner/client.go index 9ff7888ef00a..bacfd2c4e8cb 100644 --- a/utils/crypto/bls/signer/rpcsigner/client.go +++ b/utils/crypto/bls/signer/rpcsigner/client.go @@ -7,6 +7,7 @@ import ( "context" "errors" "fmt" + "time" "google.golang.org/grpc" "google.golang.org/grpc/backoff" @@ -28,6 +29,8 @@ func NewClient(ctx context.Context, url string) (*Client, func() error, error) { // TODO: figure out the best parameters here given the target block-time opts := grpc.WithConnectParams(grpc.ConnectParams{ Backoff: backoff.DefaultConfig, + // same as grpc default + MinConnectTimeout: 20 * time.Second, }) // the rpc-signer client should call a proxy server (on the same machine) that forwards From 99c3a481b65ed31290acace76b7a97165c21f06e Mon Sep 17 00:00:00 2001 From: Geoff Stuart Date: Thu, 15 May 2025 11:47:13 -0400 Subject: [PATCH 22/37] Reconfigure signer setup --- config/config.go | 243 ++++++------------ config/config_test.go | 134 +--------- config/node/config.go | 26 +- main/main.go | 12 +- node/node.go | 27 +- utils/crypto/bls/signer.go | 1 + .../bls/signer/localsigner/localsigner.go | 5 + utils/crypto/bls/signer/rpcsigner/client.go | 28 +- utils/crypto/bls/signer/signer.go | 116 +++++++++ utils/crypto/bls/signer/signer_test.go | 150 +++++++++++ 10 files changed, 411 insertions(+), 331 deletions(-) create mode 100644 utils/crypto/bls/signer/signer.go create mode 100644 utils/crypto/bls/signer/signer_test.go diff --git a/config/config.go b/config/config.go index 8557200eafdd..8d6d07e5bc30 100644 --- a/config/config.go +++ b/config/config.go @@ -4,13 +4,11 @@ package config import ( - "context" "crypto/tls" "encoding/base64" "encoding/json" "errors" "fmt" - "io/fs" "math" "os" "path/filepath" @@ -38,9 +36,6 @@ import ( "github.com/ava-labs/avalanchego/utils/bag" "github.com/ava-labs/avalanchego/utils/compression" "github.com/ava-labs/avalanchego/utils/constants" - "github.com/ava-labs/avalanchego/utils/crypto/bls" - "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/localsigner" - "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/rpcsigner" "github.com/ava-labs/avalanchego/utils/ips" "github.com/ava-labs/avalanchego/utils/logging" "github.com/ava-labs/avalanchego/utils/perms" @@ -82,6 +77,7 @@ var ( errStakingKeyContentUnset = fmt.Errorf("%s key not set but %s set", StakingTLSKeyContentKey, StakingCertContentKey) errStakingCertContentUnset = fmt.Errorf("%s key set but %s not set", StakingTLSKeyContentKey, StakingCertContentKey) errMissingStakingSigningKeyFile = errors.New("missing staking signing key file") + errTracingEndpointEmpty = fmt.Errorf("%s cannot be empty", TracingEndpointKey) errPluginDirNotADirectory = errors.New("plugin dir is not a directory") errCannotReadDirectory = errors.New("cannot read directory") errUnmarshalling = errors.New("unmarshalling failed") @@ -643,144 +639,66 @@ func getStakingTLSCert(v *viper.Viper) (tls.Certificate, error) { } } -func getStakingSigner(ctx context.Context, v *viper.Viper) (bls.Signer, func() error, error) { - ephemeralSignerEnabled := v.GetBool(StakingEphemeralSignerEnabledKey) - contentKeyIsSet := v.IsSet(StakingSignerKeyContentKey) - keyPathIsSet := v.IsSet(StakingSignerKeyPathKey) - rpcSignerURLIsSet := v.IsSet(StakingRPCSignerKey) - - signingKeyPath := getExpandedArg(v, StakingSignerKeyPathKey) - - bools := bag.Of(ephemeralSignerEnabled, contentKeyIsSet, keyPathIsSet, rpcSignerURLIsSet) - if bools.Count(true) > 1 { - return nil, nil, errInvalidSignerConfig - } - - cleanup := func() error { return nil } - - switch { - case ephemeralSignerEnabled: - signer, err := localsigner.New() - if err != nil { - return nil, nil, fmt.Errorf("couldn't generate ephemeral signer: %w", err) - } - - return signer, cleanup, nil - - case contentKeyIsSet: - signerKeyRawContent := v.GetString(StakingSignerKeyContentKey) - signerKeyContent, err := base64.StdEncoding.DecodeString(signerKeyRawContent) - if err != nil { - return nil, nil, fmt.Errorf("unable to decode base64 content: %w", err) - } - - signer, err := localsigner.FromBytes(signerKeyContent) - if err != nil { - return nil, nil, fmt.Errorf("couldn't parse signing key: %w", err) - } - - return signer, cleanup, nil - - case keyPathIsSet: - // If the key is set, but a user-file isn't provided, we don't create one. - // The siging key is only stored to the default file-location if it's created - // and saved by the current application run. - signer, err := createSignerFromFile(signingKeyPath) - if errors.Is(err, fs.ErrNotExist) { - return nil, nil, errMissingStakingSigningKeyFile - } - - if err != nil { - return nil, nil, fmt.Errorf("couldn't parse signing key: %w", err) - } - - return signer, cleanup, nil - - case rpcSignerURLIsSet: - rpcSignerURL := v.GetString(StakingRPCSignerKey) - - signer, cleanup, err := rpcsigner.NewClient(ctx, rpcSignerURL) - if err != nil { - return nil, nil, fmt.Errorf("couldn't create rpc signer client: %w", err) - } - - return signer, cleanup, nil - - default: - signerFromFile, err := createSignerFromFile(signingKeyPath) - if err != nil && !errors.Is(err, fs.ErrNotExist) { - return nil, nil, err - } - - if signerFromFile != nil { - return signerFromFile, cleanup, nil - } - - signer, err := localsigner.New() - if err != nil { - return nil, nil, fmt.Errorf("couldn't generate new signing key: %w", err) - } - - if err := os.MkdirAll(filepath.Dir(signingKeyPath), perms.ReadWriteExecute); err != nil { - return nil, nil, fmt.Errorf("couldn't create path for signing key at %s: %w", signingKeyPath, err) - } - - keyBytes := signer.ToBytes() - if err := os.WriteFile(signingKeyPath, keyBytes, perms.ReadWrite); err != nil { - return nil, nil, fmt.Errorf("couldn't write new signing key to %s: %w", signingKeyPath, err) - } - - if err := os.Chmod(signingKeyPath, perms.ReadOnly); err != nil { - return nil, nil, fmt.Errorf("couldn't restrict permissions on new signing key at %s: %w", signingKeyPath, err) - } - - return signer, cleanup, nil - } -} - -func createSignerFromFile(signingKeyPath string) (bls.Signer, error) { - signingKeyBytes, err := os.ReadFile(signingKeyPath) - if err != nil { - return nil, err - } - - signer, err := localsigner.FromBytes(signingKeyBytes) - if err != nil { - return nil, fmt.Errorf("couldn't parse signing key: %w", err) - } - - return signer, nil -} - -func getStakingConfig(ctx context.Context, v *viper.Viper, networkID uint32) (node.StakingConfig, func() error, error) { +func getStakingConfig(v *viper.Viper, networkID uint32) (node.StakingConfig, error) { config := node.StakingConfig{ SybilProtectionEnabled: v.GetBool(SybilProtectionEnabledKey), SybilProtectionDisabledWeight: v.GetUint64(SybilProtectionDisabledWeightKey), PartialSyncPrimaryNetwork: v.GetBool(PartialSyncPrimaryNetworkKey), - StakingKeyPath: getExpandedArg(v, StakingTLSKeyPathKey), - StakingCertPath: getExpandedArg(v, StakingCertPathKey), - StakingSignerPath: getExpandedArg(v, StakingSignerKeyPathKey), - StakingSignerRPC: getExpandedArg(v, StakingRPCSignerKey), + StakingTLSKeyPath: getExpandedArg(v, StakingTLSKeyPathKey), + StakingTLSCertPath: getExpandedArg(v, StakingCertPathKey), } if !config.SybilProtectionEnabled && config.SybilProtectionDisabledWeight == 0 { - return node.StakingConfig{}, nil, errSybilProtectionDisabledStakerWeights + return node.StakingConfig{}, errSybilProtectionDisabledStakerWeights } if !config.SybilProtectionEnabled && (networkID == constants.MainnetID || networkID == constants.FujiID) { - return node.StakingConfig{}, nil, errSybilProtectionDisabledOnPublicNetwork + return node.StakingConfig{}, errSybilProtectionDisabledOnPublicNetwork } var err error config.StakingTLSCert, err = getStakingTLSCert(v) if err != nil { - return node.StakingConfig{}, nil, err + return node.StakingConfig{}, err } - var cleanup func() error - config.StakingSigningKey, cleanup, err = getStakingSigner(ctx, v) - if err != nil { - return node.StakingConfig{}, nil, err + // A maximum of one signer option can be set + bools := bag.Of( + v.GetBool(StakingEphemeralSignerEnabledKey), + v.IsSet(StakingSignerKeyContentKey), + v.IsSet(StakingSignerKeyPathKey), + v.IsSet(StakingRPCSignerKey), + ) + if bools.Count(true) > 1 { + return node.StakingConfig{}, errInvalidSignerConfig + } + + if v.GetBool(StakingEphemeralSignerEnabledKey) { + config.StakingSignerConfig = node.EphemeralSignerConfig{} + } + + if v.IsSet(StakingSignerKeyContentKey) { + config.StakingSignerConfig = node.ContentKeyConfig{ + SignerKeyRawContent: getExpandedArg(v, StakingSignerKeyContentKey), + } + } + + if v.IsSet(StakingRPCSignerKey) { + config.StakingSignerConfig = node.RPCSignerConfig{ + StakingSignerRPC: getExpandedArg(v, StakingRPCSignerKey), + } + } + + if v.IsSet(StakingSignerKeyPathKey) { + config.StakingSignerConfig = node.SignerPathConfig{ + SigningKeyPath: getExpandedArg(v, StakingSignerKeyPathKey), + SignerPathIsSet: true, + } + } else { + config.StakingSignerConfig = node.SignerPathConfig{ + SigningKeyPath: getExpandedArg(v, StakingSignerKeyPathKey), + SignerPathIsSet: false, + } } if networkID != constants.MainnetID && networkID != constants.FujiID { @@ -817,13 +735,13 @@ func getStakingConfig(ctx context.Context, v *viper.Viper, networkID uint32) (no } if err != nil { - return node.StakingConfig{}, nil, errors.Join(err, cleanup()) + return node.StakingConfig{}, err } } else { config.StakingConfig = genesis.GetStakingConfig(networkID) } - return config, cleanup, nil + return config, nil } func getTxFeeConfig(v *viper.Viper, networkID uint32) genesis.TxFeeConfig { @@ -1303,7 +1221,7 @@ func getPluginDir(v *viper.Viper) (string, error) { return pluginDir, nil } -func GetNodeConfig(ctx context.Context, v *viper.Viper) (node.Config, func() error, error) { +func GetNodeConfig(v *viper.Viper) (node.Config, error) { var ( nodeConfig node.Config err error @@ -1311,24 +1229,24 @@ func GetNodeConfig(ctx context.Context, v *viper.Viper) (node.Config, func() err nodeConfig.PluginDir, err = getPluginDir(v) if err != nil { - return node.Config{}, nil, err + return node.Config{}, err } nodeConfig.ConsensusShutdownTimeout = v.GetDuration(ConsensusShutdownTimeoutKey) if nodeConfig.ConsensusShutdownTimeout < 0 { - return node.Config{}, nil, fmt.Errorf("%q must be >= 0", ConsensusShutdownTimeoutKey) + return node.Config{}, fmt.Errorf("%q must be >= 0", ConsensusShutdownTimeoutKey) } // Gossiping nodeConfig.FrontierPollFrequency = v.GetDuration(ConsensusFrontierPollFrequencyKey) if nodeConfig.FrontierPollFrequency < 0 { - return node.Config{}, nil, fmt.Errorf("%s must be >= 0", ConsensusFrontierPollFrequencyKey) + return node.Config{}, fmt.Errorf("%s must be >= 0", ConsensusFrontierPollFrequencyKey) } // App handling nodeConfig.ConsensusAppConcurrency = int(v.GetUint(ConsensusAppConcurrencyKey)) if nodeConfig.ConsensusAppConcurrency <= 0 { - return node.Config{}, nil, fmt.Errorf("%s must be > 0", ConsensusAppConcurrencyKey) + return node.Config{}, fmt.Errorf("%s must be > 0", ConsensusAppConcurrencyKey) } nodeConfig.UseCurrentHeight = v.GetBool(ProposerVMUseCurrentHeightKey) @@ -1336,63 +1254,62 @@ func GetNodeConfig(ctx context.Context, v *viper.Viper) (node.Config, func() err // Logging nodeConfig.LoggingConfig, err = getLoggingConfig(v) if err != nil { - return node.Config{}, nil, err + return node.Config{}, err } // Network ID nodeConfig.NetworkID, err = constants.NetworkID(v.GetString(NetworkNameKey)) if err != nil { - return node.Config{}, nil, err + return node.Config{}, err } // Database nodeConfig.DatabaseConfig, err = getDatabaseConfig(v, nodeConfig.NetworkID) if err != nil { - return node.Config{}, nil, err + return node.Config{}, err } // IP configuration nodeConfig.IPConfig, err = getIPConfig(v) if err != nil { - return node.Config{}, nil, err + return node.Config{}, err } // Staking - var cleanup func() error - nodeConfig.StakingConfig, cleanup, err = getStakingConfig(ctx, v, nodeConfig.NetworkID) + nodeConfig.StakingConfig, err = getStakingConfig(v, nodeConfig.NetworkID) if err != nil { - return node.Config{}, nil, err + return node.Config{}, err } // Tracked Subnets nodeConfig.TrackedSubnets, err = getTrackedSubnets(v) if err != nil { - return node.Config{}, nil, errors.Join(err, cleanup()) + return node.Config{}, err } // HTTP APIs nodeConfig.HTTPConfig, err = getHTTPConfig(v) if err != nil { - return node.Config{}, nil, errors.Join(err, cleanup()) + return node.Config{}, err } // Health nodeConfig.HealthCheckFreq = v.GetDuration(HealthCheckFreqKey) if nodeConfig.HealthCheckFreq < 0 { err = fmt.Errorf("%s must be positive", HealthCheckFreqKey) - return node.Config{}, nil, errors.Join(err, cleanup()) + return node.Config{}, err } // Halflife of continuous averager used in health checks healthCheckAveragerHalflife := v.GetDuration(HealthCheckAveragerHalflifeKey) if healthCheckAveragerHalflife <= 0 { err = fmt.Errorf("%s must be positive", HealthCheckAveragerHalflifeKey) - return node.Config{}, nil, errors.Join(err, cleanup()) + return node.Config{}, err } // Router nodeConfig.RouterHealthConfig, err = getRouterHealthConfig(v, healthCheckAveragerHalflife) if err != nil { - return node.Config{}, nil, errors.Join(err, cleanup()) + return node.Config{}, err } // Metrics @@ -1401,13 +1318,13 @@ func GetNodeConfig(ctx context.Context, v *viper.Viper) (node.Config, func() err // Adaptive Timeout Config nodeConfig.AdaptiveTimeoutConfig, err = getAdaptiveTimeoutConfig(v) if err != nil { - return node.Config{}, nil, errors.Join(err, cleanup()) + return node.Config{}, err } // Upgrade config nodeConfig.UpgradeConfig, err = getUpgradeConfig(v, nodeConfig.NetworkID) if err != nil { - return node.Config{}, nil, errors.Join(err, cleanup()) + return node.Config{}, err } // Network Config @@ -1418,20 +1335,20 @@ func GetNodeConfig(ctx context.Context, v *viper.Viper) (node.Config, func() err healthCheckAveragerHalflife, ) if err != nil { - return node.Config{}, nil, errors.Join(err, cleanup()) + return node.Config{}, err } // Subnet Configs subnetConfigs, err := getSubnetConfigs(v, nodeConfig.TrackedSubnets.List()) if err != nil { err = fmt.Errorf("couldn't read subnet configs: %w", err) - return node.Config{}, nil, errors.Join(err, cleanup()) + return node.Config{}, err } primaryNetworkConfig := getDefaultSubnetConfig(v) if err := primaryNetworkConfig.Valid(); err != nil { err = fmt.Errorf("invalid consensus parameters: %w", err) - return node.Config{}, nil, errors.Join(err, cleanup()) + return node.Config{}, err } subnetConfigs[constants.PrimaryNetworkID] = primaryNetworkConfig @@ -1440,7 +1357,7 @@ func GetNodeConfig(ctx context.Context, v *viper.Viper) (node.Config, func() err // Benchlist nodeConfig.BenchlistConfig, err = getBenchlistConfig(v, primaryNetworkConfig.ConsensusParameters) if err != nil { - return node.Config{}, nil, errors.Join(err, cleanup()) + return node.Config{}, err } // File Descriptor Limit @@ -1454,43 +1371,43 @@ func GetNodeConfig(ctx context.Context, v *viper.Viper) (node.Config, func() err nodeConfig.GenesisBytes, nodeConfig.AvaxAssetID, err = getGenesisData(v, nodeConfig.NetworkID, &genesisStakingCfg) if err != nil { err = fmt.Errorf("unable to load genesis file: %w", err) - return node.Config{}, nil, errors.Join(err, cleanup()) + return node.Config{}, err } // StateSync Configs nodeConfig.StateSyncConfig, err = getStateSyncConfig(v) if err != nil { - return node.Config{}, nil, errors.Join(err, cleanup()) + return node.Config{}, err } // Bootstrap Configs nodeConfig.BootstrapConfig, err = getBootstrapConfig(v, nodeConfig.NetworkID) if err != nil { - return node.Config{}, nil, errors.Join(err, cleanup()) + return node.Config{}, err } // Chain Configs nodeConfig.ChainConfigs, err = getChainConfigs(v) if err != nil { err = fmt.Errorf("couldn't read chain configs: %w", err) - return node.Config{}, nil, errors.Join(err, cleanup()) + return node.Config{}, err } // Profiler nodeConfig.ProfilerConfig, err = getProfilerConfig(v) if err != nil { - return node.Config{}, nil, errors.Join(err, cleanup()) + return node.Config{}, err } // VM Aliases nodeConfig.VMAliases, err = getVMAliases(v) if err != nil { - return node.Config{}, nil, errors.Join(err, cleanup()) + return node.Config{}, err } // Chain aliases nodeConfig.ChainAliases, err = getChainAliases(v) if err != nil { - return node.Config{}, nil, errors.Join(err, cleanup()) + return node.Config{}, err } nodeConfig.SystemTrackerFrequency = v.GetDuration(SystemTrackerFrequencyKey) @@ -1500,22 +1417,22 @@ func GetNodeConfig(ctx context.Context, v *viper.Viper) (node.Config, func() err nodeConfig.RequiredAvailableDiskSpace, nodeConfig.WarningThresholdAvailableDiskSpace, err = getDiskSpaceConfig(v) if err != nil { - return node.Config{}, nil, errors.Join(err, cleanup()) + return node.Config{}, err } nodeConfig.CPUTargeterConfig, err = getCPUTargeterConfig(v) if err != nil { - return node.Config{}, nil, errors.Join(err, cleanup()) + return node.Config{}, err } nodeConfig.DiskTargeterConfig, err = getDiskTargeterConfig(v) if err != nil { - return node.Config{}, nil, errors.Join(err, cleanup()) + return node.Config{}, err } nodeConfig.TraceConfig, err = getTraceConfig(v) if err != nil { - return node.Config{}, nil, errors.Join(err, cleanup()) + return node.Config{}, err } nodeConfig.ChainDataDir = getExpandedArg(v, ChainDataDirKey) @@ -1524,7 +1441,7 @@ func GetNodeConfig(ctx context.Context, v *viper.Viper) (node.Config, func() err nodeConfig.ProvidedFlags = providedFlags(v) - return nodeConfig, cleanup, nil + return nodeConfig, nil } func providedFlags(v *viper.Viper) map[string]interface{} { diff --git a/config/config_test.go b/config/config_test.go index ab97eaba8682..6ebc1dc654f1 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -4,30 +4,22 @@ package config import ( - "context" "encoding/base64" "encoding/json" "fmt" "log" - "net" "os" "path/filepath" "testing" + "github.com/ava-labs/avalanchego/subnets" "github.com/spf13/pflag" "github.com/spf13/viper" "github.com/stretchr/testify/require" - "google.golang.org/grpc" "github.com/ava-labs/avalanchego/chains" "github.com/ava-labs/avalanchego/ids" - "github.com/ava-labs/avalanchego/proto/pb/signer" "github.com/ava-labs/avalanchego/snow/consensus/snowball" - "github.com/ava-labs/avalanchego/subnets" - "github.com/ava-labs/avalanchego/utils/crypto/bls" - "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/localsigner" - "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/rpcsigner" - "github.com/ava-labs/avalanchego/utils/perms" ) const chainConfigFilenameExtension = ".ex" @@ -557,130 +549,6 @@ func TestGetSubnetConfigsFromFlags(t *testing.T) { } } -type signerServer struct { - signer.UnimplementedSignerServer -} - -func (*signerServer) PublicKey(context.Context, *signer.PublicKeyRequest) (*signer.PublicKeyResponse, error) { - // for tests to pass, this must be the base64 encoding of a 32 byte public key - // but it does not need to be associated with any private key - bytes, err := base64.StdEncoding.DecodeString("j8Ndzc1I6EYWYUWAdhcwpQ1I2xX/i4fdwgJIaxbHlf9yQKMT0jlReiiLYsydgaS1") - if err != nil { - return nil, err - } - - return &signer.PublicKeyResponse{ - PublicKey: bytes, - }, nil -} - -func TestGetStakingSigner(t *testing.T) { - testKey := "HLimS3vRibTMk9lZD4b+Z+GLuSBShvgbsu0WTLt2Kd4=" - rpcServer := grpc.NewServer() - defer rpcServer.GracefulStop() - - signer.RegisterSignerServer(rpcServer, &signerServer{}) - - listener, err := net.Listen("tcp", "[::1]:0") - require.NoError(t, err) - - go func() { - require.NoError(t, rpcServer.Serve(listener)) - }() - - type config map[string]any - - tests := []struct { - name string - viperKeys string - config config - expectedSignerType bls.Signer - expectedErr error - }{ - { - name: "default-signer", - expectedSignerType: &localsigner.LocalSigner{}, - }, - { - name: "ephemeral-signer", - config: config{StakingEphemeralSignerEnabledKey: true}, - expectedSignerType: &localsigner.LocalSigner{}, - }, - { - name: "content-key", - config: config{StakingSignerKeyContentKey: testKey}, - expectedSignerType: &localsigner.LocalSigner{}, - }, - { - name: "file-key", - config: config{ - StakingSignerKeyPathKey: func() string { - filePath := filepath.Join(t.TempDir(), "signer.key") - bytes, err := base64.StdEncoding.DecodeString(testKey) - require.NoError(t, err) - require.NoError(t, os.WriteFile(filePath, bytes, perms.ReadWrite)) - return filePath - }(), - }, - expectedSignerType: &localsigner.LocalSigner{}, - }, - { - name: "rpc-signer", - config: config{StakingRPCSignerKey: listener.Addr().String()}, - expectedSignerType: &rpcsigner.Client{}, - }, - { - name: "multiple-configurations-set", - config: config{ - StakingEphemeralSignerEnabledKey: true, - StakingSignerKeyContentKey: testKey, - }, - expectedErr: errInvalidSignerConfig, - }, - } - - // required for proper write permissions for the default signer-key location - t.Setenv("HOME", t.TempDir()) - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - require := require.New(t) - v := setupViperFlags() - - for key, value := range tt.config { - v.Set(key, value) - } - - config, cleanup, err := GetNodeConfig(context.Background(), v) - defer func() { - if err == nil { - _ = cleanup() - } - }() - - require.ErrorIs(err, tt.expectedErr) - require.IsType(tt.expectedSignerType, config.StakingSigningKey) - }) - } -} - -func TestDefaultConfigInitializationUsesExistingDefaultKey(t *testing.T) { - t.Setenv("HOME", t.TempDir()) - - require := require.New(t) - v := setupViperFlags() - - config1, cleanup1, err := GetNodeConfig(context.Background(), v) - defer func() { _ = cleanup1() }() - require.NoError(err) - - config2, cleanup2, err := GetNodeConfig(context.Background(), v) - defer func() { _ = cleanup2() }() - require.NoError(err) - - require.Equal(config1.StakingSigningKey.PublicKey(), config2.StakingSigningKey.PublicKey()) -} - // setups config json file and writes content func setupConfigJSON(t *testing.T, rootPath string, value string) string { configFilePath := filepath.Join(rootPath, "config.json") diff --git a/config/node/config.go b/config/node/config.go index ad8945413a8c..27f8419ba958 100644 --- a/config/node/config.go +++ b/config/node/config.go @@ -19,7 +19,6 @@ import ( "github.com/ava-labs/avalanchego/subnets" "github.com/ava-labs/avalanchego/trace" "github.com/ava-labs/avalanchego/upgrade" - "github.com/ava-labs/avalanchego/utils/crypto/bls" "github.com/ava-labs/avalanchego/utils/logging" "github.com/ava-labs/avalanchego/utils/profiler" "github.com/ava-labs/avalanchego/utils/set" @@ -76,13 +75,26 @@ type StakingConfig struct { SybilProtectionEnabled bool `json:"sybilProtectionEnabled"` PartialSyncPrimaryNetwork bool `json:"partialSyncPrimaryNetwork"` StakingTLSCert tls.Certificate `json:"-"` - StakingSigningKey bls.Signer `json:"-"` SybilProtectionDisabledWeight uint64 `json:"sybilProtectionDisabledWeight"` - // not accessed but used for logging - StakingKeyPath string `json:"stakingKeyPath"` - StakingCertPath string `json:"stakingCertPath"` - StakingSignerPath string `json:"stakingSignerPath"` - StakingSignerRPC string `json:"stakingSignerRPC"` + StakingTLSKeyPath string `json:"stakingTLSKeyPath"` + StakingTLSCertPath string `json:"stakingTLSCertPath"` + + StakingSignerConfig interface{} `json:"stakingSignerConfig"` +} + +type EphemeralSignerConfig struct{} + +type ContentKeyConfig struct { + SignerKeyRawContent string `json:"signerKeyRawContent"` +} + +type SignerPathConfig struct { + SignerPathIsSet bool `json:"signerPathIsSet"` + SigningKeyPath string `json:"signingKeyPath"` +} + +type RPCSignerConfig struct { + StakingSignerRPC string `json:"stakingSignerRPC"` } type StateSyncConfig struct { diff --git a/main/main.go b/main/main.go index 72fa23778740..88de72dadbbe 100644 --- a/main/main.go +++ b/main/main.go @@ -4,12 +4,10 @@ package main import ( - "context" "encoding/json" "errors" "fmt" "os" - "time" "github.com/spf13/pflag" "golang.org/x/term" @@ -53,9 +51,7 @@ func main() { os.Exit(0) } - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - nodeConfig, cleanup, err := config.GetNodeConfig(ctx, v) - cancel() + nodeConfig, err := config.GetNodeConfig(v) if err != nil { fmt.Printf("couldn't load node config: %s\n", err) os.Exit(1) @@ -68,15 +64,9 @@ func main() { nodeApp, err := app.New(nodeConfig) if err != nil { fmt.Printf("couldn't start node: %s\n", err) - if err := cleanup(); err != nil { - fmt.Printf("error cleaning up: %s\n", err) - } os.Exit(1) } exitCode := app.Run(nodeApp) - if err := cleanup(); err != nil { - fmt.Printf("error cleaning up: %s\n", err) - } os.Exit(exitCode) } diff --git a/node/node.go b/node/node.go index 80f3b59ab09b..64671899ba9c 100644 --- a/node/node.go +++ b/node/node.go @@ -58,6 +58,7 @@ import ( "github.com/ava-labs/avalanchego/utils" "github.com/ava-labs/avalanchego/utils/constants" "github.com/ava-labs/avalanchego/utils/crypto/bls" + blssigner "github.com/ava-labs/avalanchego/utils/crypto/bls/signer" "github.com/ava-labs/avalanchego/utils/dynamicip" "github.com/ava-labs/avalanchego/utils/filesystem" "github.com/ava-labs/avalanchego/utils/hashing" @@ -125,18 +126,23 @@ func New( return nil, fmt.Errorf("invalid staking certificate: %w", err) } + stakingSigner, err := blssigner.GetStakingSigner( + config.StakingSignerConfig, + ) + n := &Node{ Log: logger, LogFactory: logFactory, StakingTLSSigner: config.StakingTLSCert.PrivateKey.(crypto.Signer), StakingTLSCert: stakingCert, ID: ids.NodeIDFromCert(stakingCert), + StakingSigner: stakingSigner, Config: config, } n.DoneShuttingDown.Add(1) - pop, err := signer.NewProofOfPossession(n.Config.StakingSigningKey) + pop, err := signer.NewProofOfPossession(n.StakingSigner) if err != nil { return nil, fmt.Errorf("problem creating proof of possession: %w", err) } @@ -287,6 +293,7 @@ type Node struct { StakingTLSSigner crypto.Signer StakingTLSCert *staking.Certificate + StakingSigner bls.Signer // Storage for this node DB database.Database @@ -578,7 +585,7 @@ func (n *Node) initNetworking(reg prometheus.Registerer) error { err := n.vdrs.AddStaker( constants.PrimaryNetworkID, n.ID, - n.Config.StakingSigningKey.PublicKey(), + n.StakingSigner.PublicKey(), dummyTxID, n.Config.SybilProtectionDisabledWeight, ) @@ -617,7 +624,7 @@ func (n *Node) initNetworking(reg prometheus.Registerer) error { n.Config.NetworkConfig.Beacons = n.bootstrappers n.Config.NetworkConfig.TLSConfig = tlsConfig n.Config.NetworkConfig.TLSKey = tlsKey - n.Config.NetworkConfig.BLSKey = n.Config.StakingSigningKey + n.Config.NetworkConfig.BLSKey = n.StakingSigner n.Config.NetworkConfig.TrackedSubnets = n.Config.TrackedSubnets n.Config.NetworkConfig.UptimeCalculator = n.uptimeCalculator n.Config.NetworkConfig.UptimeRequirement = n.Config.UptimeRequirement @@ -1109,7 +1116,7 @@ func (n *Node) initChainManager(avaxAssetID ids.ID) error { SybilProtectionEnabled: n.Config.SybilProtectionEnabled, StakingTLSSigner: n.StakingTLSSigner, StakingTLSCert: n.StakingTLSCert, - StakingBLSKey: n.Config.StakingSigningKey, + StakingBLSKey: n.StakingSigner, Log: n.Log, LogFactory: n.LogFactory, VMManager: n.VMManager, @@ -1353,7 +1360,7 @@ func (n *Node) initInfoAPI() error { n.Log.Info("initializing info API") - pop, err := signer.NewProofOfPossession(n.Config.StakingSigningKey) + pop, err := signer.NewProofOfPossession(n.StakingSigner) if err != nil { return fmt.Errorf("problem creating proof of possession: %w", err) } @@ -1464,7 +1471,7 @@ func (n *Node) initHealthAPI() error { return "validator doesn't have a BLS key", nil } - nodePK := n.Config.StakingSigningKey.PublicKey() + nodePK := n.StakingSigner.PublicKey() if nodePK.Equals(vdrPK) { return "node has the correct BLS key", nil } @@ -1659,6 +1666,14 @@ func (n *Node) shutdown() { time.Sleep(n.Config.ShutdownWait) } + if n.StakingSigner != nil { + if err := n.StakingSigner.Shutdown(); err != nil { + n.Log.Debug( + "error during staking signer shutdown", + zap.Error(err), + ) + } + } if n.resourceManager != nil { n.resourceManager.Shutdown() } diff --git a/utils/crypto/bls/signer.go b/utils/crypto/bls/signer.go index 499b24d2cf3d..4d8f5cccbdc6 100644 --- a/utils/crypto/bls/signer.go +++ b/utils/crypto/bls/signer.go @@ -7,4 +7,5 @@ type Signer interface { PublicKey() *PublicKey Sign(msg []byte) (*Signature, error) SignProofOfPossession(msg []byte) (*Signature, error) + Shutdown() error } diff --git a/utils/crypto/bls/signer/localsigner/localsigner.go b/utils/crypto/bls/signer/localsigner/localsigner.go index 570f22736446..0d8ca7f0d1f8 100644 --- a/utils/crypto/bls/signer/localsigner/localsigner.go +++ b/utils/crypto/bls/signer/localsigner/localsigner.go @@ -75,3 +75,8 @@ func (s *LocalSigner) Sign(msg []byte) (*bls.Signature, error) { func (s *LocalSigner) SignProofOfPossession(msg []byte) (*bls.Signature, error) { return new(bls.Signature).Sign(s.sk, msg, bls.CiphersuiteProofOfPossession.Bytes()), nil } + +// Sign [msg] to prove the ownership +func (s *LocalSigner) Shutdown() error { + return nil +} diff --git a/utils/crypto/bls/signer/rpcsigner/client.go b/utils/crypto/bls/signer/rpcsigner/client.go index bacfd2c4e8cb..52b492862b9a 100644 --- a/utils/crypto/bls/signer/rpcsigner/client.go +++ b/utils/crypto/bls/signer/rpcsigner/client.go @@ -21,11 +21,12 @@ import ( var _ bls.Signer = (*Client)(nil) type Client struct { - client pb.SignerClient - pk *bls.PublicKey + client pb.SignerClient + pk *bls.PublicKey + connection *grpc.ClientConn } -func NewClient(ctx context.Context, url string) (*Client, func() error, error) { +func NewClient(ctx context.Context, url string) (*Client, error) { // TODO: figure out the best parameters here given the target block-time opts := grpc.WithConnectParams(grpc.ConnectParams{ Backoff: backoff.DefaultConfig, @@ -37,26 +38,27 @@ func NewClient(ctx context.Context, url string) (*Client, func() error, error) { // the request to the actual signer instead of relying on tls-credentials conn, err := grpc.NewClient(url, opts, grpc.WithTransportCredentials(insecure.NewCredentials())) if err != nil { - return nil, nil, fmt.Errorf("failed to create rpc signer client: %w", err) + return nil, fmt.Errorf("failed to create rpc signer client: %w", err) } client := pb.NewSignerClient(conn) pubkeyResponse, err := client.PublicKey(ctx, &pb.PublicKeyRequest{}) if err != nil { - return nil, nil, errors.Join(err, conn.Close()) + return nil, errors.Join(fmt.Errorf("failed to get pubkey response: %w", err), conn.Close()) } pkBytes := pubkeyResponse.GetPublicKey() pk, err := bls.PublicKeyFromCompressedBytes(pkBytes) if err != nil { - return nil, nil, errors.Join(err, conn.Close()) + return nil, errors.Join(fmt.Errorf("failed to uncompress public key bytes: %w", err), conn.Close()) } return &Client{ - client: client, - pk: pk, - }, conn.Close, nil + client: client, + pk: pk, + connection: conn, + }, nil } func (c *Client) PublicKey() *bls.PublicKey { @@ -67,7 +69,7 @@ func (c *Client) PublicKey() *bls.PublicKey { func (c *Client) Sign(message []byte) (*bls.Signature, error) { resp, err := c.client.Sign(context.TODO(), &pb.SignRequest{Message: message}) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to sign message: %w", err) } sigBytes := resp.GetSignature() @@ -79,9 +81,13 @@ func (c *Client) Sign(message []byte) (*bls.Signature, error) { func (c *Client) SignProofOfPossession(message []byte) (*bls.Signature, error) { resp, err := c.client.SignProofOfPossession(context.TODO(), &pb.SignProofOfPossessionRequest{Message: message}) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to sign proof of possession: %w", err) } sigBytes := resp.GetSignature() return bls.SignatureFromBytes(sigBytes) } + +func (c *Client) Shutdown() error { + return c.connection.Close() +} diff --git a/utils/crypto/bls/signer/signer.go b/utils/crypto/bls/signer/signer.go new file mode 100644 index 000000000000..95b4a9396fa8 --- /dev/null +++ b/utils/crypto/bls/signer/signer.go @@ -0,0 +1,116 @@ +package signer + +import ( + "context" + "encoding/base64" + "errors" + "fmt" + "io/fs" + "os" + "path/filepath" + "time" + + "github.com/ava-labs/avalanchego/config/node" + "github.com/ava-labs/avalanchego/utils/crypto/bls" + "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/localsigner" + "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/rpcsigner" + "github.com/ava-labs/avalanchego/utils/perms" +) + +var ( + errMissingStakingSigningKeyFile = errors.New("missing staking signing key file") +) + +// GetStakingSigner returns a BLS signer based on the provided configuration. +// Valdiation should be done on the config before calling this function so that at most one of the +// ephemeralSignerEnabled, signerKeyRawContent, stakingSignerPath, or stakingSignerRPC is set. +func GetStakingSigner( + config interface{}, +) (bls.Signer, error) { + + switch cfg := config.(type) { + case node.EphemeralSignerConfig: + signer, err := localsigner.New() + if err != nil { + return nil, fmt.Errorf("couldn't generate ephemeral signer: %w", err) + } + + return signer, nil + + case node.ContentKeyConfig: + signerKeyContent, err := base64.StdEncoding.DecodeString(cfg.SignerKeyRawContent) + if err != nil { + return nil, fmt.Errorf("unable to decode base64 content: %w", err) + } + + signer, err := localsigner.FromBytes(signerKeyContent) + if err != nil { + return nil, fmt.Errorf("couldn't parse signing key: %w", err) + } + + return signer, nil + + case node.RPCSignerConfig: + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + signer, err := rpcsigner.NewClient(ctx, cfg.StakingSignerRPC) + cancel() + if err != nil { + return nil, fmt.Errorf("couldn't create rpc signer client: %w", err) + } + + return signer, nil + + case node.SignerPathConfig: + _, err := os.Stat(cfg.SigningKeyPath) + if !errors.Is(err, fs.ErrNotExist) { + signingKeyBytes, err := os.ReadFile(cfg.SigningKeyPath) + if err != nil { + return nil, err + } + signer, err := localsigner.FromBytes(signingKeyBytes) + if err != nil { + return nil, fmt.Errorf("couldn't parse signing key: %w", err) + } + return signer, nil + } + + if cfg.SignerPathIsSet { + return nil, errMissingStakingSigningKeyFile + } + + signer, err := localsigner.New() + if err != nil { + return nil, fmt.Errorf("couldn't generate new signing key: %w", err) + } + + if err := os.MkdirAll(filepath.Dir(cfg.SigningKeyPath), perms.ReadWriteExecute); err != nil { + return nil, fmt.Errorf("couldn't create path for signing key at %s: %w", cfg.SigningKeyPath, err) + } + + keyBytes := signer.ToBytes() + if err := os.WriteFile(cfg.SigningKeyPath, keyBytes, perms.ReadWrite); err != nil { + return nil, fmt.Errorf("couldn't write new signing key to %s: %w", cfg.SigningKeyPath, err) + } + if err := os.Chmod(cfg.SigningKeyPath, perms.ReadOnly); err != nil { + return nil, fmt.Errorf("couldn't restrict permissions on new signing key at %s: %w", cfg.SigningKeyPath, err) + } + return signer, nil + + default: + return nil, fmt.Errorf("unsupported signer type: %T", cfg) + } +} + +func createSignerFromFile(signingKeyPath string) (bls.Signer, error) { + signingKeyBytes, err := os.ReadFile(signingKeyPath) + if err != nil { + return nil, err + } + + signer, err := localsigner.FromBytes(signingKeyBytes) + if err != nil { + return nil, fmt.Errorf("couldn't parse signing key: %w", err) + } + + return signer, nil +} diff --git a/utils/crypto/bls/signer/signer_test.go b/utils/crypto/bls/signer/signer_test.go new file mode 100644 index 000000000000..a477401b205b --- /dev/null +++ b/utils/crypto/bls/signer/signer_test.go @@ -0,0 +1,150 @@ +package signer + +import ( + "context" + "encoding/base64" + "net" + "os" + "path/filepath" + "testing" + "log" + + "google.golang.org/grpc" + + "github.com/ava-labs/avalanchego/config" + "github.com/ava-labs/avalanchego/proto/pb/signer" + "github.com/ava-labs/avalanchego/utils/crypto/bls" + "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/localsigner" + "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/rpcsigner" + "github.com/ava-labs/avalanchego/utils/perms" + "github.com/spf13/viper" + "github.com/spf13/pflag" + "github.com/stretchr/testify/require" +) + +type signerServer struct { + signer.UnimplementedSignerServer +} + +func (*signerServer) PublicKey(context.Context, *signer.PublicKeyRequest) (*signer.PublicKeyResponse, error) { + // for tests to pass, this must be the base64 encoding of a 32 byte public key + // but it does not need to be associated with any private key + bytes, err := base64.StdEncoding.DecodeString("j8Ndzc1I6EYWYUWAdhcwpQ1I2xX/i4fdwgJIaxbHlf9yQKMT0jlReiiLYsydgaS1") + if err != nil { + return nil, err + } + + return &signer.PublicKeyResponse{ + PublicKey: bytes, + }, nil +} + +func TestGetStakingSigner(t *testing.T) { + testKey := "HLimS3vRibTMk9lZD4b+Z+GLuSBShvgbsu0WTLt2Kd4=" + rpcServer := grpc.NewServer() + defer rpcServer.GracefulStop() + + signer.RegisterSignerServer(rpcServer, &signerServer{}) + + listener, err := net.Listen("tcp", "[::1]:0") + require.NoError(t, err) + + go func() { + require.NoError(t, rpcServer.Serve(listener)) + }() + + type cfg map[string]any + + tests := []struct { + name string + viperKeys string + config cfg + expectedSignerType bls.Signer + expectedErr error + }{ + { + name: "default-signer", + expectedSignerType: &localsigner.LocalSigner{}, + }, + { + name: "ephemeral-signer", + config: cfg{config.StakingEphemeralSignerEnabledKey: true}, + expectedSignerType: &localsigner.LocalSigner{}, + }, + { + name: "content-key", + config: cfg{config.StakingSignerKeyContentKey: testKey}, + expectedSignerType: &localsigner.LocalSigner{}, + }, + { + name: "file-key", + config: cfg{ + config.StakingSignerKeyPathKey: func() string { + filePath := filepath.Join(t.TempDir(), "signer.key") + bytes, err := base64.StdEncoding.DecodeString(testKey) + require.NoError(t, err) + require.NoError(t, os.WriteFile(filePath, bytes, perms.ReadWrite)) + return filePath + }(), + }, + expectedSignerType: &localsigner.LocalSigner{}, + }, + { + name: "rpc-signer", + config: cfg{config.StakingRPCSignerKey: listener.Addr().String()}, + expectedSignerType: &rpcsigner.Client{}, + }, + { + name: "multiple-configurations-set", + config: cfg{ + config.StakingEphemeralSignerEnabledKey: true, + config.StakingSignerKeyContentKey: testKey, + }, + expectedErr: errInvalidSignerConfig, + }, + } + + // required for proper write permissions for the default signer-key location + t.Setenv("HOME", t.TempDir()) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require := require.New(t) + v := setupViperFlags() + + for key, value := range tt.config { + v.Set(key, value) + } + + config, err := config.GetNodeConfig(v) + + require.ErrorIs(err, tt.expectedErr) + require.IsType(tt.expectedSignerType, config.StakingSigningKey) + }) + } +} + +func TestDefaultConfigInitializationUsesExistingDefaultKey(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + require := require.New(t) + v := setupViperFlags() + + config1, err := config.GetNodeConfig(v) + require.NoError(err) + + config2, err := config.GetNodeConfig(v) + require.NoError(err) + + require.Equal(config1.StakingSigningKey.PublicKey(), config2.StakingSigningKey.PublicKey()) +} + +func setupViperFlags() *viper.Viper { + v := viper.New() + fs := config.BuildFlagSet() + pflag.Parse() + if err := v.BindPFlags(fs); err != nil { + log.Fatal(err) + } + return v +} \ No newline at end of file From e408079d60b974cc2ffb144820e7caeae9736c24 Mon Sep 17 00:00:00 2001 From: Geoff Stuart Date: Thu, 15 May 2025 12:29:40 -0400 Subject: [PATCH 23/37] Refactor config --- config/config.go | 81 ++++++++++++++++++++++++++---------------------- node/node.go | 10 +++--- 2 files changed, 49 insertions(+), 42 deletions(-) diff --git a/config/config.go b/config/config.go index 8d6d07e5bc30..33fcb08ee2ab 100644 --- a/config/config.go +++ b/config/config.go @@ -662,43 +662,9 @@ func getStakingConfig(v *viper.Viper, networkID uint32) (node.StakingConfig, err return node.StakingConfig{}, err } - // A maximum of one signer option can be set - bools := bag.Of( - v.GetBool(StakingEphemeralSignerEnabledKey), - v.IsSet(StakingSignerKeyContentKey), - v.IsSet(StakingSignerKeyPathKey), - v.IsSet(StakingRPCSignerKey), - ) - if bools.Count(true) > 1 { - return node.StakingConfig{}, errInvalidSignerConfig - } - - if v.GetBool(StakingEphemeralSignerEnabledKey) { - config.StakingSignerConfig = node.EphemeralSignerConfig{} - } - - if v.IsSet(StakingSignerKeyContentKey) { - config.StakingSignerConfig = node.ContentKeyConfig{ - SignerKeyRawContent: getExpandedArg(v, StakingSignerKeyContentKey), - } - } - - if v.IsSet(StakingRPCSignerKey) { - config.StakingSignerConfig = node.RPCSignerConfig{ - StakingSignerRPC: getExpandedArg(v, StakingRPCSignerKey), - } - } - - if v.IsSet(StakingSignerKeyPathKey) { - config.StakingSignerConfig = node.SignerPathConfig{ - SigningKeyPath: getExpandedArg(v, StakingSignerKeyPathKey), - SignerPathIsSet: true, - } - } else { - config.StakingSignerConfig = node.SignerPathConfig{ - SigningKeyPath: getExpandedArg(v, StakingSignerKeyPathKey), - SignerPathIsSet: false, - } + config.StakingSignerConfig, err = getStakingSignerConfig(v) + if err != nil { + return node.StakingConfig{}, err } if networkID != constants.MainnetID && networkID != constants.FujiID { @@ -744,6 +710,47 @@ func getStakingConfig(v *viper.Viper, networkID uint32) (node.StakingConfig, err return config, nil } +func getStakingSignerConfig(v *viper.Viper) (interface{}, error) { + // A maximum of one signer option can be set + bools := bag.Of( + v.GetBool(StakingEphemeralSignerEnabledKey), + v.IsSet(StakingSignerKeyContentKey), + v.IsSet(StakingSignerKeyPathKey), + v.IsSet(StakingRPCSignerKey), + ) + if bools.Count(true) > 1 { + return node.StakingConfig{}, errInvalidSignerConfig + } + + switch { + case v.GetBool(StakingEphemeralSignerEnabledKey): + return node.EphemeralSignerConfig{}, nil + + case v.IsSet(StakingSignerKeyContentKey): + return node.ContentKeyConfig{ + SignerKeyRawContent: getExpandedArg(v, StakingSignerKeyContentKey), + }, nil + + case v.IsSet(StakingRPCSignerKey): + return node.RPCSignerConfig{ + StakingSignerRPC: getExpandedArg(v, StakingRPCSignerKey), + }, nil + + case v.IsSet(StakingSignerKeyPathKey): + return node.SignerPathConfig{ + SigningKeyPath: getExpandedArg(v, StakingSignerKeyPathKey), + SignerPathIsSet: true, + }, nil + + default: + return node.SignerPathConfig{ + SigningKeyPath: getExpandedArg(v, StakingSignerKeyPathKey), + SignerPathIsSet: false, + }, nil + + } +} + func getTxFeeConfig(v *viper.Viper, networkID uint32) genesis.TxFeeConfig { if networkID != constants.MainnetID && networkID != constants.FujiID { return genesis.TxFeeConfig{ diff --git a/node/node.go b/node/node.go index 64671899ba9c..98aa5df491d8 100644 --- a/node/node.go +++ b/node/node.go @@ -126,17 +126,12 @@ func New( return nil, fmt.Errorf("invalid staking certificate: %w", err) } - stakingSigner, err := blssigner.GetStakingSigner( - config.StakingSignerConfig, - ) - n := &Node{ Log: logger, LogFactory: logFactory, StakingTLSSigner: config.StakingTLSCert.PrivateKey.(crypto.Signer), StakingTLSCert: stakingCert, ID: ids.NodeIDFromCert(stakingCert), - StakingSigner: stakingSigner, Config: config, } @@ -157,6 +152,11 @@ func New( zap.Reflect("config", n.Config), ) + n.StakingSigner, err = blssigner.GetStakingSigner(config.StakingSignerConfig) + if err != nil { + return nil, fmt.Errorf("problem initializing staking signer: %w", err) + } + n.VMFactoryLog, err = logFactory.Make("vm-factory") if err != nil { return nil, fmt.Errorf("problem creating vm logger: %w", err) From c34b691b8c3de6186257d5dac4c497799b5f1714 Mon Sep 17 00:00:00 2001 From: Geoff Stuart Date: Thu, 15 May 2025 12:41:13 -0400 Subject: [PATCH 24/37] Reduce diff --- config/config.go | 27 ++++++++++----------------- utils/crypto/bls/signer/signer.go | 2 -- 2 files changed, 10 insertions(+), 19 deletions(-) diff --git a/config/config.go b/config/config.go index 33fcb08ee2ab..dc436df6afd3 100644 --- a/config/config.go +++ b/config/config.go @@ -680,28 +680,23 @@ func getStakingConfig(v *viper.Viper, networkID uint32) (node.StakingConfig, err config.RewardConfig.SupplyCap = v.GetUint64(StakeSupplyCapKey) config.MinDelegationFee = v.GetUint32(MinDelegatorFeeKey) - var err error switch { case config.UptimeRequirement < 0 || config.UptimeRequirement > 1: - err = errInvalidUptimeRequirement + return node.StakingConfig{}, errInvalidUptimeRequirement case config.MinValidatorStake > config.MaxValidatorStake: - err = errMinValidatorStakeAboveMax + return node.StakingConfig{}, errMinValidatorStakeAboveMax case config.MinDelegationFee > 1_000_000: - err = errInvalidDelegationFee + return node.StakingConfig{}, errInvalidDelegationFee case config.MinStakeDuration <= 0: - err = errInvalidMinStakeDuration + return node.StakingConfig{}, errInvalidMinStakeDuration case config.MaxStakeDuration < config.MinStakeDuration: - err = errMinStakeDurationAboveMax + return node.StakingConfig{}, errMinStakeDurationAboveMax case config.RewardConfig.MaxConsumptionRate > reward.PercentDenominator: - err = errStakeMaxConsumptionTooLarge + return node.StakingConfig{}, errStakeMaxConsumptionTooLarge case config.RewardConfig.MaxConsumptionRate < config.RewardConfig.MinConsumptionRate: - err = errStakeMaxConsumptionBelowMin + return node.StakingConfig{}, errStakeMaxConsumptionBelowMin case config.RewardConfig.MintingPeriod < config.MaxStakeDuration: - err = errStakeMintingPeriodBelowMin - } - - if err != nil { - return node.StakingConfig{}, err + return node.StakingConfig{}, errStakeMintingPeriodBelowMin } } else { config.StakingConfig = genesis.GetStakingConfig(networkID) @@ -1377,8 +1372,7 @@ func GetNodeConfig(v *viper.Viper) (node.Config, error) { genesisStakingCfg := nodeConfig.StakingConfig.StakingConfig nodeConfig.GenesisBytes, nodeConfig.AvaxAssetID, err = getGenesisData(v, nodeConfig.NetworkID, &genesisStakingCfg) if err != nil { - err = fmt.Errorf("unable to load genesis file: %w", err) - return node.Config{}, err + return node.Config{}, fmt.Errorf("unable to load genesis file: %w", err) } // StateSync Configs @@ -1396,8 +1390,7 @@ func GetNodeConfig(v *viper.Viper) (node.Config, error) { // Chain Configs nodeConfig.ChainConfigs, err = getChainConfigs(v) if err != nil { - err = fmt.Errorf("couldn't read chain configs: %w", err) - return node.Config{}, err + return node.Config{}, fmt.Errorf("couldn't read chain configs: %w", err) } // Profiler diff --git a/utils/crypto/bls/signer/signer.go b/utils/crypto/bls/signer/signer.go index 95b4a9396fa8..94aebd265ba7 100644 --- a/utils/crypto/bls/signer/signer.go +++ b/utils/crypto/bls/signer/signer.go @@ -22,8 +22,6 @@ var ( ) // GetStakingSigner returns a BLS signer based on the provided configuration. -// Valdiation should be done on the config before calling this function so that at most one of the -// ephemeralSignerEnabled, signerKeyRawContent, stakingSignerPath, or stakingSignerRPC is set. func GetStakingSigner( config interface{}, ) (bls.Signer, error) { From de822f344d59853ae5dc6f71f95960afcbfbe57d Mon Sep 17 00:00:00 2001 From: Geoff Stuart Date: Tue, 27 May 2025 10:31:05 -0400 Subject: [PATCH 25/37] Fix tests --- config/config_test.go | 75 +++++++++++++++ utils/crypto/bls/signer/signer.go | 1 - utils/crypto/bls/signer/signer_test.go | 127 ++----------------------- 3 files changed, 82 insertions(+), 121 deletions(-) diff --git a/config/config_test.go b/config/config_test.go index 6ebc1dc654f1..7dab894765c1 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -13,11 +13,13 @@ import ( "testing" "github.com/ava-labs/avalanchego/subnets" + "github.com/ava-labs/avalanchego/utils/perms" "github.com/spf13/pflag" "github.com/spf13/viper" "github.com/stretchr/testify/require" "github.com/ava-labs/avalanchego/chains" + "github.com/ava-labs/avalanchego/config/node" "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/snow/consensus/snowball" ) @@ -549,6 +551,79 @@ func TestGetSubnetConfigsFromFlags(t *testing.T) { } } +func TestGetStakingSigner(t *testing.T) { + testKey := "HLimS3vRibTMk9lZD4b+Z+GLuSBShvgbsu0WTLt2Kd4=" + type cfg map[string]any + + tests := []struct { + name string + viperKeys string + config cfg + expectedSignerConfigType interface{} + expectedErr error + }{ + { + name: "default-signer", + expectedSignerConfigType: node.SignerPathConfig{}, + }, + { + name: "ephemeral-signer", + config: cfg{StakingEphemeralSignerEnabledKey: true}, + expectedSignerConfigType: node.EphemeralSignerConfig{}, + }, + { + name: "content-key", + config: cfg{StakingSignerKeyContentKey: testKey}, + expectedSignerConfigType: node.ContentKeyConfig{}, + }, + { + name: "file-key", + config: cfg{ + StakingSignerKeyPathKey: func() string { + filePath := filepath.Join(t.TempDir(), "signer.key") + bytes, err := base64.StdEncoding.DecodeString(testKey) + require.NoError(t, err) + require.NoError(t, os.WriteFile(filePath, bytes, perms.ReadWrite)) + return filePath + }(), + }, + expectedSignerConfigType: node.SignerPathConfig{}, + }, + { + name: "rpc-signer", + config: cfg{StakingRPCSignerKey: "localhost"}, + expectedSignerConfigType: node.RPCSignerConfig{}, + }, + { + name: "multiple-configurations-set", + config: cfg{ + StakingEphemeralSignerEnabledKey: true, + StakingSignerKeyContentKey: testKey, + }, + expectedErr: errInvalidSignerConfig, + }, + } + + // required for proper write permissions for the default signer-key location + t.Setenv("HOME", t.TempDir()) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require := require.New(t) + v := setupViperFlags() + + for key, value := range tt.config { + v.Set(key, value) + } + + config, err := GetNodeConfig(v) + + require.ErrorIs(err, tt.expectedErr) + require.IsType(tt.expectedSignerConfigType, config.StakingSignerConfig) + }) + } +} + // setups config json file and writes content func setupConfigJSON(t *testing.T, rootPath string, value string) string { configFilePath := filepath.Join(rootPath, "config.json") diff --git a/utils/crypto/bls/signer/signer.go b/utils/crypto/bls/signer/signer.go index 94aebd265ba7..996ce39faaf2 100644 --- a/utils/crypto/bls/signer/signer.go +++ b/utils/crypto/bls/signer/signer.go @@ -25,7 +25,6 @@ var ( func GetStakingSigner( config interface{}, ) (bls.Signer, error) { - switch cfg := config.(type) { case node.EphemeralSignerConfig: signer, err := localsigner.New() diff --git a/utils/crypto/bls/signer/signer_test.go b/utils/crypto/bls/signer/signer_test.go index a477401b205b..1524ef90b2d7 100644 --- a/utils/crypto/bls/signer/signer_test.go +++ b/utils/crypto/bls/signer/signer_test.go @@ -1,129 +1,15 @@ package signer import ( - "context" - "encoding/base64" - "net" - "os" - "path/filepath" - "testing" "log" - - "google.golang.org/grpc" + "testing" "github.com/ava-labs/avalanchego/config" - "github.com/ava-labs/avalanchego/proto/pb/signer" - "github.com/ava-labs/avalanchego/utils/crypto/bls" - "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/localsigner" - "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/rpcsigner" - "github.com/ava-labs/avalanchego/utils/perms" - "github.com/spf13/viper" "github.com/spf13/pflag" + "github.com/spf13/viper" "github.com/stretchr/testify/require" ) -type signerServer struct { - signer.UnimplementedSignerServer -} - -func (*signerServer) PublicKey(context.Context, *signer.PublicKeyRequest) (*signer.PublicKeyResponse, error) { - // for tests to pass, this must be the base64 encoding of a 32 byte public key - // but it does not need to be associated with any private key - bytes, err := base64.StdEncoding.DecodeString("j8Ndzc1I6EYWYUWAdhcwpQ1I2xX/i4fdwgJIaxbHlf9yQKMT0jlReiiLYsydgaS1") - if err != nil { - return nil, err - } - - return &signer.PublicKeyResponse{ - PublicKey: bytes, - }, nil -} - -func TestGetStakingSigner(t *testing.T) { - testKey := "HLimS3vRibTMk9lZD4b+Z+GLuSBShvgbsu0WTLt2Kd4=" - rpcServer := grpc.NewServer() - defer rpcServer.GracefulStop() - - signer.RegisterSignerServer(rpcServer, &signerServer{}) - - listener, err := net.Listen("tcp", "[::1]:0") - require.NoError(t, err) - - go func() { - require.NoError(t, rpcServer.Serve(listener)) - }() - - type cfg map[string]any - - tests := []struct { - name string - viperKeys string - config cfg - expectedSignerType bls.Signer - expectedErr error - }{ - { - name: "default-signer", - expectedSignerType: &localsigner.LocalSigner{}, - }, - { - name: "ephemeral-signer", - config: cfg{config.StakingEphemeralSignerEnabledKey: true}, - expectedSignerType: &localsigner.LocalSigner{}, - }, - { - name: "content-key", - config: cfg{config.StakingSignerKeyContentKey: testKey}, - expectedSignerType: &localsigner.LocalSigner{}, - }, - { - name: "file-key", - config: cfg{ - config.StakingSignerKeyPathKey: func() string { - filePath := filepath.Join(t.TempDir(), "signer.key") - bytes, err := base64.StdEncoding.DecodeString(testKey) - require.NoError(t, err) - require.NoError(t, os.WriteFile(filePath, bytes, perms.ReadWrite)) - return filePath - }(), - }, - expectedSignerType: &localsigner.LocalSigner{}, - }, - { - name: "rpc-signer", - config: cfg{config.StakingRPCSignerKey: listener.Addr().String()}, - expectedSignerType: &rpcsigner.Client{}, - }, - { - name: "multiple-configurations-set", - config: cfg{ - config.StakingEphemeralSignerEnabledKey: true, - config.StakingSignerKeyContentKey: testKey, - }, - expectedErr: errInvalidSignerConfig, - }, - } - - // required for proper write permissions for the default signer-key location - t.Setenv("HOME", t.TempDir()) - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - require := require.New(t) - v := setupViperFlags() - - for key, value := range tt.config { - v.Set(key, value) - } - - config, err := config.GetNodeConfig(v) - - require.ErrorIs(err, tt.expectedErr) - require.IsType(tt.expectedSignerType, config.StakingSigningKey) - }) - } -} - func TestDefaultConfigInitializationUsesExistingDefaultKey(t *testing.T) { t.Setenv("HOME", t.TempDir()) @@ -132,11 +18,12 @@ func TestDefaultConfigInitializationUsesExistingDefaultKey(t *testing.T) { config1, err := config.GetNodeConfig(v) require.NoError(err) - - config2, err := config.GetNodeConfig(v) + signer1, err := GetStakingSigner(config1.StakingSignerConfig) + require.NoError(err) + signer2, err := GetStakingSigner(config1.StakingSignerConfig) require.NoError(err) - require.Equal(config1.StakingSigningKey.PublicKey(), config2.StakingSigningKey.PublicKey()) + require.Equal(signer1.PublicKey(), signer2.PublicKey()) } func setupViperFlags() *viper.Viper { @@ -147,4 +34,4 @@ func setupViperFlags() *viper.Viper { log.Fatal(err) } return v -} \ No newline at end of file +} From 80ed48573b978d510d4dfda771507f66fcd77230 Mon Sep 17 00:00:00 2001 From: Geoff Stuart Date: Tue, 27 May 2025 10:55:08 -0400 Subject: [PATCH 26/37] lint --- config/config.go | 3 --- config/config_test.go | 4 ++-- node/node.go | 2 +- .../bls/signer/localsigner/localsigner.go | 2 +- utils/crypto/bls/signer/signer.go | 17 +++++------------ utils/crypto/bls/signer/signer_test.go | 6 +++++- 6 files changed, 14 insertions(+), 20 deletions(-) diff --git a/config/config.go b/config/config.go index dc436df6afd3..5fb2ca1de751 100644 --- a/config/config.go +++ b/config/config.go @@ -76,8 +76,6 @@ var ( errCannotTrackPrimaryNetwork = errors.New("cannot track primary network") errStakingKeyContentUnset = fmt.Errorf("%s key not set but %s set", StakingTLSKeyContentKey, StakingCertContentKey) errStakingCertContentUnset = fmt.Errorf("%s key set but %s not set", StakingTLSKeyContentKey, StakingCertContentKey) - errMissingStakingSigningKeyFile = errors.New("missing staking signing key file") - errTracingEndpointEmpty = fmt.Errorf("%s cannot be empty", TracingEndpointKey) errPluginDirNotADirectory = errors.New("plugin dir is not a directory") errCannotReadDirectory = errors.New("cannot read directory") errUnmarshalling = errors.New("unmarshalling failed") @@ -742,7 +740,6 @@ func getStakingSignerConfig(v *viper.Viper) (interface{}, error) { SigningKeyPath: getExpandedArg(v, StakingSignerKeyPathKey), SignerPathIsSet: false, }, nil - } } diff --git a/config/config_test.go b/config/config_test.go index 7dab894765c1..a82a108cef28 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -12,8 +12,6 @@ import ( "path/filepath" "testing" - "github.com/ava-labs/avalanchego/subnets" - "github.com/ava-labs/avalanchego/utils/perms" "github.com/spf13/pflag" "github.com/spf13/viper" "github.com/stretchr/testify/require" @@ -22,6 +20,8 @@ import ( "github.com/ava-labs/avalanchego/config/node" "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/snow/consensus/snowball" + "github.com/ava-labs/avalanchego/subnets" + "github.com/ava-labs/avalanchego/utils/perms" ) const chainConfigFilenameExtension = ".ex" diff --git a/node/node.go b/node/node.go index 4c2d79e5659f..fdc3dd4b889e 100644 --- a/node/node.go +++ b/node/node.go @@ -58,7 +58,6 @@ import ( "github.com/ava-labs/avalanchego/utils" "github.com/ava-labs/avalanchego/utils/constants" "github.com/ava-labs/avalanchego/utils/crypto/bls" - blssigner "github.com/ava-labs/avalanchego/utils/crypto/bls/signer" "github.com/ava-labs/avalanchego/utils/dynamicip" "github.com/ava-labs/avalanchego/utils/filesystem" "github.com/ava-labs/avalanchego/utils/hashing" @@ -79,6 +78,7 @@ import ( "github.com/ava-labs/avalanchego/vms/rpcchainvm/runtime" databasefactory "github.com/ava-labs/avalanchego/database/factory" + blssigner "github.com/ava-labs/avalanchego/utils/crypto/bls/signer" avmconfig "github.com/ava-labs/avalanchego/vms/avm/config" platformconfig "github.com/ava-labs/avalanchego/vms/platformvm/config" coreth "github.com/ava-labs/coreth/plugin/evm" diff --git a/utils/crypto/bls/signer/localsigner/localsigner.go b/utils/crypto/bls/signer/localsigner/localsigner.go index 0d8ca7f0d1f8..2da27a7c59ab 100644 --- a/utils/crypto/bls/signer/localsigner/localsigner.go +++ b/utils/crypto/bls/signer/localsigner/localsigner.go @@ -77,6 +77,6 @@ func (s *LocalSigner) SignProofOfPossession(msg []byte) (*bls.Signature, error) } // Sign [msg] to prove the ownership -func (s *LocalSigner) Shutdown() error { +func (*LocalSigner) Shutdown() error { return nil } diff --git a/utils/crypto/bls/signer/signer.go b/utils/crypto/bls/signer/signer.go index 996ce39faaf2..c4f75bf9208c 100644 --- a/utils/crypto/bls/signer/signer.go +++ b/utils/crypto/bls/signer/signer.go @@ -1,3 +1,6 @@ +// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + package signer import ( @@ -17,9 +20,7 @@ import ( "github.com/ava-labs/avalanchego/utils/perms" ) -var ( - errMissingStakingSigningKeyFile = errors.New("missing staking signing key file") -) +var errMissingStakingSigningKeyFile = errors.New("missing staking signing key file") // GetStakingSigner returns a BLS signer based on the provided configuration. func GetStakingSigner( @@ -60,15 +61,7 @@ func GetStakingSigner( case node.SignerPathConfig: _, err := os.Stat(cfg.SigningKeyPath) if !errors.Is(err, fs.ErrNotExist) { - signingKeyBytes, err := os.ReadFile(cfg.SigningKeyPath) - if err != nil { - return nil, err - } - signer, err := localsigner.FromBytes(signingKeyBytes) - if err != nil { - return nil, fmt.Errorf("couldn't parse signing key: %w", err) - } - return signer, nil + return createSignerFromFile(cfg.SigningKeyPath) } if cfg.SignerPathIsSet { diff --git a/utils/crypto/bls/signer/signer_test.go b/utils/crypto/bls/signer/signer_test.go index 1524ef90b2d7..7d1315ae660e 100644 --- a/utils/crypto/bls/signer/signer_test.go +++ b/utils/crypto/bls/signer/signer_test.go @@ -1,13 +1,17 @@ +// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + package signer import ( "log" "testing" - "github.com/ava-labs/avalanchego/config" "github.com/spf13/pflag" "github.com/spf13/viper" "github.com/stretchr/testify/require" + + "github.com/ava-labs/avalanchego/config" ) func TestDefaultConfigInitializationUsesExistingDefaultKey(t *testing.T) { From a6d8add405e8ac814db488e63cacd6033127370c Mon Sep 17 00:00:00 2001 From: Geoff Stuart Date: Tue, 27 May 2025 13:48:25 -0400 Subject: [PATCH 27/37] Add logging to signer creation --- config/config.go | 12 ++++-------- utils/crypto/bls/signer/signer.go | 7 +++++++ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/config/config.go b/config/config.go index 5fb2ca1de751..52ca2040bebc 100644 --- a/config/config.go +++ b/config/config.go @@ -1295,14 +1295,12 @@ func GetNodeConfig(v *viper.Viper) (node.Config, error) { // Health nodeConfig.HealthCheckFreq = v.GetDuration(HealthCheckFreqKey) if nodeConfig.HealthCheckFreq < 0 { - err = fmt.Errorf("%s must be positive", HealthCheckFreqKey) - return node.Config{}, err + return node.Config{}, fmt.Errorf("%s must be positive", HealthCheckFreqKey) } // Halflife of continuous averager used in health checks healthCheckAveragerHalflife := v.GetDuration(HealthCheckAveragerHalflifeKey) if healthCheckAveragerHalflife <= 0 { - err = fmt.Errorf("%s must be positive", HealthCheckAveragerHalflifeKey) - return node.Config{}, err + return node.Config{}, fmt.Errorf("%s must be positive", HealthCheckAveragerHalflifeKey) } // Router @@ -1340,14 +1338,12 @@ func GetNodeConfig(v *viper.Viper) (node.Config, error) { // Subnet Configs subnetConfigs, err := getSubnetConfigs(v, nodeConfig.TrackedSubnets.List()) if err != nil { - err = fmt.Errorf("couldn't read subnet configs: %w", err) - return node.Config{}, err + return node.Config{}, fmt.Errorf("couldn't read subnet configs: %w", err) } primaryNetworkConfig := getDefaultSubnetConfig(v) if err := primaryNetworkConfig.Valid(); err != nil { - err = fmt.Errorf("invalid consensus parameters: %w", err) - return node.Config{}, err + return node.Config{}, fmt.Errorf("invalid consensus parameters: %w", err) } subnetConfigs[constants.PrimaryNetworkID] = primaryNetworkConfig diff --git a/utils/crypto/bls/signer/signer.go b/utils/crypto/bls/signer/signer.go index c4f75bf9208c..566a9462c380 100644 --- a/utils/crypto/bls/signer/signer.go +++ b/utils/crypto/bls/signer/signer.go @@ -18,6 +18,7 @@ import ( "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/localsigner" "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/rpcsigner" "github.com/ava-labs/avalanchego/utils/perms" + "github.com/ava-labs/libevm/log" ) var errMissingStakingSigningKeyFile = errors.New("missing staking signing key file") @@ -28,6 +29,7 @@ func GetStakingSigner( ) (bls.Signer, error) { switch cfg := config.(type) { case node.EphemeralSignerConfig: + log.Info("creating ephemeral signer") signer, err := localsigner.New() if err != nil { return nil, fmt.Errorf("couldn't generate ephemeral signer: %w", err) @@ -36,6 +38,7 @@ func GetStakingSigner( return signer, nil case node.ContentKeyConfig: + log.Info("creating signer from key content") signerKeyContent, err := base64.StdEncoding.DecodeString(cfg.SignerKeyRawContent) if err != nil { return nil, fmt.Errorf("unable to decode base64 content: %w", err) @@ -49,6 +52,7 @@ func GetStakingSigner( return signer, nil case node.RPCSignerConfig: + log.Info("creating RPC signer") ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) signer, err := rpcsigner.NewClient(ctx, cfg.StakingSignerRPC) cancel() @@ -61,6 +65,7 @@ func GetStakingSigner( case node.SignerPathConfig: _, err := os.Stat(cfg.SigningKeyPath) if !errors.Is(err, fs.ErrNotExist) { + log.Info("creating signer from file") return createSignerFromFile(cfg.SigningKeyPath) } @@ -68,6 +73,8 @@ func GetStakingSigner( return nil, errMissingStakingSigningKeyFile } + log.Info("creating signer from new key") + signer, err := localsigner.New() if err != nil { return nil, fmt.Errorf("couldn't generate new signing key: %w", err) From d79916bd9cfd22c11443034d1ad43830b485a2ca Mon Sep 17 00:00:00 2001 From: Geoff Stuart Date: Tue, 27 May 2025 14:02:26 -0400 Subject: [PATCH 28/37] Fix ordering --- node/node.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/node/node.go b/node/node.go index fdc3dd4b889e..54fdf0b768e5 100644 --- a/node/node.go +++ b/node/node.go @@ -137,6 +137,11 @@ func New( n.DoneShuttingDown.Add(1) + n.StakingSigner, err = blssigner.GetStakingSigner(config.StakingSignerConfig) + if err != nil { + return nil, fmt.Errorf("problem initializing staking signer: %w", err) + } + pop, err := signer.NewProofOfPossession(n.StakingSigner) if err != nil { return nil, fmt.Errorf("problem creating proof of possession: %w", err) @@ -152,11 +157,6 @@ func New( zap.Reflect("config", n.Config), ) - n.StakingSigner, err = blssigner.GetStakingSigner(config.StakingSignerConfig) - if err != nil { - return nil, fmt.Errorf("problem initializing staking signer: %w", err) - } - n.VMFactoryLog, err = logFactory.Make("vm-factory") if err != nil { return nil, fmt.Errorf("problem creating vm logger: %w", err) From 14c55259ae239a81e5f0e437acc7d06032a172a7 Mon Sep 17 00:00:00 2001 From: Geoff Stuart Date: Tue, 27 May 2025 14:10:11 -0400 Subject: [PATCH 29/37] Lint --- utils/crypto/bls/signer/signer.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/utils/crypto/bls/signer/signer.go b/utils/crypto/bls/signer/signer.go index 566a9462c380..0bf046098cd6 100644 --- a/utils/crypto/bls/signer/signer.go +++ b/utils/crypto/bls/signer/signer.go @@ -13,12 +13,13 @@ import ( "path/filepath" "time" + "github.com/ava-labs/libevm/log" + "github.com/ava-labs/avalanchego/config/node" "github.com/ava-labs/avalanchego/utils/crypto/bls" "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/localsigner" "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/rpcsigner" "github.com/ava-labs/avalanchego/utils/perms" - "github.com/ava-labs/libevm/log" ) var errMissingStakingSigningKeyFile = errors.New("missing staking signing key file") From 8fa5525981a7ec562145b58990727e51711cf12a Mon Sep 17 00:00:00 2001 From: Geoff Stuart Date: Wed, 28 May 2025 15:52:13 -0400 Subject: [PATCH 30/37] Remove json tags from fields that are not user supplied --- config/node/config.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/config/node/config.go b/config/node/config.go index 27f8419ba958..6fb504480826 100644 --- a/config/node/config.go +++ b/config/node/config.go @@ -79,22 +79,23 @@ type StakingConfig struct { StakingTLSKeyPath string `json:"stakingTLSKeyPath"` StakingTLSCertPath string `json:"stakingTLSCertPath"` - StakingSignerConfig interface{} `json:"stakingSignerConfig"` + // Not user supplied. This is set in order to instatiate the correct signer type. + StakingSignerConfig interface{} } type EphemeralSignerConfig struct{} type ContentKeyConfig struct { - SignerKeyRawContent string `json:"signerKeyRawContent"` + SignerKeyRawContent string } type SignerPathConfig struct { - SignerPathIsSet bool `json:"signerPathIsSet"` - SigningKeyPath string `json:"signingKeyPath"` + SignerPathIsSet bool + SigningKeyPath string } type RPCSignerConfig struct { - StakingSignerRPC string `json:"stakingSignerRPC"` + StakingSignerRPC string } type StateSyncConfig struct { From 0c159855919655decf6e24b54ec2a66a9736347a Mon Sep 17 00:00:00 2001 From: Geoff Stuart Date: Thu, 29 May 2025 10:01:51 -0400 Subject: [PATCH 31/37] Remove stray comment --- utils/crypto/bls/signer/localsigner/localsigner.go | 1 - 1 file changed, 1 deletion(-) diff --git a/utils/crypto/bls/signer/localsigner/localsigner.go b/utils/crypto/bls/signer/localsigner/localsigner.go index 2da27a7c59ab..be4e4c36d0c9 100644 --- a/utils/crypto/bls/signer/localsigner/localsigner.go +++ b/utils/crypto/bls/signer/localsigner/localsigner.go @@ -76,7 +76,6 @@ func (s *LocalSigner) SignProofOfPossession(msg []byte) (*bls.Signature, error) return new(bls.Signature).Sign(s.sk, msg, bls.CiphersuiteProofOfPossession.Bytes()), nil } -// Sign [msg] to prove the ownership func (*LocalSigner) Shutdown() error { return nil } From 58cc0c18764f5c7ac19b9e15e22fc2fb002bad35 Mon Sep 17 00:00:00 2001 From: Geoff Stuart Date: Tue, 17 Jun 2025 11:59:28 -0400 Subject: [PATCH 32/37] Update utils/crypto/bls/signer/rpcsigner/client.go Co-authored-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com> Signed-off-by: Geoff Stuart --- utils/crypto/bls/signer/rpcsigner/client.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/utils/crypto/bls/signer/rpcsigner/client.go b/utils/crypto/bls/signer/rpcsigner/client.go index 52b492862b9a..3962d0d5cbb2 100644 --- a/utils/crypto/bls/signer/rpcsigner/client.go +++ b/utils/crypto/bls/signer/rpcsigner/client.go @@ -45,7 +45,10 @@ func NewClient(ctx context.Context, url string) (*Client, error) { pubkeyResponse, err := client.PublicKey(ctx, &pb.PublicKeyRequest{}) if err != nil { - return nil, errors.Join(fmt.Errorf("failed to get pubkey response: %w", err), conn.Close()) + return nil, errors.Join( + fmt.Errorf("failed to get pubkey response: %w", err), + conn.Close(), + ) } pkBytes := pubkeyResponse.GetPublicKey() From f92d3bec1a4c27ae5f71341472267a9c046c3ffa Mon Sep 17 00:00:00 2001 From: Geoff Stuart Date: Tue, 17 Jun 2025 16:14:57 -0400 Subject: [PATCH 33/37] Review fixes --- config/config.go | 18 +++--- config/config_test.go | 66 +++++++++++-------- config/flags.go | 2 +- config/keys.go | 2 +- config/node/config.go | 11 ++-- node/node.go | 2 +- utils/crypto/bls/signer/rpcsigner/client.go | 25 +++++--- utils/crypto/bls/signer/signer.go | 70 +++++++++------------ utils/crypto/bls/signer/signer_test.go | 4 +- 9 files changed, 109 insertions(+), 91 deletions(-) diff --git a/config/config.go b/config/config.go index 52ca2040bebc..7d11795cd182 100644 --- a/config/config.go +++ b/config/config.go @@ -80,7 +80,7 @@ var ( errCannotReadDirectory = errors.New("cannot read directory") errUnmarshalling = errors.New("unmarshalling failed") errFileDoesNotExist = errors.New("file does not exist") - errInvalidSignerConfig = fmt.Errorf("only one of the following flags can be set: %s, %s, %s, %s", StakingEphemeralSignerEnabledKey, StakingSignerKeyContentKey, StakingSignerKeyPathKey, StakingRPCSignerKey) + errInvalidSignerConfig = fmt.Errorf("only one of the following flags can be set: %s, %s, %s, %s", StakingEphemeralSignerEnabledKey, StakingSignerKeyContentKey, StakingSignerKeyPathKey, StakingRPCSignerEndpointKey) ) func getConsensusConfig(v *viper.Viper) snowball.Parameters { @@ -703,13 +703,13 @@ func getStakingConfig(v *viper.Viper, networkID uint32) (node.StakingConfig, err return config, nil } -func getStakingSignerConfig(v *viper.Viper) (interface{}, error) { +func getStakingSignerConfig(v *viper.Viper) (any, error) { // A maximum of one signer option can be set bools := bag.Of( v.GetBool(StakingEphemeralSignerEnabledKey), v.IsSet(StakingSignerKeyContentKey), v.IsSet(StakingSignerKeyPathKey), - v.IsSet(StakingRPCSignerKey), + v.IsSet(StakingRPCSignerEndpointKey), ) if bools.Count(true) > 1 { return node.StakingConfig{}, errInvalidSignerConfig @@ -724,21 +724,19 @@ func getStakingSignerConfig(v *viper.Viper) (interface{}, error) { SignerKeyRawContent: getExpandedArg(v, StakingSignerKeyContentKey), }, nil - case v.IsSet(StakingRPCSignerKey): + case v.IsSet(StakingRPCSignerEndpointKey): return node.RPCSignerConfig{ - StakingSignerRPC: getExpandedArg(v, StakingRPCSignerKey), + StakingSignerRPC: getExpandedArg(v, StakingRPCSignerEndpointKey), }, nil case v.IsSet(StakingSignerKeyPathKey): return node.SignerPathConfig{ - SigningKeyPath: getExpandedArg(v, StakingSignerKeyPathKey), - SignerPathIsSet: true, + SignerKeyPath: getExpandedArg(v, StakingSignerKeyPathKey), }, nil default: - return node.SignerPathConfig{ - SigningKeyPath: getExpandedArg(v, StakingSignerKeyPathKey), - SignerPathIsSet: false, + return node.DefaultSignerConfig{ + SignerKeyPath: getExpandedArg(v, StakingSignerKeyPathKey), }, nil } } diff --git a/config/config_test.go b/config/config_test.go index a82a108cef28..af88fb86f724 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -10,6 +10,7 @@ import ( "log" "os" "path/filepath" + "strings" "testing" "github.com/spf13/pflag" @@ -553,50 +554,63 @@ func TestGetSubnetConfigsFromFlags(t *testing.T) { func TestGetStakingSigner(t *testing.T) { testKey := "HLimS3vRibTMk9lZD4b+Z+GLuSBShvgbsu0WTLt2Kd4=" - type cfg map[string]any + testKeyPath1 := filepath.Join(t.TempDir(), ".avalanchego/staking/signer.key") + testKeyPath2 := strings.Replace(testKeyPath1, "001", "002", 1) // Anticipate the new temp dir that will be created tests := []struct { - name string - viperKeys string - config cfg - expectedSignerConfigType interface{} - expectedErr error + name string + viperKeys string + config map[string]any + expectedSignerConfig any + expectedErr error }{ { - name: "default-signer", - expectedSignerConfigType: node.SignerPathConfig{}, + name: "default signer", + expectedSignerConfig: node.DefaultSignerConfig{ + SignerKeyPath: testKeyPath2, + }, }, { - name: "ephemeral-signer", - config: cfg{StakingEphemeralSignerEnabledKey: true}, - expectedSignerConfigType: node.EphemeralSignerConfig{}, + name: "ephemeral signer", + config: map[string]any{StakingEphemeralSignerEnabledKey: true}, + expectedSignerConfig: node.EphemeralSignerConfig{}, }, { - name: "content-key", - config: cfg{StakingSignerKeyContentKey: testKey}, - expectedSignerConfigType: node.ContentKeyConfig{}, + name: "content key", + config: map[string]any{StakingSignerKeyContentKey: testKey}, + expectedSignerConfig: node.ContentKeyConfig{ + SignerKeyRawContent: testKey, + }, }, { - name: "file-key", - config: cfg{ + name: "file key", + config: map[string]any{ StakingSignerKeyPathKey: func() string { - filePath := filepath.Join(t.TempDir(), "signer.key") + require.NoError(t, os.MkdirAll( + filepath.Dir(testKeyPath1), + os.ModePerm, + )) + bytes, err := base64.StdEncoding.DecodeString(testKey) require.NoError(t, err) - require.NoError(t, os.WriteFile(filePath, bytes, perms.ReadWrite)) - return filePath + require.NoError(t, os.WriteFile(testKeyPath1, bytes, perms.ReadWrite)) + return testKeyPath1 }(), }, - expectedSignerConfigType: node.SignerPathConfig{}, + expectedSignerConfig: node.SignerPathConfig{ + SignerKeyPath: testKeyPath1, + }, }, { - name: "rpc-signer", - config: cfg{StakingRPCSignerKey: "localhost"}, - expectedSignerConfigType: node.RPCSignerConfig{}, + name: "rpc signer", + config: map[string]any{StakingRPCSignerEndpointKey: "localhost"}, + expectedSignerConfig: node.RPCSignerConfig{ + StakingSignerRPC: "localhost", + }, }, { - name: "multiple-configurations-set", - config: cfg{ + name: "multiple configurations set", + config: map[string]any{ StakingEphemeralSignerEnabledKey: true, StakingSignerKeyContentKey: testKey, }, @@ -619,7 +633,7 @@ func TestGetStakingSigner(t *testing.T) { config, err := GetNodeConfig(v) require.ErrorIs(err, tt.expectedErr) - require.IsType(tt.expectedSignerConfigType, config.StakingSignerConfig) + require.Equal(tt.expectedSignerConfig, config.StakingSignerConfig) }) } } diff --git a/config/flags.go b/config/flags.go index e9a95bc4a9df..a5bde8f53d32 100644 --- a/config/flags.go +++ b/config/flags.go @@ -264,7 +264,7 @@ func addNodeFlags(fs *pflag.FlagSet) { fs.Bool(StakingEphemeralSignerEnabledKey, false, "If true, the node uses an ephemeral staking signer key") fs.String(StakingSignerKeyPathKey, defaultStakingSignerKeyPath, fmt.Sprintf("Path to the signer private key for staking. Ignored if %s is specified", StakingSignerKeyContentKey)) fs.String(StakingSignerKeyContentKey, "", "Specifies base64 encoded signer private key for staking") - fs.String(StakingRPCSignerKey, "", "Specifies the RPC endpoint of the staking signer") + fs.String(StakingRPCSignerEndpointKey, "", "Specifies the RPC endpoint of the staking signer") fs.Bool(SybilProtectionEnabledKey, true, "Enables sybil protection. If enabled, Network TLS is required") fs.Uint64(SybilProtectionDisabledWeightKey, 100, "Weight to provide to each peer when sybil protection is disabled") fs.Bool(PartialSyncPrimaryNetworkKey, false, "Only sync the P-chain on the Primary Network. If the node is a Primary Network validator, it will report unhealthy") diff --git a/config/keys.go b/config/keys.go index 814f8fdfd29d..9a98d206657f 100644 --- a/config/keys.go +++ b/config/keys.go @@ -85,7 +85,7 @@ const ( StakingEphemeralSignerEnabledKey = "staking-ephemeral-signer-enabled" StakingSignerKeyPathKey = "staking-signer-key-file" StakingSignerKeyContentKey = "staking-signer-key-file-content" - StakingRPCSignerKey = "staking-rpc-signer" + StakingRPCSignerEndpointKey = "staking-rpc-signer-endpoint" SybilProtectionEnabledKey = "sybil-protection-enabled" SybilProtectionDisabledWeightKey = "sybil-protection-disabled-weight" NetworkInitialTimeoutKey = "network-initial-timeout" diff --git a/config/node/config.go b/config/node/config.go index 6fb504480826..50c8a5c1188e 100644 --- a/config/node/config.go +++ b/config/node/config.go @@ -79,8 +79,8 @@ type StakingConfig struct { StakingTLSKeyPath string `json:"stakingTLSKeyPath"` StakingTLSCertPath string `json:"stakingTLSCertPath"` - // Not user supplied. This is set in order to instatiate the correct signer type. - StakingSignerConfig interface{} + // This is set in order to instatiate the correct signer type at runtime. + StakingSignerConfig any } type EphemeralSignerConfig struct{} @@ -90,8 +90,11 @@ type ContentKeyConfig struct { } type SignerPathConfig struct { - SignerPathIsSet bool - SigningKeyPath string + SignerKeyPath string +} + +type DefaultSignerConfig struct { + SignerKeyPath string } type RPCSignerConfig struct { diff --git a/node/node.go b/node/node.go index 9c9ca18884af..83bdfe1b0f5d 100644 --- a/node/node.go +++ b/node/node.go @@ -135,7 +135,7 @@ func New( Config: config, } - n.StakingSigner, err = blssigner.GetStakingSigner(config.StakingSignerConfig) + n.StakingSigner, err = blssigner.NewStakingSigner(config.StakingSignerConfig) if err != nil { return nil, fmt.Errorf("problem initializing staking signer: %w", err) } diff --git a/utils/crypto/bls/signer/rpcsigner/client.go b/utils/crypto/bls/signer/rpcsigner/client.go index 52b492862b9a..db73913f5e33 100644 --- a/utils/crypto/bls/signer/rpcsigner/client.go +++ b/utils/crypto/bls/signer/rpcsigner/client.go @@ -21,8 +21,9 @@ import ( var _ bls.Signer = (*Client)(nil) type Client struct { - client pb.SignerClient - pk *bls.PublicKey + client pb.SignerClient + pk *bls.PublicKey + // grpc.ClientConn handles transient connection errors. connection *grpc.ClientConn } @@ -51,7 +52,10 @@ func NewClient(ctx context.Context, url string) (*Client, error) { pkBytes := pubkeyResponse.GetPublicKey() pk, err := bls.PublicKeyFromCompressedBytes(pkBytes) if err != nil { - return nil, errors.Join(fmt.Errorf("failed to uncompress public key bytes: %w", err), conn.Close()) + return nil, errors.Join( + fmt.Errorf("failed to uncompress public key bytes: %w", err), + conn.Close(), + ) } return &Client{ @@ -65,7 +69,6 @@ func (c *Client) PublicKey() *bls.PublicKey { return c.pk } -// Sign a message. The [Client] already handles transient connection errors. func (c *Client) Sign(message []byte) (*bls.Signature, error) { resp, err := c.client.Sign(context.TODO(), &pb.SignRequest{Message: message}) if err != nil { @@ -73,10 +76,14 @@ func (c *Client) Sign(message []byte) (*bls.Signature, error) { } sigBytes := resp.GetSignature() - return bls.SignatureFromBytes(sigBytes) + sig, err := bls.SignatureFromBytes(sigBytes) + if err != nil { + return nil, fmt.Errorf("failed to parse signature: %w", err) + } + return sig, nil } -// [SignProofOfPossession] has the same behavior as [Sign] but will product a different signature. +// SignProofOfPossession produces a ProofOfPossession signature. // See BLS spec for more details. func (c *Client) SignProofOfPossession(message []byte) (*bls.Signature, error) { resp, err := c.client.SignProofOfPossession(context.TODO(), &pb.SignProofOfPossessionRequest{Message: message}) @@ -85,7 +92,11 @@ func (c *Client) SignProofOfPossession(message []byte) (*bls.Signature, error) { } sigBytes := resp.GetSignature() - return bls.SignatureFromBytes(sigBytes) + sig, err := bls.SignatureFromBytes(sigBytes) + if err != nil { + return nil, fmt.Errorf("failed to parse signature: %w", err) + } + return sig, nil } func (c *Client) Shutdown() error { diff --git a/utils/crypto/bls/signer/signer.go b/utils/crypto/bls/signer/signer.go index 0bf046098cd6..1e897f59fa14 100644 --- a/utils/crypto/bls/signer/signer.go +++ b/utils/crypto/bls/signer/signer.go @@ -13,8 +13,6 @@ import ( "path/filepath" "time" - "github.com/ava-labs/libevm/log" - "github.com/ava-labs/avalanchego/config/node" "github.com/ava-labs/avalanchego/utils/crypto/bls" "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/localsigner" @@ -22,15 +20,12 @@ import ( "github.com/ava-labs/avalanchego/utils/perms" ) -var errMissingStakingSigningKeyFile = errors.New("missing staking signing key file") - // GetStakingSigner returns a BLS signer based on the provided configuration. -func GetStakingSigner( - config interface{}, +func NewStakingSigner( + config any, ) (bls.Signer, error) { switch cfg := config.(type) { case node.EphemeralSignerConfig: - log.Info("creating ephemeral signer") signer, err := localsigner.New() if err != nil { return nil, fmt.Errorf("couldn't generate ephemeral signer: %w", err) @@ -39,7 +34,6 @@ func GetStakingSigner( return signer, nil case node.ContentKeyConfig: - log.Info("creating signer from key content") signerKeyContent, err := base64.StdEncoding.DecodeString(cfg.SignerKeyRawContent) if err != nil { return nil, fmt.Errorf("unable to decode base64 content: %w", err) @@ -53,7 +47,6 @@ func GetStakingSigner( return signer, nil case node.RPCSignerConfig: - log.Info("creating RPC signer") ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) signer, err := rpcsigner.NewClient(ctx, cfg.StakingSignerRPC) cancel() @@ -64,45 +57,24 @@ func GetStakingSigner( return signer, nil case node.SignerPathConfig: - _, err := os.Stat(cfg.SigningKeyPath) - if !errors.Is(err, fs.ErrNotExist) { - log.Info("creating signer from file") - return createSignerFromFile(cfg.SigningKeyPath) - } - - if cfg.SignerPathIsSet { - return nil, errMissingStakingSigningKeyFile - } - - log.Info("creating signer from new key") - - signer, err := localsigner.New() - if err != nil { - return nil, fmt.Errorf("couldn't generate new signing key: %w", err) - } - - if err := os.MkdirAll(filepath.Dir(cfg.SigningKeyPath), perms.ReadWriteExecute); err != nil { - return nil, fmt.Errorf("couldn't create path for signing key at %s: %w", cfg.SigningKeyPath, err) - } + return createSignerFromFile(cfg.SignerKeyPath) - keyBytes := signer.ToBytes() - if err := os.WriteFile(cfg.SigningKeyPath, keyBytes, perms.ReadWrite); err != nil { - return nil, fmt.Errorf("couldn't write new signing key to %s: %w", cfg.SigningKeyPath, err) - } - if err := os.Chmod(cfg.SigningKeyPath, perms.ReadOnly); err != nil { - return nil, fmt.Errorf("couldn't restrict permissions on new signing key at %s: %w", cfg.SigningKeyPath, err) + case node.DefaultSignerConfig: + _, err := os.Stat(cfg.SignerKeyPath) + if !errors.Is(err, fs.ErrNotExist) { + return createSignerFromFile(cfg.SignerKeyPath) } - return signer, nil + return createSignerFromNewKey(cfg.SignerKeyPath) default: return nil, fmt.Errorf("unsupported signer type: %T", cfg) } } -func createSignerFromFile(signingKeyPath string) (bls.Signer, error) { - signingKeyBytes, err := os.ReadFile(signingKeyPath) +func createSignerFromFile(signerKeyPath string) (bls.Signer, error) { + signingKeyBytes, err := os.ReadFile(signerKeyPath) if err != nil { - return nil, err + return nil, fmt.Errorf("couldn't read signing key from %s: %w", signerKeyPath, err) } signer, err := localsigner.FromBytes(signingKeyBytes) @@ -112,3 +84,23 @@ func createSignerFromFile(signingKeyPath string) (bls.Signer, error) { return signer, nil } + +func createSignerFromNewKey(signerKeyPath string) (bls.Signer, error) { + signer, err := localsigner.New() + if err != nil { + return nil, fmt.Errorf("couldn't generate new signing key: %w", err) + } + + if err := os.MkdirAll(filepath.Dir(signerKeyPath), perms.ReadWriteExecute); err != nil { + return nil, fmt.Errorf("couldn't create path for signing key at %s: %w", signerKeyPath, err) + } + + keyBytes := signer.ToBytes() + if err := os.WriteFile(signerKeyPath, keyBytes, perms.ReadWrite); err != nil { + return nil, fmt.Errorf("couldn't write new signing key to %s: %w", signerKeyPath, err) + } + if err := os.Chmod(signerKeyPath, perms.ReadOnly); err != nil { + return nil, fmt.Errorf("couldn't restrict permissions on new signing key at %s: %w", signerKeyPath, err) + } + return signer, nil +} diff --git a/utils/crypto/bls/signer/signer_test.go b/utils/crypto/bls/signer/signer_test.go index 7d1315ae660e..10a293cc8bd2 100644 --- a/utils/crypto/bls/signer/signer_test.go +++ b/utils/crypto/bls/signer/signer_test.go @@ -22,9 +22,9 @@ func TestDefaultConfigInitializationUsesExistingDefaultKey(t *testing.T) { config1, err := config.GetNodeConfig(v) require.NoError(err) - signer1, err := GetStakingSigner(config1.StakingSignerConfig) + signer1, err := NewStakingSigner(config1.StakingSignerConfig) require.NoError(err) - signer2, err := GetStakingSigner(config1.StakingSignerConfig) + signer2, err := NewStakingSigner(config1.StakingSignerConfig) require.NoError(err) require.Equal(signer1.PublicKey(), signer2.PublicKey()) From 60526b112f4af6d3539513dafe41a01af8ff746a Mon Sep 17 00:00:00 2001 From: Geoff Stuart Date: Tue, 17 Jun 2025 16:23:26 -0400 Subject: [PATCH 34/37] Move signer to node package --- node/node.go | 3 +- utils/crypto/bls/signer/rpcsigner/client.go | 4 +- utils/crypto/bls/signer/signer.go | 106 -------------------- utils/crypto/bls/signer/signer_test.go | 41 -------- 4 files changed, 3 insertions(+), 151 deletions(-) delete mode 100644 utils/crypto/bls/signer/signer.go delete mode 100644 utils/crypto/bls/signer/signer_test.go diff --git a/node/node.go b/node/node.go index 83bdfe1b0f5d..4548cb7e77a1 100644 --- a/node/node.go +++ b/node/node.go @@ -78,7 +78,6 @@ import ( "github.com/ava-labs/avalanchego/vms/rpcchainvm/runtime" databasefactory "github.com/ava-labs/avalanchego/database/factory" - blssigner "github.com/ava-labs/avalanchego/utils/crypto/bls/signer" avmconfig "github.com/ava-labs/avalanchego/vms/avm/config" platformconfig "github.com/ava-labs/avalanchego/vms/platformvm/config" coreth "github.com/ava-labs/coreth/plugin/evm" @@ -135,7 +134,7 @@ func New( Config: config, } - n.StakingSigner, err = blssigner.NewStakingSigner(config.StakingSignerConfig) + n.StakingSigner, err = NewStakingSigner(config.StakingSignerConfig) if err != nil { return nil, fmt.Errorf("problem initializing staking signer: %w", err) } diff --git a/utils/crypto/bls/signer/rpcsigner/client.go b/utils/crypto/bls/signer/rpcsigner/client.go index b94e6553dd45..aa3c098882c1 100644 --- a/utils/crypto/bls/signer/rpcsigner/client.go +++ b/utils/crypto/bls/signer/rpcsigner/client.go @@ -47,8 +47,8 @@ func NewClient(ctx context.Context, url string) (*Client, error) { pubkeyResponse, err := client.PublicKey(ctx, &pb.PublicKeyRequest{}) if err != nil { return nil, errors.Join( - fmt.Errorf("failed to get pubkey response: %w", err), - conn.Close(), + fmt.Errorf("failed to get pubkey response: %w", err), + conn.Close(), ) } diff --git a/utils/crypto/bls/signer/signer.go b/utils/crypto/bls/signer/signer.go deleted file mode 100644 index 1e897f59fa14..000000000000 --- a/utils/crypto/bls/signer/signer.go +++ /dev/null @@ -1,106 +0,0 @@ -// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. -// See the file LICENSE for licensing terms. - -package signer - -import ( - "context" - "encoding/base64" - "errors" - "fmt" - "io/fs" - "os" - "path/filepath" - "time" - - "github.com/ava-labs/avalanchego/config/node" - "github.com/ava-labs/avalanchego/utils/crypto/bls" - "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/localsigner" - "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/rpcsigner" - "github.com/ava-labs/avalanchego/utils/perms" -) - -// GetStakingSigner returns a BLS signer based on the provided configuration. -func NewStakingSigner( - config any, -) (bls.Signer, error) { - switch cfg := config.(type) { - case node.EphemeralSignerConfig: - signer, err := localsigner.New() - if err != nil { - return nil, fmt.Errorf("couldn't generate ephemeral signer: %w", err) - } - - return signer, nil - - case node.ContentKeyConfig: - signerKeyContent, err := base64.StdEncoding.DecodeString(cfg.SignerKeyRawContent) - if err != nil { - return nil, fmt.Errorf("unable to decode base64 content: %w", err) - } - - signer, err := localsigner.FromBytes(signerKeyContent) - if err != nil { - return nil, fmt.Errorf("couldn't parse signing key: %w", err) - } - - return signer, nil - - case node.RPCSignerConfig: - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - signer, err := rpcsigner.NewClient(ctx, cfg.StakingSignerRPC) - cancel() - if err != nil { - return nil, fmt.Errorf("couldn't create rpc signer client: %w", err) - } - - return signer, nil - - case node.SignerPathConfig: - return createSignerFromFile(cfg.SignerKeyPath) - - case node.DefaultSignerConfig: - _, err := os.Stat(cfg.SignerKeyPath) - if !errors.Is(err, fs.ErrNotExist) { - return createSignerFromFile(cfg.SignerKeyPath) - } - return createSignerFromNewKey(cfg.SignerKeyPath) - - default: - return nil, fmt.Errorf("unsupported signer type: %T", cfg) - } -} - -func createSignerFromFile(signerKeyPath string) (bls.Signer, error) { - signingKeyBytes, err := os.ReadFile(signerKeyPath) - if err != nil { - return nil, fmt.Errorf("couldn't read signing key from %s: %w", signerKeyPath, err) - } - - signer, err := localsigner.FromBytes(signingKeyBytes) - if err != nil { - return nil, fmt.Errorf("couldn't parse signing key: %w", err) - } - - return signer, nil -} - -func createSignerFromNewKey(signerKeyPath string) (bls.Signer, error) { - signer, err := localsigner.New() - if err != nil { - return nil, fmt.Errorf("couldn't generate new signing key: %w", err) - } - - if err := os.MkdirAll(filepath.Dir(signerKeyPath), perms.ReadWriteExecute); err != nil { - return nil, fmt.Errorf("couldn't create path for signing key at %s: %w", signerKeyPath, err) - } - - keyBytes := signer.ToBytes() - if err := os.WriteFile(signerKeyPath, keyBytes, perms.ReadWrite); err != nil { - return nil, fmt.Errorf("couldn't write new signing key to %s: %w", signerKeyPath, err) - } - if err := os.Chmod(signerKeyPath, perms.ReadOnly); err != nil { - return nil, fmt.Errorf("couldn't restrict permissions on new signing key at %s: %w", signerKeyPath, err) - } - return signer, nil -} diff --git a/utils/crypto/bls/signer/signer_test.go b/utils/crypto/bls/signer/signer_test.go deleted file mode 100644 index 10a293cc8bd2..000000000000 --- a/utils/crypto/bls/signer/signer_test.go +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. -// See the file LICENSE for licensing terms. - -package signer - -import ( - "log" - "testing" - - "github.com/spf13/pflag" - "github.com/spf13/viper" - "github.com/stretchr/testify/require" - - "github.com/ava-labs/avalanchego/config" -) - -func TestDefaultConfigInitializationUsesExistingDefaultKey(t *testing.T) { - t.Setenv("HOME", t.TempDir()) - - require := require.New(t) - v := setupViperFlags() - - config1, err := config.GetNodeConfig(v) - require.NoError(err) - signer1, err := NewStakingSigner(config1.StakingSignerConfig) - require.NoError(err) - signer2, err := NewStakingSigner(config1.StakingSignerConfig) - require.NoError(err) - - require.Equal(signer1.PublicKey(), signer2.PublicKey()) -} - -func setupViperFlags() *viper.Viper { - v := viper.New() - fs := config.BuildFlagSet() - pflag.Parse() - if err := v.BindPFlags(fs); err != nil { - log.Fatal(err) - } - return v -} From ba4977a2c83ec7cc53c627f711a41ca21e35534f Mon Sep 17 00:00:00 2001 From: Geoff Stuart Date: Wed, 18 Jun 2025 10:54:17 -0400 Subject: [PATCH 35/37] Add missing files --- node/signer.go | 106 ++++++++++++++++++++++++++++++++++++++++++++ node/signer_test.go | 41 +++++++++++++++++ 2 files changed, 147 insertions(+) create mode 100644 node/signer.go create mode 100644 node/signer_test.go diff --git a/node/signer.go b/node/signer.go new file mode 100644 index 000000000000..c8ce0f42ed6f --- /dev/null +++ b/node/signer.go @@ -0,0 +1,106 @@ +// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package node + +import ( + "context" + "encoding/base64" + "errors" + "fmt" + "io/fs" + "os" + "path/filepath" + "time" + + "github.com/ava-labs/avalanchego/config/node" + "github.com/ava-labs/avalanchego/utils/crypto/bls" + "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/localsigner" + "github.com/ava-labs/avalanchego/utils/crypto/bls/signer/rpcsigner" + "github.com/ava-labs/avalanchego/utils/perms" +) + +// NewStakingSigner returns a BLS signer based on the provided configuration. +func NewStakingSigner( + config any, +) (bls.Signer, error) { + switch cfg := config.(type) { + case node.EphemeralSignerConfig: + signer, err := localsigner.New() + if err != nil { + return nil, fmt.Errorf("couldn't generate ephemeral signer: %w", err) + } + + return signer, nil + + case node.ContentKeyConfig: + signerKeyContent, err := base64.StdEncoding.DecodeString(cfg.SignerKeyRawContent) + if err != nil { + return nil, fmt.Errorf("unable to decode base64 content: %w", err) + } + + signer, err := localsigner.FromBytes(signerKeyContent) + if err != nil { + return nil, fmt.Errorf("couldn't parse signing key: %w", err) + } + + return signer, nil + + case node.RPCSignerConfig: + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + signer, err := rpcsigner.NewClient(ctx, cfg.StakingSignerRPC) + cancel() + if err != nil { + return nil, fmt.Errorf("couldn't create rpc signer client: %w", err) + } + + return signer, nil + + case node.SignerPathConfig: + return createSignerFromFile(cfg.SignerKeyPath) + + case node.DefaultSignerConfig: + _, err := os.Stat(cfg.SignerKeyPath) + if !errors.Is(err, fs.ErrNotExist) { + return createSignerFromFile(cfg.SignerKeyPath) + } + return createSignerFromNewKey(cfg.SignerKeyPath) + + default: + return nil, fmt.Errorf("unsupported signer type: %T", cfg) + } +} + +func createSignerFromFile(signerKeyPath string) (bls.Signer, error) { + signingKeyBytes, err := os.ReadFile(signerKeyPath) + if err != nil { + return nil, fmt.Errorf("couldn't read signing key from %s: %w", signerKeyPath, err) + } + + signer, err := localsigner.FromBytes(signingKeyBytes) + if err != nil { + return nil, fmt.Errorf("couldn't parse signing key: %w", err) + } + + return signer, nil +} + +func createSignerFromNewKey(signerKeyPath string) (bls.Signer, error) { + signer, err := localsigner.New() + if err != nil { + return nil, fmt.Errorf("couldn't generate new signing key: %w", err) + } + + if err := os.MkdirAll(filepath.Dir(signerKeyPath), perms.ReadWriteExecute); err != nil { + return nil, fmt.Errorf("couldn't create path for signing key at %s: %w", signerKeyPath, err) + } + + keyBytes := signer.ToBytes() + if err := os.WriteFile(signerKeyPath, keyBytes, perms.ReadWrite); err != nil { + return nil, fmt.Errorf("couldn't write new signing key to %s: %w", signerKeyPath, err) + } + if err := os.Chmod(signerKeyPath, perms.ReadOnly); err != nil { + return nil, fmt.Errorf("couldn't restrict permissions on new signing key at %s: %w", signerKeyPath, err) + } + return signer, nil +} diff --git a/node/signer_test.go b/node/signer_test.go new file mode 100644 index 000000000000..471e5ce26e11 --- /dev/null +++ b/node/signer_test.go @@ -0,0 +1,41 @@ +// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package node + +import ( + "log" + "testing" + + "github.com/spf13/pflag" + "github.com/spf13/viper" + "github.com/stretchr/testify/require" + + "github.com/ava-labs/avalanchego/config" +) + +func TestDefaultConfigInitializationUsesExistingDefaultKey(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + require := require.New(t) + v := setupViperFlags() + + config1, err := config.GetNodeConfig(v) + require.NoError(err) + signer1, err := NewStakingSigner(config1.StakingSignerConfig) + require.NoError(err) + signer2, err := NewStakingSigner(config1.StakingSignerConfig) + require.NoError(err) + + require.Equal(signer1.PublicKey(), signer2.PublicKey()) +} + +func setupViperFlags() *viper.Viper { + v := viper.New() + fs := config.BuildFlagSet() + pflag.Parse() + if err := v.BindPFlags(fs); err != nil { + log.Fatal(err) + } + return v +} From 3283340bc1455f891ceba7d482b47ffa790013a7 Mon Sep 17 00:00:00 2001 From: Geoff Stuart Date: Tue, 24 Jun 2025 10:55:22 -0400 Subject: [PATCH 36/37] Wrap error --- utils/crypto/bls/signer/rpcsigner/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/crypto/bls/signer/rpcsigner/client.go b/utils/crypto/bls/signer/rpcsigner/client.go index aa3c098882c1..ec8ff5ee9c97 100644 --- a/utils/crypto/bls/signer/rpcsigner/client.go +++ b/utils/crypto/bls/signer/rpcsigner/client.go @@ -103,5 +103,5 @@ func (c *Client) SignProofOfPossession(message []byte) (*bls.Signature, error) { } func (c *Client) Shutdown() error { - return c.connection.Close() + return fmt.Errorf("failed to close connection: %w",c.connection.Close()) } From d6512c23ef4e3f52df45784b0d89543fd2c58339 Mon Sep 17 00:00:00 2001 From: Geoff Stuart Date: Tue, 24 Jun 2025 11:23:05 -0400 Subject: [PATCH 37/37] Lint --- utils/crypto/bls/signer/rpcsigner/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/crypto/bls/signer/rpcsigner/client.go b/utils/crypto/bls/signer/rpcsigner/client.go index ec8ff5ee9c97..a5fa14eb54b6 100644 --- a/utils/crypto/bls/signer/rpcsigner/client.go +++ b/utils/crypto/bls/signer/rpcsigner/client.go @@ -103,5 +103,5 @@ func (c *Client) SignProofOfPossession(message []byte) (*bls.Signature, error) { } func (c *Client) Shutdown() error { - return fmt.Errorf("failed to close connection: %w",c.connection.Close()) + return fmt.Errorf("failed to close connection: %w", c.connection.Close()) }