From c0e48e1c43f7054aea1a58aaceb17d354ccf98b5 Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Mon, 29 Jun 2020 16:48:41 -0400 Subject: [PATCH] Update ante handlers to use SignatureV2 (#6428) * Update ante handlers to use SignatureV2 * WIP on test fixes * WIP on test fixes * Debugging CLI Tests * CHANGELOG.md * CHANGELOG * Add missing tests for ante * Add tests for verify signatures * Update verify tests * Fix register codec issue * debug * Cleanup * Fix verify signature tests * Remove viper * remove keyring usage * Fix review changes * Fix simapp * Fix ante tests * Add reference issue * Add test for multisignature * Wrap sign error with sig * Fix verify sign test * Fix CHANGELOG.md Co-authored-by: sahith-narahari Co-authored-by: anilCSE Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Co-authored-by: Alexander Bezobchuk --- CHANGELOG.md | 4 ++ simapp/app.go | 1 + x/auth/ante/ante.go | 4 +- x/auth/ante/ante_test.go | 49 ++++++++------ x/auth/ante/sigverify.go | 119 ++++++++++++++++++++++++---------- x/auth/ante/sigverify_test.go | 18 +++-- x/auth/signing/verify.go | 42 ++++++++++++ x/auth/signing/verify_test.go | 100 ++++++++++++++++++++++++++++ x/auth/types/stdtx.go | 29 +++++---- x/auth/types/stdtx_test.go | 23 +++++++ 10 files changed, 310 insertions(+), 79 deletions(-) create mode 100644 x/auth/signing/verify.go create mode 100644 x/auth/signing/verify_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index bde6d010d329..6c22ad3160ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -143,6 +143,10 @@ be used to retrieve the actual proposal `Content`. Also the `NewMsgSubmitProposa * (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 the `GetSignBytes` method. ### Features diff --git a/simapp/app.go b/simapp/app.go index d142f9a6a138..c6d606f5fafb 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -373,6 +373,7 @@ func NewSimApp( app.SetAnteHandler( ante.NewAnteHandler( app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, + authtypes.LegacyAminoJSONHandler{}, ), ) app.SetEndBlocker(app.EndBlocker) diff --git a/x/auth/ante/ante.go b/x/auth/ante/ante.go index ab9e804b958f..6e62dbea63cf 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" "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" @@ -13,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 @@ -24,7 +26,7 @@ func NewAnteHandler( NewValidateSigCountDecorator(ak), NewDeductFeeDecorator(ak, bankKeeper), NewSigGasConsumeDecorator(ak, sigGasConsumer), - NewSigVerificationDecorator(ak), + 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 bff57793df30..1c77e44baa7f 100644 --- a/x/auth/ante/ante_test.go +++ b/x/auth/ante/ante_test.go @@ -4,10 +4,11 @@ import ( "encoding/json" "errors" "fmt" - "math/rand" "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" @@ -38,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) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, types.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -92,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) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, types.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -142,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) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, types.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -201,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) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, types.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -259,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) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, types.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -335,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) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, types.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -377,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) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, types.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -417,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) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, types.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -467,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) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, types.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -544,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) + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer, types.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -597,11 +598,17 @@ 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, https://github.com/cosmos/cosmos-sdk/issues/4789 + // 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) } @@ -662,7 +669,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, types.LegacyAminoJSONHandler{}) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -703,15 +710,15 @@ 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 default: return sdkerrors.Wrapf(sdkerrors.ErrInvalidPubKey, "unrecognized public key type: %T", pubkey) } - }) + }, types.LegacyAminoJSONHandler{}) // verify that an secp256k1 account gets rejected priv1, _, addr1 := types.KeyTestPubAddr() @@ -761,7 +768,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, types.LegacyAminoJSONHandler{}) // test that operations skipped on recheck do not run diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index 29a561479fee..b7d7d46dea31 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 } - err = sgcd.sigGasConsumer(ctx.GasMeter(), sig, pubKey, params) + + // make a SignatureV2 with PubKey filled in from above + sig = signing.SignatureV2{ + PubKey: pubKey, + Data: sig.Data, + } + + err = sgcd.sigGasConsumer(ctx.GasMeter(), sig, params) if err != nil { return ctx, err } @@ -157,12 +170,14 @@ 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 { +func NewSigVerificationDecorator(ak AccountKeeper, signModeHandler authsigning.SignModeHandler) SigVerificationDecorator { return SigVerificationDecorator{ - ak: ak, + ak: ak, + signModeHandler: signModeHandler, } } @@ -178,7 +193,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 +209,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 +222,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 +325,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 +338,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 +356,28 @@ 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) - sigIndex++ + if !sig.BitArray.GetIndex(i) { + continue + } + 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/ante/sigverify_test.go b/x/auth/ante/sigverify_test.go index 33781020af06..2f65478aff5d 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) @@ -133,7 +137,7 @@ func TestSigVerification(t *testing.T) { fee := types.NewTestStdFee() spkd := ante.NewSetPubKeyDecorator(app.AccountKeeper) - svd := ante.NewSigVerificationDecorator(app.AccountKeeper) + svd := ante.NewSigVerificationDecorator(app.AccountKeeper, types.LegacyAminoJSONHandler{}) antehandler := sdk.ChainAnteDecorators(spkd, svd) type testCase struct { @@ -210,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) + svd := ante.NewSigVerificationDecorator(app.AccountKeeper, types.LegacyAminoJSONHandler{}) antehandler := sdk.ChainAnteDecorators(spkd, svgc, svd) // Determine gas consumption of antehandler with default params diff --git a/x/auth/signing/verify.go b/x/auth/signing/verify.go new file mode 100644 index 000000000000..99a6d39b48f0 --- /dev/null +++ b/x/auth/signing/verify.go @@ -0,0 +1,42 @@ +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" +) + +// 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: + 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/signing/verify_test.go b/x/auth/signing/verify_test.go new file mode 100644 index 000000000000..e93aae75f632 --- /dev/null +++ b/x/auth/signing/verify_test.go @@ -0,0 +1,100 @@ +package signing_test + +import ( + "testing" + + "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" + "github.com/cosmos/cosmos-sdk/x/auth/signing" + "github.com/cosmos/cosmos-sdk/x/auth/types" +) + +func TestVerifySignature(t *testing.T) { + priv, pubKey, addr := types.KeyTestPubAddr() + priv1, pubKey1, addr1 := types.KeyTestPubAddr() + + const ( + memo = "testmemo" + chainId = "test-chain" + ) + + app, ctx := createTestApp(false) + ctx = ctx.WithBlockHeight(1) + + 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.NewAccountWithAddress(ctx, addr1) + 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) + 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)}) + signerData := signing.SignerData{ + 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) + 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.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.go b/x/auth/types/stdtx.go index 1667a9499833..7bcc550f93af 100644 --- a/x/auth/types/stdtx.go +++ b/x/auth/types/stdtx.go @@ -237,6 +237,21 @@ 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)) + + for i, sig := range tx.Signatures { + var err error + res[i], err = StdSignatureToSignatureV2(legacy.Cdc, sig) + if err != nil { + return nil, sdkerrors.Wrapf(err, "Unable to convert signature %v to V2", sig) + } + } + + 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 { @@ -249,20 +264,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 } diff --git a/x/auth/types/stdtx_test.go b/x/auth/types/stdtx_test.go index df43ccda9268..bdd94d3336e4 100644 --- a/x/auth/types/stdtx_test.go +++ b/x/auth/types/stdtx_test.go @@ -227,3 +227,26 @@ 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) + + 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(), + }) +}