ADR 101: Add Close method to gRPC client (#1251) (#1474)

* grpc: Add close method to client



* grpc: Add close method to privileged client



* test/e2e: Close gRPC clients after testing



---------

Signed-off-by: Thane Thomson <connect@thanethomson.com>
Co-authored-by: Thane Thomson <connect@thanethomson.com>

rpc/grpc/client/client.go

@@ -9,7 +9,6 @@ import (
 	"fmt"
 	"net"
 
-	"github.com/cosmos/gogoproto/grpc"
 	ggrpc "google.golang.org/grpc"
 	"google.golang.org/grpc/credentials/insecure"
 
@@ -24,6 +23,9 @@ type Client interface {
 	VersionServiceClient
 	BlockServiceClient
 	BlockResultsServiceClient
+
+	// Close the connection to the server. Any subsequent requests will fail.
+	Close() error
 }
 
 type clientBuilder struct {
@@ -50,13 +52,18 @@ func defaultDialerFunc(ctx context.Context, addr string) (net.Conn, error) {
 }
 
 type client struct {
-	conn grpc.ClientConn
+	conn *ggrpc.ClientConn
 
 	VersionServiceClient
 	BlockServiceClient
 	BlockResultsServiceClient
 }
 
+// Close implements Client.
+func (c *client) Close() error {
+	return c.conn.Close()
+}
+
 // WithInsecure disables transport security for the underlying client
 // connection.
 //
@@ -126,11 +133,10 @@ func New(ctx context.Context, addr string, opts ...Option) (Client, error) {
 	if builder.blockResultsServiceEnabled {
 		blockResultServiceClient = newBlockResultsServiceClient(conn)
 	}
-	client := &client{
+	return &client{
 		conn:                      conn,
 		VersionServiceClient:      versionServiceClient,
 		BlockServiceClient:        blockServiceClient,
 		BlockResultsServiceClient: blockResultServiceClient,
-	}
-	return client, nil
+	}, nil
 }

rpc/grpc/client/privileged/privileged.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"net"
 
-	"github.com/cosmos/gogoproto/grpc"
 	ggrpc "google.golang.org/grpc"
 	"google.golang.org/grpc/credentials/insecure"
 
@@ -18,6 +17,9 @@ type Option func(*clientBuilder)
 // a CometBFT node via the privileged gRPC server.
 type Client interface {
 	PruningServiceClient
+
+	// Close the connection to the server. Any subsequent requests will fail.
+	Close() error
 }
 
 type clientBuilder struct {
@@ -40,11 +42,16 @@ func defaultDialerFunc(ctx context.Context, addr string) (net.Conn, error) {
 }
 
 type client struct {
-	conn grpc.ClientConn
+	conn *ggrpc.ClientConn
 
 	PruningServiceClient
 }
 
+// Close implements Client.
+func (c *client) Close() error {
+	return c.conn.Close()
+}
+
 // WithInsecure disables transport security for the underlying client
 // connection.
 //
@@ -96,9 +103,8 @@ func New(ctx context.Context, addr string, opts ...Option) (Client, error) {
 	if builder.pruningServiceEnabled {
 		pruningServiceClient = newPruningServiceClient(conn)
 	}
-	client := &client{
+	return &client{
 		conn:                 conn,
 		PruningServiceClient: pruningServiceClient,
-	}
-	return client, nil
+	}, nil
 }

test/e2e/tests/grpc_test.go

@@ -19,6 +19,7 @@ func TestGRPC_Version(t *testing.T) {
 		defer ctxCancel()
 		client, err := node.GRPCClient(ctx)
 		require.NoError(t, err)
+		defer client.Close()
 
 		res, err := client.GetVersion(ctx)
 		require.NoError(t, err)
@@ -63,6 +64,7 @@ func TestGRPC_Block_GetByHeight(t *testing.T) {
 		defer ctxCancel()
 		gRPCClient, err := node.GRPCClient(ctx)
 		require.NoError(t, err)
+		defer gRPCClient.Close()
 
 		for _, block := range blocks {
 			if block.Header.Height < first {
@@ -102,6 +104,7 @@ func TestGRPC_Block_GetLatest(t *testing.T) {
 
 		gclient, err := node.GRPCClient(ctx)
 		require.NoError(t, err)
+		defer gclient.Close()
 
 		resultCh, err := gclient.GetLatestHeight(ctx)
 		require.NoError(t, err)
@@ -132,6 +135,7 @@ func TestGRPC_Block_GetLatestHeight(t *testing.T) {
 
 		gclient, err := node.GRPCClient(ctx)
 		require.NoError(t, err)
+		defer gclient.Close()
 
 		resultCh, err := gclient.GetLatestHeight(ctx)
 		require.NoError(t, err)
@@ -163,6 +167,7 @@ func TestGRPC_GetBlockResults(t *testing.T) {
 		defer ctxCancel()
 		gRPCClient, err := node.GRPCClient(ctx)
 		require.NoError(t, err)
+		defer gRPCClient.Close()
 
 		// GetLatestBlockResults
 		latestBlockResults, err := gRPCClient.GetLatestBlockResults(ctx)
@@ -208,6 +213,7 @@ func TestGRPC_BlockRetainHeight(t *testing.T) {
 		defer ctxCancel()
 		grpcClient, err := node.GRPCPrivilegedClient(ctx)
 		require.NoError(t, err)
+		defer grpcClient.Close()
 
 		client, err := node.Client()
 		require.NoError(t, err)
@@ -232,19 +238,21 @@ func TestGRPC_BlockResultsRetainHeight(t *testing.T) {
 
 		ctx, ctxCancel := context.WithTimeout(context.Background(), time.Minute)
 		defer ctxCancel()
+
 		grpcClient, err := node.GRPCPrivilegedClient(ctx)
 		require.NoError(t, err)
+		defer grpcClient.Close()
+
 		client, err := node.Client()
 		require.NoError(t, err)
+
 		status, err := client.Status(ctx)
 		require.NoError(t, err)
 
 		err = grpcClient.SetBlockResultsRetainHeight(ctx, uint64(status.SyncInfo.LatestBlockHeight)-1)
-
 		require.NoError(t, err, "Unexpected error for SetBlockResultsRetainHeight")
 
 		height, err := grpcClient.GetBlockResultsRetainHeight(ctx)
-
 		require.NoError(t, err, "Unexpected error for GetBlockRetainHeight")
 		require.Equal(t, height, uint64(status.SyncInfo.LatestBlockHeight)-1)
 	})