From 7bbad196cc8a52a7457fc1eaa1e4e86a5abd36b5 Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Fri, 12 Jun 2020 16:29:49 -0400 Subject: [PATCH 01/23] Update ante handlers to use SignatureV2 --- x/auth/ante/sigverify.go | 109 +++++++++++++++++++++++++++------------ x/auth/signing/verify.go | 40 ++++++++++++++ x/auth/types/stdtx.go | 28 +++++----- 3 files changed, 131 insertions(+), 46 deletions(-) create mode 100644 x/auth/signing/verify.go diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index 29a561479fee..0995ef88f7b7 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -3,12 +3,16 @@ package ante import ( "bytes" "encoding/hex" + "fmt" + + authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" + + "github.com/cosmos/cosmos-sdk/types/tx/signing" "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/ed25519" "github.com/tendermint/tendermint/crypto/secp256k1" - "github.com/cosmos/cosmos-sdk/codec/legacy" "github.com/cosmos/cosmos-sdk/crypto/types/multisig" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" @@ -32,15 +36,15 @@ func init() { // SignatureVerificationGasConsumer is the type of function that is used to both // consume gas when verifying signatures and also to accept or reject different types of pubkeys // This is where apps can define their own PubKey -type SignatureVerificationGasConsumer = func(meter sdk.GasMeter, sig []byte, pubkey crypto.PubKey, params types.Params) error +type SignatureVerificationGasConsumer = func(meter sdk.GasMeter, sig signing.SignatureV2, params types.Params) error // SigVerifiableTx defines a Tx interface for all signature verification decorators type SigVerifiableTx interface { sdk.Tx - GetSignatures() [][]byte GetSigners() []sdk.AccAddress GetPubKeys() []crypto.PubKey // If signer already has pubkey in context, this list will have nil in its place - GetSignBytes(ctx sdk.Context, acc types.AccountI) []byte + GetSignatures() [][]byte + GetSignaturesV2() ([]signing.SignatureV2, error) } // SetPubKeyDecorator sets PubKeys in context for any signer which does not already have pubkey set @@ -120,7 +124,10 @@ func (sgcd SigGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula } params := sgcd.ak.GetParams(ctx) - sigs := sigTx.GetSignatures() + sigs, err := sigTx.GetSignaturesV2() + if err != nil { + return ctx, err + } // stdSigs contains the sequence number, account number, and signatures. // When simulating, this would just be a 0-length slice. @@ -131,18 +138,24 @@ func (sgcd SigGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula if err != nil { return ctx, err } + pubKey := signerAcc.GetPubKey() + // In simulate mode the transaction comes with no signatures, thus if the + // account's pubkey is nil, both signature verification and gasKVStore.Set() + // shall consume the largest amount, i.e. it takes more gas to verify + // secp256k1 keys than ed25519 ones. if simulate && pubKey == nil { - // In simulate mode the transaction comes with no signatures, thus if the - // account's pubkey is nil, both signature verification and gasKVStore.Set() - // shall consume the largest amount, i.e. it takes more gas to verify - // secp256k1 keys than ed25519 ones. - if pubKey == nil { - pubKey = simSecp256k1Pubkey - } + pubKey = simSecp256k1Pubkey + } + + // make a SignatureV2 with PubKey filled in from above + sig = signing.SignatureV2{ + PubKey: pubKey, + Data: sig.Data, } - err = sgcd.sigGasConsumer(ctx.GasMeter(), sig, pubKey, params) + + err = sgcd.sigGasConsumer(ctx.GasMeter(), sig, params) if err != nil { return ctx, err } @@ -157,7 +170,8 @@ func (sgcd SigGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula // CONTRACT: Pubkeys are set in context for all signers before this decorator runs // CONTRACT: Tx must implement SigVerifiableTx interface type SigVerificationDecorator struct { - ak AccountKeeper + ak AccountKeeper + signModeHandler authsigning.SignModeHandler } func NewSigVerificationDecorator(ak AccountKeeper) SigVerificationDecorator { @@ -178,7 +192,10 @@ func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul // stdSigs contains the sequence number, account number, and signatures. // When simulating, this would just be a 0-length slice. - sigs := sigTx.GetSignatures() + sigs, err := sigTx.GetSignaturesV2() + if err != nil { + return ctx, err + } // stdSigs contains the sequence number, account number, and signatures. // When simulating, this would just be a 0-length slice. @@ -191,13 +208,12 @@ func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul } for i, sig := range sigs { - signerAccs[i], err = GetSignerAcc(ctx, svd.ak, signerAddrs[i]) + acc, err := GetSignerAcc(ctx, svd.ak, signerAddrs[i]) if err != nil { return ctx, err } - // retrieve signBytes of tx - signBytes := sigTx.GetSignBytes(ctx, signerAccs[i]) + signerAccs[i] = acc // retrieve pubkey pubKey := signerAccs[i].GetPubKey() @@ -205,11 +221,26 @@ func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidPubKey, "pubkey on account is not set") } - // verify signature - if !simulate && !pubKey.VerifyBytes(signBytes, sig) { - return ctx, sdkerrors.Wrapf( - sdkerrors.ErrUnauthorized, - "signature verification failed; verify correct account sequence (%d) and chain-id (%s)", signerAccs[i].GetSequence(), ctx.ChainID()) + // retrieve signer data + genesis := ctx.BlockHeight() == 0 + chainID := ctx.ChainID() + var accNum uint64 + if !genesis { + accNum = acc.GetAccountNumber() + } + signerData := authsigning.SignerData{ + ChainID: chainID, + AccountNumber: accNum, + AccountSequence: acc.GetSequence(), + } + + if !simulate { + err := authsigning.VerifySignature(pubKey, signerData, sig.Data, svd.signModeHandler, tx) + if err != nil { + return ctx, sdkerrors.Wrapf( + sdkerrors.ErrUnauthorized, + "signature verification failed; verify correct account sequence (%d) and chain-id (%s)", signerAccs[i].GetSequence(), ctx.ChainID()) + } } } @@ -293,8 +324,9 @@ func (vscd ValidateSigCountDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, sim // for signature verification based upon the public key type. The cost is fetched from the given params and is matched // by the concrete type. func DefaultSigVerificationGasConsumer( - meter sdk.GasMeter, sig []byte, pubkey crypto.PubKey, params types.Params, + meter sdk.GasMeter, sig signing.SignatureV2, params types.Params, ) error { + pubkey := sig.PubKey switch pubkey := pubkey.(type) { case ed25519.PubKeyEd25519: @@ -305,11 +337,15 @@ func DefaultSigVerificationGasConsumer( meter.ConsumeGas(params.SigVerifyCostSecp256k1, "ante verify: secp256k1") return nil - case multisig.PubKeyMultisigThreshold: - var multisignature multisig.AminoMultisignature - legacy.Cdc.MustUnmarshalBinaryBare(sig, &multisignature) - - ConsumeMultisignatureVerificationGas(meter, multisignature, pubkey, params) + case multisig.PubKey: + multisignature, ok := sig.Data.(*signing.MultiSignatureData) + if !ok { + return fmt.Errorf("expected %T, got, %T", &signing.MultiSignatureData{}, sig.Data) + } + err := ConsumeMultisignatureVerificationGas(meter, multisignature, pubkey, params) + if err != nil { + return err + } return nil default: @@ -319,18 +355,27 @@ func DefaultSigVerificationGasConsumer( // ConsumeMultisignatureVerificationGas consumes gas from a GasMeter for verifying a multisig pubkey signature func ConsumeMultisignatureVerificationGas( - meter sdk.GasMeter, sig multisig.AminoMultisignature, pubkey multisig.PubKeyMultisigThreshold, params types.Params, -) { + meter sdk.GasMeter, sig *signing.MultiSignatureData, pubkey multisig.PubKey, params types.Params, +) error { size := sig.BitArray.Size() sigIndex := 0 for i := 0; i < size; i++ { if sig.BitArray.GetIndex(i) { - DefaultSigVerificationGasConsumer(meter, sig.Sigs[sigIndex], pubkey.PubKeys[i], params) + sigV2 := signing.SignatureV2{ + PubKey: pubkey.GetPubKeys()[i], + Data: sig.Signatures[sigIndex], + } + err := DefaultSigVerificationGasConsumer(meter, sigV2, params) + if err != nil { + return err + } sigIndex++ } } + + return nil } // GetSignerAcc returns an account for a given address that is expected to sign diff --git a/x/auth/signing/verify.go b/x/auth/signing/verify.go new file mode 100644 index 000000000000..fb834dc31c62 --- /dev/null +++ b/x/auth/signing/verify.go @@ -0,0 +1,40 @@ +package signing + +import ( + "fmt" + + "github.com/tendermint/tendermint/crypto" + + "github.com/cosmos/cosmos-sdk/crypto/types/multisig" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/tx/signing" +) + +func VerifySignature(pubKey crypto.PubKey, signingData SignerData, sigData signing.SignatureData, handler SignModeHandler, tx sdk.Tx) error { + switch data := sigData.(type) { + case *signing.SingleSignatureData: + signBytes, err := handler.GetSignBytes(data.SignMode, signingData, tx) + if err != nil { + return err + } + if !pubKey.VerifyBytes(signBytes, data.Signature) { + return fmt.Errorf("unable to verify single signer signature") + } + return nil + + case *signing.MultiSignatureData: + multiPK, ok := pubKey.(multisig.PubKey) + if !ok { + return fmt.Errorf("expected %T, got %T", (multisig.PubKey)(nil), pubKey) + } + err := multiPK.VerifyMultisignature(func(mode signing.SignMode) ([]byte, error) { + return handler.GetSignBytes(mode, signingData, tx) + }, data) + if err != nil { + return err + } + return nil + default: + return fmt.Errorf("unexpected SignatureData %T", sigData) + } +} diff --git a/x/auth/types/stdtx.go b/x/auth/types/stdtx.go index c81c3cd91cbb..40d2baaecf65 100644 --- a/x/auth/types/stdtx.go +++ b/x/auth/types/stdtx.go @@ -235,6 +235,20 @@ func (tx StdTx) GetSignatures() [][]byte { return sigs } +func (tx StdTx) GetSignaturesV2() ([]signing.SignatureV2, error) { + res := make([]signing.SignatureV2, len(tx.Signatures)) + + for i, sig := range tx.Signatures { + var err error + res[i], err = StdSignatureToSignatureV2(legacy.Cdc, sig) + if err != nil { + return nil, err + } + } + + return res, nil +} + // GetPubkeys returns the pubkeys of signers if the pubkey is included in the signature // If pubkey is not included in the signature, then nil is in the slice instead func (tx StdTx) GetPubKeys() []crypto.PubKey { @@ -247,20 +261,6 @@ func (tx StdTx) GetPubKeys() []crypto.PubKey { return pks } -// GetSignBytes returns the signBytes of the tx for a given signer -func (tx StdTx) GetSignBytes(ctx sdk.Context, acc AccountI) []byte { - genesis := ctx.BlockHeight() == 0 - chainID := ctx.ChainID() - var accNum uint64 - if !genesis { - accNum = acc.GetAccountNumber() - } - - return StdSignBytes( - chainID, accNum, acc.GetSequence(), tx.Fee, tx.Msgs, tx.Memo, - ) -} - // GetGas returns the Gas in StdFee func (tx StdTx) GetGas() uint64 { return tx.Fee.Gas } From d754371b1ac978b08bdac2e58b8dee9d9cae4aa9 Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Mon, 15 Jun 2020 14:28:15 -0400 Subject: [PATCH 02/23] WIP on test fixes --- x/auth/ante/ante.go | 3 ++- x/auth/ante/ante_test.go | 6 ++++-- x/auth/ante/sigverify.go | 5 +++-- x/auth/ante/sigverify_test.go | 14 +++++++++----- 4 files changed, 18 insertions(+), 10 deletions(-) diff --git a/x/auth/ante/ante.go b/x/auth/ante/ante.go index ab9e804b958f..9debe1a8b6b6 100644 --- a/x/auth/ante/ante.go +++ b/x/auth/ante/ante.go @@ -2,6 +2,7 @@ package ante import ( sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/auth/signing/amino" "github.com/cosmos/cosmos-sdk/x/auth/types" ibcante "github.com/cosmos/cosmos-sdk/x/ibc/ante" ibckeeper "github.com/cosmos/cosmos-sdk/x/ibc/keeper" @@ -24,7 +25,7 @@ func NewAnteHandler( NewValidateSigCountDecorator(ak), NewDeductFeeDecorator(ak, bankKeeper), NewSigGasConsumeDecorator(ak, sigGasConsumer), - NewSigVerificationDecorator(ak), + NewSigVerificationDecorator(ak, amino.LegacyAminoJSONHandler{}), NewIncrementSequenceDecorator(ak), ibcante.NewProofVerificationDecorator(ibcKeeper.ClientKeeper, ibcKeeper.ChannelKeeper), // innermost AnteDecorator ) diff --git a/x/auth/ante/ante_test.go b/x/auth/ante/ante_test.go index bff57793df30..970c4196fea4 100644 --- a/x/auth/ante/ante_test.go +++ b/x/auth/ante/ante_test.go @@ -8,6 +8,8 @@ import ( "strings" "testing" + "github.com/cosmos/cosmos-sdk/types/tx/signing" + "github.com/stretchr/testify/require" "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/ed25519" @@ -703,8 +705,8 @@ func TestCustomSignatureVerificationGasConsumer(t *testing.T) { app, ctx := createTestApp(true) ctx = ctx.WithBlockHeight(1) // setup an ante handler that only accepts PubKeyEd25519 - anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, func(meter sdk.GasMeter, sig []byte, pubkey crypto.PubKey, params types.Params) error { - switch pubkey := pubkey.(type) { + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, func(meter sdk.GasMeter, sig signing.SignatureV2, params types.Params) error { + switch pubkey := sig.PubKey.(type) { case ed25519.PubKeyEd25519: meter.ConsumeGas(params.SigVerifyCostED25519, "ante verify: ed25519") return nil diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index 0995ef88f7b7..1842be6b1ba8 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -174,9 +174,10 @@ type SigVerificationDecorator struct { signModeHandler authsigning.SignModeHandler } -func NewSigVerificationDecorator(ak AccountKeeper) SigVerificationDecorator { +func NewSigVerificationDecorator(ak AccountKeeper, signModeHandler authsigning.SignModeHandler) SigVerificationDecorator { return SigVerificationDecorator{ - ak: ak, + ak: ak, + signModeHandler: signModeHandler, } } diff --git a/x/auth/ante/sigverify_test.go b/x/auth/ante/sigverify_test.go index 33781020af06..3b360d0b2a5e 100644 --- a/x/auth/ante/sigverify_test.go +++ b/x/auth/ante/sigverify_test.go @@ -4,6 +4,8 @@ import ( "fmt" "testing" + "github.com/cosmos/cosmos-sdk/types/tx/signing" + "github.com/cosmos/cosmos-sdk/simapp" "github.com/stretchr/testify/require" @@ -73,12 +75,10 @@ func TestConsumeSignatureVerificationGas(t *testing.T) { err = multisig.AddSignatureV2(multisignature1, sigV2, pkSet1) require.NoError(t, err) } - aminoMultisignature1, err := types.SignatureDataToAminoSignature(cdc, multisignature1) - require.NoError(t, err) type args struct { meter sdk.GasMeter - sig []byte + sig signing.SignatureData pubkey crypto.PubKey params types.Params } @@ -90,13 +90,17 @@ func TestConsumeSignatureVerificationGas(t *testing.T) { }{ {"PubKeyEd25519", args{sdk.NewInfiniteGasMeter(), nil, ed25519.GenPrivKey().PubKey(), params}, types.DefaultSigVerifyCostED25519, true}, {"PubKeySecp256k1", args{sdk.NewInfiniteGasMeter(), nil, secp256k1.GenPrivKey().PubKey(), params}, types.DefaultSigVerifyCostSecp256k1, false}, - {"Multisig", args{sdk.NewInfiniteGasMeter(), aminoMultisignature1, multisigKey1, params}, expectedCost1, false}, + {"Multisig", args{sdk.NewInfiniteGasMeter(), multisignature1, multisigKey1, params}, expectedCost1, false}, {"unknown key", args{sdk.NewInfiniteGasMeter(), nil, nil, params}, 0, true}, } for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - err := ante.DefaultSigVerificationGasConsumer(tt.args.meter, tt.args.sig, tt.args.pubkey, tt.args.params) + sigV2 := signing.SignatureV2{ + PubKey: tt.args.pubkey, + Data: tt.args.sig, + } + err := ante.DefaultSigVerificationGasConsumer(tt.args.meter, sigV2, tt.args.params) if tt.shouldErr { require.NotNil(t, err) From 30b23cc5be20540d9524b9bdab637f630f336fb0 Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Mon, 15 Jun 2020 15:31:17 -0400 Subject: [PATCH 03/23] WIP on test fixes --- simapp/app.go | 3 +++ x/auth/ante/ante.go | 5 +++-- x/auth/ante/ante_test.go | 28 +++++++++++++++------------- x/auth/ante/sigverify_test.go | 6 ++++-- 4 files changed, 25 insertions(+), 17 deletions(-) diff --git a/simapp/app.go b/simapp/app.go index da8a19b075eb..261b6cf06b43 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -4,6 +4,8 @@ import ( "io" "os" + "github.com/cosmos/cosmos-sdk/x/auth/signing/amino" + abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/libs/log" tmos "github.com/tendermint/tendermint/libs/os" @@ -372,6 +374,7 @@ func NewSimApp( app.SetAnteHandler( ante.NewAnteHandler( app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, + amino.LegacyAminoJSONHandler{}, ), ) app.SetEndBlocker(app.EndBlocker) diff --git a/x/auth/ante/ante.go b/x/auth/ante/ante.go index 9debe1a8b6b6..6e62dbea63cf 100644 --- a/x/auth/ante/ante.go +++ b/x/auth/ante/ante.go @@ -2,7 +2,7 @@ package ante import ( sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/auth/signing/amino" + "github.com/cosmos/cosmos-sdk/x/auth/signing" "github.com/cosmos/cosmos-sdk/x/auth/types" ibcante "github.com/cosmos/cosmos-sdk/x/ibc/ante" ibckeeper "github.com/cosmos/cosmos-sdk/x/ibc/keeper" @@ -14,6 +14,7 @@ import ( func NewAnteHandler( ak AccountKeeper, bankKeeper types.BankKeeper, ibcKeeper ibckeeper.Keeper, sigGasConsumer SignatureVerificationGasConsumer, + signModeHandler signing.SignModeHandler, ) sdk.AnteHandler { return sdk.ChainAnteDecorators( NewSetUpContextDecorator(), // outermost AnteDecorator. SetUpContext must be called first @@ -25,7 +26,7 @@ func NewAnteHandler( NewValidateSigCountDecorator(ak), NewDeductFeeDecorator(ak, bankKeeper), NewSigGasConsumeDecorator(ak, sigGasConsumer), - NewSigVerificationDecorator(ak, amino.LegacyAminoJSONHandler{}), + NewSigVerificationDecorator(ak, signModeHandler), NewIncrementSequenceDecorator(ak), ibcante.NewProofVerificationDecorator(ibcKeeper.ClientKeeper, ibcKeeper.ChannelKeeper), // innermost AnteDecorator ) diff --git a/x/auth/ante/ante_test.go b/x/auth/ante/ante_test.go index 970c4196fea4..6befbac1002b 100644 --- a/x/auth/ante/ante_test.go +++ b/x/auth/ante/ante_test.go @@ -8,6 +8,8 @@ import ( "strings" "testing" + "github.com/cosmos/cosmos-sdk/x/auth/signing/amino" + "github.com/cosmos/cosmos-sdk/types/tx/signing" "github.com/stretchr/testify/require" @@ -40,7 +42,7 @@ func TestSimulateGasCost(t *testing.T) { // setup app, ctx := createTestApp(true) ctx = ctx.WithBlockHeight(1) - anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, amino.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -94,7 +96,7 @@ func TestSimulateGasCost(t *testing.T) { func TestAnteHandlerSigErrors(t *testing.T) { // setup app, ctx := createTestApp(true) - anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, amino.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -144,7 +146,7 @@ func TestAnteHandlerAccountNumbers(t *testing.T) { // setup app, ctx := createTestApp(false) ctx = ctx.WithBlockHeight(1) - anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, amino.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -203,7 +205,7 @@ func TestAnteHandlerAccountNumbersAtBlockHeightZero(t *testing.T) { // setup app, ctx := createTestApp(false) ctx = ctx.WithBlockHeight(0) - anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, amino.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -261,7 +263,7 @@ func TestAnteHandlerSequences(t *testing.T) { // setup app, ctx := createTestApp(false) ctx = ctx.WithBlockHeight(1) - anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, amino.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -337,7 +339,7 @@ func TestAnteHandlerSequences(t *testing.T) { func TestAnteHandlerFees(t *testing.T) { // setup app, ctx := createTestApp(true) - anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, amino.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -379,7 +381,7 @@ func TestAnteHandlerMemoGas(t *testing.T) { // setup app, ctx := createTestApp(true) ctx = ctx.WithBlockHeight(1) - anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, amino.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -419,7 +421,7 @@ func TestAnteHandlerMultiSigner(t *testing.T) { // setup app, ctx := createTestApp(false) ctx = ctx.WithBlockHeight(1) - anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, amino.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -469,7 +471,7 @@ func TestAnteHandlerBadSignBytes(t *testing.T) { // setup app, ctx := createTestApp(true) ctx = ctx.WithBlockHeight(1) - anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, amino.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -546,7 +548,7 @@ func TestAnteHandlerSetPubKey(t *testing.T) { // setup app, ctx := createTestApp(true) ctx = ctx.WithBlockHeight(1) - anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, amino.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -664,7 +666,7 @@ func TestAnteHandlerSigLimitExceeded(t *testing.T) { // setup app, ctx := createTestApp(true) ctx = ctx.WithBlockHeight(1) - anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, amino.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -713,7 +715,7 @@ func TestCustomSignatureVerificationGasConsumer(t *testing.T) { default: return sdkerrors.Wrapf(sdkerrors.ErrInvalidPubKey, "unrecognized public key type: %T", pubkey) } - }) + }, amino.LegacyAminoJSONHandler{}) // verify that an secp256k1 account gets rejected priv1, _, addr1 := types.KeyTestPubAddr() @@ -763,7 +765,7 @@ func TestAnteHandlerReCheck(t *testing.T) { app.AccountKeeper.SetAccount(ctx, acc1) app.BankKeeper.SetBalances(ctx, addr1, types.NewTestCoins()) - antehandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer) + antehandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, amino.LegacyAminoJSONHandler{}) // test that operations skipped on recheck do not run diff --git a/x/auth/ante/sigverify_test.go b/x/auth/ante/sigverify_test.go index 3b360d0b2a5e..9d0f24c3a859 100644 --- a/x/auth/ante/sigverify_test.go +++ b/x/auth/ante/sigverify_test.go @@ -4,6 +4,8 @@ import ( "fmt" "testing" + "github.com/cosmos/cosmos-sdk/x/auth/signing/amino" + "github.com/cosmos/cosmos-sdk/types/tx/signing" "github.com/cosmos/cosmos-sdk/simapp" @@ -137,7 +139,7 @@ func TestSigVerification(t *testing.T) { fee := types.NewTestStdFee() spkd := ante.NewSetPubKeyDecorator(app.AccountKeeper) - svd := ante.NewSigVerificationDecorator(app.AccountKeeper) + svd := ante.NewSigVerificationDecorator(app.AccountKeeper, amino.LegacyAminoJSONHandler{}) antehandler := sdk.ChainAnteDecorators(spkd, svd) type testCase struct { @@ -214,7 +216,7 @@ func runSigDecorators(t *testing.T, params types.Params, _ bool, privs ...crypto spkd := ante.NewSetPubKeyDecorator(app.AccountKeeper) svgc := ante.NewSigGasConsumeDecorator(app.AccountKeeper, ante.DefaultSigVerificationGasConsumer) - svd := ante.NewSigVerificationDecorator(app.AccountKeeper) + svd := ante.NewSigVerificationDecorator(app.AccountKeeper, amino.LegacyAminoJSONHandler{}) antehandler := sdk.ChainAnteDecorators(spkd, svgc, svd) // Determine gas consumption of antehandler with default params From 08e4eeced0140470ab800834467461109c597985 Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Mon, 15 Jun 2020 15:54:29 -0400 Subject: [PATCH 04/23] Debugging CLI Tests --- CHANGELOG.md | 3 +++ x/auth/ante/ante_test.go | 16 ++++++++++------ x/auth/signing/verify.go | 2 ++ x/auth/types/stdtx.go | 1 + 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e2c3d552c89..fa5a00da0647 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -122,6 +122,9 @@ be used to retrieve the actual proposal `Content`. Also the `NewMsgSubmitProposa * (modules) [\#6326](https://github.com/cosmos/cosmos-sdk/pull/6326) `AppModuleBasic.GetQueryCmd` now takes a single `CLIContext` parameter. * (modules) [\#6336](https://github.com/cosmos/cosmos-sdk/pull/6336) `AppModuleBasic.RegisterQueryService` method was added to support gRPC queries, and `QuerierRoute` and `NewQuerierHandler` were deprecated. * (modules) [\#6311](https://github.com/cosmos/cosmos-sdk/issues/6311) Remove `alias.go` usage +* (x/auth) [\#6428](https://github.com/cosmos/cosmos-sdk/issues/6428) `NewAnteHandler` and `NewSigVerificationDecorator` both now take a `SignModeHandler` parameter. +`SignatureVerificationGasConsumer` now has the signature: `func(meter sdk.GasMeter, sig signing.SignatureV2, params types.Params) error`. +The `SigVerifiableTx` interface now has a `GetSignaturesV2() ([]signing.SignatureV2, error)` method and no longer has `GetSignatures` and `GetSignBytes` methods. Migration guide: diff --git a/x/auth/ante/ante_test.go b/x/auth/ante/ante_test.go index 6befbac1002b..a79e7bd09233 100644 --- a/x/auth/ante/ante_test.go +++ b/x/auth/ante/ante_test.go @@ -4,7 +4,6 @@ import ( "encoding/json" "errors" "fmt" - "math/rand" "strings" "testing" @@ -601,11 +600,16 @@ func generatePubKeysAndSignatures(n int, msg []byte, _ bool) (pubkeys []crypto.P signatures = make([][]byte, n) for i := 0; i < n; i++ { var privkey crypto.PrivKey - if rand.Int63()%2 == 0 { - privkey = ed25519.GenPrivKey() - } else { - privkey = secp256k1.GenPrivKey() - } + privkey = secp256k1.GenPrivKey() + + // TODO: also generate ed25519 keys as below when ed25519 keys are actually supported, + // for now this fails: + //if rand.Int63()%2 == 0 { + // privkey = ed25519.GenPrivKey() + //} else { + // privkey = secp256k1.GenPrivKey() + //} + pubkeys[i] = privkey.PubKey() signatures[i], _ = privkey.Sign(msg) } diff --git a/x/auth/signing/verify.go b/x/auth/signing/verify.go index fb834dc31c62..99a6d39b48f0 100644 --- a/x/auth/signing/verify.go +++ b/x/auth/signing/verify.go @@ -10,6 +10,8 @@ import ( "github.com/cosmos/cosmos-sdk/types/tx/signing" ) +// VerifySignature verifies a transaction signature contained in SignatureData abstracting over different signing modes +// and single vs multi-signatures. func VerifySignature(pubKey crypto.PubKey, signingData SignerData, sigData signing.SignatureData, handler SignModeHandler, tx sdk.Tx) error { switch data := sigData.(type) { case *signing.SingleSignatureData: diff --git a/x/auth/types/stdtx.go b/x/auth/types/stdtx.go index 40d2baaecf65..b453a564ebd1 100644 --- a/x/auth/types/stdtx.go +++ b/x/auth/types/stdtx.go @@ -235,6 +235,7 @@ func (tx StdTx) GetSignatures() [][]byte { return sigs } +// GetSignaturesV2 implements SigVerifiableTx.GetSignaturesV2 func (tx StdTx) GetSignaturesV2() ([]signing.SignatureV2, error) { res := make([]signing.SignatureV2, len(tx.Signatures)) From 5ea5719afae79dd9c1910556ac7e23879118ec03 Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Mon, 15 Jun 2020 15:56:19 -0400 Subject: [PATCH 05/23] CHANGELOG.md --- CHANGELOG.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fa5a00da0647..12fa40ab0e6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -122,9 +122,7 @@ be used to retrieve the actual proposal `Content`. Also the `NewMsgSubmitProposa * (modules) [\#6326](https://github.com/cosmos/cosmos-sdk/pull/6326) `AppModuleBasic.GetQueryCmd` now takes a single `CLIContext` parameter. * (modules) [\#6336](https://github.com/cosmos/cosmos-sdk/pull/6336) `AppModuleBasic.RegisterQueryService` method was added to support gRPC queries, and `QuerierRoute` and `NewQuerierHandler` were deprecated. * (modules) [\#6311](https://github.com/cosmos/cosmos-sdk/issues/6311) Remove `alias.go` usage -* (x/auth) [\#6428](https://github.com/cosmos/cosmos-sdk/issues/6428) `NewAnteHandler` and `NewSigVerificationDecorator` both now take a `SignModeHandler` parameter. -`SignatureVerificationGasConsumer` now has the signature: `func(meter sdk.GasMeter, sig signing.SignatureV2, params types.Params) error`. -The `SigVerifiableTx` interface now has a `GetSignaturesV2() ([]signing.SignatureV2, error)` method and no longer has `GetSignatures` and `GetSignBytes` methods. +* (x/auth) [\#6311](https://github.com/cosmos/cosmos-sdk/issues/6311) Remove `alias.go` usage Migration guide: @@ -143,6 +141,10 @@ The `SigVerifiableTx` interface now has a `GetSignaturesV2() ([]signing.Signatur * (codec) [\#6330](https://github.com/cosmos/cosmos-sdk/pull/6330) `codec.RegisterCrypto` has been moved to the `crypto/codec` package and the global `codec.Cdc` Amino instance has been deprecated and moved to the `codec/legacy_global` package. * (x/ibc) [\#6374](https://github.com/cosmos/cosmos-sdk/pull/6374) `VerifyMembership` and `VerifyNonMembership` now take a `specs []string` argument to specify the proof format used for verification. Most SDK chains can simply use `commitmenttypes.GetSDKSpecs()` for this argument. * (crypto/types/multisig) [\#6373](https://github.com/cosmos/cosmos-sdk/pull/6373) `multisig.Multisignature` has been renamed to `AminoMultisignature` +* (x/auth) [\#6428](https://github.com/cosmos/cosmos-sdk/issues/6428): + * `NewAnteHandler` and `NewSigVerificationDecorator` both now take a `SignModeHandler` parameter. + * `SignatureVerificationGasConsumer` now has the signature: `func(meter sdk.GasMeter, sig signing.SignatureV2, params types.Params) error`. + * The `SigVerifiableTx` interface now has a `GetSignaturesV2() ([]signing.SignatureV2, error)` method and no longer has `GetSignatures` and `GetSignBytes` methods. ### Features From 82f0262db4a2c4a4cd24e37783c855c88c4579f8 Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Mon, 15 Jun 2020 15:57:56 -0400 Subject: [PATCH 06/23] CHANGELOG --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12fa40ab0e6b..add87b2140c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -122,7 +122,6 @@ be used to retrieve the actual proposal `Content`. Also the `NewMsgSubmitProposa * (modules) [\#6326](https://github.com/cosmos/cosmos-sdk/pull/6326) `AppModuleBasic.GetQueryCmd` now takes a single `CLIContext` parameter. * (modules) [\#6336](https://github.com/cosmos/cosmos-sdk/pull/6336) `AppModuleBasic.RegisterQueryService` method was added to support gRPC queries, and `QuerierRoute` and `NewQuerierHandler` were deprecated. * (modules) [\#6311](https://github.com/cosmos/cosmos-sdk/issues/6311) Remove `alias.go` usage -* (x/auth) [\#6311](https://github.com/cosmos/cosmos-sdk/issues/6311) Remove `alias.go` usage Migration guide: From ca7464fe0d213af881d9a45ddad6bb3ceec1bbe8 Mon Sep 17 00:00:00 2001 From: sahith-narahari Date: Tue, 16 Jun 2020 22:17:23 +0530 Subject: [PATCH 07/23] Add missing tests for ante --- x/auth/ante/sigverify.go | 4 ++- x/auth/signing/verify_test.go | 66 +++++++++++++++++++++++++++++++++++ x/auth/types/stdtx_test.go | 20 +++++++++++ 3 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 x/auth/signing/verify_test.go diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index 1842be6b1ba8..bacf5b117659 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -363,7 +363,9 @@ func ConsumeMultisignatureVerificationGas( sigIndex := 0 for i := 0; i < size; i++ { - if sig.BitArray.GetIndex(i) { + if !sig.BitArray.GetIndex(i) { + continue + } else { sigV2 := signing.SignatureV2{ PubKey: pubkey.GetPubKeys()[i], Data: sig.Signatures[sigIndex], diff --git a/x/auth/signing/verify_test.go b/x/auth/signing/verify_test.go new file mode 100644 index 000000000000..475b05fc1029 --- /dev/null +++ b/x/auth/signing/verify_test.go @@ -0,0 +1,66 @@ +package signing_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + abci "github.com/tendermint/tendermint/abci/types" + + "github.com/cosmos/cosmos-sdk/codec" + "github.com/cosmos/cosmos-sdk/simapp" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/auth/ante" + "github.com/cosmos/cosmos-sdk/x/auth/signing" + "github.com/cosmos/cosmos-sdk/x/auth/types" +) + +func TestVerifySignature(t *testing.T) { + msg := []byte{1, 2, 3, 4} + _, pubKey, _ := types.KeyTestPubAddr() + addr := sdk.AccAddress(pubKey.Address()) + app, ctx := createTestApp(false) + ctx = ctx.WithBlockHeight(1) + + cdc := codec.New() + sdk.RegisterCodec(cdc) + types.RegisterCodec(cdc) + + acc1 := app.AccountKeeper.NewAccountWithAddress(ctx, addr) + app.AccountKeeper.SetAccount(ctx, acc1) + balances := sdk.NewCoins(sdk.NewInt64Coin("atom", 200)) + require.NoError(t, app.BankKeeper.SetBalances(ctx, addr, balances)) + + fee := types.NewStdFee(50000, sdk.Coins{sdk.NewInt64Coin("atom", 150)}) + sig := types.StdSignature{PubKey: pubKey.Bytes(), Signature: msg} + stdTx := types.NewStdTx([]sdk.Msg{types.NewTestMsg(addr)}, fee, []types.StdSignature{sig}, "testsigs") + + acc, err := ante.GetSignerAcc(ctx, app.AccountKeeper, addr) + require.Nil(t, err) + genesis := ctx.BlockHeight() == 0 + chainID := ctx.ChainID() + var accNum uint64 + if !genesis { + accNum = acc.GetAccountNumber() + } + signerData := signing.SignerData{ + ChainID: chainID, + AccountNumber: accNum, + AccountSequence: acc.GetSequence(), + } + + sigV2, err := types.StdSignatureToSignatureV2(cdc, sig) + handler := MakeTestHandlerMap() + + err = signing.VerifySignature(pubKey, signerData, sigV2.Data, handler, stdTx) + t.Log(err) + require.NoError(t, err) +} + +// returns context and app with params set on account keeper +func createTestApp(isCheckTx bool) (*simapp.SimApp, sdk.Context) { + app := simapp.Setup(isCheckTx) + ctx := app.BaseApp.NewContext(isCheckTx, abci.Header{}) + app.AccountKeeper.SetParams(ctx, types.DefaultParams()) + + return app, ctx +} diff --git a/x/auth/types/stdtx_test.go b/x/auth/types/stdtx_test.go index df43ccda9268..995db6b47b76 100644 --- a/x/auth/types/stdtx_test.go +++ b/x/auth/types/stdtx_test.go @@ -227,3 +227,23 @@ func TestSignatureV2Conversions(t *testing.T) { require.Equal(t, multiPK, sigV2.PubKey) require.Equal(t, msigData, sigV2.Data) } + +func TestGetSignaturesV2(t *testing.T) { + _, pubKey, _ := KeyTestPubAddr() + dummy := []byte("dummySig") + + cdc := codec.New() + sdk.RegisterCodec(cdc) + RegisterCodec(cdc) + + fee := NewStdFee(50000, sdk.Coins{sdk.NewInt64Coin("atom", 150)}) + sig := StdSignature{PubKey: pubKey.Bytes(), Signature: dummy} + stdTx := NewStdTx([]sdk.Msg{NewTestMsg()}, fee, []StdSignature{sig}, "testsigs") + + sigs, err := stdTx.GetSignaturesV2() + require.Nil(t, err) + require.Equal(t, len(sigs), 1) + + sigV2, err := StdSignatureToSignatureV2(cdc, sig) + require.Equal(t, sigs[0], sigV2) +} From 311386074e0710d3fc175d78c3f030088e17d67a Mon Sep 17 00:00:00 2001 From: sahith-narahari Date: Wed, 17 Jun 2020 19:03:43 +0530 Subject: [PATCH 08/23] Add tests for verify signatures --- x/auth/signing/verify_test.go | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/x/auth/signing/verify_test.go b/x/auth/signing/verify_test.go index 475b05fc1029..356d24bd4b12 100644 --- a/x/auth/signing/verify_test.go +++ b/x/auth/signing/verify_test.go @@ -15,7 +15,6 @@ import ( ) func TestVerifySignature(t *testing.T) { - msg := []byte{1, 2, 3, 4} _, pubKey, _ := types.KeyTestPubAddr() addr := sdk.AccAddress(pubKey.Address()) app, ctx := createTestApp(false) @@ -30,11 +29,19 @@ func TestVerifySignature(t *testing.T) { balances := sdk.NewCoins(sdk.NewInt64Coin("atom", 200)) require.NoError(t, app.BankKeeper.SetBalances(ctx, addr, balances)) + acc, err := ante.GetSignerAcc(ctx, app.AccountKeeper, addr) + msgs := []sdk.Msg{types.NewTestMsg(addr)} + fee := types.NewStdFee(50000, sdk.Coins{sdk.NewInt64Coin("atom", 150)}) - sig := types.StdSignature{PubKey: pubKey.Bytes(), Signature: msg} - stdTx := types.NewStdTx([]sdk.Msg{types.NewTestMsg(addr)}, fee, []types.StdSignature{sig}, "testsigs") + txBuilder := types.NewTxBuilder(types.DefaultTxEncoder(codec.New()), acc.GetAccountNumber(), acc.GetSequence(), + 200000, 1.1, false, "test-chain", "hello", sdk.NewCoins(), + sdk.DecCoins{sdk.NewDecCoinFromDec(sdk.DefaultBondDenom, sdk.NewDecWithPrec(10000, sdk.Precision))}, + ) + signBytes, err := txBuilder.BuildSignMsg(msgs) + sign, err := txBuilder.Sign("addr", signBytes) + stdSig := types.StdSignature{PubKey: pubKey.Bytes(), Signature: sign} + stdTx := types.NewStdTx(msgs, fee, []types.StdSignature{}, "testsigs") - acc, err := ante.GetSignerAcc(ctx, app.AccountKeeper, addr) require.Nil(t, err) genesis := ctx.BlockHeight() == 0 chainID := ctx.ChainID() @@ -48,7 +55,7 @@ func TestVerifySignature(t *testing.T) { AccountSequence: acc.GetSequence(), } - sigV2, err := types.StdSignatureToSignatureV2(cdc, sig) + sigV2, err := types.StdSignatureToSignatureV2(cdc, stdSig) handler := MakeTestHandlerMap() err = signing.VerifySignature(pubKey, signerData, sigV2.Data, handler, stdTx) From 0756f34ca2bebb72badcc2454320ad42a333277b Mon Sep 17 00:00:00 2001 From: sahith-narahari Date: Thu, 18 Jun 2020 22:59:57 +0530 Subject: [PATCH 09/23] Update verify tests --- x/auth/ante/sigverify.go | 19 +++++++++---------- x/auth/signing/verify_test.go | 23 +++++++++++++++++++++-- 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index bacf5b117659..b7d7d46dea31 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -365,17 +365,16 @@ func ConsumeMultisignatureVerificationGas( for i := 0; i < size; i++ { if !sig.BitArray.GetIndex(i) { continue - } else { - sigV2 := signing.SignatureV2{ - PubKey: pubkey.GetPubKeys()[i], - Data: sig.Signatures[sigIndex], - } - err := DefaultSigVerificationGasConsumer(meter, sigV2, params) - if err != nil { - return err - } - sigIndex++ } + sigV2 := signing.SignatureV2{ + PubKey: pubkey.GetPubKeys()[i], + Data: sig.Signatures[sigIndex], + } + err := DefaultSigVerificationGasConsumer(meter, sigV2, params) + if err != nil { + return err + } + sigIndex++ } return nil diff --git a/x/auth/signing/verify_test.go b/x/auth/signing/verify_test.go index 356d24bd4b12..691f10707856 100644 --- a/x/auth/signing/verify_test.go +++ b/x/auth/signing/verify_test.go @@ -1,6 +1,9 @@ package signing_test import ( + "github.com/cosmos/cosmos-sdk/crypto/hd" + "github.com/cosmos/cosmos-sdk/crypto/keyring" + "github.com/cosmos/cosmos-sdk/tests" "testing" "github.com/stretchr/testify/require" @@ -37,12 +40,28 @@ func TestVerifySignature(t *testing.T) { 200000, 1.1, false, "test-chain", "hello", sdk.NewCoins(), sdk.DecCoins{sdk.NewDecCoinFromDec(sdk.DefaultBondDenom, sdk.NewDecWithPrec(10000, sdk.Precision))}, ) + + dir, clean := tests.NewTestCaseDir(t) + t.Cleanup(clean) + + path := hd.CreateHDPath(118, 0, 0).String() + kr, err := keyring.New(sdk.KeyringServiceName(), "test", dir, nil) + require.NoError(t, err) + + var from = "test_sign" + + _, _, err = kr.NewMnemonic(from, keyring.English, path, hd.Secp256k1) + require.NoError(t, err) + txBuilder.WithKeybase(kr) signBytes, err := txBuilder.BuildSignMsg(msgs) - sign, err := txBuilder.Sign("addr", signBytes) + require.Nil(t, err) + + sign, err := txBuilder.Sign(from, signBytes) + require.Nil(t, err) + stdSig := types.StdSignature{PubKey: pubKey.Bytes(), Signature: sign} stdTx := types.NewStdTx(msgs, fee, []types.StdSignature{}, "testsigs") - require.Nil(t, err) genesis := ctx.BlockHeight() == 0 chainID := ctx.ChainID() var accNum uint64 From 4556e62df904300f9379ce8c5f7bec0592f0451f Mon Sep 17 00:00:00 2001 From: anilCSE Date: Sun, 21 Jun 2020 01:35:28 +0530 Subject: [PATCH 10/23] Fix register codec issue --- x/auth/signing/verify_test.go | 37 +++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/x/auth/signing/verify_test.go b/x/auth/signing/verify_test.go index 691f10707856..7701d1e82b68 100644 --- a/x/auth/signing/verify_test.go +++ b/x/auth/signing/verify_test.go @@ -1,9 +1,11 @@ package signing_test import ( + "github.com/cosmos/cosmos-sdk/client/flags" "github.com/cosmos/cosmos-sdk/crypto/hd" "github.com/cosmos/cosmos-sdk/crypto/keyring" "github.com/cosmos/cosmos-sdk/tests" + "github.com/spf13/viper" "testing" "github.com/stretchr/testify/require" @@ -23,9 +25,26 @@ func TestVerifySignature(t *testing.T) { app, ctx := createTestApp(false) ctx = ctx.WithBlockHeight(1) + dir, clean := tests.NewTestCaseDir(t) + t.Cleanup(clean) + + viper.Set(flags.FlagKeyringBackend, "test") + path := hd.CreateHDPath(118, 0, 0).String() + kr, err := keyring.New(sdk.KeyringServiceName(), "test", dir, nil) + require.NoError(t, err) + + var from = "test_sign" + _, _, err = kr.NewMnemonic(from, keyring.English, path, hd.Secp256k1) + require.NoError(t, err) + + viper.Set(flags.FlagFrom, from) + viper.Set(flags.FlagChainID, "test-chain") + viper.Set(flags.FlagHome, dir) + cdc := codec.New() sdk.RegisterCodec(cdc) types.RegisterCodec(cdc) + cdc.RegisterConcrete(sdk.TestMsg{}, "cosmos-sdk/Test", nil) acc1 := app.AccountKeeper.NewAccountWithAddress(ctx, addr) app.AccountKeeper.SetAccount(ctx, acc1) @@ -36,23 +55,11 @@ func TestVerifySignature(t *testing.T) { msgs := []sdk.Msg{types.NewTestMsg(addr)} fee := types.NewStdFee(50000, sdk.Coins{sdk.NewInt64Coin("atom", 150)}) - txBuilder := types.NewTxBuilder(types.DefaultTxEncoder(codec.New()), acc.GetAccountNumber(), acc.GetSequence(), + txBuilder := types.NewTxBuilder(types.DefaultTxEncoder(cdc), acc.GetAccountNumber(), acc.GetSequence(), 200000, 1.1, false, "test-chain", "hello", sdk.NewCoins(), sdk.DecCoins{sdk.NewDecCoinFromDec(sdk.DefaultBondDenom, sdk.NewDecWithPrec(10000, sdk.Precision))}, - ) + ).WithKeybase(kr) - dir, clean := tests.NewTestCaseDir(t) - t.Cleanup(clean) - - path := hd.CreateHDPath(118, 0, 0).String() - kr, err := keyring.New(sdk.KeyringServiceName(), "test", dir, nil) - require.NoError(t, err) - - var from = "test_sign" - - _, _, err = kr.NewMnemonic(from, keyring.English, path, hd.Secp256k1) - require.NoError(t, err) - txBuilder.WithKeybase(kr) signBytes, err := txBuilder.BuildSignMsg(msgs) require.Nil(t, err) @@ -60,7 +67,7 @@ func TestVerifySignature(t *testing.T) { require.Nil(t, err) stdSig := types.StdSignature{PubKey: pubKey.Bytes(), Signature: sign} - stdTx := types.NewStdTx(msgs, fee, []types.StdSignature{}, "testsigs") + stdTx := types.NewStdTx(msgs, fee, []types.StdSignature{stdSig}, "testsigs") genesis := ctx.BlockHeight() == 0 chainID := ctx.ChainID() From deffd9995f9ef8b89afb6dfd3e6788f1fe4d64c6 Mon Sep 17 00:00:00 2001 From: anilCSE Date: Sun, 21 Jun 2020 01:45:31 +0530 Subject: [PATCH 11/23] debug --- x/auth/signing/verify_test.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/x/auth/signing/verify_test.go b/x/auth/signing/verify_test.go index 7701d1e82b68..2becea2ed7ad 100644 --- a/x/auth/signing/verify_test.go +++ b/x/auth/signing/verify_test.go @@ -56,7 +56,7 @@ func TestVerifySignature(t *testing.T) { fee := types.NewStdFee(50000, sdk.Coins{sdk.NewInt64Coin("atom", 150)}) txBuilder := types.NewTxBuilder(types.DefaultTxEncoder(cdc), acc.GetAccountNumber(), acc.GetSequence(), - 200000, 1.1, false, "test-chain", "hello", sdk.NewCoins(), + 200000, 1.1, false, "test-chain", "testsigs", sdk.NewCoins(), sdk.DecCoins{sdk.NewDecCoinFromDec(sdk.DefaultBondDenom, sdk.NewDecWithPrec(10000, sdk.Precision))}, ).WithKeybase(kr) @@ -67,17 +67,13 @@ func TestVerifySignature(t *testing.T) { require.Nil(t, err) stdSig := types.StdSignature{PubKey: pubKey.Bytes(), Signature: sign} - stdTx := types.NewStdTx(msgs, fee, []types.StdSignature{stdSig}, "testsigs") + stdTx := types.NewStdTx(msgs, fee, []types.StdSignature{}, "testsigs") - genesis := ctx.BlockHeight() == 0 chainID := ctx.ChainID() - var accNum uint64 - if !genesis { - accNum = acc.GetAccountNumber() - } + signerData := signing.SignerData{ ChainID: chainID, - AccountNumber: accNum, + AccountNumber: acc.GetAccountNumber(), AccountSequence: acc.GetSequence(), } From 25e28a67ef978a5f21cc99cce1e4a8e85c6000e0 Mon Sep 17 00:00:00 2001 From: anilCSE Date: Sun, 21 Jun 2020 01:56:01 +0530 Subject: [PATCH 12/23] Cleanup --- x/auth/signing/verify_test.go | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/x/auth/signing/verify_test.go b/x/auth/signing/verify_test.go index 2becea2ed7ad..45842bf5fe38 100644 --- a/x/auth/signing/verify_test.go +++ b/x/auth/signing/verify_test.go @@ -21,6 +21,14 @@ import ( func TestVerifySignature(t *testing.T) { _, pubKey, _ := types.KeyTestPubAddr() + + const ( + from = "test_sign" + backend = "test" + memo = "testmemo" + chainId = "test-chain" + ) + addr := sdk.AccAddress(pubKey.Address()) app, ctx := createTestApp(false) ctx = ctx.WithBlockHeight(1) @@ -28,17 +36,17 @@ func TestVerifySignature(t *testing.T) { dir, clean := tests.NewTestCaseDir(t) t.Cleanup(clean) - viper.Set(flags.FlagKeyringBackend, "test") + viper.Set(flags.FlagKeyringBackend, backend) path := hd.CreateHDPath(118, 0, 0).String() - kr, err := keyring.New(sdk.KeyringServiceName(), "test", dir, nil) + kr, err := keyring.New(sdk.KeyringServiceName(), backend, dir, nil) require.NoError(t, err) - var from = "test_sign" + _, _, err = kr.NewMnemonic(from, keyring.English, path, hd.Secp256k1) require.NoError(t, err) viper.Set(flags.FlagFrom, from) - viper.Set(flags.FlagChainID, "test-chain") + viper.Set(flags.FlagChainID, chainId) viper.Set(flags.FlagHome, dir) cdc := codec.New() @@ -56,8 +64,10 @@ func TestVerifySignature(t *testing.T) { fee := types.NewStdFee(50000, sdk.Coins{sdk.NewInt64Coin("atom", 150)}) txBuilder := types.NewTxBuilder(types.DefaultTxEncoder(cdc), acc.GetAccountNumber(), acc.GetSequence(), - 200000, 1.1, false, "test-chain", "testsigs", sdk.NewCoins(), - sdk.DecCoins{sdk.NewDecCoinFromDec(sdk.DefaultBondDenom, sdk.NewDecWithPrec(10000, sdk.Precision))}, + fee.Gas, 0, false, chainId, + memo, + fee.Amount, + nil, ).WithKeybase(kr) signBytes, err := txBuilder.BuildSignMsg(msgs) @@ -67,7 +77,7 @@ func TestVerifySignature(t *testing.T) { require.Nil(t, err) stdSig := types.StdSignature{PubKey: pubKey.Bytes(), Signature: sign} - stdTx := types.NewStdTx(msgs, fee, []types.StdSignature{}, "testsigs") + stdTx := types.NewStdTx(msgs, fee, []types.StdSignature{}, memo) chainID := ctx.ChainID() From e5e144697811ea023cd1c3e42fc7ace7fc3c6132 Mon Sep 17 00:00:00 2001 From: anilCSE Date: Tue, 23 Jun 2020 02:04:45 +0530 Subject: [PATCH 13/23] Fix verify signature tests --- x/auth/signing/verify_test.go | 36 ++++++++++------------------------- 1 file changed, 10 insertions(+), 26 deletions(-) diff --git a/x/auth/signing/verify_test.go b/x/auth/signing/verify_test.go index 45842bf5fe38..6b92e7562a3b 100644 --- a/x/auth/signing/verify_test.go +++ b/x/auth/signing/verify_test.go @@ -20,7 +20,7 @@ import ( ) func TestVerifySignature(t *testing.T) { - _, pubKey, _ := types.KeyTestPubAddr() + priv, pubKey, addr := types.KeyTestPubAddr() const ( from = "test_sign" @@ -29,7 +29,6 @@ func TestVerifySignature(t *testing.T) { chainId = "test-chain" ) - addr := sdk.AccAddress(pubKey.Address()) app, ctx := createTestApp(false) ctx = ctx.WithBlockHeight(1) @@ -41,7 +40,6 @@ func TestVerifySignature(t *testing.T) { kr, err := keyring.New(sdk.KeyringServiceName(), backend, dir, nil) require.NoError(t, err) - _, _, err = kr.NewMnemonic(from, keyring.English, path, hd.Secp256k1) require.NoError(t, err) @@ -58,40 +56,26 @@ func TestVerifySignature(t *testing.T) { app.AccountKeeper.SetAccount(ctx, acc1) balances := sdk.NewCoins(sdk.NewInt64Coin("atom", 200)) require.NoError(t, app.BankKeeper.SetBalances(ctx, addr, balances)) - acc, err := ante.GetSignerAcc(ctx, app.AccountKeeper, addr) - msgs := []sdk.Msg{types.NewTestMsg(addr)} + require.NoError(t, app.BankKeeper.SetBalances(ctx, addr, balances)) + msgs := []sdk.Msg{types.NewTestMsg(addr)} fee := types.NewStdFee(50000, sdk.Coins{sdk.NewInt64Coin("atom", 150)}) - txBuilder := types.NewTxBuilder(types.DefaultTxEncoder(cdc), acc.GetAccountNumber(), acc.GetSequence(), - fee.Gas, 0, false, chainId, - memo, - fee.Amount, - nil, - ).WithKeybase(kr) - - signBytes, err := txBuilder.BuildSignMsg(msgs) - require.Nil(t, err) - - sign, err := txBuilder.Sign(from, signBytes) - require.Nil(t, err) - - stdSig := types.StdSignature{PubKey: pubKey.Bytes(), Signature: sign} - stdTx := types.NewStdTx(msgs, fee, []types.StdSignature{}, memo) - - chainID := ctx.ChainID() - signerData := signing.SignerData{ - ChainID: chainID, + ChainID: chainId, AccountNumber: acc.GetAccountNumber(), AccountSequence: acc.GetSequence(), } + signBytes := types.StdSignBytes(signerData.ChainID, signerData.AccountNumber, signerData.AccountSequence, + fee, msgs, memo) + signature, err := priv.Sign(signBytes) + require.NoError(t, err) + stdSig := types.StdSignature{PubKey: pubKey.Bytes(), Signature: signature} sigV2, err := types.StdSignatureToSignatureV2(cdc, stdSig) handler := MakeTestHandlerMap() - + stdTx := types.NewStdTx(msgs, fee, []types.StdSignature{stdSig}, memo) err = signing.VerifySignature(pubKey, signerData, sigV2.Data, handler, stdTx) - t.Log(err) require.NoError(t, err) } From 488ad4424fc8989007afd4422d5318c76f6b8acf Mon Sep 17 00:00:00 2001 From: anilCSE Date: Tue, 23 Jun 2020 02:26:24 +0530 Subject: [PATCH 14/23] Remove viper --- x/auth/signing/verify_test.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/x/auth/signing/verify_test.go b/x/auth/signing/verify_test.go index 6b92e7562a3b..188c51996a64 100644 --- a/x/auth/signing/verify_test.go +++ b/x/auth/signing/verify_test.go @@ -1,11 +1,9 @@ package signing_test import ( - "github.com/cosmos/cosmos-sdk/client/flags" "github.com/cosmos/cosmos-sdk/crypto/hd" "github.com/cosmos/cosmos-sdk/crypto/keyring" "github.com/cosmos/cosmos-sdk/tests" - "github.com/spf13/viper" "testing" "github.com/stretchr/testify/require" @@ -35,7 +33,6 @@ func TestVerifySignature(t *testing.T) { dir, clean := tests.NewTestCaseDir(t) t.Cleanup(clean) - viper.Set(flags.FlagKeyringBackend, backend) path := hd.CreateHDPath(118, 0, 0).String() kr, err := keyring.New(sdk.KeyringServiceName(), backend, dir, nil) require.NoError(t, err) @@ -43,10 +40,6 @@ func TestVerifySignature(t *testing.T) { _, _, err = kr.NewMnemonic(from, keyring.English, path, hd.Secp256k1) require.NoError(t, err) - viper.Set(flags.FlagFrom, from) - viper.Set(flags.FlagChainID, chainId) - viper.Set(flags.FlagHome, dir) - cdc := codec.New() sdk.RegisterCodec(cdc) types.RegisterCodec(cdc) From e5a79cf8fb3c45e22c1bfac810555fcae1b1acb6 Mon Sep 17 00:00:00 2001 From: anilCSE Date: Tue, 23 Jun 2020 02:27:53 +0530 Subject: [PATCH 15/23] remove keyring usage --- x/auth/signing/verify_test.go | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/x/auth/signing/verify_test.go b/x/auth/signing/verify_test.go index 188c51996a64..acf14ddfa849 100644 --- a/x/auth/signing/verify_test.go +++ b/x/auth/signing/verify_test.go @@ -1,9 +1,6 @@ package signing_test import ( - "github.com/cosmos/cosmos-sdk/crypto/hd" - "github.com/cosmos/cosmos-sdk/crypto/keyring" - "github.com/cosmos/cosmos-sdk/tests" "testing" "github.com/stretchr/testify/require" @@ -30,16 +27,6 @@ func TestVerifySignature(t *testing.T) { app, ctx := createTestApp(false) ctx = ctx.WithBlockHeight(1) - dir, clean := tests.NewTestCaseDir(t) - t.Cleanup(clean) - - path := hd.CreateHDPath(118, 0, 0).String() - kr, err := keyring.New(sdk.KeyringServiceName(), backend, dir, nil) - require.NoError(t, err) - - _, _, err = kr.NewMnemonic(from, keyring.English, path, hd.Secp256k1) - require.NoError(t, err) - cdc := codec.New() sdk.RegisterCodec(cdc) types.RegisterCodec(cdc) From b20c3f552e48e6ee9d2f0faee5cec6979699c23e Mon Sep 17 00:00:00 2001 From: anilCSE Date: Wed, 24 Jun 2020 02:40:56 +0530 Subject: [PATCH 16/23] Fix review changes --- x/auth/signing/verify_test.go | 2 -- x/auth/types/stdtx_test.go | 7 +++++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/x/auth/signing/verify_test.go b/x/auth/signing/verify_test.go index acf14ddfa849..c6f2c7cb1534 100644 --- a/x/auth/signing/verify_test.go +++ b/x/auth/signing/verify_test.go @@ -18,8 +18,6 @@ func TestVerifySignature(t *testing.T) { priv, pubKey, addr := types.KeyTestPubAddr() const ( - from = "test_sign" - backend = "test" memo = "testmemo" chainId = "test-chain" ) diff --git a/x/auth/types/stdtx_test.go b/x/auth/types/stdtx_test.go index 995db6b47b76..bdd94d3336e4 100644 --- a/x/auth/types/stdtx_test.go +++ b/x/auth/types/stdtx_test.go @@ -244,6 +244,9 @@ func TestGetSignaturesV2(t *testing.T) { require.Nil(t, err) require.Equal(t, len(sigs), 1) - sigV2, err := StdSignatureToSignatureV2(cdc, sig) - require.Equal(t, sigs[0], sigV2) + require.Equal(t, sigs[0].PubKey.Bytes(), sig.GetPubKey().Bytes()) + require.Equal(t, sigs[0].Data, &signing.SingleSignatureData{ + SignMode: signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, + Signature: sig.GetSignature(), + }) } From 8500d1fda16db81be62cdc6fcdc8691d9da4deb5 Mon Sep 17 00:00:00 2001 From: sahith-narahari Date: Fri, 26 Jun 2020 00:32:23 +0530 Subject: [PATCH 17/23] Fix simapp --- simapp/app.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/simapp/app.go b/simapp/app.go index d2410f4aaa75..c6d606f5fafb 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -4,8 +4,6 @@ import ( "io" "os" - "github.com/cosmos/cosmos-sdk/x/auth/signing/amino" - abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/libs/log" tmos "github.com/tendermint/tendermint/libs/os" @@ -375,7 +373,7 @@ func NewSimApp( app.SetAnteHandler( ante.NewAnteHandler( app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, - amino.LegacyAminoJSONHandler{}, + authtypes.LegacyAminoJSONHandler{}, ), ) app.SetEndBlocker(app.EndBlocker) From d36ce64193854752ecdce41ce87e3b776f1d6b03 Mon Sep 17 00:00:00 2001 From: sahith-narahari Date: Fri, 26 Jun 2020 01:17:16 +0530 Subject: [PATCH 18/23] Fix ante tests --- x/auth/ante/ante_test.go | 28 +++++++++++++--------------- x/auth/ante/sigverify_test.go | 6 ++---- 2 files changed, 15 insertions(+), 19 deletions(-) diff --git a/x/auth/ante/ante_test.go b/x/auth/ante/ante_test.go index a79e7bd09233..05a91e46bef4 100644 --- a/x/auth/ante/ante_test.go +++ b/x/auth/ante/ante_test.go @@ -7,8 +7,6 @@ import ( "strings" "testing" - "github.com/cosmos/cosmos-sdk/x/auth/signing/amino" - "github.com/cosmos/cosmos-sdk/types/tx/signing" "github.com/stretchr/testify/require" @@ -41,7 +39,7 @@ func TestSimulateGasCost(t *testing.T) { // setup app, ctx := createTestApp(true) ctx = ctx.WithBlockHeight(1) - anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, amino.LegacyAminoJSONHandler{}) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, types.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -95,7 +93,7 @@ func TestSimulateGasCost(t *testing.T) { func TestAnteHandlerSigErrors(t *testing.T) { // setup app, ctx := createTestApp(true) - anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, amino.LegacyAminoJSONHandler{}) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, types.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -145,7 +143,7 @@ func TestAnteHandlerAccountNumbers(t *testing.T) { // setup app, ctx := createTestApp(false) ctx = ctx.WithBlockHeight(1) - anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, amino.LegacyAminoJSONHandler{}) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, types.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -204,7 +202,7 @@ func TestAnteHandlerAccountNumbersAtBlockHeightZero(t *testing.T) { // setup app, ctx := createTestApp(false) ctx = ctx.WithBlockHeight(0) - anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, amino.LegacyAminoJSONHandler{}) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, types.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -262,7 +260,7 @@ func TestAnteHandlerSequences(t *testing.T) { // setup app, ctx := createTestApp(false) ctx = ctx.WithBlockHeight(1) - anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, amino.LegacyAminoJSONHandler{}) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, types.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -338,7 +336,7 @@ func TestAnteHandlerSequences(t *testing.T) { func TestAnteHandlerFees(t *testing.T) { // setup app, ctx := createTestApp(true) - anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, amino.LegacyAminoJSONHandler{}) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, types.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -380,7 +378,7 @@ func TestAnteHandlerMemoGas(t *testing.T) { // setup app, ctx := createTestApp(true) ctx = ctx.WithBlockHeight(1) - anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, amino.LegacyAminoJSONHandler{}) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, types.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -420,7 +418,7 @@ func TestAnteHandlerMultiSigner(t *testing.T) { // setup app, ctx := createTestApp(false) ctx = ctx.WithBlockHeight(1) - anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, amino.LegacyAminoJSONHandler{}) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, types.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -470,7 +468,7 @@ func TestAnteHandlerBadSignBytes(t *testing.T) { // setup app, ctx := createTestApp(true) ctx = ctx.WithBlockHeight(1) - anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, amino.LegacyAminoJSONHandler{}) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, types.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -547,7 +545,7 @@ func TestAnteHandlerSetPubKey(t *testing.T) { // setup app, ctx := createTestApp(true) ctx = ctx.WithBlockHeight(1) - anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, amino.LegacyAminoJSONHandler{}) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, types.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -670,7 +668,7 @@ func TestAnteHandlerSigLimitExceeded(t *testing.T) { // setup app, ctx := createTestApp(true) ctx = ctx.WithBlockHeight(1) - anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, amino.LegacyAminoJSONHandler{}) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, types.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -719,7 +717,7 @@ func TestCustomSignatureVerificationGasConsumer(t *testing.T) { default: return sdkerrors.Wrapf(sdkerrors.ErrInvalidPubKey, "unrecognized public key type: %T", pubkey) } - }, amino.LegacyAminoJSONHandler{}) + }, types.LegacyAminoJSONHandler{}) // verify that an secp256k1 account gets rejected priv1, _, addr1 := types.KeyTestPubAddr() @@ -769,7 +767,7 @@ func TestAnteHandlerReCheck(t *testing.T) { app.AccountKeeper.SetAccount(ctx, acc1) app.BankKeeper.SetBalances(ctx, addr1, types.NewTestCoins()) - antehandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, amino.LegacyAminoJSONHandler{}) + antehandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, types.LegacyAminoJSONHandler{}) // test that operations skipped on recheck do not run diff --git a/x/auth/ante/sigverify_test.go b/x/auth/ante/sigverify_test.go index 9d0f24c3a859..2f65478aff5d 100644 --- a/x/auth/ante/sigverify_test.go +++ b/x/auth/ante/sigverify_test.go @@ -4,8 +4,6 @@ import ( "fmt" "testing" - "github.com/cosmos/cosmos-sdk/x/auth/signing/amino" - "github.com/cosmos/cosmos-sdk/types/tx/signing" "github.com/cosmos/cosmos-sdk/simapp" @@ -139,7 +137,7 @@ func TestSigVerification(t *testing.T) { fee := types.NewTestStdFee() spkd := ante.NewSetPubKeyDecorator(app.AccountKeeper) - svd := ante.NewSigVerificationDecorator(app.AccountKeeper, amino.LegacyAminoJSONHandler{}) + svd := ante.NewSigVerificationDecorator(app.AccountKeeper, types.LegacyAminoJSONHandler{}) antehandler := sdk.ChainAnteDecorators(spkd, svd) type testCase struct { @@ -216,7 +214,7 @@ func runSigDecorators(t *testing.T, params types.Params, _ bool, privs ...crypto spkd := ante.NewSetPubKeyDecorator(app.AccountKeeper) svgc := ante.NewSigGasConsumeDecorator(app.AccountKeeper, ante.DefaultSigVerificationGasConsumer) - svd := ante.NewSigVerificationDecorator(app.AccountKeeper, amino.LegacyAminoJSONHandler{}) + svd := ante.NewSigVerificationDecorator(app.AccountKeeper, types.LegacyAminoJSONHandler{}) antehandler := sdk.ChainAnteDecorators(spkd, svgc, svd) // Determine gas consumption of antehandler with default params From 836ab64bf0c7ed50dc511f9b58f72ec9b8872963 Mon Sep 17 00:00:00 2001 From: sahith-narahari Date: Fri, 26 Jun 2020 11:39:07 +0530 Subject: [PATCH 19/23] Add reference issue --- x/auth/ante/ante_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x/auth/ante/ante_test.go b/x/auth/ante/ante_test.go index 05a91e46bef4..1c77e44baa7f 100644 --- a/x/auth/ante/ante_test.go +++ b/x/auth/ante/ante_test.go @@ -600,7 +600,8 @@ func generatePubKeysAndSignatures(n int, msg []byte, _ bool) (pubkeys []crypto.P var privkey crypto.PrivKey privkey = secp256k1.GenPrivKey() - // TODO: also generate ed25519 keys as below when ed25519 keys are actually supported, + // TODO: also generate ed25519 keys as below when ed25519 keys are + // actually supported, https://github.com/cosmos/cosmos-sdk/issues/4789 // for now this fails: //if rand.Int63()%2 == 0 { // privkey = ed25519.GenPrivKey() From be4ba5fca3848a9139aef336bd014c5e87f51f9a Mon Sep 17 00:00:00 2001 From: sahith-narahari Date: Fri, 26 Jun 2020 23:50:07 +0530 Subject: [PATCH 20/23] Add test for multisignature --- x/auth/signing/verify_test.go | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/x/auth/signing/verify_test.go b/x/auth/signing/verify_test.go index c6f2c7cb1534..9d67458abe9d 100644 --- a/x/auth/signing/verify_test.go +++ b/x/auth/signing/verify_test.go @@ -5,8 +5,10 @@ import ( "github.com/stretchr/testify/require" abci "github.com/tendermint/tendermint/abci/types" + "github.com/tendermint/tendermint/crypto" "github.com/cosmos/cosmos-sdk/codec" + "github.com/cosmos/cosmos-sdk/crypto/types/multisig" "github.com/cosmos/cosmos-sdk/simapp" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/auth/ante" @@ -16,9 +18,10 @@ import ( func TestVerifySignature(t *testing.T) { priv, pubKey, addr := types.KeyTestPubAddr() + priv1, pubKey1, addr1 := types.KeyTestPubAddr() const ( - memo = "testmemo" + memo = "testmemo" chainId = "test-chain" ) @@ -31,6 +34,7 @@ func TestVerifySignature(t *testing.T) { cdc.RegisterConcrete(sdk.TestMsg{}, "cosmos-sdk/Test", nil) acc1 := app.AccountKeeper.NewAccountWithAddress(ctx, addr) + _ = app.AccountKeeper.NewAccountWithAddress(ctx, addr1) app.AccountKeeper.SetAccount(ctx, acc1) balances := sdk.NewCoins(sdk.NewInt64Coin("atom", 200)) require.NoError(t, app.BankKeeper.SetBalances(ctx, addr, balances)) @@ -51,10 +55,39 @@ func TestVerifySignature(t *testing.T) { stdSig := types.StdSignature{PubKey: pubKey.Bytes(), Signature: signature} sigV2, err := types.StdSignatureToSignatureV2(cdc, stdSig) + require.NoError(t, err) + handler := MakeTestHandlerMap() stdTx := types.NewStdTx(msgs, fee, []types.StdSignature{stdSig}, memo) err = signing.VerifySignature(pubKey, signerData, sigV2.Data, handler, stdTx) require.NoError(t, err) + + pkSet := []crypto.PubKey{pubKey, pubKey1} + multisigKey := multisig.NewPubKeyMultisigThreshold(2, pkSet) + multisignature := multisig.NewMultisig(2) + msgs = []sdk.Msg{types.NewTestMsg(addr, addr1)} + multiSignBytes := types.StdSignBytes(signerData.ChainID, signerData.AccountNumber, signerData.AccountSequence, + fee, msgs, memo) + + sig1, err := priv.Sign(multiSignBytes) + require.NoError(t, err) + stdSig1 := types.StdSignature{PubKey: pubKey.Bytes(), Signature: sig1} + sig1V2, err := types.StdSignatureToSignatureV2(cdc, stdSig1) + require.NoError(t, err) + + sig2, err := priv1.Sign(multiSignBytes) + require.NoError(t, err) + stdSig2 := types.StdSignature{PubKey: pubKey.Bytes(), Signature: sig2} + sig2V2, err := types.StdSignatureToSignatureV2(cdc, stdSig2) + require.NoError(t, err) + + err = multisig.AddSignatureFromPubKey(multisignature, sig1V2.Data, pkSet[0], pkSet) + require.NoError(t, err) + err = multisig.AddSignatureFromPubKey(multisignature, sig2V2.Data, pkSet[1], pkSet) + require.NoError(t, err) + + err = signing.VerifySignature(multisigKey, signerData, multisignature, handler, stdTx) + require.Error(t, err) } // returns context and app with params set on account keeper From 36e5f293f62f224ba3be7797f7e10d9832a6cf2c Mon Sep 17 00:00:00 2001 From: sahith-narahari Date: Sat, 27 Jun 2020 00:56:06 +0530 Subject: [PATCH 21/23] Wrap sign error with sig --- x/auth/types/stdtx.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/auth/types/stdtx.go b/x/auth/types/stdtx.go index 20efb40fcac0..7bcc550f93af 100644 --- a/x/auth/types/stdtx.go +++ b/x/auth/types/stdtx.go @@ -245,7 +245,7 @@ func (tx StdTx) GetSignaturesV2() ([]signing.SignatureV2, error) { var err error res[i], err = StdSignatureToSignatureV2(legacy.Cdc, sig) if err != nil { - return nil, err + return nil, sdkerrors.Wrapf(err, "Unable to convert signature %v to V2", sig) } } From cdde4c11bd1e160d6e9a39b56b038ed160f96fa2 Mon Sep 17 00:00:00 2001 From: sahith-narahari Date: Sat, 27 Jun 2020 16:53:58 +0530 Subject: [PATCH 22/23] Fix verify sign test --- x/auth/signing/verify_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/auth/signing/verify_test.go b/x/auth/signing/verify_test.go index 9d67458abe9d..e93aae75f632 100644 --- a/x/auth/signing/verify_test.go +++ b/x/auth/signing/verify_test.go @@ -87,7 +87,7 @@ func TestVerifySignature(t *testing.T) { require.NoError(t, err) err = signing.VerifySignature(multisigKey, signerData, multisignature, handler, stdTx) - require.Error(t, err) + require.NoError(t, err) } // returns context and app with params set on account keeper From 8d653de2210bb6c15e315b3907b694b806655af2 Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Mon, 29 Jun 2020 16:43:51 -0400 Subject: [PATCH 23/23] Fix CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9214cf7ee22b..6c22ad3160ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -146,7 +146,7 @@ be used to retrieve the actual proposal `Content`. Also the `NewMsgSubmitProposa * (x/auth) [\#6428](https://github.com/cosmos/cosmos-sdk/issues/6428): * `NewAnteHandler` and `NewSigVerificationDecorator` both now take a `SignModeHandler` parameter. * `SignatureVerificationGasConsumer` now has the signature: `func(meter sdk.GasMeter, sig signing.SignatureV2, params types.Params) error`. - * The `SigVerifiableTx` interface now has a `GetSignaturesV2() ([]signing.SignatureV2, error)` method and no longer has `GetSignatures` and `GetSignBytes` methods. + * The `SigVerifiableTx` interface now has a `GetSignaturesV2() ([]signing.SignatureV2, error)` method and no longer has the `GetSignBytes` method. ### Features