Change tx.Handler interface to remove code duplication

[amaury1093]

Summary

Make the middleware's tx.Handler interface as simple as possible.

Problem

The current tx.Handler interface has 3 methods, and because of their signatures, it creates code duplication.

Current interface:

cosmos-sdk/types/tx/middleware.go

Lines 24 to 30 in 4e718a8

	// TxHandler defines the baseapp's CheckTx, DeliverTx and Simulate respective
	// handlers. It is designed as a middleware stack.
	type Handler interface {
	CheckTx(ctx context.Context, tx sdk.Tx, req abci.RequestCheckTx) (abci.ResponseCheckTx, error)
	DeliverTx(ctx context.Context, tx sdk.Tx, req abci.RequestDeliverTx) (abci.ResponseDeliverTx, error)
	SimulateTx(ctx context.Context, tx sdk.Tx, req RequestSimulateTx) (ResponseSimulateTx, error)
	}

Example of duplication here and #10208 (comment).

Solution

Proposal:

We propose a common tx.Response type for the 3 methods

package tx // types/tx

type Handler interface {
  func CheckTx(ctx context.Content, tx sdk.Tx, checkReq tx.CheckRequest) (res tx.Response, checkRes tx.CheckResponse, error error)
  func DeliverTx(ctx context.Content, tx sdk.Tx) (res tx.Response, error error)
  func SimulateTx(ctx context.Content, tx sdk.Tx) (res tx.Response, error error)
}

type Response struct {
  GasInfo sdk.GasInfo
  MsgResponses  codectypes.Any[] // Represents each Msg service handler's response type. Will get proto-serialized into the `Data` field in ABCI, see note #2
  Log string
  Events sdk.Events
}

type CheckRequest {
  Type CheckTxType // Check or Recheck
}

type CheckResponse {
  Priority uint32
}

Notes

1. Alternative solution is to merge the tx types in ABCI Can we merge CheckTx and DeliverTx response types tendermint/spec#342
2. For rationale to use Any, see [types.TxMsgData, types.MsgData]: consider using google.Protobuf.Any instead of sdk.MsgData #10496

[aaronc]

This seems reasonable.

We could even go a step further and lift GasInfo and Events into Response.

type Response struct {
  GasWanted uint64
  GasUsed uint64
  MsgResponses  codectypes.Any[] // Represents each Msg service handler's response type. Will get proto-serialized into the `Data` field in ABCI, see note #2
  Log string
  Events []Event
}

Events still depends on tendermint types, and also there has been some recent discussion about bypassing tendermint events altogether for efficiency. But maybe that's a separate conversation.

[atheeshp]

@aaronc we need TxBytes in few middleware handlers (ref), @AmauryM and I are aligning on a solution to add tx.Request like below

type tx.Request struct {
    Tx sdk.Tx 
    TxBytes []byte
}

Hence the tx.Handler interface look like

type Handler interface {
  func CheckTx(ctx context.Content, req tx.Request, checkReq tx.CheckRequest) (res tx.Response, checkRes tx.CheckResponse, error error)
  func DeliverTx(ctx context.Content, req tx.Request) (res tx.Response, error error)
  func SimulateTx(ctx context.Content, req tx.Request) (res tx.Response, error error)
}

wdyt?

[aaronc]

@aaronc we need TxBytes in few middleware handlers (ref), @AmauryM and I are aligning on a solution to add tx.Request like below

type tx.Request struct {
    Tx sdk.Tx 
    TxBytes []byte
}

Hence the tx.Handler interface look like

type Handler interface {
  func CheckTx(ctx context.Content, req tx.Request, checkReq tx.CheckRequest) (res tx.Response, checkRes tx.CheckResponse, error error)
  func DeliverTx(ctx context.Content, req tx.Request) (res tx.Response, error error)
  func SimulateTx(ctx context.Content, req tx.Request) (res tx.Response, error error)
}

wdyt?

This sounds like the right plan @atheeshp.

I would also like to pull TxDecoder out of BaseApp and into the Middleware so this solution will allow that. Basically BaseApp will call the root Handler with only TxBytes and Tx set to nil and the first middleware will do the decoding. There is actually a use case for having one middleware before tx decoding that we can handle with this approach.

[amaury1093]

Makes sense. The refactor PR is getting big, I would propose to the the TxDecoderMiddleware in a separate PR.

#10610