From b3bb21d0c4747d915e26f28dd41b4971b01be7ab Mon Sep 17 00:00:00 2001 From: HuangYi Date: Tue, 14 Dec 2021 18:01:35 +0800 Subject: [PATCH] revert tx when block gas limit exceeded Closes: #10769 Solution: - create `PersistTxMiddleware` - tx state change is reverted if any error happens Update x/auth/middleware/recovery.go Co-authored-by: Aleksandr Bezobchuk snapshot middleware SimulateTx --- x/auth/middleware/middleware.go | 2 + x/auth/middleware/run_msgs.go | 31 +------------- x/auth/middleware/snapshot.go | 76 +++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 30 deletions(-) create mode 100644 x/auth/middleware/snapshot.go diff --git a/x/auth/middleware/middleware.go b/x/auth/middleware/middleware.go index 53d16938e21..ef520305950 100644 --- a/x/auth/middleware/middleware.go +++ b/x/auth/middleware/middleware.go @@ -83,6 +83,8 @@ func NewDefaultTxHandler(options TxHandlerOptions) (tx.Handler, error) { // that reads the GasMeter. In our case, the Recovery middleware reads // the GasMeter to populate GasInfo. GasTxMiddleware, + // Branch out MultiStore in DeliverTx and commit if no error returned. + PersistTxMiddleware, // Recover from panics. Panics outside of this middleware won't be // caught, be careful! RecoveryTxMiddleware, diff --git a/x/auth/middleware/run_msgs.go b/x/auth/middleware/run_msgs.go index c90d222a907..d333e591899 100644 --- a/x/auth/middleware/run_msgs.go +++ b/x/auth/middleware/run_msgs.go @@ -2,11 +2,8 @@ package middleware import ( "context" - "fmt" "strings" - "github.com/tendermint/tendermint/crypto/tmhash" - codectypes "github.com/cosmos/cosmos-sdk/codec/types" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" @@ -50,11 +47,6 @@ func (txh runMsgsTxHandler) SimulateTx(ctx context.Context, req tx.Request) (tx. // Handler does not exist for a given message route. Otherwise, a reference to a // Result is returned. The caller must not commit state if an error is returned. func (txh runMsgsTxHandler) runMsgs(sdkCtx sdk.Context, msgs []sdk.Msg, txBytes []byte) (tx.Response, error) { - // Create a new Context based off of the existing Context with a MultiStore branch - // in case message processing fails. At this point, the MultiStore - // is a branch of a branch. - runMsgCtx, msCache := cacheTxContext(sdkCtx, txBytes) - // Attempt to execute all messages and only update state if all messages pass // and we're in DeliverTx. Note, runMsgs will never return a reference to a // Result if any single message fails or does not have a registered Handler. @@ -72,7 +64,7 @@ func (txh runMsgsTxHandler) runMsgs(sdkCtx sdk.Context, msgs []sdk.Msg, txBytes if handler := txh.msgServiceRouter.Handler(msg); handler != nil { // ADR 031 request type routing - msgResult, err = handler(runMsgCtx, msg) + msgResult, err = handler(sdkCtx, msg) eventMsgName = sdk.MsgTypeURL(msg) } else if legacyMsg, ok := msg.(legacytx.LegacyMsg); ok { // legacy sdk.Msg routing @@ -116,8 +108,6 @@ func (txh runMsgsTxHandler) runMsgs(sdkCtx sdk.Context, msgs []sdk.Msg, txBytes msgLogs = append(msgLogs, sdk.NewABCIMessageLog(uint32(i), msgResult.Log, msgEvents)) } - msCache.Write() - return tx.Response{ // GasInfo will be populated by the Gas middleware. Log: strings.TrimSpace(msgLogs.String()), @@ -125,22 +115,3 @@ func (txh runMsgsTxHandler) runMsgs(sdkCtx sdk.Context, msgs []sdk.Msg, txBytes MsgResponses: msgResponses, }, nil } - -// cacheTxContext returns a new context based off of the provided context with -// a branched multi-store. -func cacheTxContext(sdkCtx sdk.Context, txBytes []byte) (sdk.Context, sdk.CacheMultiStore) { - ms := sdkCtx.MultiStore() - // TODO: https://github.com/cosmos/cosmos-sdk/issues/2824 - msCache := ms.CacheMultiStore() - if msCache.TracingEnabled() { - msCache = msCache.SetTracingContext( - sdk.TraceContext( - map[string]interface{}{ - "txHash": fmt.Sprintf("%X", tmhash.Sum(txBytes)), - }, - ), - ).(sdk.CacheMultiStore) - } - - return sdkCtx.WithMultiStore(msCache), msCache -} diff --git a/x/auth/middleware/snapshot.go b/x/auth/middleware/snapshot.go new file mode 100644 index 00000000000..d6ab690b8ab --- /dev/null +++ b/x/auth/middleware/snapshot.go @@ -0,0 +1,76 @@ +package middleware + +import ( + "context" + "fmt" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/tx" + "github.com/tendermint/tendermint/crypto/tmhash" +) + +type persistHandler struct { + next tx.Handler +} + +// PersistTxMiddleware branch out MultiStore in DeliverTx and commit if no error returned. +func PersistTxMiddleware(txh tx.Handler) tx.Handler { + return persistHandler{next: txh} +} + +// CheckTx implements tx.Handler.CheckTx method. +func (sh persistHandler) CheckTx(ctx context.Context, req tx.Request, checkReq tx.RequestCheckTx) (tx.Response, tx.ResponseCheckTx, error) { + // Do nothing during CheckTx. + return sh.next.CheckTx(ctx, req, checkReq) +} + +// DeliverTx implements tx.Handler.DeliverTx method. +func (sh persistHandler) DeliverTx(ctx context.Context, req tx.Request) (tx.Response, error) { + // Create a new Context based off of the existing Context with a MultiStore branch + // in case message processing fails. At this point, the MultiStore + // is a branch of a branch. + sdkCtx := sdk.UnwrapSDKContext(ctx) + runMsgCtx, msCache := cacheTxContext(sdkCtx, req.TxBytes) + + rsp, err := sh.next.DeliverTx(sdk.WrapSDKContext(runMsgCtx), req) + if err != nil { + msCache.Write() + } + + return rsp, err +} + +// SimulateTx implements tx.Handler.SimulateTx method. +func (sh persistHandler) SimulateTx(ctx context.Context, req tx.Request) (tx.Response, error) { + // Create a new Context based off of the existing Context with a MultiStore branch + // in case message processing fails. At this point, the MultiStore + // is a branch of a branch. + sdkCtx := sdk.UnwrapSDKContext(ctx) + runMsgCtx, msCache := cacheTxContext(sdkCtx, req.TxBytes) + + rsp, err := sh.next.SimulateTx(sdk.WrapSDKContext(runMsgCtx), req) + if err != nil { + msCache.Write() + } + + return rsp, err +} + +// cacheTxContext returns a new context based off of the provided context with +// a branched multi-store. +func cacheTxContext(sdkCtx sdk.Context, txBytes []byte) (sdk.Context, sdk.CacheMultiStore) { + ms := sdkCtx.MultiStore() + // TODO: https://github.com/cosmos/cosmos-sdk/issues/2824 + msCache := ms.CacheMultiStore() + if msCache.TracingEnabled() { + msCache = msCache.SetTracingContext( + sdk.TraceContext( + map[string]interface{}{ + "txHash": fmt.Sprintf("%X", tmhash.Sum(txBytes)), + }, + ), + ).(sdk.CacheMultiStore) + } + + return sdkCtx.WithMultiStore(msCache), msCache +}