From 9069bb62b030069a745e134bccfbe5fbbf32d94d Mon Sep 17 00:00:00 2001 From: bizk Date: Tue, 17 Oct 2023 18:54:01 -0300 Subject: [PATCH 01/21] Renamed missleading variable name --- crypto/keys/secp256k1/secp256k1.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/crypto/keys/secp256k1/secp256k1.go b/crypto/keys/secp256k1/secp256k1.go index c9c4864339b..2055a560e8a 100644 --- a/crypto/keys/secp256k1/secp256k1.go +++ b/crypto/keys/secp256k1/secp256k1.go @@ -9,7 +9,7 @@ import ( "math/big" "github.com/cometbft/cometbft/crypto" - secp256k1 "github.com/decred/dcrd/dcrec/secp256k1/v4" + secp256k1dcrd "github.com/decred/dcrd/dcrec/secp256k1/v4" "golang.org/x/crypto/ripemd160" //nolint: staticcheck // keep around for backwards compatibility errorsmod "cosmossdk.io/errors" @@ -39,7 +39,8 @@ func (privKey *PrivKey) Bytes() []byte { // PubKey performs the point-scalar multiplication from the privKey on the // generator point to get the pubkey. func (privKey *PrivKey) PubKey() cryptotypes.PubKey { - pubkeyObject := secp256k1.PrivKeyFromBytes(privKey.Key).PubKey() + pubkeyObject := secp256k1dcrd.PrivKeyFromBytes(privKey.Key).PubKey() + pk := pubkeyObject.SerializeCompressed() return &PubKey{Key: pk} } @@ -100,7 +101,7 @@ func genPrivKey(rand io.Reader) []byte { d.SetBytes(privKeyBytes[:]) // break if we found a valid point (i.e. > 0 and < N == curverOrder) - isValidFieldElement := 0 < d.Sign() && d.Cmp(secp256k1.S256().N) < 0 + isValidFieldElement := 0 < d.Sign() && d.Cmp(secp256k1dcrd.S256().N) < 0 if isValidFieldElement { break } @@ -128,7 +129,7 @@ func GenPrivKeyFromSecret(secret []byte) *PrivKey { // https://apps.nsa.gov/iaarchive/library/ia-guidance/ia-solutions-for-classified/algorithm-guidance/suite-b-implementers-guide-to-fips-186-3-ecdsa.cfm // see also https://github.com/golang/go/blob/0380c9ad38843d523d9c9804fe300cb7edd7cd3c/src/crypto/ecdsa/ecdsa.go#L89-L101 fe := new(big.Int).SetBytes(secHash[:]) - n := new(big.Int).Sub(secp256k1.S256().N, one) + n := new(big.Int).Sub(secp256k1dcrd.S256().N, one) fe.Mod(fe, n) fe.Add(fe, one) From fd1600f52ebb2c917e439c4791ef4ce694ee3495 Mon Sep 17 00:00:00 2001 From: bizk Date: Tue, 17 Oct 2023 20:28:49 -0300 Subject: [PATCH 02/21] added onCurve verification --- x/auth/ante/sigverify.go | 43 ++++++++++++++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 6 deletions(-) diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index 9f7c18d57cb..c22c38b1294 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -5,6 +5,7 @@ import ( "encoding/base64" "encoding/hex" "fmt" + secp256k1dcrd "github.com/decred/dcrd/dcrec/secp256k1/v4" "google.golang.org/protobuf/types/known/anypb" @@ -250,6 +251,40 @@ func OnlyLegacyAminoSigners(sigData signing.SignatureData) bool { } } +func verifyIsOnCurve(pubKey cryptotypes.PubKey) (err error) { + switch pubKey.(type) { + case *ed25519.PubKey: + return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "ED25519 public keys are unsupported") + case *secp256k1.PubKey: + pubKey, err := secp256k1dcrd.ParsePubKey(key) + if err != nil { + return err + } + if pubKey.IsOnCurve() == false { + return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "secp256r1 key is not in curve") + } + case *secp256r1.PubKey: + pubKey := pubKey.(*secp256r1.PubKey).Key.PublicKey + if pubKey.IsOnCurve(pubKey.X, pubKey.Y) != false { + return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "secp256r1 key is not on curve") + } + case multisig.PubKey: + for _, pubKeyObject := range pubKey.(multisig.PubKey).GetPubKeys() { + err = verifyIsOnCurve(pubKeyObject) + if err != nil { + break + } + } + if err != nil { + return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "Multisig public key is not on curve") + } + default: + return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "unrecognized public key type") + } + + return nil +} + func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { sigTx, ok := tx.(authsigning.Tx) if !ok { @@ -285,12 +320,8 @@ func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "pubkey on account is not set") } - // Check account sequence number. - if sig.Sequence != acc.GetSequence() { - return ctx, errorsmod.Wrapf( - sdkerrors.ErrWrongSequence, - "account sequence mismatch, expected %d, got %d", acc.GetSequence(), sig.Sequence, - ) + if verifyIsOnCurve(pubKey) != nil { + return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "failed to verify if pub key is on curve.") } // retrieve signer data From f6150a820be9004e90a81698cb4e62e489d12163 Mon Sep 17 00:00:00 2001 From: bizk Date: Fri, 20 Oct 2023 16:41:50 -0300 Subject: [PATCH 03/21] added oncurve verification and tests --- testutil/testdata/tx.go | 9 +++ x/auth/ante/sigverify.go | 29 ++++++--- x/auth/ante/sigverify_test.go | 111 ++++++++++++++++++++++++++++++++++ 3 files changed, 141 insertions(+), 8 deletions(-) diff --git a/testutil/testdata/tx.go b/testutil/testdata/tx.go index 7ceb62431de..ed270eddd7b 100644 --- a/testutil/testdata/tx.go +++ b/testutil/testdata/tx.go @@ -1,6 +1,7 @@ package testdata import ( + "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" "testing" "gotest.tools/v3/assert" @@ -52,6 +53,14 @@ func KeyTestPubAddrSecp256R1(t *testing.T) (cryptotypes.PrivKey, cryptotypes.Pub return key, pub, addr } +// KeyTestPubAddrED25519 generates a new ed25519 keypair. +func KeyTestPubAddrED25519() (cryptotypes.PrivKey, cryptotypes.PubKey, sdk.AccAddress) { + key := ed25519.GenPrivKey() + pub := key.PubKey() + addr := sdk.AccAddress(pub.Address()) + return key, pub, addr +} + // NewTestFeeAmount is a test fee amount. func NewTestFeeAmount() sdk.Coins { return sdk.NewCoins(sdk.NewInt64Coin("atom", 150)) diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index c22c38b1294..82bc2b59378 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -95,6 +95,9 @@ func (spkd SetPubKeyDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate b return ctx, errorsmod.Wrapf(sdkerrors.ErrInvalidPubKey, "pubKey does not match signer address %s with signer index: %d", signerStrs[i], i) } + if err := verifyIsOnCurve(pk); err != nil { + return ctx, err + } acc, err := GetSignerAcc(ctx, spkd.ak, signers[i]) if err != nil { @@ -188,6 +191,9 @@ func (sgcd SigGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula } pubKey := signerAcc.GetPubKey() + if err := verifyIsOnCurve(pubKey); err != nil { + return ctx, err + } // In simulate mode the transaction comes with no signatures, thus if the // account's pubkey is nil, both signature verification and gasKVStore.Set() @@ -256,22 +262,24 @@ func verifyIsOnCurve(pubKey cryptotypes.PubKey) (err error) { case *ed25519.PubKey: return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "ED25519 public keys are unsupported") case *secp256k1.PubKey: - pubKey, err := secp256k1dcrd.ParsePubKey(key) + pubKeyObject, err := secp256k1dcrd.ParsePubKey(pubKey.Bytes()) if err != nil { + if err.(secp256k1dcrd.Error).Err.Error() == "ErrPubKeyNotOnCurve" { + return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "secp256k1 key is not on curve") + } return err } - if pubKey.IsOnCurve() == false { - return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "secp256r1 key is not in curve") + if !pubKeyObject.IsOnCurve() { + return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "secp256k1 key is not on curve") } case *secp256r1.PubKey: pubKey := pubKey.(*secp256r1.PubKey).Key.PublicKey - if pubKey.IsOnCurve(pubKey.X, pubKey.Y) != false { + if !pubKey.IsOnCurve(pubKey.X, pubKey.Y) { return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "secp256r1 key is not on curve") } case multisig.PubKey: for _, pubKeyObject := range pubKey.(multisig.PubKey).GetPubKeys() { - err = verifyIsOnCurve(pubKeyObject) - if err != nil { + if err := verifyIsOnCurve(pubKeyObject); err != nil { break } } @@ -320,8 +328,8 @@ func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "pubkey on account is not set") } - if verifyIsOnCurve(pubKey) != nil { - return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "failed to verify if pub key is on curve.") + if err := verifyIsOnCurve(pubKey); err != nil { + return ctx, err } // retrieve signer data @@ -407,6 +415,11 @@ func (isd IncrementSequenceDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, sim panic(err) } + pubKey := acc.GetPubKey() + if err := verifyIsOnCurve(pubKey); err != nil { + return ctx, err + } + isd.ak.SetAccount(ctx, acc) } diff --git a/x/auth/ante/sigverify_test.go b/x/auth/ante/sigverify_test.go index 45049aaedf0..52bb129a1a5 100644 --- a/x/auth/ante/sigverify_test.go +++ b/x/auth/ante/sigverify_test.go @@ -2,6 +2,7 @@ package ante_test import ( "fmt" + secp256k1dcrd "github.com/decred/dcrd/dcrec/secp256k1/v4" "testing" "github.com/golang/mock/gomock" @@ -359,3 +360,113 @@ func TestIncrementSequenceDecorator(t *testing.T) { require.Equal(t, tc.expectedSeq, suite.accountKeeper.GetAccount(suite.ctx, addr).GetSequence()) } } + +func TestAnteHandlerChecks(t *testing.T) { + suite := SetupTestSuite(t, true) + suite.txBankKeeper.EXPECT().DenomMetadataV2(gomock.Any(), gomock.Any()).Return(&bankv1beta1.QueryDenomMetadataResponse{}, nil).AnyTimes() + suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder() + + feeAmount := testdata.NewTestFeeAmount() + gasLimit := testdata.NewTestGasLimit() + suite.txBuilder.SetFeeAmount(feeAmount) + suite.txBuilder.SetGasLimit(gasLimit) + enabledSignModes := []signing.SignMode{signing.SignMode_SIGN_MODE_DIRECT, signing.SignMode_SIGN_MODE_TEXTUAL, signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON} + // Since TEXTUAL is not enabled by default, we create a custom TxConfig + // here which includes it. + txConfigOpts := authtx.ConfigOptions{ + TextualCoinMetadataQueryFn: txmodule.NewGRPCCoinMetadataQueryFn(suite.clientCtx), + EnabledSignModes: enabledSignModes, + } + + anteTxConfig, err := authtx.NewTxConfigWithOptions( + codec.NewProtoCodec(suite.encCfg.InterfaceRegistry), + txConfigOpts, + ) + require.NoError(t, err) + + // make block height non-zero to ensure account numbers part of signBytes + suite.ctx = suite.ctx.WithBlockHeight(1) + + // keys and addresses + priv1, _, addr1 := testdata.KeyTestPubAddr() + + secp256k1NotOnCurve, _ := secp256k1dcrd.GeneratePrivateKey() + secp256k1NotOnCurve.Key.SetInt(0) + priv12 := &secp256k1.PrivKey{Key: secp256k1NotOnCurve.Serialize()} + addr12 := sdk.AccAddress(priv12.PubKey().Address()) + + priv2, _, addr2 := testdata.KeyTestPubAddrSecp256R1(t) + //priv21 := priv1 + //addr21 := sdk.AccAddress(priv21.PubKey().Address()) + + priv3, _, addr3 := testdata.KeyTestPubAddrED25519() + + addrs := []sdk.AccAddress{addr1, addr12, addr2, addr3} + + msgs := make([]sdk.Msg, len(addrs)) + accs := make([]sdk.AccountI, len(addrs)) + // set accounts and create msg for each address + for i, addr := range addrs { + acc := suite.accountKeeper.NewAccountWithAddress(suite.ctx, addr) + require.NoError(t, acc.SetAccountNumber(uint64(i)+1000)) + suite.accountKeeper.SetAccount(suite.ctx, acc) + msgs[i] = testdata.NewTestMsg(addr) + accs[i] = acc + } + + //All decorators where we check if the public key is on curve + setPubKeyDecorator := ante.NewSetPubKeyDecorator(suite.accountKeeper) + sigGasConsumeDecorator := ante.NewSigGasConsumeDecorator(suite.accountKeeper, ante.DefaultSigVerificationGasConsumer) + sigVerificationDecorator := ante.NewSigVerificationDecorator(suite.accountKeeper, anteTxConfig.SignModeHandler()) + IncrementSequenceDecorator := ante.NewIncrementSequenceDecorator(suite.accountKeeper) + + anteHandler := sdk.ChainAnteDecorators(setPubKeyDecorator, sigGasConsumeDecorator, sigVerificationDecorator, IncrementSequenceDecorator) + require.NoError(t, err) + + type testCase struct { + name string + privs []cryptotypes.PrivKey + msg sdk.Msg + accNums []uint64 + accSeqs []uint64 + shouldErr bool + suported bool + } + testCases := []testCase{ + {"secp256k1_onCurve", []cryptotypes.PrivKey{priv1}, msgs[0], []uint64{accs[0].GetAccountNumber()}, []uint64{0}, false, true}, + {"secp256k1_NotOnCurve", []cryptotypes.PrivKey{priv12}, msgs[1], []uint64{accs[1].GetAccountNumber()}, []uint64{1}, true, true}, + {"secp256r1_onCurve", []cryptotypes.PrivKey{priv2}, msgs[2], []uint64{accs[2].GetAccountNumber()}, []uint64{2}, false, true}, + //{"secp256r1_notOnCurve", []cryptotypes.PrivKey{priv2}, msgs[3], []uint64{accs[1].GetAccountNumber()}, []uint64{1}, true, true}, + {"ed255619", []cryptotypes.PrivKey{priv3}, msgs[3], []uint64{accs[2].GetAccountNumber()}, []uint64{3}, true, false}, + } + + for i, tc := range testCases { + t.Run(fmt.Sprintf("%s key", tc.name), func(t *testing.T) { + suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder() // Create new txBuilder for each test + + require.NoError(t, suite.txBuilder.SetMsgs(tc.msg)) + + suite.txBuilder.SetFeeAmount(feeAmount) + suite.txBuilder.SetGasLimit(gasLimit) + + tx, err := suite.CreateTestTx(suite.ctx, tc.privs, tc.accNums, tc.accSeqs, suite.ctx.ChainID(), signing.SignMode_SIGN_MODE_DIRECT) + require.NoError(t, err) + + txBytes, err := suite.clientCtx.TxConfig.TxEncoder()(tx) + require.NoError(t, err) + + byteCtx := suite.ctx.WithTxBytes(txBytes) + _, err = anteHandler(byteCtx, tx, true) + if tc.shouldErr { + require.NotNil(t, err, "TestCase %d: %s did not error as expected", i, tc.name) + if tc.suported { + require.ErrorContains(t, err, "is not on curve") + } else { + require.ErrorContains(t, err, "public keys are unsupported") + } + } else { + require.Nil(t, err, "TestCase %d: %s errored unexpectedly. Err: %v", i, tc.name, err) + } + }) + } +} From f74ef04b83657fc5cab0dce3218bccb6a8cbb3e1 Mon Sep 17 00:00:00 2001 From: bizk Date: Fri, 20 Oct 2023 17:44:06 -0300 Subject: [PATCH 04/21] removed multisig verification --- x/auth/ante/sigverify.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index 82bc2b59378..014d1784355 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -277,15 +277,6 @@ func verifyIsOnCurve(pubKey cryptotypes.PubKey) (err error) { if !pubKey.IsOnCurve(pubKey.X, pubKey.Y) { return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "secp256r1 key is not on curve") } - case multisig.PubKey: - for _, pubKeyObject := range pubKey.(multisig.PubKey).GetPubKeys() { - if err := verifyIsOnCurve(pubKeyObject); err != nil { - break - } - } - if err != nil { - return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "Multisig public key is not on curve") - } default: return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "unrecognized public key type") } From 767e1d10c5608b98165840dfc7b049e8c125a0d5 Mon Sep 17 00:00:00 2001 From: bizk Date: Fri, 20 Oct 2023 19:10:51 -0300 Subject: [PATCH 05/21] lint fix --- x/auth/ante/sigverify.go | 12 ++++++------ x/auth/ante/sigverify_test.go | 8 ++------ 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index 014d1784355..658e4e86d14 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -5,8 +5,8 @@ import ( "encoding/base64" "encoding/hex" "fmt" - secp256k1dcrd "github.com/decred/dcrd/dcrec/secp256k1/v4" + secp256k1dcrd "github.com/decred/dcrd/dcrec/secp256k1/v4" "google.golang.org/protobuf/types/known/anypb" errorsmod "cosmossdk.io/errors" @@ -258,13 +258,13 @@ func OnlyLegacyAminoSigners(sigData signing.SignatureData) bool { } func verifyIsOnCurve(pubKey cryptotypes.PubKey) (err error) { - switch pubKey.(type) { + switch typedPubKey := pubKey.(type) { case *ed25519.PubKey: return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "ED25519 public keys are unsupported") case *secp256k1.PubKey: - pubKeyObject, err := secp256k1dcrd.ParsePubKey(pubKey.Bytes()) + pubKeyObject, err := secp256k1dcrd.ParsePubKey(typedPubKey.Bytes()) if err != nil { - if err.(secp256k1dcrd.Error).Err.Error() == "ErrPubKeyNotOnCurve" { + if err.(secp256k1dcrd.Error).Err.Error() == "1ErrPubKeyNotOnCurve" { return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "secp256k1 key is not on curve") } return err @@ -273,8 +273,8 @@ func verifyIsOnCurve(pubKey cryptotypes.PubKey) (err error) { return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "secp256k1 key is not on curve") } case *secp256r1.PubKey: - pubKey := pubKey.(*secp256r1.PubKey).Key.PublicKey - if !pubKey.IsOnCurve(pubKey.X, pubKey.Y) { + pubKeyObject := typedPubKey.Key.PublicKey + if !pubKeyObject.IsOnCurve(pubKeyObject.X, pubKeyObject.Y) { return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "secp256r1 key is not on curve") } default: diff --git a/x/auth/ante/sigverify_test.go b/x/auth/ante/sigverify_test.go index 52bb129a1a5..2b4208a4adb 100644 --- a/x/auth/ante/sigverify_test.go +++ b/x/auth/ante/sigverify_test.go @@ -2,9 +2,8 @@ package ante_test import ( "fmt" - secp256k1dcrd "github.com/decred/dcrd/dcrec/secp256k1/v4" "testing" - + secp256k1dcrd "github.com/decred/dcrd/dcrec/secp256k1/v4" "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" @@ -396,8 +395,6 @@ func TestAnteHandlerChecks(t *testing.T) { addr12 := sdk.AccAddress(priv12.PubKey().Address()) priv2, _, addr2 := testdata.KeyTestPubAddrSecp256R1(t) - //priv21 := priv1 - //addr21 := sdk.AccAddress(priv21.PubKey().Address()) priv3, _, addr3 := testdata.KeyTestPubAddrED25519() @@ -414,7 +411,6 @@ func TestAnteHandlerChecks(t *testing.T) { accs[i] = acc } - //All decorators where we check if the public key is on curve setPubKeyDecorator := ante.NewSetPubKeyDecorator(suite.accountKeeper) sigGasConsumeDecorator := ante.NewSigGasConsumeDecorator(suite.accountKeeper, ante.DefaultSigVerificationGasConsumer) sigVerificationDecorator := ante.NewSigVerificationDecorator(suite.accountKeeper, anteTxConfig.SignModeHandler()) @@ -436,7 +432,7 @@ func TestAnteHandlerChecks(t *testing.T) { {"secp256k1_onCurve", []cryptotypes.PrivKey{priv1}, msgs[0], []uint64{accs[0].GetAccountNumber()}, []uint64{0}, false, true}, {"secp256k1_NotOnCurve", []cryptotypes.PrivKey{priv12}, msgs[1], []uint64{accs[1].GetAccountNumber()}, []uint64{1}, true, true}, {"secp256r1_onCurve", []cryptotypes.PrivKey{priv2}, msgs[2], []uint64{accs[2].GetAccountNumber()}, []uint64{2}, false, true}, - //{"secp256r1_notOnCurve", []cryptotypes.PrivKey{priv2}, msgs[3], []uint64{accs[1].GetAccountNumber()}, []uint64{1}, true, true}, + // {"secp256r1_notOnCurve", []cryptotypes.PrivKey{priv2}, msgs[3], []uint64{accs[1].GetAccountNumber()}, []uint64{1}, true, true}, {"ed255619", []cryptotypes.PrivKey{priv3}, msgs[3], []uint64{accs[2].GetAccountNumber()}, []uint64{3}, true, false}, } From 68ef7eb2567c5cfa9bc181b9628823fe22b76084 Mon Sep 17 00:00:00 2001 From: bizk Date: Mon, 23 Oct 2023 11:16:43 -0300 Subject: [PATCH 06/21] added back sequence check --- x/auth/ante/sigverify.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index 658e4e86d14..bf8cfd01f61 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -319,6 +319,13 @@ func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "pubkey on account is not set") } + if sig.Sequence != acc.GetSequence() { + return ctx, errorsmod.Wrapf( + sdkerrors.ErrWrongSequence, + "account sequence mismatch, expected %d, got %d", acc.GetSequence(), sig.Sequence, + ) + } + if err := verifyIsOnCurve(pubKey); err != nil { return ctx, err } From 4715f21cfb73c59559b0bc020efb0b4645908dfd Mon Sep 17 00:00:00 2001 From: bizk Date: Tue, 24 Oct 2023 13:07:53 -0300 Subject: [PATCH 07/21] fixed some tests and improved error handling --- x/auth/ante/sigverify.go | 17 ++++++++++++----- x/auth/ante/sigverify_test.go | 25 ++++++++++++------------- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index bf8cfd01f61..8d2a7e3b204 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -5,6 +5,7 @@ import ( "encoding/base64" "encoding/hex" "fmt" + "strings" secp256k1dcrd "github.com/decred/dcrd/dcrec/secp256k1/v4" "google.golang.org/protobuf/types/known/anypb" @@ -191,6 +192,9 @@ func (sgcd SigGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula } pubKey := signerAcc.GetPubKey() + if !simulate && pubKey == nil { + return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "pubkey on account is not set") + } if err := verifyIsOnCurve(pubKey); err != nil { return ctx, err } @@ -264,7 +268,7 @@ func verifyIsOnCurve(pubKey cryptotypes.PubKey) (err error) { case *secp256k1.PubKey: pubKeyObject, err := secp256k1dcrd.ParsePubKey(typedPubKey.Bytes()) if err != nil { - if err.(secp256k1dcrd.Error).Err.Error() == "1ErrPubKeyNotOnCurve" { + if strings.Contains(err.Error(), "not on the secp256k1 curve") { return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "secp256k1 key is not on curve") } return err @@ -319,6 +323,10 @@ func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "pubkey on account is not set") } + if err := verifyIsOnCurve(pubKey); err != nil { + return ctx, err + } + if sig.Sequence != acc.GetSequence() { return ctx, errorsmod.Wrapf( sdkerrors.ErrWrongSequence, @@ -326,10 +334,6 @@ func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul ) } - if err := verifyIsOnCurve(pubKey); err != nil { - return ctx, err - } - // retrieve signer data genesis := ctx.BlockHeight() == 0 chainID := ctx.ChainID() @@ -414,6 +418,9 @@ func (isd IncrementSequenceDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, sim } pubKey := acc.GetPubKey() + if !simulate && pubKey == nil { + return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "pubkey on account is not set") + } if err := verifyIsOnCurve(pubKey); err != nil { return ctx, err } diff --git a/x/auth/ante/sigverify_test.go b/x/auth/ante/sigverify_test.go index 2b4208a4adb..3b070eaab80 100644 --- a/x/auth/ante/sigverify_test.go +++ b/x/auth/ante/sigverify_test.go @@ -2,10 +2,10 @@ package ante_test import ( "fmt" - "testing" secp256k1dcrd "github.com/decred/dcrd/dcrec/secp256k1/v4" "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" + "testing" bankv1beta1 "cosmossdk.io/api/cosmos/bank/v1beta1" storetypes "cosmossdk.io/store/types" @@ -338,8 +338,9 @@ func TestIncrementSequenceDecorator(t *testing.T) { tx, err := suite.CreateTestTx(suite.ctx, privs, accNums, accSeqs, suite.ctx.ChainID(), signing.SignMode_SIGN_MODE_DIRECT) require.NoError(t, err) - isd := ante.NewIncrementSequenceDecorator(suite.accountKeeper) - antehandler := sdk.ChainAnteDecorators(isd) + pubKeyDecorator := ante.NewSetPubKeyDecorator(suite.accountKeeper) + IncrementSequenceDecorator := ante.NewIncrementSequenceDecorator(suite.accountKeeper) + antehandler := sdk.ChainAnteDecorators(pubKeyDecorator, IncrementSequenceDecorator) testCases := []struct { ctx sdk.Context @@ -354,21 +355,20 @@ func TestIncrementSequenceDecorator(t *testing.T) { } 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, suite.accountKeeper.GetAccount(suite.ctx, addr).GetSequence()) + t.Run(fmt.Sprintf("%d test", i), func(t *testing.T) { + _, err = antehandler(tc.ctx, tx, tc.simulate) + require.NoError(t, err, "unexpected error; tc #%d, %v", i, tc) + require.Equal(t, tc.expectedSeq, suite.accountKeeper.GetAccount(suite.ctx, addr).GetSequence()) + }) } } func TestAnteHandlerChecks(t *testing.T) { suite := SetupTestSuite(t, true) suite.txBankKeeper.EXPECT().DenomMetadataV2(gomock.Any(), gomock.Any()).Return(&bankv1beta1.QueryDenomMetadataResponse{}, nil).AnyTimes() - suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder() feeAmount := testdata.NewTestFeeAmount() gasLimit := testdata.NewTestGasLimit() - suite.txBuilder.SetFeeAmount(feeAmount) - suite.txBuilder.SetGasLimit(gasLimit) enabledSignModes := []signing.SignMode{signing.SignMode_SIGN_MODE_DIRECT, signing.SignMode_SIGN_MODE_TEXTUAL, signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON} // Since TEXTUAL is not enabled by default, we create a custom TxConfig // here which includes it. @@ -395,7 +395,6 @@ func TestAnteHandlerChecks(t *testing.T) { addr12 := sdk.AccAddress(priv12.PubKey().Address()) priv2, _, addr2 := testdata.KeyTestPubAddrSecp256R1(t) - priv3, _, addr3 := testdata.KeyTestPubAddrED25519() addrs := []sdk.AccAddress{addr1, addr12, addr2, addr3} @@ -417,7 +416,6 @@ func TestAnteHandlerChecks(t *testing.T) { IncrementSequenceDecorator := ante.NewIncrementSequenceDecorator(suite.accountKeeper) anteHandler := sdk.ChainAnteDecorators(setPubKeyDecorator, sigGasConsumeDecorator, sigVerificationDecorator, IncrementSequenceDecorator) - require.NoError(t, err) type testCase struct { name string @@ -428,11 +426,12 @@ func TestAnteHandlerChecks(t *testing.T) { shouldErr bool suported bool } + + // Secp256r1 keys that are not on curve will fail before even doing any operation i.e when trying to get the pubkey testCases := []testCase{ {"secp256k1_onCurve", []cryptotypes.PrivKey{priv1}, msgs[0], []uint64{accs[0].GetAccountNumber()}, []uint64{0}, false, true}, {"secp256k1_NotOnCurve", []cryptotypes.PrivKey{priv12}, msgs[1], []uint64{accs[1].GetAccountNumber()}, []uint64{1}, true, true}, - {"secp256r1_onCurve", []cryptotypes.PrivKey{priv2}, msgs[2], []uint64{accs[2].GetAccountNumber()}, []uint64{2}, false, true}, - // {"secp256r1_notOnCurve", []cryptotypes.PrivKey{priv2}, msgs[3], []uint64{accs[1].GetAccountNumber()}, []uint64{1}, true, true}, + {"secp256r1_onCurve", []cryptotypes.PrivKey{priv2}, msgs[2], []uint64{accs[2].GetAccountNumber()}, []uint64{0}, false, true}, {"ed255619", []cryptotypes.PrivKey{priv3}, msgs[3], []uint64{accs[2].GetAccountNumber()}, []uint64{3}, true, false}, } From b4edd4afbfbbc9d0a93a42e97db7ff2b62a9b229 Mon Sep 17 00:00:00 2001 From: bizk Date: Tue, 24 Oct 2023 13:39:26 -0300 Subject: [PATCH 08/21] lint-fix --- x/auth/ante/sigverify_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x/auth/ante/sigverify_test.go b/x/auth/ante/sigverify_test.go index 181e39540d7..57b6ecc88cb 100644 --- a/x/auth/ante/sigverify_test.go +++ b/x/auth/ante/sigverify_test.go @@ -2,10 +2,11 @@ package ante_test import ( "fmt" + "testing" + secp256k1dcrd "github.com/decred/dcrd/dcrec/secp256k1/v4" "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" - "testing" bankv1beta1 "cosmossdk.io/api/cosmos/bank/v1beta1" storetypes "cosmossdk.io/store/types" From f9ba81810bf523ddea96a07c46c16ad1b173dd1f Mon Sep 17 00:00:00 2001 From: bizk Date: Tue, 24 Oct 2023 14:57:55 -0300 Subject: [PATCH 09/21] added multisig handling --- x/auth/ante/sigverify.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index 664743077bc..2ab87b3f120 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -281,6 +281,19 @@ func verifyIsOnCurve(pubKey cryptotypes.PubKey) (err error) { if !pubKeyObject.IsOnCurve(pubKeyObject.X, pubKeyObject.Y) { return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "secp256r1 key is not on curve") } + case multisig.PubKey: + pubKeysObjects := typedPubKey.GetPubKeys() + ok := true + for _, pubKeyObject := range pubKeysObjects { + if err := verifyIsOnCurve(pubKeyObject); err != nil { + ok = false + break + } + } + if !ok { + return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "some keys in multisig are not on curve") + } + default: return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "unrecognized public key type") } From e4af4f0ea08b23bfa26984f5649847f18456390d Mon Sep 17 00:00:00 2001 From: bizk Date: Tue, 24 Oct 2023 15:30:30 -0300 Subject: [PATCH 10/21] lint fix --- testutil/testdata/tx.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/testutil/testdata/tx.go b/testutil/testdata/tx.go index ed270eddd7b..43874da6a95 100644 --- a/testutil/testdata/tx.go +++ b/testutil/testdata/tx.go @@ -1,18 +1,17 @@ package testdata import ( - "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" "testing" - "gotest.tools/v3/assert" - "pgregory.net/rapid" - + "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" "github.com/cosmos/cosmos-sdk/crypto/keys/secp256r1" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/types/query" + "gotest.tools/v3/assert" + "pgregory.net/rapid" ) // AddressGenerator creates and returns a random address generator using rapid. @@ -44,7 +43,7 @@ func KeyTestPubAddr() (cryptotypes.PrivKey, cryptotypes.PubKey, sdk.AccAddress) return key, pub, addr } -// KeyTestPubAddr generates a new secp256r1 keypair. +// KeyTestPubAddrSecp256R1 generates a new secp256r1 keypair. func KeyTestPubAddrSecp256R1(t *testing.T) (cryptotypes.PrivKey, cryptotypes.PubKey, sdk.AccAddress) { key, err := secp256r1.GenPrivKey() assert.NilError(t, err) From dcf7704659f7181eff21ac8ab9844daa20d7cb16 Mon Sep 17 00:00:00 2001 From: bizk Date: Tue, 24 Oct 2023 19:25:27 -0300 Subject: [PATCH 11/21] lint fix --- testutil/testdata/grpc_query.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/testutil/testdata/grpc_query.go b/testutil/testdata/grpc_query.go index 522e28d59c2..9b61713c560 100644 --- a/testutil/testdata/grpc_query.go +++ b/testutil/testdata/grpc_query.go @@ -5,12 +5,11 @@ import ( "fmt" "testing" + "github.com/cosmos/cosmos-sdk/codec/types" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/gogoproto/proto" grpc "google.golang.org/grpc" "gotest.tools/v3/assert" - - "github.com/cosmos/cosmos-sdk/codec/types" - sdk "github.com/cosmos/cosmos-sdk/types" ) // iterCount defines the number of iterations to run on each query to test From b72057bddaf5f5e56c871fd4f55e353af07bea19 Mon Sep 17 00:00:00 2001 From: bizk Date: Tue, 24 Oct 2023 19:29:42 -0300 Subject: [PATCH 12/21] lint fix --- testutil/testdata/animal.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/testutil/testdata/animal.go b/testutil/testdata/animal.go index 428d3d4b2b9..7d5afa61528 100644 --- a/testutil/testdata/animal.go +++ b/testutil/testdata/animal.go @@ -1,7 +1,5 @@ package testdata -// nolint - import ( "fmt" From fe9f412063855b2cd57bfc4520ea6654ec36dc16 Mon Sep 17 00:00:00 2001 From: bizk Date: Tue, 24 Oct 2023 20:07:29 -0300 Subject: [PATCH 13/21] linter fixes --- .../bank/keeper/deterministic_test.go | 40 ++++++------- .../staking/keeper/deterministic_test.go | 56 +++++++++---------- testutil/testdata/grpc_query.go | 2 +- x/auth/keeper/deterministic_test.go | 28 +++++----- 4 files changed, 63 insertions(+), 63 deletions(-) diff --git a/tests/integration/bank/keeper/deterministic_test.go b/tests/integration/bank/keeper/deterministic_test.go index 5f7a48199bd..cd5125f75e4 100644 --- a/tests/integration/bank/keeper/deterministic_test.go +++ b/tests/integration/bank/keeper/deterministic_test.go @@ -147,12 +147,12 @@ func TestGRPCQueryBalance(t *testing.T) { req := banktypes.NewQueryBalanceRequest(addr, coin.GetDenom()) - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.Balance, 0, true) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.Balance, 0, true) }) fundAccount(f, addr1, coin1) req := banktypes.NewQueryBalanceRequest(addr1, coin1.GetDenom()) - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.Balance, 1087, false) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.Balance, 1087, false) } func TestGRPCQueryAllBalances(t *testing.T) { @@ -174,7 +174,7 @@ func TestGRPCQueryAllBalances(t *testing.T) { fundAccount(f, addr, coins...) req := banktypes.NewQueryAllBalancesRequest(addr, testdata.PaginationGenerator(rt, uint64(numCoins)).Draw(rt, "pagination"), false) - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.AllBalances, 0, true) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.AllBalances, 0, true) }) coins := sdk.NewCoins( @@ -185,7 +185,7 @@ func TestGRPCQueryAllBalances(t *testing.T) { fundAccount(f, addr1, coins...) req := banktypes.NewQueryAllBalancesRequest(addr1, nil, false) - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.AllBalances, 357, false) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.AllBalances, 357, false) } func TestGRPCQuerySpendableBalances(t *testing.T) { @@ -212,7 +212,7 @@ func TestGRPCQuerySpendableBalances(t *testing.T) { assert.NilError(t, err) req := banktypes.NewQuerySpendableBalancesRequest(addr, testdata.PaginationGenerator(rt, uint64(len(denoms))).Draw(rt, "pagination")) - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.SpendableBalances, 0, true) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.SpendableBalances, 0, true) }) coins := sdk.NewCoins( @@ -224,7 +224,7 @@ func TestGRPCQuerySpendableBalances(t *testing.T) { assert.NilError(t, err) req := banktypes.NewQuerySpendableBalancesRequest(addr1, nil) - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.SpendableBalances, 2032, false) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.SpendableBalances, 2032, false) } func TestGRPCQueryTotalSupply(t *testing.T) { @@ -256,7 +256,7 @@ func TestGRPCQueryTotalSupply(t *testing.T) { Pagination: testdata.PaginationGenerator(rt, uint64(len(initialSupply))).Draw(rt, "pagination"), } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.TotalSupply, 0, true) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.TotalSupply, 0, true) }) f = initDeterministicFixture(t) // reset @@ -269,7 +269,7 @@ func TestGRPCQueryTotalSupply(t *testing.T) { assert.NilError(t, f.bankKeeper.MintCoins(f.ctx, minttypes.ModuleName, coins)) req := &banktypes.QueryTotalSupplyRequest{} - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.TotalSupply, 150, false) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.TotalSupply, 150, false) } func TestGRPCQueryTotalSupplyOf(t *testing.T) { @@ -285,14 +285,14 @@ func TestGRPCQueryTotalSupplyOf(t *testing.T) { assert.NilError(t, f.bankKeeper.MintCoins(f.ctx, minttypes.ModuleName, sdk.NewCoins(coin))) req := &banktypes.QuerySupplyOfRequest{Denom: coin.GetDenom()} - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.SupplyOf, 0, true) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.SupplyOf, 0, true) }) coin := sdk.NewCoin("bar", math.NewInt(100)) assert.NilError(t, f.bankKeeper.MintCoins(f.ctx, minttypes.ModuleName, sdk.NewCoins(coin))) req := &banktypes.QuerySupplyOfRequest{Denom: coin.GetDenom()} - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.SupplyOf, 1021, false) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.SupplyOf, 1021, false) } func TestGRPCQueryParams(t *testing.T) { @@ -314,7 +314,7 @@ func TestGRPCQueryParams(t *testing.T) { assert.NilError(t, err) req := &banktypes.QueryParamsRequest{} - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.Params, 0, true) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.Params, 0, true) }) enabledStatus := banktypes.SendEnabled{ @@ -330,7 +330,7 @@ func TestGRPCQueryParams(t *testing.T) { err := f.bankKeeper.SetParams(f.ctx, params) assert.NilError(t, err) req := &banktypes.QueryParamsRequest{} - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.Params, 1003, false) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.Params, 1003, false) } func createAndReturnMetadatas(t *rapid.T, count int) []banktypes.Metadata { @@ -385,7 +385,7 @@ func TestGRPCDenomsMetadata(t *testing.T) { Pagination: testdata.PaginationGenerator(rt, uint64(count)).Draw(rt, "pagination"), } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.DenomsMetadata, 0, true) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.DenomsMetadata, 0, true) }) f = initDeterministicFixture(t) // reset @@ -393,7 +393,7 @@ func TestGRPCDenomsMetadata(t *testing.T) { f.bankKeeper.SetDenomMetaData(f.ctx, metadataAtom) req := &banktypes.QueryDenomsMetadataRequest{} - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.DenomsMetadata, 660, false) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.DenomsMetadata, 660, false) } func TestGRPCDenomMetadata(t *testing.T) { @@ -409,7 +409,7 @@ func TestGRPCDenomMetadata(t *testing.T) { Denom: denomMetadata[0].Base, } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.DenomMetadata, 0, true) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.DenomMetadata, 0, true) }) f.bankKeeper.SetDenomMetaData(f.ctx, metadataAtom) @@ -418,7 +418,7 @@ func TestGRPCDenomMetadata(t *testing.T) { Denom: metadataAtom.Base, } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.DenomMetadata, 1300, false) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.DenomMetadata, 1300, false) } func TestGRPCSendEnabled(t *testing.T) { @@ -448,7 +448,7 @@ func TestGRPCSendEnabled(t *testing.T) { // Pagination is only taken into account when `denoms` is an empty array Pagination: testdata.PaginationGenerator(rt, uint64(len(allDenoms))).Draw(rt, "pagination"), } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.SendEnabled, 0, true) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.SendEnabled, 0, true) }) coin1 := banktypes.SendEnabled{ @@ -467,7 +467,7 @@ func TestGRPCSendEnabled(t *testing.T) { Denoms: []string{coin1.GetDenom(), coin2.GetDenom()}, } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.SendEnabled, 4063, false) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.SendEnabled, 4063, false) } func TestGRPCDenomOwners(t *testing.T) { @@ -493,7 +493,7 @@ func TestGRPCDenomOwners(t *testing.T) { Denom: denom, Pagination: testdata.PaginationGenerator(rt, uint64(numAddr)).Draw(rt, "pagination"), } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.DenomOwners, 0, true) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.DenomOwners, 0, true) }) denomOwners := []*banktypes.DenomOwner{ @@ -518,5 +518,5 @@ func TestGRPCDenomOwners(t *testing.T) { req := &banktypes.QueryDenomOwnersRequest{ Denom: coin1.GetDenom(), } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.DenomOwners, 2516, false) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.DenomOwners, 2516, false) } diff --git a/tests/integration/staking/keeper/deterministic_test.go b/tests/integration/staking/keeper/deterministic_test.go index d0bd1405819..446a87afb42 100644 --- a/tests/integration/staking/keeper/deterministic_test.go +++ b/tests/integration/staking/keeper/deterministic_test.go @@ -348,7 +348,7 @@ func TestGRPCValidator(t *testing.T) { ValidatorAddr: val.OperatorAddress, } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.Validator, 0, true) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.Validator, 0, true) }) f = initDeterministicFixture(t) // reset @@ -357,7 +357,7 @@ func TestGRPCValidator(t *testing.T) { ValidatorAddr: val.OperatorAddress, } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.Validator, 1915, false) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.Validator, 1915, false) } func TestGRPCValidators(t *testing.T) { @@ -376,14 +376,14 @@ func TestGRPCValidators(t *testing.T) { Pagination: testdata.PaginationGenerator(rt, uint64(valsCount)).Draw(rt, "pagination"), } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.Validators, 0, true) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.Validators, 0, true) }) f = initDeterministicFixture(t) // reset getStaticValidator(t, f) getStaticValidator2(t, f) - testdata.DeterministicIterations(f.ctx, t, &stakingtypes.QueryValidatorsRequest{}, f.queryClient.Validators, 2862, false) + testdata.DeterministicIterations(t, f.ctx, &stakingtypes.QueryValidatorsRequest{}, f.queryClient.Validators, 2862, false) } func TestGRPCValidatorDelegations(t *testing.T) { @@ -405,7 +405,7 @@ func TestGRPCValidatorDelegations(t *testing.T) { Pagination: testdata.PaginationGenerator(rt, uint64(numDels)).Draw(rt, "pagination"), } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.ValidatorDelegations, 0, true) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.ValidatorDelegations, 0, true) }) f = initDeterministicFixture(t) // reset @@ -422,7 +422,7 @@ func TestGRPCValidatorDelegations(t *testing.T) { ValidatorAddr: validator.OperatorAddress, } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.ValidatorDelegations, 14637, false) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.ValidatorDelegations, 14637, false) } func TestGRPCValidatorUnbondingDelegations(t *testing.T) { @@ -448,7 +448,7 @@ func TestGRPCValidatorUnbondingDelegations(t *testing.T) { Pagination: testdata.PaginationGenerator(rt, uint64(numDels)).Draw(rt, "pagination"), } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.ValidatorUnbondingDelegations, 0, true) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.ValidatorUnbondingDelegations, 0, true) }) f = initDeterministicFixture(t) // reset @@ -470,7 +470,7 @@ func TestGRPCValidatorUnbondingDelegations(t *testing.T) { ValidatorAddr: validator.OperatorAddress, } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.ValidatorUnbondingDelegations, 3719, false) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.ValidatorUnbondingDelegations, 3719, false) } func TestGRPCDelegation(t *testing.T) { @@ -488,7 +488,7 @@ func TestGRPCDelegation(t *testing.T) { DelegatorAddr: delegator.String(), } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.Delegation, 0, true) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.Delegation, 0, true) }) f = initDeterministicFixture(t) // reset @@ -502,7 +502,7 @@ func TestGRPCDelegation(t *testing.T) { DelegatorAddr: delegator1, } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.Delegation, 4689, false) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.Delegation, 4689, false) } func TestGRPCUnbondingDelegation(t *testing.T) { @@ -525,7 +525,7 @@ func TestGRPCUnbondingDelegation(t *testing.T) { DelegatorAddr: delegator.String(), } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.UnbondingDelegation, 0, true) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.UnbondingDelegation, 0, true) }) f = initDeterministicFixture(t) // reset @@ -542,7 +542,7 @@ func TestGRPCUnbondingDelegation(t *testing.T) { DelegatorAddr: delegator1, } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.UnbondingDelegation, 1621, false) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.UnbondingDelegation, 1621, false) } func TestGRPCDelegatorDelegations(t *testing.T) { @@ -564,7 +564,7 @@ func TestGRPCDelegatorDelegations(t *testing.T) { Pagination: testdata.PaginationGenerator(rt, uint64(numVals)).Draw(rt, "pagination"), } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.DelegatorDelegations, 0, true) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.DelegatorDelegations, 0, true) }) f = initDeterministicFixture(t) // reset @@ -577,7 +577,7 @@ func TestGRPCDelegatorDelegations(t *testing.T) { DelegatorAddr: delegator1, } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.DelegatorDelegations, 4292, false) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.DelegatorDelegations, 4292, false) } func TestGRPCDelegatorValidator(t *testing.T) { @@ -596,7 +596,7 @@ func TestGRPCDelegatorValidator(t *testing.T) { ValidatorAddr: validator.OperatorAddress, } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.DelegatorValidator, 0, true) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.DelegatorValidator, 0, true) }) f = initDeterministicFixture(t) // reset @@ -611,7 +611,7 @@ func TestGRPCDelegatorValidator(t *testing.T) { ValidatorAddr: validator.OperatorAddress, } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.DelegatorValidator, 3563, false) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.DelegatorValidator, 3563, false) } func TestGRPCDelegatorUnbondingDelegations(t *testing.T) { @@ -637,7 +637,7 @@ func TestGRPCDelegatorUnbondingDelegations(t *testing.T) { Pagination: testdata.PaginationGenerator(rt, uint64(numVals)).Draw(rt, "pagination"), } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.DelegatorUnbondingDelegations, 0, true) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.DelegatorUnbondingDelegations, 0, true) }) f = initDeterministicFixture(t) // reset @@ -653,7 +653,7 @@ func TestGRPCDelegatorUnbondingDelegations(t *testing.T) { DelegatorAddr: delegator1, } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.DelegatorUnbondingDelegations, 1302, false) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.DelegatorUnbondingDelegations, 1302, false) } func TestGRPCHistoricalInfo(t *testing.T) { @@ -675,7 +675,7 @@ func TestGRPCHistoricalInfo(t *testing.T) { Height: height, } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.HistoricalInfo, 0, true) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.HistoricalInfo, 0, true) }) f = initDeterministicFixture(t) // reset @@ -694,7 +694,7 @@ func TestGRPCHistoricalInfo(t *testing.T) { Height: height, } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.HistoricalInfo, 1027, false) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.HistoricalInfo, 1027, false) } func TestGRPCDelegatorValidators(t *testing.T) { @@ -716,7 +716,7 @@ func TestGRPCDelegatorValidators(t *testing.T) { Pagination: testdata.PaginationGenerator(rt, uint64(numVals)).Draw(rt, "pagination"), } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.DelegatorValidators, 0, true) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.DelegatorValidators, 0, true) }) f = initDeterministicFixture(t) // reset @@ -727,7 +727,7 @@ func TestGRPCDelegatorValidators(t *testing.T) { assert.NilError(t, err) req := &stakingtypes.QueryDelegatorValidatorsRequest{DelegatorAddr: delegator1} - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.DelegatorValidators, 3166, false) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.DelegatorValidators, 3166, false) } func TestGRPCPool(t *testing.T) { @@ -737,12 +737,12 @@ func TestGRPCPool(t *testing.T) { rapid.Check(t, func(rt *rapid.T) { createAndSetValidator(t, rt, f) - testdata.DeterministicIterations(f.ctx, t, &stakingtypes.QueryPoolRequest{}, f.queryClient.Pool, 0, true) + testdata.DeterministicIterations(t, f.ctx, &stakingtypes.QueryPoolRequest{}, f.queryClient.Pool, 0, true) }) f = initDeterministicFixture(t) // reset getStaticValidator(t, f) - testdata.DeterministicIterations(f.ctx, t, &stakingtypes.QueryPoolRequest{}, f.queryClient.Pool, 6296, false) + testdata.DeterministicIterations(t, f.ctx, &stakingtypes.QueryPoolRequest{}, f.queryClient.Pool, 6296, false) } func TestGRPCRedelegations(t *testing.T) { @@ -788,7 +788,7 @@ func TestGRPCRedelegations(t *testing.T) { } req.Pagination = testdata.PaginationGenerator(rt, uint64(numDels)).Draw(rt, "pagination") - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.Redelegations, 0, true) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.Redelegations, 0, true) }) f = initDeterministicFixture(t) // reset @@ -807,7 +807,7 @@ func TestGRPCRedelegations(t *testing.T) { DstValidatorAddr: validator2, } - testdata.DeterministicIterations(f.ctx, t, req, f.queryClient.Redelegations, 3920, false) + testdata.DeterministicIterations(t, f.ctx, req, f.queryClient.Redelegations, 3920, false) } func TestGRPCParams(t *testing.T) { @@ -829,7 +829,7 @@ func TestGRPCParams(t *testing.T) { err := f.stakingKeeper.Params.Set(f.ctx, params) assert.NilError(t, err) - testdata.DeterministicIterations(f.ctx, t, &stakingtypes.QueryParamsRequest{}, f.queryClient.Params, 0, true) + testdata.DeterministicIterations(t, f.ctx, &stakingtypes.QueryParamsRequest{}, f.queryClient.Params, 0, true) }) params := stakingtypes.Params{ @@ -845,5 +845,5 @@ func TestGRPCParams(t *testing.T) { err := f.stakingKeeper.Params.Set(f.ctx, params) assert.NilError(t, err) - testdata.DeterministicIterations(f.ctx, t, &stakingtypes.QueryParamsRequest{}, f.queryClient.Params, 1162, false) + testdata.DeterministicIterations(t, f.ctx, &stakingtypes.QueryParamsRequest{}, f.queryClient.Params, 1162, false) } diff --git a/testutil/testdata/grpc_query.go b/testutil/testdata/grpc_query.go index 9b61713c560..75609263454 100644 --- a/testutil/testdata/grpc_query.go +++ b/testutil/testdata/grpc_query.go @@ -66,8 +66,8 @@ func (m *TestAnyResponse) UnpackInterfaces(unpacker types.AnyUnpacker) error { // `gasOverwrite` is set to true, we also check that this consumed // gas value is equal to the hardcoded `gasConsumed`. func DeterministicIterations[request, response proto.Message]( - ctx sdk.Context, t *testing.T, + ctx sdk.Context, req request, grpcFn func(context.Context, request, ...grpc.CallOption) (response, error), gasConsumed uint64, diff --git a/x/auth/keeper/deterministic_test.go b/x/auth/keeper/deterministic_test.go index 630fa173fd4..b1dcd78a184 100644 --- a/x/auth/keeper/deterministic_test.go +++ b/x/auth/keeper/deterministic_test.go @@ -120,7 +120,7 @@ func (suite *DeterministicTestSuite) TestGRPCQueryAccount() { rapid.Check(suite.T(), func(t *rapid.T) { accs := suite.createAndSetAccounts(t, 1) req := &types.QueryAccountRequest{Address: accs[0].GetAddress().String()} - testdata.DeterministicIterations(suite.ctx, suite.T(), req, suite.queryClient.Account, 0, true) + testdata.DeterministicIterations(suite.T(), suite.ctx, req, suite.queryClient.Account, 0, true) }) // Regression tests @@ -132,7 +132,7 @@ func (suite *DeterministicTestSuite) TestGRPCQueryAccount() { req := &types.QueryAccountRequest{Address: acc1.GetAddress().String()} - testdata.DeterministicIterations(suite.ctx, suite.T(), req, suite.queryClient.Account, 1543, false) + testdata.DeterministicIterations(suite.T(), suite.ctx, req, suite.queryClient.Account, 1543, false) } // pubkeyGenerator creates and returns a random pubkey generator using rapid. @@ -149,7 +149,7 @@ func (suite *DeterministicTestSuite) TestGRPCQueryAccounts() { accs := suite.createAndSetAccounts(t, numAccs) req := &types.QueryAccountsRequest{Pagination: testdata.PaginationGenerator(t, uint64(numAccs)).Draw(t, "accounts")} - testdata.DeterministicIterations(suite.ctx, suite.T(), req, suite.queryClient.Accounts, 0, true) + testdata.DeterministicIterations(suite.T(), suite.ctx, req, suite.queryClient.Accounts, 0, true) for i := 0; i < numAccs; i++ { suite.accountKeeper.RemoveAccount(suite.ctx, accs[i]) @@ -174,14 +174,14 @@ func (suite *DeterministicTestSuite) TestGRPCQueryAccounts() { suite.accountKeeper.SetAccount(suite.ctx, acc2) req := &types.QueryAccountsRequest{} - testdata.DeterministicIterations(suite.ctx, suite.T(), req, suite.queryClient.Accounts, 1716, false) + testdata.DeterministicIterations(suite.T(), suite.ctx, req, suite.queryClient.Accounts, 1716, false) } func (suite *DeterministicTestSuite) TestGRPCQueryAccountAddressByID() { rapid.Check(suite.T(), func(t *rapid.T) { accs := suite.createAndSetAccounts(t, 1) req := &types.QueryAccountAddressByIDRequest{AccountId: accs[0].GetAccountNumber()} - testdata.DeterministicIterations(suite.ctx, suite.T(), req, suite.queryClient.AccountAddressByID, 0, true) + testdata.DeterministicIterations(suite.T(), suite.ctx, req, suite.queryClient.AccountAddressByID, 0, true) }) // Regression test @@ -192,7 +192,7 @@ func (suite *DeterministicTestSuite) TestGRPCQueryAccountAddressByID() { suite.accountKeeper.SetAccount(suite.ctx, acc1) req := &types.QueryAccountAddressByIDRequest{AccountId: accNum} - testdata.DeterministicIterations(suite.ctx, suite.T(), req, suite.queryClient.AccountAddressByID, 1123, false) + testdata.DeterministicIterations(suite.T(), suite.ctx, req, suite.queryClient.AccountAddressByID, 1123, false) } func (suite *DeterministicTestSuite) TestGRPCQueryParameters() { @@ -208,7 +208,7 @@ func (suite *DeterministicTestSuite) TestGRPCQueryParameters() { suite.Require().NoError(err) req := &types.QueryParamsRequest{} - testdata.DeterministicIterations(suite.ctx, suite.T(), req, suite.queryClient.Params, 0, true) + testdata.DeterministicIterations(suite.T(), suite.ctx, req, suite.queryClient.Params, 0, true) }) // Regression test @@ -218,7 +218,7 @@ func (suite *DeterministicTestSuite) TestGRPCQueryParameters() { suite.Require().NoError(err) req := &types.QueryParamsRequest{} - testdata.DeterministicIterations(suite.ctx, suite.T(), req, suite.queryClient.Params, 1042, false) + testdata.DeterministicIterations(suite.T(), suite.ctx, req, suite.queryClient.Params, 1042, false) } func (suite *DeterministicTestSuite) TestGRPCQueryAccountInfo() { @@ -227,7 +227,7 @@ func (suite *DeterministicTestSuite) TestGRPCQueryAccountInfo() { suite.Require().Len(accs, 1) req := &types.QueryAccountInfoRequest{Address: accs[0].GetAddress().String()} - testdata.DeterministicIterations(suite.ctx, suite.T(), req, suite.queryClient.AccountInfo, 0, true) + testdata.DeterministicIterations(suite.T(), suite.ctx, req, suite.queryClient.AccountInfo, 0, true) }) // Regression test @@ -238,7 +238,7 @@ func (suite *DeterministicTestSuite) TestGRPCQueryAccountInfo() { suite.accountKeeper.SetAccount(suite.ctx, acc) req := &types.QueryAccountInfoRequest{Address: acc.GetAddress().String()} - testdata.DeterministicIterations(suite.ctx, suite.T(), req, suite.queryClient.AccountInfo, 1543, false) + testdata.DeterministicIterations(suite.T(), suite.ctx, req, suite.queryClient.AccountInfo, 1543, false) } func (suite *DeterministicTestSuite) createAndReturnQueryClient(ak keeper.AccountKeeper) types.QueryClient { @@ -301,7 +301,7 @@ func (suite *DeterministicTestSuite) TestGRPCQueryModuleAccounts() { queryClient := suite.createAndReturnQueryClient(ak) req := &types.QueryModuleAccountsRequest{} - testdata.DeterministicIterations(suite.ctx, suite.T(), req, queryClient.ModuleAccounts, 0, true) + testdata.DeterministicIterations(suite.T(), suite.ctx, req, queryClient.ModuleAccounts, 0, true) }) maccs := make([]string, 0, len(suite.maccPerms)) @@ -313,7 +313,7 @@ func (suite *DeterministicTestSuite) TestGRPCQueryModuleAccounts() { queryClient := suite.createAndReturnQueryClient(suite.accountKeeper) req := &types.QueryModuleAccountsRequest{} - testdata.DeterministicIterations(suite.ctx, suite.T(), req, queryClient.ModuleAccounts, 8565, false) + testdata.DeterministicIterations(suite.T(), suite.ctx, req, queryClient.ModuleAccounts, 8565, false) } func (suite *DeterministicTestSuite) TestGRPCQueryModuleAccountByName() { @@ -348,7 +348,7 @@ func (suite *DeterministicTestSuite) TestGRPCQueryModuleAccountByName() { queryClient := suite.createAndReturnQueryClient(ak) req := &types.QueryModuleAccountByNameRequest{Name: mName} - testdata.DeterministicIterations(suite.ctx, suite.T(), req, queryClient.ModuleAccountByName, 0, true) + testdata.DeterministicIterations(suite.T(), suite.ctx, req, queryClient.ModuleAccountByName, 0, true) }) maccs := make([]string, 0, len(suite.maccPerms)) @@ -360,5 +360,5 @@ func (suite *DeterministicTestSuite) TestGRPCQueryModuleAccountByName() { queryClient := suite.createAndReturnQueryClient(suite.accountKeeper) req := &types.QueryModuleAccountByNameRequest{Name: "mint"} - testdata.DeterministicIterations(suite.ctx, suite.T(), req, queryClient.ModuleAccountByName, 1372, false) + testdata.DeterministicIterations(suite.T(), suite.ctx, req, queryClient.ModuleAccountByName, 1372, false) } From 62c708554f98cd6d4968dbc1e3b614b09087bf82 Mon Sep 17 00:00:00 2001 From: bizk Date: Wed, 25 Oct 2023 10:28:54 -0300 Subject: [PATCH 14/21] added t.helper in the respective functions --- testutil/testdata/grpc_query.go | 1 + testutil/testdata/tx.go | 1 + 2 files changed, 2 insertions(+) diff --git a/testutil/testdata/grpc_query.go b/testutil/testdata/grpc_query.go index 75609263454..7a3bbafafce 100644 --- a/testutil/testdata/grpc_query.go +++ b/testutil/testdata/grpc_query.go @@ -73,6 +73,7 @@ func DeterministicIterations[request, response proto.Message]( gasConsumed uint64, gasOverwrite bool, ) { + t.Helper() before := ctx.GasMeter().GasConsumed() prevRes, err := grpcFn(ctx, req) assert.NilError(t, err) diff --git a/testutil/testdata/tx.go b/testutil/testdata/tx.go index 43874da6a95..e75b14b0aee 100644 --- a/testutil/testdata/tx.go +++ b/testutil/testdata/tx.go @@ -45,6 +45,7 @@ func KeyTestPubAddr() (cryptotypes.PrivKey, cryptotypes.PubKey, sdk.AccAddress) // KeyTestPubAddrSecp256R1 generates a new secp256r1 keypair. func KeyTestPubAddrSecp256R1(t *testing.T) (cryptotypes.PrivKey, cryptotypes.PubKey, sdk.AccAddress) { + t.Helper() key, err := secp256r1.GenPrivKey() assert.NilError(t, err) pub := key.PubKey() From 9c460f201bc4fcec5d952644b34b0fa8117dc25d Mon Sep 17 00:00:00 2001 From: bizk Date: Wed, 25 Oct 2023 10:59:14 -0300 Subject: [PATCH 15/21] run gci write on testutil --- testutil/testdata/animal.go | 3 +-- testutil/testdata/codec.go | 3 +-- testutil/testdata/query.pb.go | 7 ++++--- testutil/testdata/testdata.pb.go | 7 ++++--- testutil/testdata/testpb/query.pulsar.go | 7 ++++--- testutil/testdata/testpb/query_grpc.pb.go | 1 + testutil/testdata/testpb/testdata.pulsar.go | 7 ++++--- testutil/testdata/testpb/tx.pulsar.go | 9 +++++---- testutil/testdata/testpb/tx_grpc.pb.go | 1 + testutil/testdata/testpb/unknonwnproto.pulsar.go | 13 +++++++------ testutil/testdata/tx.pb.go | 7 ++++--- testutil/testdata/unknonwnproto.pb.go | 7 ++++--- 12 files changed, 40 insertions(+), 32 deletions(-) diff --git a/testutil/testdata/animal.go b/testutil/testdata/animal.go index 7d5afa61528..b3333dd1406 100644 --- a/testutil/testdata/animal.go +++ b/testutil/testdata/animal.go @@ -3,9 +3,8 @@ package testdata import ( "fmt" - "github.com/cosmos/gogoproto/proto" - "github.com/cosmos/cosmos-sdk/codec/types" + "github.com/cosmos/gogoproto/proto" ) type Animal interface { diff --git a/testutil/testdata/codec.go b/testutil/testdata/codec.go index 6e4dcc253b5..d05d3873410 100644 --- a/testutil/testdata/codec.go +++ b/testutil/testdata/codec.go @@ -1,12 +1,11 @@ package testdata import ( - amino "github.com/tendermint/go-amino" - "github.com/cosmos/cosmos-sdk/codec/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/msgservice" tx "github.com/cosmos/cosmos-sdk/types/tx" + amino "github.com/tendermint/go-amino" ) func NewTestInterfaceRegistry() types.InterfaceRegistry { diff --git a/testutil/testdata/query.pb.go b/testutil/testdata/query.pb.go index d104840c5dc..0bb946d1a9a 100644 --- a/testutil/testdata/query.pb.go +++ b/testutil/testdata/query.pb.go @@ -6,15 +6,16 @@ package testdata import ( context "context" fmt "fmt" + io "io" + math "math" + math_bits "math/bits" + types "github.com/cosmos/cosmos-sdk/codec/types" grpc1 "github.com/cosmos/gogoproto/grpc" proto "github.com/cosmos/gogoproto/proto" grpc "google.golang.org/grpc" codes "google.golang.org/grpc/codes" status "google.golang.org/grpc/status" - io "io" - math "math" - math_bits "math/bits" ) // Reference imports to suppress errors if they are not otherwise used. diff --git a/testutil/testdata/testdata.pb.go b/testutil/testdata/testdata.pb.go index 3db89888b05..93017231c60 100644 --- a/testutil/testdata/testdata.pb.go +++ b/testutil/testdata/testdata.pb.go @@ -5,12 +5,13 @@ package testdata import ( fmt "fmt" - types "github.com/cosmos/cosmos-sdk/codec/types" - _ "github.com/cosmos/gogoproto/gogoproto" - proto "github.com/cosmos/gogoproto/proto" io "io" math "math" math_bits "math/bits" + + types "github.com/cosmos/cosmos-sdk/codec/types" + _ "github.com/cosmos/gogoproto/gogoproto" + proto "github.com/cosmos/gogoproto/proto" ) // Reference imports to suppress errors if they are not otherwise used. diff --git a/testutil/testdata/testpb/query.pulsar.go b/testutil/testdata/testpb/query.pulsar.go index 9b2a04bab5c..f4e9bad1266 100644 --- a/testutil/testdata/testpb/query.pulsar.go +++ b/testutil/testdata/testpb/query.pulsar.go @@ -3,14 +3,15 @@ package testpb import ( fmt "fmt" + io "io" + reflect "reflect" + sync "sync" + runtime "github.com/cosmos/cosmos-proto/runtime" protoreflect "google.golang.org/protobuf/reflect/protoreflect" protoiface "google.golang.org/protobuf/runtime/protoiface" protoimpl "google.golang.org/protobuf/runtime/protoimpl" anypb "google.golang.org/protobuf/types/known/anypb" - io "io" - reflect "reflect" - sync "sync" ) var ( diff --git a/testutil/testdata/testpb/query_grpc.pb.go b/testutil/testdata/testpb/query_grpc.pb.go index 565a93499d4..e01aae23a5c 100644 --- a/testutil/testdata/testpb/query_grpc.pb.go +++ b/testutil/testdata/testpb/query_grpc.pb.go @@ -8,6 +8,7 @@ package testpb import ( context "context" + grpc "google.golang.org/grpc" codes "google.golang.org/grpc/codes" status "google.golang.org/grpc/status" diff --git a/testutil/testdata/testpb/testdata.pulsar.go b/testutil/testdata/testpb/testdata.pulsar.go index 8802ea9cd31..00473c4fd0e 100644 --- a/testutil/testdata/testpb/testdata.pulsar.go +++ b/testutil/testdata/testpb/testdata.pulsar.go @@ -3,15 +3,16 @@ package testpb import ( fmt "fmt" + io "io" + reflect "reflect" + sync "sync" + runtime "github.com/cosmos/cosmos-proto/runtime" _ "github.com/cosmos/gogoproto/gogoproto" protoreflect "google.golang.org/protobuf/reflect/protoreflect" protoiface "google.golang.org/protobuf/runtime/protoiface" protoimpl "google.golang.org/protobuf/runtime/protoimpl" anypb "google.golang.org/protobuf/types/known/anypb" - io "io" - reflect "reflect" - sync "sync" ) var ( diff --git a/testutil/testdata/testpb/tx.pulsar.go b/testutil/testdata/testpb/tx.pulsar.go index 5e4213c4802..d1a26a980dc 100644 --- a/testutil/testdata/testpb/tx.pulsar.go +++ b/testutil/testdata/testpb/tx.pulsar.go @@ -2,17 +2,18 @@ package testpb import ( + fmt "fmt" + io "io" + reflect "reflect" + sync "sync" + _ "cosmossdk.io/api/amino" _ "cosmossdk.io/api/cosmos/msg/v1" - fmt "fmt" runtime "github.com/cosmos/cosmos-proto/runtime" _ "github.com/cosmos/gogoproto/gogoproto" protoreflect "google.golang.org/protobuf/reflect/protoreflect" protoiface "google.golang.org/protobuf/runtime/protoiface" protoimpl "google.golang.org/protobuf/runtime/protoimpl" - io "io" - reflect "reflect" - sync "sync" ) var ( diff --git a/testutil/testdata/testpb/tx_grpc.pb.go b/testutil/testdata/testpb/tx_grpc.pb.go index 3971c84d770..103111c993a 100644 --- a/testutil/testdata/testpb/tx_grpc.pb.go +++ b/testutil/testdata/testpb/tx_grpc.pb.go @@ -8,6 +8,7 @@ package testpb import ( context "context" + grpc "google.golang.org/grpc" codes "google.golang.org/grpc/codes" status "google.golang.org/grpc/status" diff --git a/testutil/testdata/testpb/unknonwnproto.pulsar.go b/testutil/testdata/testpb/unknonwnproto.pulsar.go index 84dbe6fbdb5..5e5c214b36f 100644 --- a/testutil/testdata/testpb/unknonwnproto.pulsar.go +++ b/testutil/testdata/testpb/unknonwnproto.pulsar.go @@ -2,20 +2,21 @@ package testpb import ( - v1beta1 "cosmossdk.io/api/cosmos/tx/v1beta1" binary "encoding/binary" fmt "fmt" + io "io" + math "math" + reflect "reflect" + sort "sort" + sync "sync" + + v1beta1 "cosmossdk.io/api/cosmos/tx/v1beta1" runtime "github.com/cosmos/cosmos-proto/runtime" _ "github.com/cosmos/gogoproto/gogoproto" protoreflect "google.golang.org/protobuf/reflect/protoreflect" protoiface "google.golang.org/protobuf/runtime/protoiface" protoimpl "google.golang.org/protobuf/runtime/protoimpl" anypb "google.golang.org/protobuf/types/known/anypb" - io "io" - math "math" - reflect "reflect" - sort "sort" - sync "sync" ) var ( diff --git a/testutil/testdata/tx.pb.go b/testutil/testdata/tx.pb.go index e2d416faa41..70ed5f9952b 100644 --- a/testutil/testdata/tx.pb.go +++ b/testutil/testdata/tx.pb.go @@ -6,6 +6,10 @@ package testdata import ( context "context" fmt "fmt" + io "io" + math "math" + math_bits "math/bits" + _ "github.com/cosmos/cosmos-sdk/types/msgservice" _ "github.com/cosmos/cosmos-sdk/types/tx/amino" _ "github.com/cosmos/gogoproto/gogoproto" @@ -14,9 +18,6 @@ import ( grpc "google.golang.org/grpc" codes "google.golang.org/grpc/codes" status "google.golang.org/grpc/status" - io "io" - math "math" - math_bits "math/bits" ) // Reference imports to suppress errors if they are not otherwise used. diff --git a/testutil/testdata/unknonwnproto.pb.go b/testutil/testdata/unknonwnproto.pb.go index 3522a725359..69d4059c44e 100644 --- a/testutil/testdata/unknonwnproto.pb.go +++ b/testutil/testdata/unknonwnproto.pb.go @@ -6,13 +6,14 @@ package testdata import ( encoding_binary "encoding/binary" fmt "fmt" + io "io" + math "math" + math_bits "math/bits" + types "github.com/cosmos/cosmos-sdk/codec/types" tx "github.com/cosmos/cosmos-sdk/types/tx" _ "github.com/cosmos/gogoproto/gogoproto" proto "github.com/cosmos/gogoproto/proto" - io "io" - math "math" - math_bits "math/bits" ) // Reference imports to suppress errors if they are not otherwise used. From e9f9f4fbe908d25a5fae54aa348e8f1e8f1b1e86 Mon Sep 17 00:00:00 2001 From: bizk Date: Wed, 25 Oct 2023 11:11:38 -0300 Subject: [PATCH 16/21] run gci write on testutil --- testutil/testdata/animal.go | 3 ++- testutil/testdata/codec.go | 3 ++- testutil/testdata/grpc_query.go | 5 +++-- testutil/testdata/tx.go | 5 +++-- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/testutil/testdata/animal.go b/testutil/testdata/animal.go index b3333dd1406..7d5afa61528 100644 --- a/testutil/testdata/animal.go +++ b/testutil/testdata/animal.go @@ -3,8 +3,9 @@ package testdata import ( "fmt" - "github.com/cosmos/cosmos-sdk/codec/types" "github.com/cosmos/gogoproto/proto" + + "github.com/cosmos/cosmos-sdk/codec/types" ) type Animal interface { diff --git a/testutil/testdata/codec.go b/testutil/testdata/codec.go index d05d3873410..6e4dcc253b5 100644 --- a/testutil/testdata/codec.go +++ b/testutil/testdata/codec.go @@ -1,11 +1,12 @@ package testdata import ( + amino "github.com/tendermint/go-amino" + "github.com/cosmos/cosmos-sdk/codec/types" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/msgservice" tx "github.com/cosmos/cosmos-sdk/types/tx" - amino "github.com/tendermint/go-amino" ) func NewTestInterfaceRegistry() types.InterfaceRegistry { diff --git a/testutil/testdata/grpc_query.go b/testutil/testdata/grpc_query.go index 7a3bbafafce..510e554d433 100644 --- a/testutil/testdata/grpc_query.go +++ b/testutil/testdata/grpc_query.go @@ -5,11 +5,12 @@ import ( "fmt" "testing" - "github.com/cosmos/cosmos-sdk/codec/types" - sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/gogoproto/proto" grpc "google.golang.org/grpc" "gotest.tools/v3/assert" + + "github.com/cosmos/cosmos-sdk/codec/types" + sdk "github.com/cosmos/cosmos-sdk/types" ) // iterCount defines the number of iterations to run on each query to test diff --git a/testutil/testdata/tx.go b/testutil/testdata/tx.go index e75b14b0aee..3abc02d61c6 100644 --- a/testutil/testdata/tx.go +++ b/testutil/testdata/tx.go @@ -3,6 +3,9 @@ package testdata import ( "testing" + "gotest.tools/v3/assert" + "pgregory.net/rapid" + "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" "github.com/cosmos/cosmos-sdk/crypto/keys/secp256r1" @@ -10,8 +13,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/types/query" - "gotest.tools/v3/assert" - "pgregory.net/rapid" ) // AddressGenerator creates and returns a random address generator using rapid. From 277c69c21fdd42865e0a9306b520d75d4e527158 Mon Sep 17 00:00:00 2001 From: bizk Date: Wed, 25 Oct 2023 11:44:43 -0300 Subject: [PATCH 17/21] added coment on test --- x/auth/ante/sigverify_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/auth/ante/sigverify_test.go b/x/auth/ante/sigverify_test.go index 57b6ecc88cb..ca1e144852e 100644 --- a/x/auth/ante/sigverify_test.go +++ b/x/auth/ante/sigverify_test.go @@ -394,7 +394,7 @@ func TestAnteHandlerChecks(t *testing.T) { priv1, _, addr1 := testdata.KeyTestPubAddr() secp256k1NotOnCurve, _ := secp256k1dcrd.GeneratePrivateKey() - secp256k1NotOnCurve.Key.SetInt(0) + secp256k1NotOnCurve.Key.SetInt(0) // Setting the key point to 0, results in an invalid point on the curve. priv12 := &secp256k1.PrivKey{Key: secp256k1NotOnCurve.Serialize()} addr12 := sdk.AccAddress(priv12.PubKey().Address()) From 5dcd350c979ce9095dd646d4b48ba7ec6acbc4b1 Mon Sep 17 00:00:00 2001 From: bizk Date: Thu, 2 Nov 2023 16:33:34 -0300 Subject: [PATCH 18/21] Fixed comments --- x/auth/ante/sigverify.go | 13 ++++++------- x/auth/ante/sigverify_test.go | 4 ++-- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index 2ab87b3f120..d9b98e3cb79 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -4,9 +4,8 @@ import ( "bytes" "encoding/base64" "encoding/hex" + "errors" "fmt" - "strings" - secp256k1dcrd "github.com/decred/dcrd/dcrec/secp256k1/v4" "google.golang.org/protobuf/types/known/anypb" @@ -263,12 +262,10 @@ func OnlyLegacyAminoSigners(sigData signing.SignatureData) bool { func verifyIsOnCurve(pubKey cryptotypes.PubKey) (err error) { switch typedPubKey := pubKey.(type) { - case *ed25519.PubKey: - return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "ED25519 public keys are unsupported") case *secp256k1.PubKey: pubKeyObject, err := secp256k1dcrd.ParsePubKey(typedPubKey.Bytes()) if err != nil { - if strings.Contains(err.Error(), "not on the secp256k1 curve") { + if errors.Is(err, secp256k1dcrd.ErrPubKeyNotOnCurve) { return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "secp256k1 key is not on curve") } return err @@ -276,11 +273,13 @@ func verifyIsOnCurve(pubKey cryptotypes.PubKey) (err error) { if !pubKeyObject.IsOnCurve() { return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "secp256k1 key is not on curve") } + case *secp256r1.PubKey: pubKeyObject := typedPubKey.Key.PublicKey if !pubKeyObject.IsOnCurve(pubKeyObject.X, pubKeyObject.Y) { return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "secp256r1 key is not on curve") } + case multisig.PubKey: pubKeysObjects := typedPubKey.GetPubKeys() ok := true @@ -291,11 +290,11 @@ func verifyIsOnCurve(pubKey cryptotypes.PubKey) (err error) { } } if !ok { - return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "some keys in multisig are not on curve") + return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "some keys are not on curve") } default: - return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "unrecognized public key type") + return errorsmod.Wrapf(sdkerrors.ErrInvalidPubKey, "unsupported key type: %T", typedPubKey) } return nil diff --git a/x/auth/ante/sigverify_test.go b/x/auth/ante/sigverify_test.go index ca1e144852e..066b3bb3fe3 100644 --- a/x/auth/ante/sigverify_test.go +++ b/x/auth/ante/sigverify_test.go @@ -459,9 +459,9 @@ func TestAnteHandlerChecks(t *testing.T) { if tc.shouldErr { require.NotNil(t, err, "TestCase %d: %s did not error as expected", i, tc.name) if tc.suported { - require.ErrorContains(t, err, "is not on curve") + require.ErrorContains(t, err, "not on curve") } else { - require.ErrorContains(t, err, "public keys are unsupported") + require.ErrorContains(t, err, "unsupported public key type") } } else { require.Nil(t, err, "TestCase %d: %s errored unexpectedly. Err: %v", i, tc.name, err) From b79aa2fc6a7c113683cb2dde4e4870058bb8c06a Mon Sep 17 00:00:00 2001 From: bizk Date: Thu, 2 Nov 2023 16:46:07 -0300 Subject: [PATCH 19/21] added changelog.md --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2131b19ca59..af362e01309 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,7 +69,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [#17733](https://github.com/cosmos/cosmos-sdk/pull/17733) Ensure `buf export` exports all proto dependencies * (version) [#18063](https://github.com/cosmos/cosmos-sdk/pull/18063) Include additional information in the Info struct. This change enhances the Info struct by adding support for additional information through the ExtraInfo field * [#18204](https://github.com/cosmos/cosmos-sdk/pull/18204) Use streaming json parser to parse chain-id from genesis file. - +* (crypto | x/auth) [#14372](https://github.com/cosmos/cosmos-sdk/pull/18194) Key checks on signatures antehandle + ### Bug Fixes * (server) [#18254](https://github.com/cosmos/cosmos-sdk/pull/18254) Don't hardcode gRPC address to localhost. From 69f65e04add4ece81616614e1b69c195ac27e5ae Mon Sep 17 00:00:00 2001 From: bizk Date: Thu, 2 Nov 2023 17:22:18 -0300 Subject: [PATCH 20/21] lint fix --- x/auth/ante/sigverify.go | 1 + 1 file changed, 1 insertion(+) diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index d9b98e3cb79..b036846d21d 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -6,6 +6,7 @@ import ( "encoding/hex" "errors" "fmt" + secp256k1dcrd "github.com/decred/dcrd/dcrec/secp256k1/v4" "google.golang.org/protobuf/types/known/anypb" From 12a79afee1d0c690aa21c8c31002a1cd3cd90fbb Mon Sep 17 00:00:00 2001 From: bizk Date: Fri, 3 Nov 2023 10:43:59 -0300 Subject: [PATCH 21/21] small fix on test --- x/auth/ante/sigverify_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/auth/ante/sigverify_test.go b/x/auth/ante/sigverify_test.go index 066b3bb3fe3..42c91d323fe 100644 --- a/x/auth/ante/sigverify_test.go +++ b/x/auth/ante/sigverify_test.go @@ -461,7 +461,7 @@ func TestAnteHandlerChecks(t *testing.T) { if tc.suported { require.ErrorContains(t, err, "not on curve") } else { - require.ErrorContains(t, err, "unsupported public key type") + require.ErrorContains(t, err, "unsupported key type") } } else { require.Nil(t, err, "TestCase %d: %s errored unexpectedly. Err: %v", i, tc.name, err)