Fix IncrementSequenceDecorator (#5950)

* Fix IncrementSequenceDecorator

* Update PR #

* Update godoc

* Add TestIncrementSequenceDecorator test

CHANGELOG.md

@@ -99,6 +99,7 @@ types (eg. keys) to the `auth` module internal amino codec.
 * (rest) [\#5906](https://github.com/cosmos/cosmos-sdk/pull/5906) Fix an issue that make some REST calls panic when sending
 invalid or incomplete requests.
 * (x/genutil) [\#5938](https://github.com/cosmos/cosmos-sdk/pull/5938) Fix `InitializeNodeValidatorFiles` error handling.
+* (x/auth) [\#5950](https://github.com/cosmos/cosmos-sdk/pull/5950) Fix `IncrementSequenceDecorator` to use is `IsReCheckTx` instead of `IsCheckTx` to allow account sequence incrementing.
 
 ### State Machine Breaking
 

x/auth/ante/sigverify.go

@@ -218,13 +218,13 @@ 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. 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.
+// there is no need to execute IncrementSequenceDecorator on RecheckTX since
+// CheckTx would already bump the sequence number.
 //
-// CONTRACT: The tx must implement the SigVerifiableTx interface.
+// NOTE: Since CheckTx and DeliverTx state are managed separately, subsequent and
+// sequential txs orginating from the same account cannot be handled correctly in
+// a reliable way unless sequence numbers are managed and tracked manually by a
+// client. It is recommended to instead use multiple messages in a tx.
 type IncrementSequenceDecorator struct {
 	ak keeper.AccountKeeper
 }
@@ -236,8 +236,8 @@ 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() && !simulate {
+	// no need to increment sequence on RecheckTx
+	if ctx.IsReCheckTx() && !simulate {
 		return next(ctx, tx, simulate)
 	}
 
@@ -252,6 +252,7 @@ func (isd IncrementSequenceDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, sim
 		if err := acc.SetSequence(acc.GetSequence() + 1); err != nil {
 			panic(err)
 		}
+
 		isd.ak.SetAccount(ctx, acc)
 	}
 

x/auth/ante/sigverify_test.go

@@ -212,3 +212,40 @@ func runSigDecorators(t *testing.T, params types.Params, multisig bool, privs ..
 
 	return after - before, err
 }
+
+func TestIncrementSequenceDecorator(t *testing.T) {
+	app, ctx := createTestApp(true)
+
+	priv, _, addr := types.KeyTestPubAddr()
+	acc := app.AccountKeeper.NewAccountWithAddress(ctx, addr)
+	require.NoError(t, acc.SetAccountNumber(uint64(50)))
+	app.AccountKeeper.SetAccount(ctx, acc)
+
+	msgs := []sdk.Msg{types.NewTestMsg(addr)}
+	privKeys := []crypto.PrivKey{priv}
+	accNums := []uint64{app.AccountKeeper.GetAccount(ctx, addr).GetAccountNumber()}
+	accSeqs := []uint64{app.AccountKeeper.GetAccount(ctx, addr).GetSequence()}
+	fee := types.NewTestStdFee()
+	tx := types.NewTestTx(ctx, msgs, privKeys, accNums, accSeqs, fee)
+
+	isd := ante.NewIncrementSequenceDecorator(app.AccountKeeper)
+	antehandler := sdk.ChainAnteDecorators(isd)
+
+	testCases := []struct {
+		ctx         sdk.Context
+		simulate    bool
+		expectedSeq uint64
+	}{
+		{ctx.WithIsReCheckTx(true), false, 0},
+		{ctx.WithIsCheckTx(true).WithIsReCheckTx(false), false, 1},
+		{ctx.WithIsReCheckTx(true), false, 1},
+		{ctx.WithIsReCheckTx(true), false, 1},
+		{ctx.WithIsReCheckTx(true), true, 2},
+	}
+
+	for i, tc := range testCases {
+		_, err := antehandler(tc.ctx, tx, tc.simulate)
+		require.NoError(t, err, "unexpected error; tc #%d, %v", i, tc)
+		require.Equal(t, tc.expectedSeq, app.AccountKeeper.GetAccount(ctx, addr).GetSequence())
+	}
+}