rpc: fix hash encoding in JSON parameters (#6813)

The responses from node RPCs encode hash values as hexadecimal strings. This
behaviour is stipulated in our OpenAPI documentation. In some cases, however,
hashes received as JSON parameters were being decoded as byte buffers, as is
the convention for JSON.

This resulted in the confusing situation that a hash reported by one request
(e.g., broadcast_tx_commit) could not be passed as a parameter to another
(e.g., tx) via JSON, without translating the hex-encoded output hash into the
base64 encoding used by JSON for opaque bytes.

Fixes #6802.

docs/tendermint-core/using-tendermint.md

@@ -185,51 +185,65 @@ the argument name and use `_` as a placeholder.
 
 ### Formatting
 
-The following nuances when sending/formatting transactions should be
-taken into account:
+When sending transactions to the RPC interface, the following formatting rules
+must be followed:
 
-With `GET`:
+Using `GET` (with parameters in the URL):
 
-To send a UTF8 string byte array, quote the value of the tx parameter:
+To send a UTF8 string as transaction data, enclose the value of the `tx`
+parameter in double quotes:
 
 ```sh
 curl 'http://localhost:26657/broadcast_tx_commit?tx="hello"'
 ```
 
-which sends a 5 byte transaction: "h e l l o" \[68 65 6c 6c 6f\].
+which sends a 5-byte transaction: "h e l l o" \[68 65 6c 6c 6f\].
 
-Note the URL must be wrapped with single quotes, else bash will ignore
-the double quotes. To avoid the single quotes, escape the double quotes:
+Note that the URL in this example is enclosed in single quotes to prevent the
+shell from interpreting the double quotes. Alternatively, you may escape the
+double quotes with backslashes:
 
 ```sh
 curl http://localhost:26657/broadcast_tx_commit?tx=\"hello\"
 ```
 
-Using a special character:
+The double-quoted format works with for multibyte characters, as long as they
+are valid UTF8, for example:
 
 ```sh
 curl 'http://localhost:26657/broadcast_tx_commit?tx="€5"'
 ```
 
-sends a 4 byte transaction: "€5" (UTF8) \[e2 82 ac 35\].
+sends a 4-byte transaction: "€5" (UTF8) \[e2 82 ac 35\].
 
-To send as raw hex, omit quotes AND prefix the hex string with `0x`:
+Arbitrary (non-UTF8) transaction data may also be encoded as a string of
+hexadecimal digits (2 digits per byte). To do this, omit the quotation marks
+and prefix the hex string with `0x`:
 
 ```sh
-curl http://localhost:26657/broadcast_tx_commit?tx=0x01020304
+curl http://localhost:26657/broadcast_tx_commit?tx=0x68656C6C6F
 ```
 
-which sends a 4 byte transaction: \[01 02 03 04\].
+which sends the 5-byte transaction: \[68 65 6c 6c 6f\].
 
-With `POST` (using `json`), the raw hex must be `base64` encoded:
+Using `POST` (with parameters in JSON), the transaction data are sent as a JSON
+string in base64 encoding:
 
 ```sh
-curl --data-binary '{"jsonrpc":"2.0","id":"anything","method":"broadcast_tx_commit","params": {"tx": "AQIDBA=="}}' -H 'content-type:text/plain;' http://localhost:26657
+curl http://localhost:26657 -H 'Content-Type: application/json' --data-binary '{
+  "jsonrpc": "2.0",
+  "id": "anything",
+  "method": "broadcast_tx_commit",
+  "params": {
+    "tx": "aGVsbG8="
+  }
+}'
 ```
 
-which sends the same 4 byte transaction: \[01 02 03 04\].
+which sends the same 5-byte transaction: \[68 65 6c 6c 6f\].
 
-Note that raw hex cannot be used in `POST` transactions.
+Note that the hexadecimal encoding of transaction data is _not_ supported in
+JSON (`POST`) requests.
 
 ## Reset
 

light/rpc/client.go

@@ -341,7 +341,7 @@ func (c *Client) Block(ctx context.Context, height *int64) (*ctypes.ResultBlock,
 }
 
 // BlockByHash calls rpcclient#BlockByHash and then verifies the result.
-func (c *Client) BlockByHash(ctx context.Context, hash []byte) (*ctypes.ResultBlock, error) {
+func (c *Client) BlockByHash(ctx context.Context, hash tmbytes.HexBytes) (*ctypes.ResultBlock, error) {
 	res, err := c.next.BlockByHash(ctx, hash)
 	if err != nil {
 		return nil, err
@@ -454,7 +454,7 @@ func (c *Client) Commit(ctx context.Context, height *int64) (*ctypes.ResultCommi
 
 // Tx calls rpcclient#Tx method and then verifies the proof if such was
 // requested.
-func (c *Client) Tx(ctx context.Context, hash []byte, prove bool) (*ctypes.ResultTx, error) {
+func (c *Client) Tx(ctx context.Context, hash tmbytes.HexBytes, prove bool) (*ctypes.ResultTx, error) {
 	res, err := c.next.Tx(ctx, hash, prove)
 	if err != nil || !prove {
 		return res, err

rpc/client/http/http.go

@@ -419,7 +419,7 @@ func (c *baseRPCClient) Block(ctx context.Context, height *int64) (*ctypes.Resul
 	return result, nil
 }
 
-func (c *baseRPCClient) BlockByHash(ctx context.Context, hash []byte) (*ctypes.ResultBlock, error) {
+func (c *baseRPCClient) BlockByHash(ctx context.Context, hash bytes.HexBytes) (*ctypes.ResultBlock, error) {
 	result := new(ctypes.ResultBlock)
 	params := map[string]interface{}{
 		"hash": hash,
@@ -460,7 +460,7 @@ func (c *baseRPCClient) Commit(ctx context.Context, height *int64) (*ctypes.Resu
 	return result, nil
 }
 
-func (c *baseRPCClient) Tx(ctx context.Context, hash []byte, prove bool) (*ctypes.ResultTx, error) {
+func (c *baseRPCClient) Tx(ctx context.Context, hash bytes.HexBytes, prove bool) (*ctypes.ResultTx, error) {
 	result := new(ctypes.ResultTx)
 	params := map[string]interface{}{
 		"hash":  hash,

rpc/client/interface.go

@@ -67,11 +67,11 @@ type ABCIClient interface {
 // and prove anything about the chain.
 type SignClient interface {
 	Block(ctx context.Context, height *int64) (*ctypes.ResultBlock, error)
-	BlockByHash(ctx context.Context, hash []byte) (*ctypes.ResultBlock, error)
+	BlockByHash(ctx context.Context, hash bytes.HexBytes) (*ctypes.ResultBlock, error)
 	BlockResults(ctx context.Context, height *int64) (*ctypes.ResultBlockResults, error)
 	Commit(ctx context.Context, height *int64) (*ctypes.ResultCommit, error)
 	Validators(ctx context.Context, height *int64, page, perPage *int) (*ctypes.ResultValidators, error)
-	Tx(ctx context.Context, hash []byte, prove bool) (*ctypes.ResultTx, error)
+	Tx(ctx context.Context, hash bytes.HexBytes, prove bool) (*ctypes.ResultTx, error)
 
 	// TxSearch defines a method to search for a paginated set of transactions by
 	// DeliverTx event search criteria.

rpc/client/local/local.go

@@ -166,7 +166,7 @@ func (c *Local) Block(ctx context.Context, height *int64) (*ctypes.ResultBlock,
 	return c.env.Block(c.ctx, height)
 }
 
-func (c *Local) BlockByHash(ctx context.Context, hash []byte) (*ctypes.ResultBlock, error) {
+func (c *Local) BlockByHash(ctx context.Context, hash bytes.HexBytes) (*ctypes.ResultBlock, error) {
 	return c.env.BlockByHash(c.ctx, hash)
 }
 
@@ -182,7 +182,7 @@ func (c *Local) Validators(ctx context.Context, height *int64, page, perPage *in
 	return c.env.Validators(c.ctx, height, page, perPage)
 }
 
-func (c *Local) Tx(ctx context.Context, hash []byte, prove bool) (*ctypes.ResultTx, error) {
+func (c *Local) Tx(ctx context.Context, hash bytes.HexBytes, prove bool) (*ctypes.ResultTx, error) {
 	return c.env.Tx(c.ctx, hash, prove)
 }
 

rpc/client/mock/client.go

@@ -166,7 +166,7 @@ func (c Client) Block(ctx context.Context, height *int64) (*ctypes.ResultBlock,
 	return c.env.Block(&rpctypes.Context{}, height)
 }
 
-func (c Client) BlockByHash(ctx context.Context, hash []byte) (*ctypes.ResultBlock, error) {
+func (c Client) BlockByHash(ctx context.Context, hash bytes.HexBytes) (*ctypes.ResultBlock, error) {
 	return c.env.BlockByHash(&rpctypes.Context{}, hash)
 }
 

rpc/client/mocks/client.go

@@ -115,11 +115,11 @@ func (_m *Client) Block(ctx context.Context, height *int64) (*coretypes.ResultBl
 }
 
 // BlockByHash provides a mock function with given fields: ctx, hash
-func (_m *Client) BlockByHash(ctx context.Context, hash []byte) (*coretypes.ResultBlock, error) {
+func (_m *Client) BlockByHash(ctx context.Context, hash bytes.HexBytes) (*coretypes.ResultBlock, error) {
 	ret := _m.Called(ctx, hash)
 
 	var r0 *coretypes.ResultBlock
-	if rf, ok := ret.Get(0).(func(context.Context, []byte) *coretypes.ResultBlock); ok {
+	if rf, ok := ret.Get(0).(func(context.Context, bytes.HexBytes) *coretypes.ResultBlock); ok {
 		r0 = rf(ctx, hash)
 	} else {
 		if ret.Get(0) != nil {
@@ -128,7 +128,7 @@ func (_m *Client) BlockByHash(ctx context.Context, hash []byte) (*coretypes.Resu
 	}
 
 	var r1 error
-	if rf, ok := ret.Get(1).(func(context.Context, []byte) error); ok {
+	if rf, ok := ret.Get(1).(func(context.Context, bytes.HexBytes) error); ok {
 		r1 = rf(ctx, hash)
 	} else {
 		r1 = ret.Error(1)
@@ -706,11 +706,11 @@ func (_m *Client) Subscribe(ctx context.Context, subscriber string, query string
 }
 
 // Tx provides a mock function with given fields: ctx, hash, prove
-func (_m *Client) Tx(ctx context.Context, hash []byte, prove bool) (*coretypes.ResultTx, error) {
+func (_m *Client) Tx(ctx context.Context, hash bytes.HexBytes, prove bool) (*coretypes.ResultTx, error) {
 	ret := _m.Called(ctx, hash, prove)
 
 	var r0 *coretypes.ResultTx
-	if rf, ok := ret.Get(0).(func(context.Context, []byte, bool) *coretypes.ResultTx); ok {
+	if rf, ok := ret.Get(0).(func(context.Context, bytes.HexBytes, bool) *coretypes.ResultTx); ok {
 		r0 = rf(ctx, hash, prove)
 	} else {
 		if ret.Get(0) != nil {
@@ -719,7 +719,7 @@ func (_m *Client) Tx(ctx context.Context, hash []byte, prove bool) (*coretypes.R
 	}
 
 	var r1 error
-	if rf, ok := ret.Get(1).(func(context.Context, []byte, bool) error); ok {
+	if rf, ok := ret.Get(1).(func(context.Context, bytes.HexBytes, bool) error); ok {
 		r1 = rf(ctx, hash, prove)
 	} else {
 		r1 = ret.Error(1)

rpc/core/blocks.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"sort"
 
+	"github.com/tendermint/tendermint/libs/bytes"
 	tmmath "github.com/tendermint/tendermint/libs/math"
 	tmquery "github.com/tendermint/tendermint/libs/pubsub/query"
 	ctypes "github.com/tendermint/tendermint/rpc/core/types"
@@ -107,7 +108,11 @@ func (env *Environment) Block(ctx *rpctypes.Context, heightPtr *int64) (*ctypes.
 
 // BlockByHash gets block by hash.
 // More: https://docs.tendermint.com/master/rpc/#/Info/block_by_hash
-func (env *Environment) BlockByHash(ctx *rpctypes.Context, hash []byte) (*ctypes.ResultBlock, error) {
+func (env *Environment) BlockByHash(ctx *rpctypes.Context, hash bytes.HexBytes) (*ctypes.ResultBlock, error) {
+	// N.B. The hash parameter is HexBytes so that the reflective parameter
+	// decoding logic in the HTTP service will correctly translate from JSON.
+	// See https://github.com/tendermint/tendermint/issues/6802 for context.
+
 	block := env.BlockStore.LoadBlockByHash(hash)
 	if block == nil {
 		return &ctypes.ResultBlock{BlockID: types.BlockID{}, Block: nil}, nil

rpc/core/tx.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"sort"
 
+	"github.com/tendermint/tendermint/libs/bytes"
 	tmmath "github.com/tendermint/tendermint/libs/math"
 	tmquery "github.com/tendermint/tendermint/libs/pubsub/query"
 	ctypes "github.com/tendermint/tendermint/rpc/core/types"
@@ -17,9 +18,13 @@ import (
 // transaction is in the mempool, invalidated, or was not sent in the first
 // place.
 // More: https://docs.tendermint.com/master/rpc/#/Info/tx
-func (env *Environment) Tx(ctx *rpctypes.Context, hash []byte, prove bool) (*ctypes.ResultTx, error) {
+func (env *Environment) Tx(ctx *rpctypes.Context, hash bytes.HexBytes, prove bool) (*ctypes.ResultTx, error) {
 	// if index is disabled, return error
 
+	// N.B. The hash parameter is HexBytes so that the reflective parameter
+	// decoding logic in the HTTP service will correctly translate from JSON.
+	// See https://github.com/tendermint/tendermint/issues/6802 for context.
+
 	if !indexer.KVSinkEnabled(env.EventSinks) {
 		return nil, errors.New("transaction querying is disabled due to no kvEventSink")
 	}