From aa3a64d2895895a07c535bd4863eb082c0b6c6ae Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Fri, 13 Jul 2018 10:53:12 -0700 Subject: [PATCH] Test recover in ante --- baseapp/baseapp.go | 2 +- examples/basecoin/app/app.go | 1 - x/auth/ante.go | 35 ++++++++++++++++++----------------- x/auth/ante_test.go | 23 +++++++++++------------ 4 files changed, 30 insertions(+), 31 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index b349168213..78a14414c9 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -105,7 +105,7 @@ func NewBaseApp(name string, cdc *wire.Codec, logger log.Logger, db dbm.DB, opti } // Create and name new BaseApp -// Does not set cdc and instead takes a user-defined txDecoder. If nil, BaseApp uses defaultTxDecoder +// Does not set cdc and instead takes a user-defined txDecoder. // TODO: Rename to NewBaseApp and remove above constructor once auth, wire dependencies removed func NewBaseAppNoCodec(name string, logger log.Logger, db dbm.DB, txDecoder sdk.TxDecoder, options ...func(*BaseApp)) *BaseApp { app := &BaseApp{ diff --git a/examples/basecoin/app/app.go b/examples/basecoin/app/app.go index 349952e70a..a43fea0838 100644 --- a/examples/basecoin/app/app.go +++ b/examples/basecoin/app/app.go @@ -78,7 +78,6 @@ func NewBasecoinApp(logger log.Logger, db dbm.DB, baseAppOptions ...func(*bam.Ba app.SetBeginBlocker(app.BeginBlocker) app.SetEndBlocker(app.EndBlocker) app.SetAnteHandler(auth.NewAnteHandler(app.accountMapper, app.feeCollectionKeeper)) - app.SetTxDecoder(auth.DefaultTxDecoder(cdc)) // mount the multistore and load the latest state app.MountStoresIAVL(app.keyMain, app.keyAccount, app.keyIBC) diff --git a/x/auth/ante.go b/x/auth/ante.go index 73c3df6628..9f14193962 100644 --- a/x/auth/ante.go +++ b/x/auth/ante.go @@ -21,7 +21,7 @@ func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler { return func( ctx sdk.Context, tx sdk.Tx, - ) (_ sdk.Context, res sdk.Result, abort bool) { + ) (newCtx sdk.Context, res sdk.Result, abort bool) { // This AnteHandler requires Txs to be StdTxs stdTx, ok := tx.(StdTx) @@ -29,6 +29,9 @@ func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler { return ctx, sdk.ErrInternal("tx must be StdTx").Result(), true } + // set the gas meter + newCtx = ctx.WithGasMeter(sdk.NewGasMeter(stdTx.Fee.Gas)) + // AnteHandlers must have their own defer/recover in order // for the BaseApp to know how much gas was used! // This is because the GasMeter is created in the AnteHandler, @@ -39,8 +42,9 @@ func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler { case sdk.ErrorOutOfGas: log := fmt.Sprintf("out of gas in location: %v", rType.Descriptor) res = sdk.ErrOutOfGas(log).Result() - res.GasWanted = stdTx.Fee.Gas - res.GasUsed = ctx.GasMeter().GasConsumed() + res.GasWanted = stdTx.Fee.Gas + res.GasUsed = newCtx.GasMeter().GasConsumed() + abort = true default: panic(r) } @@ -49,18 +53,15 @@ func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler { err := validateBasic(stdTx) if err != nil { - return ctx, err.Result(), true + return newCtx, err.Result(), true } sigs := stdTx.GetSignatures() signerAddrs := stdTx.GetSigners() msgs := tx.GetMsgs() - // set the gas meter - ctx = ctx.WithGasMeter(sdk.NewGasMeter(stdTx.Fee.Gas)) - // charge gas for the memo - ctx.GasMeter().ConsumeGas(memoCostPerByte*sdk.Gas(len(stdTx.GetMemo())), "memo") + newCtx.GasMeter().ConsumeGas(memoCostPerByte*sdk.Gas(len(stdTx.GetMemo())), "memo") // Get the sign bytes (requires all account & sequence numbers and the fee) sequences := make([]int64, len(sigs)) @@ -77,38 +78,38 @@ func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler { signerAddr, sig := signerAddrs[i], sigs[i] // check signature, return account with incremented nonce - signBytes := StdSignBytes(ctx.ChainID(), accNums[i], sequences[i], fee, msgs, stdTx.GetMemo()) + signBytes := StdSignBytes(newCtx.ChainID(), accNums[i], sequences[i], fee, msgs, stdTx.GetMemo()) signerAcc, res := processSig( - ctx, am, + newCtx, am, signerAddr, sig, signBytes, ) if !res.IsOK() { - return ctx, res, true + return newCtx, res, true } // first sig pays the fees // TODO: Add min fees // Can this function be moved outside of the loop? if i == 0 && !fee.Amount.IsZero() { - ctx.GasMeter().ConsumeGas(deductFeesCost, "deductFees") + newCtx.GasMeter().ConsumeGas(deductFeesCost, "deductFees") signerAcc, res = deductFees(signerAcc, fee) if !res.IsOK() { - return ctx, res, true + return newCtx, res, true } - fck.addCollectedFees(ctx, fee.Amount) + fck.addCollectedFees(newCtx, fee.Amount) } // Save the account. - am.SetAccount(ctx, signerAcc) + am.SetAccount(newCtx, signerAcc) signerAccs[i] = signerAcc } // cache the signer accounts in the context - ctx = WithSigners(ctx, signerAccs) + newCtx = WithSigners(newCtx, signerAccs) // TODO: tx tags (?) - return ctx, sdk.Result{GasWanted: stdTx.Fee.Gas}, false // continue... + return newCtx, sdk.Result{GasWanted: stdTx.Fee.Gas}, false // continue... } } diff --git a/x/auth/ante_test.go b/x/auth/ante_test.go index c30013d32f..907de17492 100644 --- a/x/auth/ante_test.go +++ b/x/auth/ante_test.go @@ -47,21 +47,20 @@ func checkValidTx(t *testing.T, anteHandler sdk.AnteHandler, ctx sdk.Context, tx // run the tx through the anteHandler and ensure it fails with the given code func checkInvalidTx(t *testing.T, anteHandler sdk.AnteHandler, ctx sdk.Context, tx sdk.Tx, code sdk.CodeType) { - defer func() { - if r := recover(); r != nil { - switch r.(type) { - case sdk.ErrorOutOfGas: - require.Equal(t, sdk.ToABCICode(sdk.CodespaceRoot, code), sdk.ToABCICode(sdk.CodespaceRoot, sdk.CodeOutOfGas), - fmt.Sprintf("Expected ErrorOutOfGas, got %v", r)) - default: - panic(r) - } - } - }() - _, result, abort := anteHandler(ctx, tx) + newCtx, result, abort := anteHandler(ctx, tx) require.True(t, abort) require.Equal(t, sdk.ToABCICode(sdk.CodespaceRoot, code), result.Code, fmt.Sprintf("Expected %v, got %v", sdk.ToABCICode(sdk.CodespaceRoot, code), result)) + + if code == sdk.CodeOutOfGas { + stdTx, ok := tx.(StdTx) + require.True(t, ok, "tx must be in form auth.StdTx") + // GasWanted set correctly + require.Equal(t, stdTx.Fee.Gas, result.GasWanted, "Gas wanted not set correctly") + require.True(t, result.GasUsed > result.GasWanted, "GasUsed not greated than GasWanted") + // Check that context is set correctly + require.Equal(t, result.GasUsed, newCtx.GasMeter().GasConsumed(), "Context not updated correctly") + } } func newTestTx(ctx sdk.Context, msgs []sdk.Msg, privs []crypto.PrivKey, accNums []int64, seqs []int64, fee StdFee) sdk.Tx {