From 8eaff8fadd408762fa5a3c5a40969bda4548057e Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Thu, 26 May 2022 23:09:12 +0200 Subject: [PATCH] fix: Make rechecking a tx check the sequence number #12060 --- x/auth/ante/ante_test.go | 1 - x/auth/ante/sigverify.go | 9 +++---- x/auth/ante/sigverify_test.go | 44 ++++++++++++++++++++++++----------- 3 files changed, 33 insertions(+), 21 deletions(-) diff --git a/x/auth/ante/ante_test.go b/x/auth/ante/ante_test.go index a868459846..6ff5532a43 100644 --- a/x/auth/ante/ante_test.go +++ b/x/auth/ante/ante_test.go @@ -1096,7 +1096,6 @@ func (suite *AnteTestSuite) TestAnteHandlerReCheck() { // since these decorators don't run on recheck, the tx should pass the antehandler txBuilder, err := suite.clientCtx.TxConfig.WrapTxBuilder(tx) suite.Require().NoError(err) - suite.Require().NoError(txBuilder.SetSignatures()) _, err = suite.anteHandler(suite.ctx, txBuilder.GetTx(), false) suite.Require().Nil(err, "AnteHandler errored on recheck unexpectedly: %v", err) diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index b32f46728f..c77632a7d0 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -195,7 +195,7 @@ func (sgcd SigGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula } // Verify all signatures for a tx and return an error if any are invalid. Note, -// the SigVerificationDecorator decorator will not get executed on ReCheck. +// the SigVerificationDecorator will not check signatures on ReCheck. // // CONTRACT: Pubkeys are set in context for all signers before this decorator runs // CONTRACT: Tx must implement SigVerifiableTx interface @@ -233,10 +233,6 @@ func OnlyLegacyAminoSigners(sigData signing.SignatureData) bool { } 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.(authsigning.SigVerifiableTx) if !ok { return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "invalid transaction type") @@ -291,7 +287,8 @@ func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul PubKey: pubKey, } - if !simulate { + // no need to verify signatures on recheck tx + if !simulate && !ctx.IsReCheckTx() { err := authsigning.VerifySignature(pubKey, signerData, sig.Data, svd.signModeHandler, tx) if err != nil { var errMsg string diff --git a/x/auth/ante/sigverify_test.go b/x/auth/ante/sigverify_test.go index 074f4c33af..c002818d3b 100644 --- a/x/auth/ante/sigverify_test.go +++ b/x/auth/ante/sigverify_test.go @@ -149,21 +149,23 @@ func (suite *AnteTestSuite) TestSigVerification() { antehandler := sdk.ChainAnteDecorators(spkd, svd) type testCase struct { - name string - privs []cryptotypes.PrivKey - accNums []uint64 - accSeqs []uint64 - recheck bool - shouldErr bool + name string + privs []cryptotypes.PrivKey + accNums []uint64 + accSeqs []uint64 + invalidSigs bool + recheck bool + shouldErr bool } + validSigs := false testCases := []testCase{ - {"no signers", []cryptotypes.PrivKey{}, []uint64{}, []uint64{}, false, true}, - {"not enough signers", []cryptotypes.PrivKey{priv1, priv2}, []uint64{0, 1}, []uint64{0, 0}, false, true}, - {"wrong order signers", []cryptotypes.PrivKey{priv3, priv2, priv1}, []uint64{2, 1, 0}, []uint64{0, 0, 0}, false, true}, - {"wrong accnums", []cryptotypes.PrivKey{priv1, priv2, priv3}, []uint64{7, 8, 9}, []uint64{0, 0, 0}, false, true}, - {"wrong sequences", []cryptotypes.PrivKey{priv1, priv2, priv3}, []uint64{0, 1, 2}, []uint64{3, 4, 5}, false, true}, - {"valid tx", []cryptotypes.PrivKey{priv1, priv2, priv3}, []uint64{0, 1, 2}, []uint64{0, 0, 0}, false, false}, - {"no err on recheck", []cryptotypes.PrivKey{}, []uint64{}, []uint64{}, true, false}, + {"no signers", []cryptotypes.PrivKey{}, []uint64{}, []uint64{}, validSigs, false, true}, + {"not enough signers", []cryptotypes.PrivKey{priv1, priv2}, []uint64{0, 1}, []uint64{0, 0}, validSigs, false, true}, + {"wrong order signers", []cryptotypes.PrivKey{priv3, priv2, priv1}, []uint64{2, 1, 0}, []uint64{0, 0, 0}, validSigs, false, true}, + {"wrong accnums", []cryptotypes.PrivKey{priv1, priv2, priv3}, []uint64{7, 8, 9}, []uint64{0, 0, 0}, validSigs, false, true}, + {"wrong sequences", []cryptotypes.PrivKey{priv1, priv2, priv3}, []uint64{0, 1, 2}, []uint64{3, 4, 5}, validSigs, false, true}, + {"valid tx", []cryptotypes.PrivKey{priv1, priv2, priv3}, []uint64{0, 1, 2}, []uint64{0, 0, 0}, validSigs, false, false}, + {"no err on recheck", []cryptotypes.PrivKey{priv1, priv2, priv3}, []uint64{0, 0, 0}, []uint64{0, 0, 0}, !validSigs, true, false}, } for i, tc := range testCases { suite.ctx = suite.ctx.WithIsReCheckTx(tc.recheck) @@ -175,6 +177,20 @@ func (suite *AnteTestSuite) TestSigVerification() { tx, err := suite.CreateTestTx(tc.privs, tc.accNums, tc.accSeqs, suite.ctx.ChainID()) suite.Require().NoError(err) + if tc.invalidSigs { + txSigs, _ := tx.GetSignaturesV2() + badSig, _ := tc.privs[0].Sign([]byte("unrelated message")) + txSigs[0] = signing.SignatureV2{ + PubKey: tc.privs[0].PubKey(), + Data: &signing.SingleSignatureData{ + SignMode: suite.clientCtx.TxConfig.SignModeHandler().DefaultMode(), + Signature: badSig, + }, + Sequence: tc.accSeqs[0], + } + suite.txBuilder.SetSignatures(txSigs...) + tx = suite.txBuilder.GetTx() + } _, err = antehandler(suite.ctx, tx, false) if tc.shouldErr { @@ -259,7 +275,7 @@ func (suite *AnteTestSuite) TestSigVerification_ExplicitAmino() { {"wrong accnums", []cryptotypes.PrivKey{priv1, priv2, priv3}, []uint64{7, 8, 9}, []uint64{0, 0, 0}, false, true}, {"wrong sequences", []cryptotypes.PrivKey{priv1, priv2, priv3}, []uint64{0, 1, 2}, []uint64{3, 4, 5}, false, true}, {"valid tx", []cryptotypes.PrivKey{priv1, priv2, priv3}, []uint64{0, 1, 2}, []uint64{0, 0, 0}, false, false}, - {"no err on recheck", []cryptotypes.PrivKey{}, []uint64{}, []uint64{}, true, false}, + {"no err on recheck", []cryptotypes.PrivKey{priv1, priv2, priv3}, []uint64{0, 1, 2}, []uint64{0, 0, 0}, true, false}, } for i, tc := range testCases { suite.ctx = suite.ctx.WithIsReCheckTx(tc.recheck)