Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Make rechecking a tx check the sequence number (backport #12060) #12062

Merged
merged 1 commit into from
May 27, 2022
Merged

fix: Make rechecking a tx check the sequence number (backport #12060) #12062

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion x/auth/ante/ante_test.go
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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