rpc: deprecate/updates to broadcast tx (#8624)

CHANGELOG_PENDING.md

@@ -22,6 +22,7 @@ Special thanks to external contributors on this release:
   - [cli] \#8081 make the reset command safe to use by intoducing `reset-state` command. Fixed by \#8259. (@marbar3778, @cmwaters)
   - [config] \#8222 default indexer configuration to null. (@creachadair)
   - [rpc] \#8570 rework timeouts to be per-method instead of global. (@creachadair)
+  - [rpc] \#8624 deprecate `broadcast_tx_commit` and `braodcast_tx_sync` and `broadcast_tx_async` in favor of `braodcast_tx`. (@tychoish)
 
 - Apps
 

UPGRADING.md

@@ -96,6 +96,18 @@ callback.
 For more detailed information, see [ADR 075](https://tinyurl.com/adr075) which
 defines and describes the new API in detail.
 
+#### BroadcastTx Methods
+
+All callers should use the new `broadcast_tx` method, which has the
+same semantics as the legacy `broadcast_tx_sync` method. The
+`broadcast_tx_sync` and `broadcast_tx_async` methods are now
+deprecated and will be removed in 0.37.
+
+Additionally the `broadcast_tx_commit` method is *also* deprecated,
+and will be removed in 0.37. Client code that submits a transaction
+and needs to wait for it to be committed to the chain, should poll
+the tendermint to observe the transaction in the committed state.
+
 ### Timeout Parameter Changes
 
 Tendermint v0.36 updates how the Tendermint consensus timing parameters are

internal/rpc/core/mempool.go

@@ -19,20 +19,24 @@ import (
 
 // BroadcastTxAsync returns right away, with no response. Does not wait for
 // CheckTx nor DeliverTx results.
-// More: https://docs.tendermint.com/master/rpc/#/Tx/broadcast_tx_async
+// More:
+// https://docs.tendermint.com/master/rpc/#/Tx/broadcast_tx_async
+// Deprecated and should be removed in 0.37
 func (env *Environment) BroadcastTxAsync(ctx context.Context, req *coretypes.RequestBroadcastTx) (*coretypes.ResultBroadcastTx, error) {
-	err := env.Mempool.CheckTx(ctx, req.Tx, nil, mempool.TxInfo{})
-	if err != nil {
-		return nil, err
-	}
+	go func() { _ = env.Mempool.CheckTx(ctx, req.Tx, nil, mempool.TxInfo{}) }()
 
 	return &coretypes.ResultBroadcastTx{Hash: req.Tx.Hash()}, nil
 }
 
-// BroadcastTxSync returns with the response from CheckTx. Does not wait for
+// Deprecated and should be remove in 0.37
+func (env *Environment) BroadcastTxSync(ctx context.Context, req *coretypes.RequestBroadcastTx) (*coretypes.ResultBroadcastTx, error) {
+	return env.BroadcastTx(ctx, req)
+}
+
+// BroadcastTx returns with the response from CheckTx. Does not wait for
 // DeliverTx result.
 // More: https://docs.tendermint.com/master/rpc/#/Tx/broadcast_tx_sync
-func (env *Environment) BroadcastTxSync(ctx context.Context, req *coretypes.RequestBroadcastTx) (*coretypes.ResultBroadcastTx, error) {
+func (env *Environment) BroadcastTx(ctx context.Context, req *coretypes.RequestBroadcastTx) (*coretypes.ResultBroadcastTx, error) {
 	resCh := make(chan *abci.ResponseCheckTx, 1)
 	err := env.Mempool.CheckTx(
 		ctx,

internal/rpc/core/routes.go

@@ -59,8 +59,11 @@ func NewRoutesMap(svc RPCService, opts *RouteOptions) RoutesMap {
 		"num_unconfirmed_txs":  rpc.NewRPCFunc(svc.NumUnconfirmedTxs),
 
 		// tx broadcast API
+		"broadcast_tx": rpc.NewRPCFunc(svc.BroadcastTx),
+		// TODO remove after 0.36
+		// deprecated broadcast tx methods:
 		"broadcast_tx_commit": rpc.NewRPCFunc(svc.BroadcastTxCommit),
-		"broadcast_tx_sync":   rpc.NewRPCFunc(svc.BroadcastTxSync),
+		"broadcast_tx_sync":   rpc.NewRPCFunc(svc.BroadcastTx),
 		"broadcast_tx_async":  rpc.NewRPCFunc(svc.BroadcastTxAsync),
 
 		// abci API
@@ -87,6 +90,7 @@ type RPCService interface {
 	BlockSearch(ctx context.Context, req *coretypes.RequestBlockSearch) (*coretypes.ResultBlockSearch, error)
 	BlockchainInfo(ctx context.Context, req *coretypes.RequestBlockchainInfo) (*coretypes.ResultBlockchainInfo, error)
 	BroadcastEvidence(ctx context.Context, req *coretypes.RequestBroadcastEvidence) (*coretypes.ResultBroadcastEvidence, error)
+	BroadcastTx(ctx context.Context, req *coretypes.RequestBroadcastTx) (*coretypes.ResultBroadcastTx, error)
 	BroadcastTxAsync(ctx context.Context, req *coretypes.RequestBroadcastTx) (*coretypes.ResultBroadcastTx, error)
 	BroadcastTxCommit(ctx context.Context, req *coretypes.RequestBroadcastTx) (*coretypes.ResultBroadcastTxCommit, error)
 	BroadcastTxSync(ctx context.Context, req *coretypes.RequestBroadcastTx) (*coretypes.ResultBroadcastTx, error)

light/proxy/routes.go

@@ -52,6 +52,10 @@ func (p proxyService) BroadcastTxAsync(ctx context.Context, req *coretypes.Reque
 	return p.Client.BroadcastTxAsync(ctx, req.Tx)
 }
 
+func (p proxyService) BroadcastTx(ctx context.Context, req *coretypes.RequestBroadcastTx) (*coretypes.ResultBroadcastTx, error) {
+	return p.Client.BroadcastTx(ctx, req.Tx)
+}
+
 func (p proxyService) BroadcastTxCommit(ctx context.Context, req *coretypes.RequestBroadcastTx) (*coretypes.ResultBroadcastTxCommit, error) {
 	return p.Client.BroadcastTxCommit(ctx, req.Tx)
 }

light/rpc/client.go

@@ -229,6 +229,10 @@ func (c *Client) BroadcastTxSync(ctx context.Context, tx types.Tx) (*coretypes.R
 	return c.next.BroadcastTxSync(ctx, tx)
 }
 
+func (c *Client) BroadcastTx(ctx context.Context, tx types.Tx) (*coretypes.ResultBroadcastTx, error) {
+	return c.next.BroadcastTx(ctx, tx)
+}
+
 func (c *Client) UnconfirmedTxs(ctx context.Context, page, perPage *int) (*coretypes.ResultUnconfirmedTxs, error) {
 	return c.next.UnconfirmedTxs(ctx, page, perPage)
 }

rpc/client/http/http.go

@@ -242,6 +242,10 @@ func (c *baseRPCClient) BroadcastTxSync(ctx context.Context, tx types.Tx) (*core
 	return c.broadcastTX(ctx, "broadcast_tx_sync", tx)
 }
 
+func (c *baseRPCClient) BroadcastTx(ctx context.Context, tx types.Tx) (*coretypes.ResultBroadcastTx, error) {
+	return c.broadcastTX(ctx, "broadcast_tx_sync", tx)
+}
+
 func (c *baseRPCClient) broadcastTX(ctx context.Context, route string, tx types.Tx) (*coretypes.ResultBroadcastTx, error) {
 	result := new(coretypes.ResultBroadcastTx)
 	if err := c.caller.Call(ctx, route, &coretypes.RequestBroadcastTx{Tx: tx}, result); err != nil {

rpc/client/interface.go

@@ -62,6 +62,8 @@ type ABCIClient interface {
 		opts ABCIQueryOptions) (*coretypes.ResultABCIQuery, error)
 
 	// Writing to abci app
+	BroadcastTx(context.Context, types.Tx) (*coretypes.ResultBroadcastTx, error)
+	// These methods are deprecated:
 	BroadcastTxCommit(context.Context, types.Tx) (*coretypes.ResultBroadcastTxCommit, error)
 	BroadcastTxAsync(context.Context, types.Tx) (*coretypes.ResultBroadcastTx, error)
 	BroadcastTxSync(context.Context, types.Tx) (*coretypes.ResultBroadcastTx, error)

rpc/client/local/local.go

@@ -87,6 +87,10 @@ func (c *Local) BroadcastTxCommit(ctx context.Context, tx types.Tx) (*coretypes.
 	return c.env.BroadcastTxCommit(ctx, &coretypes.RequestBroadcastTx{Tx: tx})
 }
 
+func (c *Local) BroadcastTx(ctx context.Context, tx types.Tx) (*coretypes.ResultBroadcastTx, error) {
+	return c.env.BroadcastTx(ctx, &coretypes.RequestBroadcastTx{Tx: tx})
+}
+
 func (c *Local) BroadcastTxAsync(ctx context.Context, tx types.Tx) (*coretypes.ResultBroadcastTx, error) {
 	return c.env.BroadcastTxAsync(ctx, &coretypes.RequestBroadcastTx{Tx: tx})
 }

rpc/client/mock/abci.go

@@ -94,6 +94,10 @@ func (a ABCIApp) BroadcastTxAsync(ctx context.Context, tx types.Tx) (*coretypes.
 }
 
 func (a ABCIApp) BroadcastTxSync(ctx context.Context, tx types.Tx) (*coretypes.ResultBroadcastTx, error) {
+	return a.BroadcastTx(ctx, tx)
+}
+
+func (a ABCIApp) BroadcastTx(ctx context.Context, tx types.Tx) (*coretypes.ResultBroadcastTx, error) {
 	c, err := a.App.CheckTx(ctx, &abci.RequestCheckTx{Tx: tx})
 	if err != nil {
 		return nil, err
@@ -158,6 +162,14 @@ func (m ABCIMock) BroadcastTxAsync(ctx context.Context, tx types.Tx) (*coretypes
 	return res.(*coretypes.ResultBroadcastTx), nil
 }
 
+func (m ABCIMock) BroadcastTx(ctx context.Context, tx types.Tx) (*coretypes.ResultBroadcastTx, error) {
+	res, err := m.Broadcast.GetResponse(tx)
+	if err != nil {
+		return nil, err
+	}
+	return res.(*coretypes.ResultBroadcastTx), nil
+}
+
 func (m ABCIMock) BroadcastTxSync(ctx context.Context, tx types.Tx) (*coretypes.ResultBroadcastTx, error) {
 	res, err := m.Broadcast.GetResponse(tx)
 	if err != nil {
@@ -252,3 +264,14 @@ func (r *ABCIRecorder) BroadcastTxSync(ctx context.Context, tx types.Tx) (*coret
 	})
 	return res, err
 }
+
+func (r *ABCIRecorder) BroadcastTx(ctx context.Context, tx types.Tx) (*coretypes.ResultBroadcastTx, error) {
+	res, err := r.Client.BroadcastTx(ctx, tx)
+	r.addCall(Call{
+		Name:     "broadcast_tx",
+		Args:     tx,
+		Response: res,
+		Error:    err,
+	})
+	return res, err
+}

rpc/client/mocks/client.go

@@ -227,6 +227,29 @@ func (_m *Client) BroadcastEvidence(_a0 context.Context, _a1 types.Evidence) (*c
 	return r0, r1
 }
 
+// BroadcastTx provides a mock function with given fields: _a0, _a1
+func (_m *Client) BroadcastTx(_a0 context.Context, _a1 types.Tx) (*coretypes.ResultBroadcastTx, error) {
+	ret := _m.Called(_a0, _a1)
+
+	var r0 *coretypes.ResultBroadcastTx
+	if rf, ok := ret.Get(0).(func(context.Context, types.Tx) *coretypes.ResultBroadcastTx); ok {
+		r0 = rf(_a0, _a1)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(*coretypes.ResultBroadcastTx)
+		}
+	}
+
+	var r1 error
+	if rf, ok := ret.Get(1).(func(context.Context, types.Tx) error); ok {
+		r1 = rf(_a0, _a1)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}
+
 // BroadcastTxAsync provides a mock function with given fields: _a0, _a1
 func (_m *Client) BroadcastTxAsync(_a0 context.Context, _a1 types.Tx) (*coretypes.ResultBroadcastTx, error) {
 	ret := _m.Called(_a0, _a1)

rpc/openapi/openapi.yaml

@@ -82,9 +82,50 @@ paths:
   /broadcast_tx_sync:
     get:
       summary: Returns with the response from CheckTx. Does not wait for DeliverTx result.
+      deprecated: true
       tags:
         - Tx
       operationId: broadcast_tx_sync
+      description: |
+        This method is deprecated in Tendermint v0.36, and will be
+        removed in v0.37. Use `broadcast_tx`, which has similar
+        semantics.
+
+        This method blocks until CheckTx returns and reports its result, but
+        does not wait for the transaction to be included in a block. To know
+        when the transaction is included in a block, check for the transaction
+        event using JSON-RPC.  See
+        https://docs.tendermint.com/master/app-dev/subscribing-to-events-via-websocket.html
+
+        See https://docs.tendermint.com/master/tendermint-core/using-tendermint.html#formatting
+        for formatting/encoding rules.
+      parameters:
+        - in: query
+          name: tx
+          required: true
+          schema:
+            type: string
+          example: "456"
+          description: The transaction
+      responses:
+        "200":
+          description: Empty
+          content:
+            application/json:
+              schema:
+                $ref: "#/components/schemas/BroadcastTxResponse"
+        "500":
+          description: Error
+          content:
+            application/json:
+              schema:
+                $ref: "#/components/schemas/ErrorResponse"
+  /broadcast_tx:
+    get:
+      summary: Returns with the response from CheckTx. Does not wait for DeliverTx result.
+      tags:
+        - Tx
+      operationId: broadcast_tx
       description: |
         This method blocks until CheckTx returns and reports its result, but
         does not wait for the transaction to be included in a block. To know
@@ -118,10 +159,14 @@ paths:
   /broadcast_tx_async:
     get:
       summary: Returns right away, with no response. Does not wait for CheckTx nor DeliverTx results.
+      deprecated: true
       tags:
         - Tx
       operationId: broadcast_tx_async
       description: |
+        This method is deprecated in Tendermint v0.36, and will be
+        removed in v0.37. Use `broadcast_tx`.
+
         This method submits the transaction and returns immediately without
         waiting for the transaction to be checked (CheckTx) or committed.  Too
         know when the transaction is included in a block, you can check for the
@@ -154,6 +199,7 @@ paths:
   /broadcast_tx_commit:
     get:
       summary: Returns with the responses from CheckTx and DeliverTx.
+      deprecated: true
       tags:
         - Tx
       operationId: broadcast_tx_commit
@@ -165,7 +211,7 @@ paths:
         succeed and report the failing (non-zero) ABCI result code.
 
         WARNING: Use this only for testing and development. For production use,
-        call broadcast_tx_sync or broadcast_tx_async.
+        call broadcast_tx.
 
         To know when a transaction is included in a block, check for the
         transaction event using JSON-RPC.  See