Skip to content

Commit

Permalink
Update ante handlers to use SignatureV2 (#6428)
Browse files Browse the repository at this point in the history 
* 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 <sahithnarahari@gmail.com>
Co-authored-by: anilCSE <anil@vitwit.com>
Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>
  • Loading branch information
@aaronc @sahith-narahari
@anilcse @fedekunze @alexanderbez
5 people authored Jun 29, 2020
1 parent 6251d28 commit c0e48e1
Show file tree
Hide file tree
Showing 10 changed files with 310 additions and 79 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions simapp/app.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,7 @@ func NewSimApp(
app.SetAnteHandler(
ante.NewAnteHandler(
app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer,
authtypes.LegacyAminoJSONHandler{},
),
)
app.SetEndBlocker(app.EndBlocker)
Expand Down
4 changes: 3 additions & 1 deletion x/auth/ante/ante.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand All @@ -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
)
Expand Down
49 changes: 28 additions & 21 deletions x/auth/ante/ante_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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

Expand Down
Loading

0 comments on commit c0e48e1

Please sign in to comment.