Make rpc-signer client handle the connection

Author: richardpringle

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"
@@ -700,19 +698,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:
@@ -748,7 +738,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

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"

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 {

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,19 +62,25 @@ 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()
 
 	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()