diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f1fc9c7bbd7..ce320d48f65d 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index 895be5373c61..c40712bb0838 100644 --- a/x/auth/ante/sigverify.go +++ b/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) } diff --git a/x/auth/ante/sigverify_test.go b/x/auth/ante/sigverify_test.go index 35aaa8c9f806..54f74b3eac02 100644 --- a/x/auth/ante/sigverify_test.go +++ b/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()) + } +}