From 420faefd51d17cd598e9f895c6bbf762127955bc Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 14 Oct 2019 22:52:54 -0700 Subject: [PATCH 01/16] Introduce Recheck optimizations --- baseapp/abci.go | 4 +++- baseapp/baseapp.go | 9 +++++++-- types/context.go | 7 +++++++ x/auth/ante/basic.go | 4 ++++ x/auth/ante/sigverify.go | 8 ++++++++ 5 files changed, 29 insertions(+), 3 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index e843b99c756b..0e6ef5183db6 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -164,8 +164,10 @@ func (app *BaseApp) CheckTx(req abci.RequestCheckTx) (res abci.ResponseCheckTx) tx, err := app.txDecoder(req.Tx) if err != nil { result = err.Result() - } else { + } else if req.Type == abci.CheckTxType_New { result = app.runTx(runTxModeCheck, req.Tx, tx) + } else { + result = app.runTx(runTxModeReCheck, req.Tx, tx) } return abci.ResponseCheckTx{ diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 4fc78546bf40..91c43f6b6353 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -24,6 +24,8 @@ import ( const ( // Check a transaction runTxModeCheck runTxMode = iota + // Recheck a transaction + runTxModeReCheck runTxMode = iota // Simulate a transaction runTxModeSimulate runTxMode = iota // Deliver a transaction @@ -481,6 +483,9 @@ func (app *BaseApp) getContextForTx(mode runTxMode, txBytes []byte) (ctx sdk.Con WithVoteInfos(app.voteInfos). WithConsensusParams(app.consensusParams) + if mode == runTxModeReCheck { + ctx = ctx.WithIsReCheckTx(true) + } if mode == runTxModeSimulate { ctx, _ = ctx.CacheContext() } @@ -653,8 +658,8 @@ func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, mode runTxMode) (re var msgResult sdk.Result - // skip actual execution for CheckTx mode - if mode != runTxModeCheck { + // skip actual execution for CheckTx and ReCheckTx mode + if mode != runTxModeCheck && mode != runTxModeReCheck { msgResult = handler(ctx, msg) } diff --git a/types/context.go b/types/context.go index 9fe8f2b67437..c7c9e552c378 100644 --- a/types/context.go +++ b/types/context.go @@ -31,6 +31,7 @@ type Context struct { gasMeter GasMeter blockGasMeter GasMeter checkTx bool + recheckTx bool minGasPrice DecCoins consParams *abci.ConsensusParams eventManager *EventManager @@ -51,6 +52,7 @@ func (c Context) VoteInfos() []abci.VoteInfo { return c.voteInfo } func (c Context) GasMeter() GasMeter { return c.gasMeter } func (c Context) BlockGasMeter() GasMeter { return c.blockGasMeter } func (c Context) IsCheckTx() bool { return c.checkTx } +func (c Context) IsReCheckTx() bool { return c.recheckTx } func (c Context) MinGasPrices() DecCoins { return c.minGasPrice } func (c Context) EventManager() *EventManager { return c.eventManager } @@ -152,6 +154,11 @@ func (c Context) WithIsCheckTx(isCheckTx bool) Context { return c } +func (c Context) WithIsReCheckTx(isRecheckTx bool) Context { + c.recheckTx = isRecheckTx + return c +} + func (c Context) WithMinGasPrices(gasPrices DecCoins) Context { c.minGasPrice = gasPrices return c diff --git a/x/auth/ante/basic.go b/x/auth/ante/basic.go index b73374a80012..4c595b2b4913 100644 --- a/x/auth/ante/basic.go +++ b/x/auth/ante/basic.go @@ -22,6 +22,10 @@ func NewValidateBasicDecorator() ValidateBasicDecorator { } func (vbd ValidateBasicDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { + // no need to validate basic on recheck tx + if ctx.IsReCheckTx() { + return next(ctx, tx, simulate) + } if err := tx.ValidateBasic(); err != nil { return ctx, err } diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index 4b340907e1e8..5d77e778e554 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -163,6 +163,10 @@ func NewSigVerificationDecorator(ak keeper.AccountKeeper) SigVerificationDecorat } func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { + // no need to verify signatures on recheck tx + if ctx.IsReCheckTx() { + return next(ctx, tx, simulate) + } sigTx, ok := tx.(SigVerifiableTx) if !ok { return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "invalid transaction type") @@ -220,6 +224,10 @@ func NewIncrementSequenceDecorator(ak keeper.AccountKeeper) IncrementSequenceDec } func (isd IncrementSequenceDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { + // no need to increment sequence on check tx + if ctx.IsCheckTx() { + return next(ctx, tx, simulate) + } sigTx, ok := tx.(SigVerifiableTx) if !ok { return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "invalid transaction type") From 0310b36b9159e2e7a74395de7b0278eedb2eeee2 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 15 Oct 2019 12:29:15 -0700 Subject: [PATCH 02/16] fix tests --- x/auth/ante/ante_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/x/auth/ante/ante_test.go b/x/auth/ante/ante_test.go index b3d22061b668..d37a31eee80b 100644 --- a/x/auth/ante/ante_test.go +++ b/x/auth/ante/ante_test.go @@ -86,7 +86,7 @@ func TestAnteHandlerSigErrors(t *testing.T) { // Test logic around account number checking with one signer and many signers. func TestAnteHandlerAccountNumbers(t *testing.T) { // setup - app, ctx := createTestApp(true) + app, ctx := createTestApp(false) ctx = ctx.WithBlockHeight(1) anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, ante.DefaultSigVerificationGasConsumer) @@ -143,7 +143,7 @@ func TestAnteHandlerAccountNumbers(t *testing.T) { // Test logic around account number checking with many signers when BlockHeight is 0. func TestAnteHandlerAccountNumbersAtBlockHeightZero(t *testing.T) { // setup - app, ctx := createTestApp(true) + app, ctx := createTestApp(false) ctx = ctx.WithBlockHeight(0) anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, ante.DefaultSigVerificationGasConsumer) @@ -199,7 +199,7 @@ func TestAnteHandlerAccountNumbersAtBlockHeightZero(t *testing.T) { // Test logic around sequence checking with one signer and many signers. func TestAnteHandlerSequences(t *testing.T) { // setup - app, ctx := createTestApp(true) + app, ctx := createTestApp(false) ctx = ctx.WithBlockHeight(1) anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, ante.DefaultSigVerificationGasConsumer) @@ -355,7 +355,7 @@ func TestAnteHandlerMemoGas(t *testing.T) { func TestAnteHandlerMultiSigner(t *testing.T) { // setup - app, ctx := createTestApp(true) + app, ctx := createTestApp(false) ctx = ctx.WithBlockHeight(1) anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, ante.DefaultSigVerificationGasConsumer) From 17b47bf8a9c80a97a29eb5ba8309c941c7619cb1 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 15 Oct 2019 13:12:36 -0700 Subject: [PATCH 03/16] add unit tests --- x/auth/ante/basic.go | 3 ++- x/auth/ante/basic_test.go | 8 ++++++++ x/auth/ante/sigverify.go | 2 ++ x/auth/ante/sigverify_test.go | 36 ++++++++++++++++++++++++++++++----- 4 files changed, 43 insertions(+), 6 deletions(-) diff --git a/x/auth/ante/basic.go b/x/auth/ante/basic.go index 4c595b2b4913..6c7fcca1ba85 100644 --- a/x/auth/ante/basic.go +++ b/x/auth/ante/basic.go @@ -15,6 +15,7 @@ var ( // ValidateBasicDecorator will call tx.ValidateBasic and return any non-nil error. // If ValidateBasic passes, decorator calls next AnteHandler in chain. +// This decorator will not get run on ReCheck since it is not dependent on application state type ValidateBasicDecorator struct{} func NewValidateBasicDecorator() ValidateBasicDecorator { @@ -22,7 +23,7 @@ func NewValidateBasicDecorator() ValidateBasicDecorator { } func (vbd ValidateBasicDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { - // no need to validate basic on recheck tx + // no need to validate basic on recheck tx, call next antehandler if ctx.IsReCheckTx() { return next(ctx, tx, simulate) } diff --git a/x/auth/ante/basic_test.go b/x/auth/ante/basic_test.go index db9c1a8b31ef..229757123c6c 100644 --- a/x/auth/ante/basic_test.go +++ b/x/auth/ante/basic_test.go @@ -39,6 +39,14 @@ func TestValidateBasic(t *testing.T) { _, err = antehandler(ctx, validTx, false) require.Nil(t, err, "ValidateBasicDecorator returned error on valid tx. err: %v", err) + + // test decorator skips on recheck + ctx = ctx.WithIsReCheckTx(true) + + // decorator should skip processing invalidTx on recheck and thus return nil-error + _, err = antehandler(ctx, invalidTx, false) + + require.Nil(t, err, "ValidateBasicDecorator ran on ReCheck") } func TestValidateMemo(t *testing.T) { diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index 5d77e778e554..6926336a31f7 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -150,6 +150,7 @@ func (sgcd SigGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula // Verify all signatures for tx and return error if any are invalid // increment sequence of each signer and set updated account back in store // Call next AnteHandler +// This decorator will not get run on ReCheck since it is not dependent on application state // CONTRACT: Pubkeys are set in context for all signers before this decorator runs // CONTRACT: Tx must implement SigVerifiableTx interface type SigVerificationDecorator struct { @@ -212,6 +213,7 @@ func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul // Increments sequences of all signers. // Use this decorator to prevent replay attacks +// No need to run this decorator on CheckTx since it is merely updating nonce // CONTRACT: Tx must implement SigVerifiableTx interface type IncrementSequenceDecorator struct { ak keeper.AccountKeeper diff --git a/x/auth/ante/sigverify_test.go b/x/auth/ante/sigverify_test.go index 82b9f8f62443..836d16eccbeb 100644 --- a/x/auth/ante/sigverify_test.go +++ b/x/auth/ante/sigverify_test.go @@ -101,6 +101,8 @@ func TestConsumeSignatureVerificationGas(t *testing.T) { func TestSigVerification(t *testing.T) { // setup app, ctx := createTestApp(true) + // make block height non-zero to ensure account numbers part of signBytes + ctx = ctx.WithBlockHeight(1) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -120,15 +122,39 @@ func TestSigVerification(t *testing.T) { fee := types.NewTestStdFee() - privs, accNums, seqs := []crypto.PrivKey{priv3, priv2, priv1}, []uint64{2, 1, 0}, []uint64{0, 0, 0} - tx := types.NewTestTx(ctx, msgs, privs, accNums, seqs, fee) - spkd := ante.NewSetPubKeyDecorator(app.AccountKeeper) svd := ante.NewSigVerificationDecorator(app.AccountKeeper) antehandler := sdk.ChainAnteDecorators(spkd, svd) - _, err := antehandler(ctx, tx, false) - require.NotNil(t, err) + type testCase struct { + name string + privs []crypto.PrivKey + accNums []uint64 + seqs []uint64 + recheck bool + shouldErr bool + } + testCases := []testCase{ + {"no signers", []crypto.PrivKey{}, []uint64{}, []uint64{}, false, true}, + {"not enough signers", []crypto.PrivKey{priv1, priv2}, []uint64{0, 1}, []uint64{0, 0}, false, true}, + {"wrong order signers", []crypto.PrivKey{priv3, priv2, priv1}, []uint64{2, 1, 0}, []uint64{0, 0, 0}, false, true}, + {"wrong accnums", []crypto.PrivKey{priv1, priv2, priv3}, []uint64{7, 8, 9}, []uint64{0, 0, 0}, false, true}, + {"wrong sequences", []crypto.PrivKey{priv1, priv2, priv3}, []uint64{0, 1, 2}, []uint64{3, 4, 5}, false, true}, + {"valid tx", []crypto.PrivKey{priv1, priv2, priv3}, []uint64{0, 1, 2}, []uint64{0, 0, 0}, false, false}, + {"no err on recheck", []crypto.PrivKey{}, []uint64{}, []uint64{}, true, false}, + } + for i, tc := range testCases { + ctx = ctx.WithIsReCheckTx(tc.recheck) + + tx := types.NewTestTx(ctx, msgs, tc.privs, tc.accNums, tc.seqs, fee) + + _, err := antehandler(ctx, tx, false) + if tc.shouldErr { + require.NotNil(t, err, "TestCase %d: %s did not error as expected", i, tc.name) + } else { + require.Nil(t, err, "TestCase %d: %s errored unexpectedly. Err: %v", i, tc.name, err) + } + } } func TestSigIntegration(t *testing.T) { From 4eb7dc8545477669c5a12902199df5fc367d2325 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 15 Oct 2019 14:31:32 -0700 Subject: [PATCH 04/16] add integration tests --- x/auth/ante/ante_test.go | 83 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/x/auth/ante/ante_test.go b/x/auth/ante/ante_test.go index d37a31eee80b..e8ea098c4f0c 100644 --- a/x/auth/ante/ante_test.go +++ b/x/auth/ante/ante_test.go @@ -1,6 +1,7 @@ package ante_test import ( + "encoding/json" "fmt" "math/rand" "strings" @@ -680,3 +681,85 @@ func TestCustomSignatureVerificationGasConsumer(t *testing.T) { tx = types.NewTestTx(ctx, msgs, privs, accnums, seqs, fee) checkValidTx(t, anteHandler, ctx, tx, false) } + +func TestAnteHandlerReCheck(t *testing.T) { + // setup + app, ctx := createTestApp(true) + // set blockheight and recheck=true + ctx = ctx.WithBlockHeight(1) + ctx = ctx.WithIsReCheckTx(true) + + // keys and addresses + priv1, _, addr1 := types.KeyTestPubAddr() + // priv2, _, addr2 := types.KeyTestPubAddr() + + // set the accounts + acc1 := app.AccountKeeper.NewAccountWithAddress(ctx, addr1) + acc1.SetCoins(types.NewTestCoins()) + require.NoError(t, acc1.SetAccountNumber(0)) + app.AccountKeeper.SetAccount(ctx, acc1) + + antehandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, ante.DefaultSigVerificationGasConsumer) + + // test that operations skipped on recheck do not run + + msg := types.NewTestMsg(addr1) + msgs := []sdk.Msg{msg} + fee := types.NewTestStdFee() + + privs, accnums, seqs := []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0} + tx := types.NewTestTxWithMemo(ctx, msgs, privs, accnums, seqs, fee, "thisisatestmemo") + + // make signature array empty which would normally cause ValidateBasicDecorator and SigVerificationDecorator fail + // since these decorators don't run on recheck, the tx should pass the antehandler + stdTx := tx.(types.StdTx) + stdTx.Signatures = []types.StdSignature{} + + _, err := antehandler(ctx, stdTx, false) + require.Nil(t, err, "AnteHandler errored on recheck unexpectedly: %v", err) + + tx = types.NewTestTxWithMemo(ctx, msgs, privs, accnums, seqs, fee, "thisisatestmemo") + txBytes, err := json.Marshal(tx) + require.Nil(t, err, "Error marshalling tx: %v", err) + ctx = ctx.WithTxBytes(txBytes) + + // require that state machine param-dependent checking is still run on recheck since parameters can change between check and recheck + testCases := []struct { + name string + params types.Params + }{ + {"memo size check", types.NewParams(0, types.DefaultTxSigLimit, types.DefaultTxSizeCostPerByte, types.DefaultSigVerifyCostED25519, types.DefaultSigVerifyCostSecp256k1)}, + {"tx sig limit check", types.NewParams(types.DefaultMaxMemoCharacters, 0, types.DefaultTxSizeCostPerByte, types.DefaultSigVerifyCostED25519, types.DefaultSigVerifyCostSecp256k1)}, + {"txsize check", types.NewParams(types.DefaultMaxMemoCharacters, types.DefaultTxSigLimit, 10000000, types.DefaultSigVerifyCostED25519, types.DefaultSigVerifyCostSecp256k1)}, + {"sig verify cost check", types.NewParams(types.DefaultMaxMemoCharacters, types.DefaultTxSigLimit, types.DefaultTxSizeCostPerByte, types.DefaultSigVerifyCostED25519, 100000000)}, + } + for _, tc := range testCases { + // set testcase parameters + app.AccountKeeper.SetParams(ctx, tc.params) + + _, err := antehandler(ctx, tx, false) + + require.NotNil(t, err, "tx does not fail on recheck with updated params in test case: %s", tc.name) + + // reset parameters to default values + app.AccountKeeper.SetParams(ctx, types.DefaultParams()) + } + + // require that local mempool fee check is still run on recheck since validator may change minFee between check and recheck + // create new minimum gas price so antehandler fails on recheck + ctx = ctx.WithMinGasPrices([]sdk.DecCoin{sdk.DecCoin{ + Denom: "dnecoin", // fee does not have this denom + Amount: sdk.NewDec(5), + }}) + _, err = antehandler(ctx, tx, false) + require.NotNil(t, err, "antehandler on recheck did not fail when mingasPrice was changed") + // reset min gasprice + ctx = ctx.WithMinGasPrices(sdk.DecCoins{}) + + // remove funds for account so antehandler fails on recheck + acc1.SetCoins(sdk.Coins{}) + app.AccountKeeper.SetAccount(ctx, acc1) + + _, err = antehandler(ctx, tx, false) + require.NotNil(t, err, "antehandler on recheck did not fail once feePayer no longer has sufficient funds") +} From 48bc5cb289232415c8fdd17bf452c46417963a79 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 15 Oct 2019 15:34:16 -0700 Subject: [PATCH 05/16] update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d91fa65022c..b4a6a7d7ff6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -122,6 +122,9 @@ that allows for arbitrary vesting periods. * `SigVerificationDecorator`: Verify each signature is valid, return if there is an error. * `ValidateSigCountDecorator`: Validate the number of signatures in tx based on app-parameters. * `IncrementSequenceDecorator`: Increments the account sequence for each signer to prevent replay attacks. + * Baseapp has a new `runTxModeReCheck` to allow applications to skip expensive and unnecessary re-checking of transaction + * Context has new `IsRecheckTx() bool` and `WithIsReCheckTx(bool) Context` methods to allow recheck boolean to be available for antehandler + * AnteDecorators have been updated to avoid unnecessary checks when `ctx.IsReCheckTx() == true` ### Improvements From af3b24db38d5f88f9bb6a7894c6b3afe23b08a8c Mon Sep 17 00:00:00 2001 From: Aditya Date: Tue, 15 Oct 2019 15:35:02 -0700 Subject: [PATCH 06/16] Update x/auth/ante/ante_test.go Co-Authored-By: Bot from GolangCI <42910462+golangcibot@users.noreply.github.com> --- x/auth/ante/ante_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/auth/ante/ante_test.go b/x/auth/ante/ante_test.go index e8ea098c4f0c..a6113d5d962f 100644 --- a/x/auth/ante/ante_test.go +++ b/x/auth/ante/ante_test.go @@ -747,7 +747,7 @@ func TestAnteHandlerReCheck(t *testing.T) { // require that local mempool fee check is still run on recheck since validator may change minFee between check and recheck // create new minimum gas price so antehandler fails on recheck - ctx = ctx.WithMinGasPrices([]sdk.DecCoin{sdk.DecCoin{ + ctx = ctx.WithMinGasPrices([]sdk.DecCoin{{ Denom: "dnecoin", // fee does not have this denom Amount: sdk.NewDec(5), }}) From 32f087bb889224ce2358810c9c281801655e6d79 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 22 Oct 2019 11:39:22 -0700 Subject: [PATCH 07/16] enforce invariant, that recheck mode has IsCheckTx() = true --- baseapp/baseapp.go | 6 +++--- types/context.go | 8 +++++++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 91c43f6b6353..c1594023a696 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -469,11 +469,11 @@ func validateBasicTxMsgs(msgs []sdk.Msg) sdk.Error { // Returns the applications's deliverState if app is in runTxModeDeliver, // otherwise it returns the application's checkstate. func (app *BaseApp) getState(mode runTxMode) *state { - if mode == runTxModeCheck || mode == runTxModeSimulate { - return app.checkState + if mode == runTxModeDeliver { + return app.deliverState } - return app.deliverState + return app.checkState } // retrieve the context for the tx w/ txBytes and other memoized values. diff --git a/types/context.go b/types/context.go index c7c9e552c378..303ecfd82b0c 100644 --- a/types/context.go +++ b/types/context.go @@ -31,7 +31,7 @@ type Context struct { gasMeter GasMeter blockGasMeter GasMeter checkTx bool - recheckTx bool + recheckTx bool // if recheckTx == true, checkTx must be true minGasPrice DecCoins consParams *abci.ConsensusParams eventManager *EventManager @@ -154,7 +154,13 @@ func (c Context) WithIsCheckTx(isCheckTx bool) Context { return c } +// WithIsRecheckTx(true) will also set checkTx = true +// in order to enforce invariant that if recheckTx = true +// then checkTx = true as well func (c Context) WithIsReCheckTx(isRecheckTx bool) Context { + if isRecheckTx { + c.checkTx = true + } c.recheckTx = isRecheckTx return c } From 14f1939811c48bce04406ae696bb67ad1713ea60 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 22 Oct 2019 12:02:26 -0700 Subject: [PATCH 08/16] address reviews --- CHANGELOG.md | 6 +++--- baseapp/abci.go | 9 ++++++--- baseapp/baseapp.go | 2 +- x/auth/ante/sigverify.go | 2 +- 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1df1610e9a0e..c36d7fc8f12c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -123,9 +123,9 @@ that allows for arbitrary vesting periods. * `SigVerificationDecorator`: Verify each signature is valid, return if there is an error. * `ValidateSigCountDecorator`: Validate the number of signatures in tx based on app-parameters. * `IncrementSequenceDecorator`: Increments the account sequence for each signer to prevent replay attacks. - * Baseapp has a new `runTxModeReCheck` to allow applications to skip expensive and unnecessary re-checking of transaction - * Context has new `IsRecheckTx() bool` and `WithIsReCheckTx(bool) Context` methods to allow recheck boolean to be available for antehandler - * AnteDecorators have been updated to avoid unnecessary checks when `ctx.IsReCheckTx() == true` + * (baseapp) [\#5196](https://github.com/cosmos/cosmos-sdk/pull/5196) Baseapp has a new `runTxModeReCheck` to allow applications to skip expensive and unnecessary re-checking of transaction + * (types) [\#5196](https://github.com/cosmos/cosmos-sdk/pull/5196) Context has new `IsRecheckTx() bool` and `WithIsReCheckTx(bool) Context` methods to allow recheck boolean to be available for antehandler + * (x/auth/ante) [\#5196](https://github.com/cosmos/cosmos-sdk/pull/5196) AnteDecorators have been updated to avoid unnecessary checks when `ctx.IsReCheckTx() == true` ### Improvements diff --git a/baseapp/abci.go b/baseapp/abci.go index 0e6ef5183db6..ef8c71d4222a 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -162,12 +162,15 @@ func (app *BaseApp) CheckTx(req abci.RequestCheckTx) (res abci.ResponseCheckTx) var result sdk.Result tx, err := app.txDecoder(req.Tx) - if err != nil { + switch { + case err != nil: result = err.Result() - } else if req.Type == abci.CheckTxType_New { + case req.Type == abci.CheckTxType_New: result = app.runTx(runTxModeCheck, req.Tx, tx) - } else { + case req.Type == abci.CheckTxType_Recheck: result = app.runTx(runTxModeReCheck, req.Tx, tx) + default: + panic(fmt.Sprintf("Unknown RequestCheckTx Type: %v", req.Type)) } return abci.ResponseCheckTx{ diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 2db83561e05a..29a4685b816a 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -24,7 +24,7 @@ import ( const ( // Check a transaction runTxModeCheck runTxMode = iota - // Recheck a transaction + // Recheck a (pending) transaction after a commit runTxModeReCheck runTxMode = iota // Simulate a transaction runTxModeSimulate runTxMode = iota diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index d4210287bca6..af52e6c39b4a 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -156,7 +156,7 @@ func (sgcd SigGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula // Verify all signatures for tx and return error if any are invalid // increment sequence of each signer and set updated account back in store // Call next AnteHandler -// This decorator will not get run on ReCheck since it is not dependent on application state +// This decorator will not get run on ReCheck // CONTRACT: Pubkeys are set in context for all signers before this decorator runs // CONTRACT: Tx must implement SigVerifiableTx interface type SigVerificationDecorator struct { From 81a59961cd28af1a9c314614c6233f5d7a65d1a4 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 22 Oct 2019 15:42:48 -0400 Subject: [PATCH 09/16] update changelog --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c36d7fc8f12c..34899095516b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -100,6 +100,9 @@ correct version via: `pkgutil --pkg-info=com.apple.pkg.CLTools_Executables`. to the new keyring. * [\#4486](https://github.com/cosmos/cosmos-sdk/issues/4486) Introduce new `PeriodicVestingAccount` vesting account type that allows for arbitrary vesting periods. +* (baseapp) [\#5196](https://github.com/cosmos/cosmos-sdk/pull/5196) Baseapp has a new `runTxModeReCheck` to allow applications to skip expensive and unnecessary re-checking of transactions. +* (types) [\#5196](https://github.com/cosmos/cosmos-sdk/pull/5196) Context has new `IsRecheckTx() bool` and `WithIsReCheckTx(bool) Context` methods to to be used in the `AnteHandler`. +* (x/auth/ante) [\#5196](https://github.com/cosmos/cosmos-sdk/pull/5196) AnteDecorators have been updated to avoid unnecessary checks when `ctx.IsReCheckTx() == true` * (x/auth) [\#5006](https://github.com/cosmos/cosmos-sdk/pull/5006) Modular `AnteHandler` via composable decorators: * The `AnteDecorator` interface has been introduced to allow users to implement modular `AnteHandler` functionality that can be composed together to create a single `AnteHandler` rather than implementing @@ -123,9 +126,6 @@ that allows for arbitrary vesting periods. * `SigVerificationDecorator`: Verify each signature is valid, return if there is an error. * `ValidateSigCountDecorator`: Validate the number of signatures in tx based on app-parameters. * `IncrementSequenceDecorator`: Increments the account sequence for each signer to prevent replay attacks. - * (baseapp) [\#5196](https://github.com/cosmos/cosmos-sdk/pull/5196) Baseapp has a new `runTxModeReCheck` to allow applications to skip expensive and unnecessary re-checking of transaction - * (types) [\#5196](https://github.com/cosmos/cosmos-sdk/pull/5196) Context has new `IsRecheckTx() bool` and `WithIsReCheckTx(bool) Context` methods to allow recheck boolean to be available for antehandler - * (x/auth/ante) [\#5196](https://github.com/cosmos/cosmos-sdk/pull/5196) AnteDecorators have been updated to avoid unnecessary checks when `ctx.IsReCheckTx() == true` ### Improvements From b5232ee477e4b942dcd6800138fc8ae53117ee6a Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 22 Oct 2019 15:48:04 -0400 Subject: [PATCH 10/16] lint: update godoc --- types/context.go | 7 +++---- x/auth/ante/basic.go | 5 +++-- x/auth/ante/sigverify.go | 18 ++++++++++-------- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/types/context.go b/types/context.go index 303ecfd82b0c..ed4b693c9770 100644 --- a/types/context.go +++ b/types/context.go @@ -31,7 +31,7 @@ type Context struct { gasMeter GasMeter blockGasMeter GasMeter checkTx bool - recheckTx bool // if recheckTx == true, checkTx must be true + recheckTx bool // if recheckTx == true, then checkTx must also be true minGasPrice DecCoins consParams *abci.ConsensusParams eventManager *EventManager @@ -154,9 +154,8 @@ func (c Context) WithIsCheckTx(isCheckTx bool) Context { return c } -// WithIsRecheckTx(true) will also set checkTx = true -// in order to enforce invariant that if recheckTx = true -// then checkTx = true as well +// WithIsRecheckTx called with true will also set true on checkTx in order to +// enforce the invariant that if recheckTx = true then checkTx = true as well. func (c Context) WithIsReCheckTx(isRecheckTx bool) Context { if isRecheckTx { c.checkTx = true diff --git a/x/auth/ante/basic.go b/x/auth/ante/basic.go index d2ce71148e84..0e999a9d4781 100644 --- a/x/auth/ante/basic.go +++ b/x/auth/ante/basic.go @@ -17,8 +17,9 @@ var ( ) // ValidateBasicDecorator will call tx.ValidateBasic and return any non-nil error. -// If ValidateBasic passes, decorator calls next AnteHandler in chain. -// This decorator will not get run on ReCheck since it is not dependent on application state +// If ValidateBasic passes, decorator calls next AnteHandler in chain. Note, +// ValidateBasicDecorator decorator will not get executed on ReCheckTx since it +// is not dependent on application state. type ValidateBasicDecorator struct{} func NewValidateBasicDecorator() ValidateBasicDecorator { diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index af52e6c39b4a..3e2daa76c7fe 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -153,10 +153,9 @@ func (sgcd SigGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula return next(ctx, tx, simulate) } -// Verify all signatures for tx and return error if any are invalid -// increment sequence of each signer and set updated account back in store -// Call next AnteHandler -// This decorator will not get run on ReCheck +// Verify all signatures for a tx and return an error if any are invalid. Note, +// the SigVerificationDecorator decorator will not get executed on ReCheck. +// // CONTRACT: Pubkeys are set in context for all signers before this decorator runs // CONTRACT: Tx must implement SigVerifiableTx interface type SigVerificationDecorator struct { @@ -217,9 +216,11 @@ func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul return next(ctx, tx, simulate) } -// Increments sequences of all signers. -// Use this decorator to prevent replay attacks -// No need to run this decorator on CheckTx since it is merely updating nonce +// IncrementSequenceDecorator handles incrementing sequences of all signers. +// Use the IncrementSequenceDecorator decorator to prevent replay attacks. Note, +// there is no need to execute IncrementSequenceDecorator on CheckTx or RecheckTX +// since it is merely updating the nonce. +// // CONTRACT: Tx must implement SigVerifiableTx interface type IncrementSequenceDecorator struct { ak keeper.AccountKeeper @@ -232,10 +233,11 @@ func NewIncrementSequenceDecorator(ak keeper.AccountKeeper) IncrementSequenceDec } func (isd IncrementSequenceDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { - // no need to increment sequence on check tx + // no need to increment sequence on CheckTx or RecheckTx if ctx.IsCheckTx() { return next(ctx, tx, simulate) } + sigTx, ok := tx.(SigVerifiableTx) if !ok { return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "invalid transaction type") From 8d2e54b4d482112814e3fbec6527fdf3a15755d3 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 22 Oct 2019 16:11:33 -0400 Subject: [PATCH 11/16] docs: update checktx section in baseapp doc --- docs/core/baseapp.md | 61 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 48 insertions(+), 13 deletions(-) diff --git a/docs/core/baseapp.md b/docs/core/baseapp.md index 72dafa94e059..bacf99fb0b05 100644 --- a/docs/core/baseapp.md +++ b/docs/core/baseapp.md @@ -27,6 +27,7 @@ functionalities of an SDK application. - [Query Routing](#query-routing) - [Main ABCI Messages](#main-abci-messages) - [CheckTx](#checktx-1) + - [RecheckTx](#rechecktx) - [DeliverTx](#delivertx-1) - [RunTx, AnteHandler and RunMsgs](#runtx-antehandler-and-runmsgs) - [RunTx](#runtx) @@ -103,7 +104,7 @@ raw transaction bytes relayed by the underlying Tendermint engine. used to persist data related to the core of the application, like consensus parameters. - [`AnteHandler`](#antehandler): This handler is used to handle signature verification, fee payment, and other pre-message execution checks when a transaction is received. It's executed during -[`CheckTx`](#checktx-1) and [`DeliverTx`](#delivertx-1). +[`CheckTx/RecheckTx`](#checktx-1) and [`DeliverTx`](#delivertx-1). - [`InitChainer`](../basics/app-anatomy.md#initchainer), [`BeginBlocker` and `EndBlocker`](../basics/app-anatomy.md#beginblocker-and-endblocker): These are the functions executed when the application receives the `InitChain`, `BeginBlock` and `EndBlock` @@ -240,18 +241,43 @@ Developers building on top of the Cosmos SDK need not implement the ABCI themsel ### CheckTx -`CheckTx` is sent by the underlying consensus engine when a new unconfirmed (i.e. not yet included in a valid block) transaction is received by a full-node. The role of `CheckTx` is to guard the full-node's mempool (where unconfirmed transactions are stored until they are included in a block) from spam transactions. Unconfirmed transactions are relayed to peers only if they pass `CheckTx`. +`CheckTx` is sent by the underlying consensus engine when a new unconfirmed (i.e. not yet included in a valid block) +transaction is received by a full-node. The role of `CheckTx` is to guard the full-node's mempool +(where unconfirmed transactions are stored until they are included in a block) from spam transactions. +Unconfirmed transactions are relayed to peers only if they pass `CheckTx`. -`CheckTx()` can perform both _stateful_ and _stateless_ checks, but developers should strive to make them lightweight. In the Cosmos SDK, after decoding transactions, `CheckTx()` is implemented to do the following checks: +`CheckTx()` can perform both _stateful_ and _stateless_ checks, but developers should strive to +make them lightweight. In the Cosmos SDK, after decoding transactions, `CheckTx()` is implemented +to do the following checks: 1. Extract the `message`s from the transaction. -2. Perform _stateless_ checks by calling `ValidateBasic()` on each of the `messages`. This is done first, as _stateless_ checks are less computationally expensive than _stateful_ checks. If `ValidateBasic()` fail, `CheckTx` returns before running _stateful_ checks, which saves resources. -3. Perform non-module related _stateful_ checks on the account. This step is mainly about checking that the `message` signatures are valid, that enough fees are provided and that the sending account has enough funds to pay for said fees. Note that no precise `gas` counting occurs here, as `message`s are not processed. Usually, the `anteHandler` will check that the `gas` provided with the transaction is superior to a minimum reference gas amount based on the raw transaction size, in order to avoid spam with transactions that provide 0 gas. -4. Ensure that a [`Route`](#message-routing) exists for each `message`, but do **not** actually process `message`s. `Message`s only need to be processed when the canonical state need to be updated, which happens during `DeliverTx`. - -Steps 2. and 3. are performed by the `anteHandler` in the [`RunTx()`](<#runtx()-,antehandler-and-runmsgs()>) function, which `CheckTx()` calls with the `runTxModeCheck` mode. During each step of `CheckTx()`, a special [volatile state](#volatile-states) called `checkState` is updated. This state is used to keep track of the temporary changes triggered by the `CheckTx()` calls of each transaction without modifying the [main canonical state](#main-state) . For example, when a transaction goes through `CheckTx()`, the transaction's fees are deducted from the sender's account in `checkState`. If a second transaction is received from the same account before the first is processed, and the account has consumed all its funds in `checkState` during the first transaction, the second transaction will fail `CheckTx`() and be rejected. In any case, the sender's account will not actually pay the fees until the transaction is actually included in a block, because `checkState` never gets committed to the main state. `checkState` is reset to the latest state of the main state each time a blocks gets [committed](#commit). - -`CheckTx` returns a response to the underlying consensus engine of type [`abci.ResponseCheckTx`](https://tendermint.com/docs/spec/abci/abci.html#messages). The response contains: +2. Perform _stateless_ checks by calling `ValidateBasic()` on each of the `messages`. This is done + first, as _stateless_ checks are less computationally expensive than _stateful_ checks. If + `ValidateBasic()` fail, `CheckTx` returns before running _stateful_ checks, which saves resources. +3. Perform non-module related _stateful_ checks on the account. This step is mainly about checking + that the `message` signatures are valid, that enough fees are provided and that the sending account + has enough funds to pay for said fees. Note that no precise `gas` counting occurs here, + as `message`s are not processed. Usually, the `anteHandler` will check that the `gas` provided + with the transaction is superior to a minimum reference gas amount based on the raw transaction size, + in order to avoid spam with transactions that provide 0 gas. +4. Ensure that a [`Route`](#message-routing) exists for each `message`, but do **not** actually + process `message`s. `Message`s only need to be processed when the canonical state need to be updated, + which happens during `DeliverTx`. + +Steps 2. and 3. are performed by the `AnteHandler` in the [`RunTx()`](<#runtx()-,antehandler-and-runmsgs()>) +function, which `CheckTx()` calls with the `runTxModeCheck` mode. During each step of `CheckTx()`, a +special [volatile state](#volatile-states) called `checkState` is updated. This state is used to keep +track of the temporary changes triggered by the `CheckTx()` calls of each transaction without modifying +the [main canonical state](#main-state) . For example, when a transaction goes through `CheckTx()`, the +transaction's fees are deducted from the sender's account in `checkState`. If a second transaction is +received from the same account before the first is processed, and the account has consumed all its +funds in `checkState` during the first transaction, the second transaction will fail `CheckTx`() and +be rejected. In any case, the sender's account will not actually pay the fees until the transaction +is actually included in a block, because `checkState` never gets committed to the main state. The +`checkState` is reset to the latest state of the main state each time a blocks gets [committed](#commit). + +`CheckTx` returns a response to the underlying consensus engine of type [`abci.ResponseCheckTx`](https://tendermint.com/docs/spec/abci/abci.html#messages). +The response contains: - `Code (uint32)`: Response Code. `0` if successful. - `Data ([]byte)`: Result bytes, if any. @@ -259,9 +285,18 @@ Steps 2. and 3. are performed by the `anteHandler` in the [`RunTx()`](<#runtx()- - `Info (string):` Additional information. May be non-deterministic. - `GasWanted (int64)`: Amount of gas requested for transaction. It is provided by users when they generate the transaction. - `GasUsed (int64)`: Amount of gas consumed by transaction. During `CheckTx`, this value is computed by multiplying the standard cost of a transaction byte by the size of the raw transaction (click [here](https://github.com/cosmos/cosmos-sdk/blob/master/x/auth/ante.go#L101) for an example). -- `Tags ([]cmn.KVPair)`: Key-Value tags for filtering and indexing transactions (eg. by account). +- `Events ([]Event)`: Key-Value events for filtering and indexing transactions (eg. by account or message type). - `Codespace (string)`: Namespace for the Code. +#### RecheckTx + +After `Commit`, `CheckTx` is run again on all transactions that remain in the node's local mempool +after filtering those included in the block. To prevent the mempool from rechecking all transactions +every time a block is committed, the configuration option `mempool.recheck=false` can be set. As of +Tendermint v0.32.1, an additional `Type` parameter is made available to the `CheckTx` function that +indicates whether an incoming transaction is new (`CheckTxType_New`), or a recheck (`CheckTxType_Recheck`). +This allows certain checks like signature verification can be skipped during `CheckTxType_Recheck`. + ### DeliverTx When the underlying consensus engine receives a block proposal, each transaction in the block needs to be processed by the application. To that end, the underlying consensus engine sends a `DeliverTx` message to the application for each transaction in a sequential order. @@ -270,8 +305,8 @@ Before the first transaction of a given block is processed, a [volatile state](# `DeliverTx` performs the **exact same steps as `CheckTx`**, with a little caveat at step 3 and the addition of a fifth step: -3. The `anteHandler` does **not** check that the transaction's `gas-prices` is sufficient. That is because the `min-gas-prices` value `gas-prices` is checked against is local to the node, and therefore what is enough for one full-node might not be for another. This means that the proposer can potentially include transactions for free, although they are not incentivised to do so, as they earn a bonus on the total fee of the block they propose. -4. For each `message` in the transaction, route to the appropriate module's `handler`. Additional _stateful_ checks are performed, and the cache-wrapped multistore held in `deliverState`'s `context` is updated by the module's `keeper`. If the `handler` returns successfully, the cache-wrapped multistore held in `context` is written to `deliverState` `CacheMultiStore`. +1. The `anteHandler` does **not** check that the transaction's `gas-prices` is sufficient. That is because the `min-gas-prices` value `gas-prices` is checked against is local to the node, and therefore what is enough for one full-node might not be for another. This means that the proposer can potentially include transactions for free, although they are not incentivised to do so, as they earn a bonus on the total fee of the block they propose. +2. For each `message` in the transaction, route to the appropriate module's `handler`. Additional _stateful_ checks are performed, and the cache-wrapped multistore held in `deliverState`'s `context` is updated by the module's `keeper`. If the `handler` returns successfully, the cache-wrapped multistore held in `context` is written to `deliverState` `CacheMultiStore`. During step 5., each read/write to the store increases the value of `GasConsumed`. You can find the default cost of each operation [here](https://github.com/cosmos/cosmos-sdk/blob/master/store/types/gas.go#L142-L150). At any point, if `GasConsumed > GasWanted`, the function returns with `Code != 0` and `DeliverTx()` fails. From 27d03939f87bbb96a6c28cab38b23c36e235249b Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 22 Oct 2019 16:15:51 -0400 Subject: [PATCH 12/16] doc: update IncrementSequenceDecorator godoc --- x/auth/ante/sigverify.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index 3e2daa76c7fe..da987527f943 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -219,9 +219,12 @@ func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul // IncrementSequenceDecorator handles incrementing sequences of all signers. // Use the IncrementSequenceDecorator decorator to prevent replay attacks. Note, // there is no need to execute IncrementSequenceDecorator on CheckTx or RecheckTX -// since it is merely updating the nonce. +// since it is merely updating the nonce. As a result, this has the side effect +// that subsequent and sequential txs orginating from the same account cannot be +// handled correctly in a reliable way. To send sequential txs orginating from the +// same account, it is recommended to instead use multiple messages in a tx. // -// CONTRACT: Tx must implement SigVerifiableTx interface +// CONTRACT: The tx must implement the SigVerifiableTx interface. type IncrementSequenceDecorator struct { ak keeper.AccountKeeper } From 08a44d64c16e2548d72287b808dfde7c18a3d266 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 23 Oct 2019 09:12:51 -0400 Subject: [PATCH 13/16] cleanup iota const --- baseapp/baseapp.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 29a4685b816a..4e38ff54b407 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -22,14 +22,10 @@ import ( ) const ( - // Check a transaction - runTxModeCheck runTxMode = iota - // Recheck a (pending) transaction after a commit - runTxModeReCheck runTxMode = iota - // Simulate a transaction - runTxModeSimulate runTxMode = iota - // Deliver a transaction - runTxModeDeliver runTxMode = iota + runTxModeCheck runTxMode = iota // Check a transaction + runTxModeReCheck // Recheck a (pending) transaction after a commit + runTxModeSimulate // Simulate a transaction + runTxModeDeliver // Deliver a transaction // MainStoreKey is the string representation of the main store MainStoreKey = "main" From 39b91bf949cb7016d89eda87160271e62e402560 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 23 Oct 2019 09:14:45 -0400 Subject: [PATCH 14/16] remove named return --- baseapp/baseapp.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 4e38ff54b407..ae1ca19db7f7 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -473,8 +473,8 @@ func (app *BaseApp) getState(mode runTxMode) *state { } // retrieve the context for the tx w/ txBytes and other memoized values. -func (app *BaseApp) getContextForTx(mode runTxMode, txBytes []byte) (ctx sdk.Context) { - ctx = app.getState(mode).ctx. +func (app *BaseApp) getContextForTx(mode runTxMode, txBytes []byte) sdk.Context { + ctx := app.getState(mode).ctx. WithTxBytes(txBytes). WithVoteInfos(app.voteInfos). WithConsensusParams(app.consensusParams) @@ -486,7 +486,7 @@ func (app *BaseApp) getContextForTx(mode runTxMode, txBytes []byte) (ctx sdk.Con ctx, _ = ctx.CacheContext() } - return + return ctx } // cacheTxContext returns a new context based off of the provided context with From 0af80d587fa4b6746f69a2a5321aa5f595ccd820 Mon Sep 17 00:00:00 2001 From: Alexander Bezobchuk Date: Wed, 23 Oct 2019 09:14:58 -0400 Subject: [PATCH 15/16] Update docs/core/baseapp.md Co-Authored-By: Federico Kunze <31522760+fedekunze@users.noreply.github.com> --- docs/core/baseapp.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/core/baseapp.md b/docs/core/baseapp.md index bacf99fb0b05..c35ec232dd91 100644 --- a/docs/core/baseapp.md +++ b/docs/core/baseapp.md @@ -257,7 +257,7 @@ to do the following checks: 3. Perform non-module related _stateful_ checks on the account. This step is mainly about checking that the `message` signatures are valid, that enough fees are provided and that the sending account has enough funds to pay for said fees. Note that no precise `gas` counting occurs here, - as `message`s are not processed. Usually, the `anteHandler` will check that the `gas` provided + as `message`s are not processed. Usually, the `AnteHandler` will check that the `gas` provided with the transaction is superior to a minimum reference gas amount based on the raw transaction size, in order to avoid spam with transactions that provide 0 gas. 4. Ensure that a [`Route`](#message-routing) exists for each `message`, but do **not** actually From a0f895fd2144583cd8118b1e42ab1a578eaa23c7 Mon Sep 17 00:00:00 2001 From: Alexander Bezobchuk Date: Wed, 23 Oct 2019 09:15:07 -0400 Subject: [PATCH 16/16] Update docs/core/baseapp.md Co-Authored-By: Federico Kunze <31522760+fedekunze@users.noreply.github.com> --- docs/core/baseapp.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/core/baseapp.md b/docs/core/baseapp.md index c35ec232dd91..2a0a06a4a836 100644 --- a/docs/core/baseapp.md +++ b/docs/core/baseapp.md @@ -305,7 +305,7 @@ Before the first transaction of a given block is processed, a [volatile state](# `DeliverTx` performs the **exact same steps as `CheckTx`**, with a little caveat at step 3 and the addition of a fifth step: -1. The `anteHandler` does **not** check that the transaction's `gas-prices` is sufficient. That is because the `min-gas-prices` value `gas-prices` is checked against is local to the node, and therefore what is enough for one full-node might not be for another. This means that the proposer can potentially include transactions for free, although they are not incentivised to do so, as they earn a bonus on the total fee of the block they propose. +1. The `AnteHandler` does **not** check that the transaction's `gas-prices` is sufficient. That is because the `min-gas-prices` value `gas-prices` is checked against is local to the node, and therefore what is enough for one full-node might not be for another. This means that the proposer can potentially include transactions for free, although they are not incentivised to do so, as they earn a bonus on the total fee of the block they propose. 2. For each `message` in the transaction, route to the appropriate module's `handler`. Additional _stateful_ checks are performed, and the cache-wrapped multistore held in `deliverState`'s `context` is updated by the module's `keeper`. If the `handler` returns successfully, the cache-wrapped multistore held in `context` is written to `deliverState` `CacheMultiStore`. During step 5., each read/write to the store increases the value of `GasConsumed`. You can find the default cost of each operation [here](https://github.com/cosmos/cosmos-sdk/blob/master/store/types/gas.go#L142-L150). At any point, if `GasConsumed > GasWanted`, the function returns with `Code != 0` and `DeliverTx()` fails.