rpc: simplify the JSON-RPC client Caller interface (#7549)

* Update Caller interface and its documentation.
* Rework MapToRequest as ParamsToRequest.

The old interface returned the result as well as populating it.  Nothing was
using this, so drop the duplicated value from the return signature. Clarify the
documentation on the Caller type.

Rework the MapToRequest helper to take an arbitrary value instead of only a
map. This is groundwork for getting rid of the custom marshaling code. For now,
however, the implementation preserves the existing behaviour for the map, until
we can replace those.

rpc/client/http/http.go

@@ -203,21 +203,17 @@ func (b *BatchHTTP) Count() int {
 
 func (c *baseRPCClient) Status(ctx context.Context) (*coretypes.ResultStatus, error) {
 	result := new(coretypes.ResultStatus)
-	_, err := c.caller.Call(ctx, "status", map[string]interface{}{}, result)
-	if err != nil {
+	if err := c.caller.Call(ctx, "status", map[string]interface{}{}, result); err != nil {
 		return nil, err
 	}
-
 	return result, nil
 }
 
 func (c *baseRPCClient) ABCIInfo(ctx context.Context) (*coretypes.ResultABCIInfo, error) {
 	result := new(coretypes.ResultABCIInfo)
-	_, err := c.caller.Call(ctx, "abci_info", map[string]interface{}{}, result)
-	if err != nil {
+	if err := c.caller.Call(ctx, "abci_info", map[string]interface{}{}, result); err != nil {
 		return nil, err
 	}
-
 	return result, nil
 }
 
@@ -235,13 +231,11 @@ func (c *baseRPCClient) ABCIQueryWithOptions(
 	data bytes.HexBytes,
 	opts rpcclient.ABCIQueryOptions) (*coretypes.ResultABCIQuery, error) {
 	result := new(coretypes.ResultABCIQuery)
-	_, err := c.caller.Call(ctx, "abci_query",
+	if err := c.caller.Call(ctx, "abci_query",
 		map[string]interface{}{"path": path, "data": data, "height": opts.Height, "prove": opts.Prove},
-		result)
-	if err != nil {
+		result); err != nil {
 		return nil, err
 	}
-
 	return result, nil
 }
 
@@ -250,8 +244,7 @@ func (c *baseRPCClient) BroadcastTxCommit(
 	tx types.Tx,
 ) (*coretypes.ResultBroadcastTxCommit, error) {
 	result := new(coretypes.ResultBroadcastTxCommit)
-	_, err := c.caller.Call(ctx, "broadcast_tx_commit", map[string]interface{}{"tx": tx}, result)
-	if err != nil {
+	if err := c.caller.Call(ctx, "broadcast_tx_commit", map[string]interface{}{"tx": tx}, result); err != nil {
 		return nil, err
 	}
 	return result, nil
@@ -277,8 +270,7 @@ func (c *baseRPCClient) broadcastTX(
 	tx types.Tx,
 ) (*coretypes.ResultBroadcastTx, error) {
 	result := new(coretypes.ResultBroadcastTx)
-	_, err := c.caller.Call(ctx, route, map[string]interface{}{"tx": tx}, result)
-	if err != nil {
+	if err := c.caller.Call(ctx, route, map[string]interface{}{"tx": tx}, result); err != nil {
 		return nil, err
 	}
 	return result, nil
@@ -293,8 +285,7 @@ func (c *baseRPCClient) UnconfirmedTxs(
 	if limit != nil {
 		params["limit"] = limit
 	}
-	_, err := c.caller.Call(ctx, "unconfirmed_txs", params, result)
-	if err != nil {
+	if err := c.caller.Call(ctx, "unconfirmed_txs", params, result); err != nil {
 		return nil, err
 	}
 	return result, nil
@@ -302,8 +293,7 @@ func (c *baseRPCClient) UnconfirmedTxs(
 
 func (c *baseRPCClient) NumUnconfirmedTxs(ctx context.Context) (*coretypes.ResultUnconfirmedTxs, error) {
 	result := new(coretypes.ResultUnconfirmedTxs)
-	_, err := c.caller.Call(ctx, "num_unconfirmed_txs", map[string]interface{}{}, result)
-	if err != nil {
+	if err := c.caller.Call(ctx, "num_unconfirmed_txs", map[string]interface{}{}, result); err != nil {
 		return nil, err
 	}
 	return result, nil
@@ -311,16 +301,14 @@ func (c *baseRPCClient) NumUnconfirmedTxs(ctx context.Context) (*coretypes.Resul
 
 func (c *baseRPCClient) CheckTx(ctx context.Context, tx types.Tx) (*coretypes.ResultCheckTx, error) {
 	result := new(coretypes.ResultCheckTx)
-	_, err := c.caller.Call(ctx, "check_tx", map[string]interface{}{"tx": tx}, result)
-	if err != nil {
+	if err := c.caller.Call(ctx, "check_tx", map[string]interface{}{"tx": tx}, result); err != nil {
 		return nil, err
 	}
 	return result, nil
 }
 
 func (c *baseRPCClient) RemoveTx(ctx context.Context, txKey types.TxKey) error {
-	_, err := c.caller.Call(ctx, "remove_tx", map[string]interface{}{"tx_key": txKey}, nil)
-	if err != nil {
+	if err := c.caller.Call(ctx, "remove_tx", map[string]interface{}{"tx_key": txKey}, nil); err != nil {
 		return err
 	}
 	return nil
@@ -328,8 +316,7 @@ func (c *baseRPCClient) RemoveTx(ctx context.Context, txKey types.TxKey) error {
 
 func (c *baseRPCClient) NetInfo(ctx context.Context) (*coretypes.ResultNetInfo, error) {
 	result := new(coretypes.ResultNetInfo)
-	_, err := c.caller.Call(ctx, "net_info", map[string]interface{}{}, result)
-	if err != nil {
+	if err := c.caller.Call(ctx, "net_info", map[string]interface{}{}, result); err != nil {
 		return nil, err
 	}
 	return result, nil
@@ -337,8 +324,7 @@ func (c *baseRPCClient) NetInfo(ctx context.Context) (*coretypes.ResultNetInfo,
 
 func (c *baseRPCClient) DumpConsensusState(ctx context.Context) (*coretypes.ResultDumpConsensusState, error) {
 	result := new(coretypes.ResultDumpConsensusState)
-	_, err := c.caller.Call(ctx, "dump_consensus_state", map[string]interface{}{}, result)
-	if err != nil {
+	if err := c.caller.Call(ctx, "dump_consensus_state", map[string]interface{}{}, result); err != nil {
 		return nil, err
 	}
 	return result, nil
@@ -346,8 +332,7 @@ func (c *baseRPCClient) DumpConsensusState(ctx context.Context) (*coretypes.Resu
 
 func (c *baseRPCClient) ConsensusState(ctx context.Context) (*coretypes.ResultConsensusState, error) {
 	result := new(coretypes.ResultConsensusState)
-	_, err := c.caller.Call(ctx, "consensus_state", map[string]interface{}{}, result)
-	if err != nil {
+	if err := c.caller.Call(ctx, "consensus_state", map[string]interface{}{}, result); err != nil {
 		return nil, err
 	}
 	return result, nil
@@ -362,8 +347,7 @@ func (c *baseRPCClient) ConsensusParams(
 	if height != nil {
 		params["height"] = height
 	}
-	_, err := c.caller.Call(ctx, "consensus_params", params, result)
-	if err != nil {
+	if err := c.caller.Call(ctx, "consensus_params", params, result); err != nil {
 		return nil, err
 	}
 	return result, nil
@@ -371,8 +355,7 @@ func (c *baseRPCClient) ConsensusParams(
 
 func (c *baseRPCClient) Health(ctx context.Context) (*coretypes.ResultHealth, error) {
 	result := new(coretypes.ResultHealth)
-	_, err := c.caller.Call(ctx, "health", map[string]interface{}{}, result)
-	if err != nil {
+	if err := c.caller.Call(ctx, "health", map[string]interface{}{}, result); err != nil {
 		return nil, err
 	}
 	return result, nil
@@ -384,10 +367,9 @@ func (c *baseRPCClient) BlockchainInfo(
 	maxHeight int64,
 ) (*coretypes.ResultBlockchainInfo, error) {
 	result := new(coretypes.ResultBlockchainInfo)
-	_, err := c.caller.Call(ctx, "blockchain",
+	if err := c.caller.Call(ctx, "blockchain",
 		map[string]interface{}{"minHeight": minHeight, "maxHeight": maxHeight},
-		result)
-	if err != nil {
+		result); err != nil {
 		return nil, err
 	}
 	return result, nil
@@ -395,8 +377,7 @@ func (c *baseRPCClient) BlockchainInfo(
 
 func (c *baseRPCClient) Genesis(ctx context.Context) (*coretypes.ResultGenesis, error) {
 	result := new(coretypes.ResultGenesis)
-	_, err := c.caller.Call(ctx, "genesis", map[string]interface{}{}, result)
-	if err != nil {
+	if err := c.caller.Call(ctx, "genesis", map[string]interface{}{}, result); err != nil {
 		return nil, err
 	}
 	return result, nil
@@ -404,8 +385,7 @@ func (c *baseRPCClient) Genesis(ctx context.Context) (*coretypes.ResultGenesis,
 
 func (c *baseRPCClient) GenesisChunked(ctx context.Context, id uint) (*coretypes.ResultGenesisChunk, error) {
 	result := new(coretypes.ResultGenesisChunk)
-	_, err := c.caller.Call(ctx, "genesis_chunked", map[string]interface{}{"chunk": id}, result)
-	if err != nil {
+	if err := c.caller.Call(ctx, "genesis_chunked", map[string]interface{}{"chunk": id}, result); err != nil {
 		return nil, err
 	}
 	return result, nil
@@ -417,8 +397,7 @@ func (c *baseRPCClient) Block(ctx context.Context, height *int64) (*coretypes.Re
 	if height != nil {
 		params["height"] = height
 	}
-	_, err := c.caller.Call(ctx, "block", params, result)
-	if err != nil {
+	if err := c.caller.Call(ctx, "block", params, result); err != nil {
 		return nil, err
 	}
 	return result, nil
@@ -429,8 +408,7 @@ func (c *baseRPCClient) BlockByHash(ctx context.Context, hash bytes.HexBytes) (*
 	params := map[string]interface{}{
 		"hash": hash,
 	}
-	_, err := c.caller.Call(ctx, "block_by_hash", params, result)
-	if err != nil {
+	if err := c.caller.Call(ctx, "block_by_hash", params, result); err != nil {
 		return nil, err
 	}
 	return result, nil
@@ -445,8 +423,7 @@ func (c *baseRPCClient) BlockResults(
 	if height != nil {
 		params["height"] = height
 	}
-	_, err := c.caller.Call(ctx, "block_results", params, result)
-	if err != nil {
+	if err := c.caller.Call(ctx, "block_results", params, result); err != nil {
 		return nil, err
 	}
 	return result, nil
@@ -458,8 +435,7 @@ func (c *baseRPCClient) Header(ctx context.Context, height *int64) (*coretypes.R
 	if height != nil {
 		params["height"] = height
 	}
-	_, err := c.caller.Call(ctx, "header", params, result)
-	if err != nil {
+	if err := c.caller.Call(ctx, "header", params, result); err != nil {
 		return nil, err
 	}
 	return result, nil
@@ -470,8 +446,7 @@ func (c *baseRPCClient) HeaderByHash(ctx context.Context, hash bytes.HexBytes) (
 	params := map[string]interface{}{
 		"hash": hash,
 	}
-	_, err := c.caller.Call(ctx, "header_by_hash", params, result)
-	if err != nil {
+	if err := c.caller.Call(ctx, "header_by_hash", params, result); err != nil {
 		return nil, err
 	}
 	return result, nil
@@ -483,8 +458,7 @@ func (c *baseRPCClient) Commit(ctx context.Context, height *int64) (*coretypes.R
 	if height != nil {
 		params["height"] = height
 	}
-	_, err := c.caller.Call(ctx, "commit", params, result)
-	if err != nil {
+	if err := c.caller.Call(ctx, "commit", params, result); err != nil {
 		return nil, err
 	}
 	return result, nil
@@ -496,8 +470,7 @@ func (c *baseRPCClient) Tx(ctx context.Context, hash bytes.HexBytes, prove bool)
 		"hash":  hash,
 		"prove": prove,
 	}
-	_, err := c.caller.Call(ctx, "tx", params, result)
-	if err != nil {
+	if err := c.caller.Call(ctx, "tx", params, result); err != nil {
 		return nil, err
 	}
 	return result, nil
@@ -526,8 +499,7 @@ func (c *baseRPCClient) TxSearch(
 		params["per_page"] = perPage
 	}
 
-	_, err := c.caller.Call(ctx, "tx_search", params, result)
-	if err != nil {
+	if err := c.caller.Call(ctx, "tx_search", params, result); err != nil {
 		return nil, err
 	}
 
@@ -554,8 +526,7 @@ func (c *baseRPCClient) BlockSearch(
 		params["per_page"] = perPage
 	}
 
-	_, err := c.caller.Call(ctx, "block_search", params, result)
-	if err != nil {
+	if err := c.caller.Call(ctx, "block_search", params, result); err != nil {
 		return nil, err
 	}
 
@@ -579,8 +550,7 @@ func (c *baseRPCClient) Validators(
 	if height != nil {
 		params["height"] = height
 	}
-	_, err := c.caller.Call(ctx, "validators", params, result)
-	if err != nil {
+	if err := c.caller.Call(ctx, "validators", params, result); err != nil {
 		return nil, err
 	}
 	return result, nil
@@ -591,8 +561,7 @@ func (c *baseRPCClient) BroadcastEvidence(
 	ev types.Evidence,
 ) (*coretypes.ResultBroadcastEvidence, error) {
 	result := new(coretypes.ResultBroadcastEvidence)
-	_, err := c.caller.Call(ctx, "broadcast_evidence", map[string]interface{}{"evidence": ev}, result)
-	if err != nil {
+	if err := c.caller.Call(ctx, "broadcast_evidence", map[string]interface{}{"evidence": ev}, result); err != nil {
 		return nil, err
 	}
 	return result, nil

rpc/jsonrpc/client/decode.go

@@ -9,33 +9,27 @@ import (
 	rpctypes "github.com/tendermint/tendermint/rpc/jsonrpc/types"
 )
 
-func unmarshalResponseBytes(
-	responseBytes []byte,
-	expectedID rpctypes.JSONRPCIntID,
-	result interface{},
-) (interface{}, error) {
-
+func unmarshalResponseBytes(responseBytes []byte, expectedID rpctypes.JSONRPCIntID, result interface{}) error {
 	// Read response.  If rpc/core/types is imported, the result will unmarshal
 	// into the correct type.
 	response := &rpctypes.RPCResponse{}
 	if err := json.Unmarshal(responseBytes, response); err != nil {
-		return nil, fmt.Errorf("error unmarshaling: %w", err)
+		return fmt.Errorf("error unmarshaling: %w", err)
 	}
 
 	if response.Error != nil {
-		return nil, response.Error
+		return response.Error
 	}
 
 	if err := validateAndVerifyID(response, expectedID); err != nil {
-		return nil, fmt.Errorf("wrong ID: %w", err)
+		return fmt.Errorf("wrong ID: %w", err)
 	}
 
 	// Unmarshal the RawMessage into the result.
 	if err := tmjson.Unmarshal(response.Result, result); err != nil {
-		return nil, fmt.Errorf("error unmarshaling result: %w", err)
+		return fmt.Errorf("error unmarshaling result: %w", err)
 	}
-
-	return result, nil
+	return nil
 }
 
 func unmarshalResponseBytesArray(

rpc/jsonrpc/client/http_json_client.go

@@ -106,9 +106,15 @@ func (u parsedURL) GetTrimmedURL() string {
 
 //-------------------------------------------------------------
 
-// Caller implementers can facilitate calling the JSON-RPC endpoint.
+// A Caller handles the round trip of a single JSON-RPC request.  The
+// implementation is responsible for assigning request IDs, marshaling
+// parameters, and unmarshaling results.
 type Caller interface {
-	Call(ctx context.Context, method string, params map[string]interface{}, result interface{}) (interface{}, error)
+	// Call sends a new request for method to the server with the given
+	// parameters. If params == nil, the request has empty parameters.
+	// If result == nil, any result value must be discarded without error.
+	// Otherwise the concrete value of result must be a pointer.
+	Call(ctx context.Context, method string, params, result interface{}) error
 }
 
 //-------------------------------------------------------------
@@ -174,28 +180,23 @@ func NewWithHTTPClient(remote string, c *http.Client) (*Client, error) {
 
 // Call issues a POST HTTP request. Requests are JSON encoded. Content-Type:
 // application/json.
-func (c *Client) Call(
-	ctx context.Context,
-	method string,
-	params map[string]interface{},
-	result interface{},
-) (interface{}, error) {
+func (c *Client) Call(ctx context.Context, method string, params, result interface{}) error {
 	id := c.nextRequestID()
 
-	request, err := rpctypes.MapToRequest(id, method, params)
+	request, err := rpctypes.ParamsToRequest(id, method, params)
 	if err != nil {
-		return nil, fmt.Errorf("failed to encode params: %w", err)
+		return fmt.Errorf("failed to encode params: %w", err)
 	}
 
 	requestBytes, err := json.Marshal(request)
 	if err != nil {
-		return nil, fmt.Errorf("failed to marshal request: %w", err)
+		return fmt.Errorf("failed to marshal request: %w", err)
 	}
 
 	requestBuf := bytes.NewBuffer(requestBytes)
 	httpRequest, err := http.NewRequestWithContext(ctx, http.MethodPost, c.address, requestBuf)
 	if err != nil {
-		return nil, fmt.Errorf("request setup failed: %w", err)
+		return fmt.Errorf("request setup failed: %w", err)
 	}
 
 	httpRequest.Header.Set("Content-Type", "application/json")
@@ -206,14 +207,14 @@ func (c *Client) Call(
 
 	httpResponse, err := c.client.Do(httpRequest)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
 	defer httpResponse.Body.Close()
 
 	responseBytes, err := io.ReadAll(httpResponse.Body)
 	if err != nil {
-		return nil, fmt.Errorf("failed to read response body: %w", err)
+		return fmt.Errorf("failed to read response body: %w", err)
 	}
 
 	return unmarshalResponseBytes(responseBytes, id, result)
@@ -340,19 +341,14 @@ func (b *RequestBatch) Send(ctx context.Context) ([]interface{}, error) {
 
 // Call enqueues a request to call the given RPC method with the specified
 // parameters, in the same way that the `Client.Call` function would.
-func (b *RequestBatch) Call(
-	_ context.Context,
-	method string,
-	params map[string]interface{},
-	result interface{},
-) (interface{}, error) {
+func (b *RequestBatch) Call(_ context.Context, method string, params, result interface{}) error {
 	id := b.client.nextRequestID()
-	request, err := rpctypes.MapToRequest(id, method, params)
+	request, err := rpctypes.ParamsToRequest(id, method, params)
 	if err != nil {
-		return nil, err
+		return err
 	}
 	b.enqueue(&jsonRPCBufferedRequest{request: request, result: result})
-	return result, nil
+	return nil
 }
 
 //-------------------------------------------------------------

rpc/jsonrpc/client/ws_client.go

@@ -217,7 +217,7 @@ func (c *WSClient) Send(ctx context.Context, request rpctypes.RPCRequest) error
 
 // Call enqueues a call request onto the Send queue. Requests are JSON encoded.
 func (c *WSClient) Call(ctx context.Context, method string, params map[string]interface{}) error {
-	request, err := rpctypes.MapToRequest(c.nextRequestID(), method, params)
+	request, err := rpctypes.ParamsToRequest(c.nextRequestID(), method, params)
 	if err != nil {
 		return err
 	}

rpc/jsonrpc/jsonrpc_test.go

@@ -148,7 +148,7 @@ func echoViaHTTP(ctx context.Context, cl client.Caller, val string) (string, err
 		"arg": val,
 	}
 	result := new(ResultEcho)
-	if _, err := cl.Call(ctx, "echo", params, result); err != nil {
+	if err := cl.Call(ctx, "echo", params, result); err != nil {
 		return "", err
 	}
 	return result.Value, nil
@@ -159,7 +159,7 @@ func echoIntViaHTTP(ctx context.Context, cl client.Caller, val int) (int, error)
 		"arg": val,
 	}
 	result := new(ResultEchoInt)
-	if _, err := cl.Call(ctx, "echo_int", params, result); err != nil {
+	if err := cl.Call(ctx, "echo_int", params, result); err != nil {
 		return 0, err
 	}
 	return result.Value, nil
@@ -170,7 +170,7 @@ func echoBytesViaHTTP(ctx context.Context, cl client.Caller, bytes []byte) ([]by
 		"arg": bytes,
 	}
 	result := new(ResultEchoBytes)
-	if _, err := cl.Call(ctx, "echo_bytes", params, result); err != nil {
+	if err := cl.Call(ctx, "echo_bytes", params, result); err != nil {
 		return []byte{}, err
 	}
 	return result.Value, nil
@@ -181,7 +181,7 @@ func echoDataBytesViaHTTP(ctx context.Context, cl client.Caller, bytes tmbytes.H
 		"arg": bytes,
 	}
 	result := new(ResultEchoDataBytes)
-	if _, err := cl.Call(ctx, "echo_data_bytes", params, result); err != nil {
+	if err := cl.Call(ctx, "echo_data_bytes", params, result); err != nil {
 		return []byte{}, err
 	}
 	return result.Value, nil

rpc/jsonrpc/server/ws_handler_test.go

@@ -28,7 +28,7 @@ func TestWebsocketManagerHandler(t *testing.T) {
 	}
 
 	// check basic functionality works
-	req, err := rpctypes.MapToRequest(
+	req, err := rpctypes.ParamsToRequest(
 		rpctypes.JSONRPCStringID("TestWebsocketManager"),
 		"c",
 		map[string]interface{}{"s": "a", "i": 10},

rpc/jsonrpc/types/types.go

@@ -98,17 +98,27 @@ func (req RPCRequest) String() string {
 	return fmt.Sprintf("RPCRequest{%s %s/%X}", req.ID, req.Method, req.Params)
 }
 
-func MapToRequest(id jsonrpcid, method string, params map[string]interface{}) (RPCRequest, error) {
-	var paramsMap = make(map[string]json.RawMessage, len(params))
-	for name, value := range params {
-		valueJSON, err := tmjson.Marshal(value)
-		if err != nil {
-			return RPCRequest{}, err
+// ParamsToRequest constructs a new RPCRequest with the given ID, method, and parameters.
+func ParamsToRequest(id jsonrpcid, method string, params interface{}) (RPCRequest, error) {
+	var payload json.RawMessage
+	var err error
+	switch t := params.(type) {
+	case map[string]interface{}:
+		// TODO(creachadair): This special case preserves existing behavior that
+		// relies on the custom JSON encoding library. Remove it once that
+		// requirement has been removed.
+		paramsMap := make(map[string]json.RawMessage, len(t))
+		for name, value := range t {
+			valueJSON, err := tmjson.Marshal(value)
+			if err != nil {
+				return RPCRequest{}, err
+			}
+			paramsMap[name] = valueJSON
 		}
-		paramsMap[name] = valueJSON
+		payload, err = json.Marshal(paramsMap)
+	default:
+		payload, err = json.Marshal(params)
 	}
-
-	payload, err := json.Marshal(paramsMap)
 	if err != nil {
 		return RPCRequest{}, err
 	}

rpc/test/helpers.go

@@ -32,7 +32,7 @@ func waitForRPC(ctx context.Context, conf *config.Config) {
 	}
 	result := new(coretypes.ResultStatus)
 	for {
-		_, err := client.Call(ctx, "status", map[string]interface{}{}, result)
+		err := client.Call(ctx, "status", map[string]interface{}{}, result)
 		if err == nil {
 			return
 		}