Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Update ante handlers to use SignatureV2 #6428

Merged
merged 31 commits into from
Jun 29, 2020
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
7bbad19
Update ante handlers to use SignatureV2
aaronc Jun 12, 2020
e48e900
Merge branch 'master' of https://github.com/cosmos/cosmos-sdk into aa…
aaronc Jun 15, 2020
9c67acc
Merge branch 'master' of https://github.com/cosmos/cosmos-sdk into aa…
aaronc Jun 15, 2020
d754371
WIP on test fixes
aaronc Jun 15, 2020
30b23cc
WIP on test fixes
aaronc Jun 15, 2020
08e4eec
Debugging CLI Tests
aaronc Jun 15, 2020
5ea5719
CHANGELOG.md
aaronc Jun 15, 2020
82f0262
CHANGELOG
aaronc Jun 15, 2020
ca7464f
Add missing tests for ante
sahith-narahari Jun 16, 2020
3113860
Add tests for verify signatures
sahith-narahari Jun 17, 2020
0756f34
Update verify tests
sahith-narahari Jun 18, 2020
4556e62
Fix register codec issue
anilcse Jun 20, 2020
deffd99
debug
anilcse Jun 20, 2020
25e28a6
Cleanup
anilcse Jun 20, 2020
e5e1446
Fix verify signature tests
anilcse Jun 22, 2020
488ad44
Remove viper
anilcse Jun 22, 2020
e5a79cf
remove keyring usage
anilcse Jun 22, 2020
b20c3f5
Fix review changes
anilcse Jun 23, 2020
5a93f28
Merge branch 'master' into aaronc/6213-ante-sig-verify
aaronc Jun 25, 2020
8500d1f
Fix simapp
sahith-narahari Jun 25, 2020
d36ce64
Fix ante tests
sahith-narahari Jun 25, 2020
836ab64
Add reference issue
sahith-narahari Jun 26, 2020
be4ba5f
Add test for multisignature
sahith-narahari Jun 26, 2020
1878fbc
Merge branch 'master' into aaronc/6213-ante-sig-verify
fedekunze Jun 26, 2020
36e5f29
Wrap sign error with sig
sahith-narahari Jun 26, 2020
4a8b09c
Merge branch 'aaronc/6213-ante-sig-verify' of github.com:cosmos/cosmo…
sahith-narahari Jun 26, 2020
cdde4c1
Fix verify sign test
sahith-narahari Jun 27, 2020
1f0aed6
Merge branch 'master' into aaronc/6213-ante-sig-verify
aaronc Jun 29, 2020
b63a0bb
Merge branch 'master' into aaronc/6213-ante-sig-verify
aaronc Jun 29, 2020
d030eb0
Merge branch 'master' into aaronc/6213-ante-sig-verify
alexanderbez Jun 29, 2020
8d653de
Fix CHANGELOG.md
aaronc Jun 29, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,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 `GetSignatures` and `GetSignBytes` methods.
aaronc marked this conversation as resolved.
Show resolved Hide resolved

### Features

Expand Down
3 changes: 3 additions & 0 deletions simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -372,6 +374,7 @@ func NewSimApp(
app.SetAnteHandler(
ante.NewAnteHandler(
app.AccountKeeper, app.BankKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer,
amino.LegacyAminoJSONHandler{},
sahith-narahari marked this conversation as resolved.
Show resolved Hide resolved
),
)
app.SetEndBlocker(app.EndBlocker)
Expand Down
4 changes: 3 additions & 1 deletion x/auth/ante/ante.go
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
50 changes: 29 additions & 21 deletions x/auth/ante/ante_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@ import (
"encoding/json"
"errors"
"fmt"
"math/rand"
"strings"
"testing"

"github.com/cosmos/cosmos-sdk/x/auth/signing/amino"

"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 +41,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()
Expand Down Expand Up @@ -92,7 +95,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()
Expand Down Expand Up @@ -142,7 +145,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()
Expand Down Expand Up @@ -201,7 +204,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()
Expand Down Expand Up @@ -259,7 +262,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()
Expand Down Expand Up @@ -335,7 +338,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()
Expand Down Expand Up @@ -377,7 +380,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()
Expand Down Expand Up @@ -417,7 +420,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()
Expand Down Expand Up @@ -467,7 +470,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()
Expand Down Expand Up @@ -544,7 +547,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()
Expand Down Expand Up @@ -597,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,
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
// 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 +670,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()
Expand Down Expand Up @@ -703,15 +711,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)
}
})
}, amino.LegacyAminoJSONHandler{})

// verify that an secp256k1 account gets rejected
priv1, _, addr1 := types.KeyTestPubAddr()
Expand Down Expand Up @@ -761,7 +769,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

Expand Down
Loading