Skip to content

Commit

Permalink
fix: Make rechecking a tx check the sequence number #12060 (#12062)
Browse files Browse the repository at this point in the history
(cherry picked from commit 8eaff8f)

Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com>
  • Loading branch information
mergify[bot] and ValarDragon authored May 27, 2022
1 parent 2921a1c commit a802a5c
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 21 deletions.
1 change: 0 additions & 1 deletion x/auth/ante/ante_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
9 changes: 3 additions & 6 deletions x/auth/ante/sigverify.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,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
Expand Down Expand Up @@ -229,10 +229,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")
Expand Down Expand Up @@ -285,7 +281,8 @@ func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul
Sequence: acc.GetSequence(),
}

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
Expand Down
44 changes: 30 additions & 14 deletions x/auth/ante/sigverify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit a802a5c

Please sign in to comment.