cosmos / cosmos-sdk

:chains: A Framework for Building High Value Public Blockchains :sparkles:
https://cosmos.network/
Apache License 2.0
6.18k stars 3.58k forks source link

Move away from Tx interfaces (sdk.Tx, sdk.FeeTx...) #10347

Open amaury1093 opened 2 years ago

amaury1093 commented 2 years ago

Problem

We used to introduce these interfaces to support both proto Txs (*tx.Tx) and amino (StdTx...)

In the middlewares, we are currently hardcoding the sdk.Tx interface, which introduces a lot of type casting in the implementation.

Ideas

Deprecate:

https://github.com/cosmos/cosmos-sdk/blob/994219a4d2f467a0bd628d60d8670c1102edbe88/types/tx_msg.go#L38-L69

and slowly remove its usage from code.

Proposal

Step 1: Change the middleware interface to use concrete Tx.

We need to do the following changes:

  1. Make the wrapper public. Probably find a better name, but we can use Wrapper for now.

  2. Change the tx.Handler interface

- CheckTx(ctx context.Context, tx sdk.Tx, req abci.RequestCheckTx) (abci.ResponseCheckTx, error)
+ CheckTx(ctx context.Context, tx tx.Wrapper, req abci.RequestCheckTx) (abci.ResponseCheckTx, error)

Make sure there's no dep cycles.

  1. In the middleware implementations, remove all type casting.

Notes

The last place where amino StdTx were used by default was in legacy API, which was removed in #9594 .

amaury1093 commented 2 years ago

Let's wait for this. It's too big a refactor, and not worth it for 0.45. ref: https://github.com/cosmos/cosmos-sdk/pull/10480#issuecomment-963205811