fix(client): Add GetHeightFromMetadataStrict API to grpc client for better error handling. (#25804)

Co-authored-by: Alex | Cosmos Labs <[email protected]>
Co-authored-by: Tyler <[email protected]>

Author: 3 people

CHANGELOG.md

@@ -95,7 +95,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (cli) [#25485](https://github.com/cosmos/cosmos-sdk/pull/25485) Avoid failed to convert address field in `withdraw-validator-commission` cmd.
 * (baseapp) [#25642](https://github.com/cosmos/cosmos-sdk/pull/25642) Mark pre-block events for indexing based on local configuration.
 * (x/bank) [#25751](https://github.com/cosmos/cosmos-sdk/pull/25751) Fix recipient address in events.
-* (server/config) [#25806] (https://github.com/cosmos/cosmos-sdk/pull/25806) fix: add missing commas in historical gRPC config template #25806.
+* (server/config) [#25806](https://github.com/cosmos/cosmos-sdk/pull/25806) fix: add missing commas in historical gRPC config template.
+* (client) [#25804](https://github.com/cosmos/cosmos-sdk/pull/25804) Add `GetHeightFromMetadataStrict` API to `grpc` client for better error handling.
 
 ### Deprecated
 

client/grpc_query.go

@@ -75,23 +75,38 @@ var fallBackCodec = codec.NewProtoCodec(types.NewInterfaceRegistry())
 
 // GetHeightFromMetadata extracts the block height from gRPC metadata in the context.
 // Returns 0 if no valid height is found.
+//
+// Deprecated: This function silently ignores parse errors and negative heights for backward compatibility.
+// Use GetHeightFromMetadataStrict for stricter error handling.
 func GetHeightFromMetadata(grpcCtx gocontext.Context) int64 {
+	height, _ := GetHeightFromMetadataStrict(grpcCtx)
+	return height
+}
+
+// GetHeightFromMetadataStrict extracts the block height from gRPC metadata in the context.
+// Returns an error if the height header is present but invalid (parse error or negative).
+// Returns 0 with no error if no height header is present.
+func GetHeightFromMetadataStrict(grpcCtx gocontext.Context) (int64, error) {
 	md, ok := metadata.FromOutgoingContext(grpcCtx)
 	if !ok {
-		return 0
+		return 0, nil
 	}
 	heights := md.Get(grpctypes.GRPCBlockHeightHeader)
 	if len(heights) == 0 {
-		return 0
+		return 0, nil
 	}
 	height, err := strconv.ParseInt(heights[0], 10, 64)
 	if err != nil {
-		return 0
+		return 0, errorsmod.Wrapf(
+			sdkerrors.ErrInvalidRequest,
+			"invalid height header %q: %v", grpctypes.GRPCBlockHeightHeader, err)
 	}
 	if height < 0 {
-		return 0
+		return 0, errorsmod.Wrapf(
+			sdkerrors.ErrInvalidRequest,
+			"height (%d) from %q must be >= 0", height, grpctypes.GRPCBlockHeightHeader)
 	}
-	return height
+	return height, nil
 }
 
 // Invoke implements the grpc ClientConn.Invoke method
@@ -102,15 +117,19 @@ func (ctx Context) Invoke(grpcCtx gocontext.Context, method string, req, reply a
 	// 2-2. or we are querying for state, in which case we call ABCI's Query if grpc client not set.
 
 	// In both cases, we don't allow empty request args (it will panic unexpectedly).
-	if reflect.ValueOf(req).IsNil() {
+	if req == nil {
+		return errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "request cannot be nil")
+	}
+	reqVal := reflect.ValueOf(req)
+	if reqVal.Kind() == reflect.Ptr && reqVal.IsNil() {
 		return errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "request cannot be nil")
 	}
 
 	// Case 1. Broadcasting a Tx.
 	if reqProto, ok := req.(*tx.BroadcastTxRequest); ok {
 		res, ok := reply.(*tx.BroadcastTxResponse)
 		if !ok {
-			return errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, "expected %T, got %T", (*tx.BroadcastTxResponse)(nil), req)
+			return errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, "expected %T, got %T", (*tx.BroadcastTxResponse)(nil), reply)
 		}
 
 		broadcastRes, err := TxServiceBroadcast(grpcCtx, ctx, reqProto)
@@ -119,7 +138,7 @@ func (ctx Context) Invoke(grpcCtx gocontext.Context, method string, req, reply a
 		}
 		*res = *broadcastRes
 
-		return err
+		return nil
 	}
 
 	if ctx.GRPCClient != nil {
@@ -128,7 +147,10 @@ func (ctx Context) Invoke(grpcCtx gocontext.Context, method string, req, reply a
 		if ctx.GRPCConnProvider != nil {
 			height := ctx.Height
 			if height <= 0 {
-				height = GetHeightFromMetadata(grpcCtx)
+				height, err = GetHeightFromMetadataStrict(grpcCtx)
+				if err != nil {
+					return err
+				}
 			}
 
 			grpcConn = ctx.GRPCConnProvider.GetGRPCConn(height)
@@ -143,11 +165,9 @@ func (ctx Context) Invoke(grpcCtx gocontext.Context, method string, req, reply a
 	}
 
 	// parse height header
-	height := GetHeightFromMetadata(grpcCtx)
-	if height < 0 {
-		return errorsmod.Wrapf(
-			sdkerrors.ErrInvalidRequest,
-			"client.Context.Invoke: height (%d) from %q must be >= 0", height, grpctypes.GRPCBlockHeightHeader)
+	height, err := GetHeightFromMetadataStrict(grpcCtx)
+	if err != nil {
+		return err
 	}
 
 	if height > 0 {

client/grpc_query_test.go

@@ -330,3 +330,54 @@ func TestGetHeightFromMetadata(t *testing.T) {
 		})
 	}
 }
+
+func TestGetHeightFromMetadataStrict(t *testing.T) {
+	tests := []struct {
+		name           string
+		setupContext   func() context.Context
+		expectedHeight int64
+		expectError    bool
+	}{
+		{
+			name: "valid height",
+			setupContext: func() context.Context {
+				md := metadata.Pairs(grpctypes.GRPCBlockHeightHeader, "123")
+				return metadata.NewOutgoingContext(context.Background(), md)
+			},
+			expectedHeight: 123,
+		},
+		{
+			name:         "no metadata",
+			setupContext: context.Background,
+		},
+		{
+			name: "negative height errors",
+			setupContext: func() context.Context {
+				md := metadata.Pairs(grpctypes.GRPCBlockHeightHeader, "-10")
+				return metadata.NewOutgoingContext(context.Background(), md)
+			},
+			expectError: true,
+		},
+		{
+			name: "invalid height errors",
+			setupContext: func() context.Context {
+				md := metadata.Pairs(grpctypes.GRPCBlockHeightHeader, "foo")
+				return metadata.NewOutgoingContext(context.Background(), md)
+			},
+			expectError: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ctx := tt.setupContext()
+			height, err := client.GetHeightFromMetadataStrict(ctx)
+			if tt.expectError {
+				require.Error(t, err)
+				return
+			}
+			require.NoError(t, err)
+			require.Equal(t, tt.expectedHeight, height)
+		})
+	}
+}