Merge PR 5196: RecheckTx Optimizations

2019-10-23 10:43:58 -04:00 · 2019-10-23 10:43:58 -04:00 · 4cd18bcf35
commit 4cd18bcf35
parent f08c30d394
10 changed files with 237 additions and 43 deletions
--- 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
--- a/baseapp/abci.go
+++ b/baseapp/abci.go
@ -162,10 +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 {
+	case req.Type == abci.CheckTxType_New:
 		result = app.runTx(runTxModeCheck, req.Tx, tx)
+	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{
--- a/baseapp/baseapp.go
+++ b/baseapp/baseapp.go
@ -22,12 +22,10 @@ import (
 )

 const (
-	// Check a transaction
-	runTxModeCheck 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"
@ -467,25 +465,28 @@ 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.
-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)

+	if mode == runTxModeReCheck {
+		ctx = ctx.WithIsReCheckTx(true)
+	}
 	if mode == runTxModeSimulate {
 		ctx, _ = ctx.CacheContext()
 	}

-	return
+	return ctx
 }

 // cacheTxContext returns a new context based off of the provided context with
@ -653,8 +654,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)
 		}

--- 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`.
+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).
+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:
+`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.

--- a/types/context.go
+++ b/types/context.go
@ -31,6 +31,7 @@ type Context struct {
 	gasMeter      GasMeter
 	blockGasMeter GasMeter
 	checkTx       bool
+	recheckTx     bool // if recheckTx == true, then checkTx must also be true
 	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,16 @@ func (c Context) WithIsCheckTx(isCheckTx bool) Context {
 	return c
 }

+// 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
+	}
+	c.recheckTx = isRecheckTx
+	return c
+}
+
 func (c Context) WithMinGasPrices(gasPrices DecCoins) Context {
 	c.minGasPrice = gasPrices
 	return c
--- 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"
@ -138,7 +139,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)

@ -195,7 +196,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)

@ -251,7 +252,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)

@ -407,7 +408,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)

@ -733,3 +734,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{{
+		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")
+}
--- a/x/auth/ante/basic.go
+++ b/x/auth/ante/basic.go
@ -17,7 +17,9 @@ var (
 )

 // ValidateBasicDecorator will call tx.ValidateBasic and return any non-nil error.
-// If ValidateBasic passes, decorator calls next AnteHandler in chain.
+// 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 {
@ -25,6 +27,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, call next antehandler
+	if ctx.IsReCheckTx() {
+		return next(ctx, tx, simulate)
+	}
 	if err := tx.ValidateBasic(); err != nil {
 		return ctx, err
 	}
--- a/x/auth/ante/basic_test.go
+++ b/x/auth/ante/basic_test.go
@ -40,6 +40,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) {
--- a/x/auth/ante/sigverify.go
+++ b/x/auth/ante/sigverify.go
@ -153,9 +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
+// 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 {
@ -169,6 +169,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")
@ -212,9 +216,15 @@ 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
-// CONTRACT: Tx must implement SigVerifiableTx interface
+// 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. 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: The tx must implement the SigVerifiableTx interface.
 type IncrementSequenceDecorator struct {
 	ak keeper.AccountKeeper
 }
@ -226,6 +236,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 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")
--- a/x/auth/ante/sigverify_test.go
+++ b/x/auth/ante/sigverify_test.go
@ -102,6 +102,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()
@ -121,15 +123,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) {