diff --git a/CHANGELOG.md b/CHANGELOG.md index 77651cc69c..fd2ae140a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,13 +38,14 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## Features -* [\#8559](https://github.com/cosmos/cosmos-sdk/pull/8559) Adding Protobuf compatible secp256r1 ECDSA signatures. +* [\#8559](https://github.com/cosmos/cosmos-sdk/pull/8559) Added Protobuf compatible secp256r1 ECDSA signatures. +* [\#8786](https://github.com/cosmos/cosmos-sdk/pull/8786) Enabled secp256r1 in x/auth. ### Client Breaking Changes * [\#8363](https://github.com/cosmos/cosmos-sdk/pull/8363) Addresses no longer have a fixed 20-byte length. From the SDK modules' point of view, any 1-255 bytes-long byte array is a valid address. * [\#8346](https://github.com/cosmos/cosmos-sdk/pull/8346) All CLI `tx` commands generate ServiceMsgs by default. Graceful Amino support has been added to ServiceMsgs to support signing legacy Msgs. -* (crypto/ed25519) [\#8690] Adopt zip1215 ed2559 verification rules. +* (crypto/ed25519) [\#8690] Adopt zip1215 ed2559 verification rules. ### API Breaking Changes @@ -55,7 +56,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/distribution) [\#8473](https://github.com/cosmos/cosmos-sdk/pull/8473) On genesis init, if the distribution module account balance, coming from bank module state, does not match the one in distribution module state, the initialization will panic. * (client/keys) [\#8500](https://github.com/cosmos/cosmos-sdk/pull/8500) `InfoImporter` interface is removed from legacy keybase. * [\#8629](https://github.com/cosmos/cosmos-sdk/pull/8629) Deprecated `SetFullFundraiserPath` from `Config` in favor of `SetPurpose` and `SetCoinType`. -* (x/upgrade) [\#8673](https://github.com/cosmos/cosmos-sdk/pull/8673) Remove IBC logic from x/upgrade. Deprecates IBC fields in an Upgrade Plan. IBC upgrade logic moved to 02-client and an IBC UpgradeProposal is added. +* (x/upgrade) [\#8673](https://github.com/cosmos/cosmos-sdk/pull/8673) Remove IBC logic from x/upgrade. Deprecates IBC fields in an Upgrade Plan. IBC upgrade logic moved to 02-client and an IBC UpgradeProposal is added. * (x/bank) [\#8517](https://github.com/cosmos/cosmos-sdk/pull/8517) `SupplyI` interface and `Supply` are removed and uses `sdk.Coins` for supply tracking ### State Machine Breaking @@ -72,7 +73,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/bank) [\#8614](https://github.com/cosmos/cosmos-sdk/issues/8614) Add `Name` and `Symbol` fields to denom metadata * (x/auth) [\#8522](https://github.com/cosmos/cosmos-sdk/pull/8522) Allow to query all stored accounts -* (x/ibc) [\#7949](https://github.com/cosmos/cosmos-sdk/issues/7949) Standardized channel `Acknowledgement` moved to its own file. Codec registration redundancy removed. +* (x/ibc) [\#7949](https://github.com/cosmos/cosmos-sdk/issues/7949) Standardized channel `Acknowledgement` moved to its own file. Codec registration redundancy removed. * (crypto/types) [\#8600](https://github.com/cosmos/cosmos-sdk/pull/8600) `CompactBitArray`: optimize the `NumTrueBitsBefore` method and add an `Equal` method. * (x/ibc) [\#8624](https://github.com/cosmos/cosmos-sdk/pull/8624) Emit full header in IBC UpdateClient message. diff --git a/crypto/keys/secp256r1/keys.pb.go b/crypto/keys/secp256r1/keys.pb.go index 898daa499b..898f19a123 100644 --- a/crypto/keys/secp256r1/keys.pb.go +++ b/crypto/keys/secp256r1/keys.pb.go @@ -26,7 +26,7 @@ const _ = proto.GoGoProtoPackageIsVersion3 // please upgrade the proto package // PubKey defines a secp256r1 ECDSA public key. type PubKey struct { // Point on secp256r1 curve in a compressed representation as specified in section - // 4.3.6 of ANSI X9.62. + // 4.3.6 of ANSI X9.62: https://webstore.ansi.org/standards/ascx9/ansix9621998 Key *ecdsaPK `protobuf:"bytes,1,opt,name=key,proto3,customtype=ecdsaPK" json:"key,omitempty"` } diff --git a/docs/core/proto-docs.md b/docs/core/proto-docs.md index 0daa9e3afc..be2ef77e18 100644 --- a/docs/core/proto-docs.md +++ b/docs/core/proto-docs.md @@ -80,7 +80,6 @@ - [Output](#cosmos.bank.v1beta1.Output) - [Params](#cosmos.bank.v1beta1.Params) - [SendEnabled](#cosmos.bank.v1beta1.SendEnabled) - - [Supply](#cosmos.bank.v1beta1.Supply) - [cosmos/bank/v1beta1/genesis.proto](#cosmos/bank/v1beta1/genesis.proto) - [Balance](#cosmos.bank.v1beta1.Balance) @@ -1561,22 +1560,6 @@ sendable). - - - -### Supply -Supply represents a struct that passively keeps track of the total supply -amounts in the network. - - -| Field | Type | Label | Description | -| ----- | ---- | ----- | ----------- | -| `total` | [cosmos.base.v1beta1.Coin](#cosmos.base.v1beta1.Coin) | repeated | | - - - - - @@ -2946,7 +2929,7 @@ PubKey defines a secp256r1 ECDSA public key. | Field | Type | Label | Description | | ----- | ---- | ----- | ----------- | -| `key` | [bytes](#bytes) | | Point on secp256r1 curve in a compressed representation as specified in section 4.3.6 of ANSI X9.62. | +| `key` | [bytes](#bytes) | | Point on secp256r1 curve in a compressed representation as specified in section 4.3.6 of ANSI X9.62: https://webstore.ansi.org/standards/ascx9/ansix9621998 | diff --git a/testutil/testdata/tx.go b/testutil/testdata/tx.go index 1538460830..cb7ae9d370 100644 --- a/testutil/testdata/tx.go +++ b/testutil/testdata/tx.go @@ -4,8 +4,10 @@ import ( "encoding/json" "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" + "github.com/cosmos/cosmos-sdk/crypto/keys/secp256r1" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/require" ) // KeyTestPubAddr generates a new secp256k1 keypair. @@ -16,6 +18,15 @@ func KeyTestPubAddr() (cryptotypes.PrivKey, cryptotypes.PubKey, sdk.AccAddress) return key, pub, addr } +// KeyTestPubAddr generates a new secp256r1 keypair. +func KeyTestPubAddrSecp256R1(require *require.Assertions) (cryptotypes.PrivKey, cryptotypes.PubKey, sdk.AccAddress) { + key, err := secp256r1.GenPrivKey() + require.NoError(err) + pub := key.PubKey() + addr := sdk.AccAddress(pub.Address()) + return key, pub, addr +} + // NewTestFeeAmount is a test fee amount. func NewTestFeeAmount() sdk.Coins { return sdk.NewCoins(sdk.NewInt64Coin("atom", 150)) diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index 79fbbcdc67..19bbdcc3a2 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -8,6 +8,7 @@ import ( "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" + "github.com/cosmos/cosmos-sdk/crypto/keys/secp256r1" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" "github.com/cosmos/cosmos-sdk/crypto/types/multisig" sdk "github.com/cosmos/cosmos-sdk/types" @@ -368,6 +369,10 @@ func DefaultSigVerificationGasConsumer( meter.ConsumeGas(params.SigVerifyCostSecp256k1, "ante verify: secp256k1") return nil + case *secp256r1.PubKey: + meter.ConsumeGas(params.SigVerifyCostSecp256r1(), "ante verify: secp256r1") + return nil + case multisig.PubKey: multisignature, ok := sig.Data.(*signing.MultiSignatureData) if !ok { diff --git a/x/auth/ante/sigverify_benchmark_test.go b/x/auth/ante/sigverify_benchmark_test.go new file mode 100644 index 0000000000..683cc261a9 --- /dev/null +++ b/x/auth/ante/sigverify_benchmark_test.go @@ -0,0 +1,42 @@ +package ante_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + tmcrypto "github.com/tendermint/tendermint/crypto" + + "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" + "github.com/cosmos/cosmos-sdk/crypto/keys/secp256r1" +) + +// This benchmark is used to asses the ante.Secp256k1ToR1GasFactor value +func BenchmarkSig(b *testing.B) { + require := require.New(b) + msg := tmcrypto.CRandBytes(1000) + + skK := secp256k1.GenPrivKey() + pkK := skK.PubKey() + skR, _ := secp256r1.GenPrivKey() + pkR := skR.PubKey() + + sigK, err := skK.Sign(msg) + require.NoError(err) + sigR, err := skR.Sign(msg) + require.NoError(err) + b.ResetTimer() + + b.Run("secp256k1", func(b *testing.B) { + for i := 0; i < b.N; i++ { + ok := pkK.VerifySignature(msg, sigK) + require.True(ok) + } + }) + + b.Run("secp256r1", func(b *testing.B) { + for i := 0; i < b.N; i++ { + ok := pkR.VerifySignature(msg, sigR) + require.True(ok) + } + }) +} diff --git a/x/auth/ante/sigverify_test.go b/x/auth/ante/sigverify_test.go index fa15e1f179..8eb42888aa 100644 --- a/x/auth/ante/sigverify_test.go +++ b/x/auth/ante/sigverify_test.go @@ -8,6 +8,7 @@ import ( "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" + "github.com/cosmos/cosmos-sdk/crypto/keys/secp256r1" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" "github.com/cosmos/cosmos-sdk/crypto/types/multisig" "github.com/cosmos/cosmos-sdk/simapp" @@ -21,12 +22,13 @@ import ( func (suite *AnteTestSuite) TestSetPubKey() { suite.SetupTest(true) // setup + require := suite.Require() suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder() // keys and addresses priv1, pub1, addr1 := testdata.KeyTestPubAddr() priv2, pub2, addr2 := testdata.KeyTestPubAddr() - priv3, pub3, addr3 := testdata.KeyTestPubAddr() + priv3, pub3, addr3 := testdata.KeyTestPubAddrSecp256R1(require) addrs := []sdk.AccAddress{addr1, addr2, addr3} pubs := []cryptotypes.PubKey{pub1, pub2, pub3} @@ -35,32 +37,30 @@ func (suite *AnteTestSuite) TestSetPubKey() { // set accounts and create msg for each address for i, addr := range addrs { acc := suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, addr) - suite.Require().NoError(acc.SetAccountNumber(uint64(i))) + require.NoError(acc.SetAccountNumber(uint64(i))) suite.app.AccountKeeper.SetAccount(suite.ctx, acc) msgs[i] = testdata.NewTestMsg(addr) } - suite.Require().NoError(suite.txBuilder.SetMsgs(msgs...)) - - feeAmount := testdata.NewTestFeeAmount() - gasLimit := testdata.NewTestGasLimit() - suite.txBuilder.SetFeeAmount(feeAmount) - suite.txBuilder.SetGasLimit(gasLimit) + require.NoError(suite.txBuilder.SetMsgs(msgs...)) + suite.txBuilder.SetFeeAmount(testdata.NewTestFeeAmount()) + suite.txBuilder.SetGasLimit(testdata.NewTestGasLimit()) privs, accNums, accSeqs := []cryptotypes.PrivKey{priv1, priv2, priv3}, []uint64{0, 1, 2}, []uint64{0, 0, 0} tx, err := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID()) - suite.Require().NoError(err) + require.NoError(err) spkd := ante.NewSetPubKeyDecorator(suite.app.AccountKeeper) antehandler := sdk.ChainAnteDecorators(spkd) ctx, err := antehandler(suite.ctx, tx, false) - suite.Require().Nil(err) + require.NoError(err) // Require that all accounts have pubkey set after Decorator runs for i, addr := range addrs { pk, err := suite.app.AccountKeeper.GetPubKey(ctx, addr) - suite.Require().Nil(err, "Error on retrieving pubkey from account") - suite.Require().Equal(pubs[i], pk, "Pubkey retrieved from account is unexpected") + require.NoError(err, "Error on retrieving pubkey from account") + require.True(pubs[i].Equals(pk), + "Wrong Pubkey retrieved from AccountKeeper, idx=%d\nexpected=%s\n got=%s", i, pubs[i], pk) } } @@ -69,6 +69,8 @@ func (suite *AnteTestSuite) TestConsumeSignatureVerificationGas() { msg := []byte{1, 2, 3, 4} cdc := simapp.MakeTestEncodingConfig().Amino + p := types.DefaultParams() + skR1, _ := secp256r1.GenPrivKey() pkSet1, sigSet1 := generatePubKeysAndSignatures(5, msg, false) multisigKey1 := kmultisig.NewLegacyAminoPubKey(2, pkSet1) multisignature1 := multisig.NewMultisig(len(pkSet1)) @@ -93,8 +95,9 @@ func (suite *AnteTestSuite) TestConsumeSignatureVerificationGas() { gasConsumed uint64 shouldErr bool }{ - {"PubKeyEd25519", args{sdk.NewInfiniteGasMeter(), nil, ed25519.GenPrivKey().PubKey(), params}, types.DefaultSigVerifyCostED25519, true}, - {"PubKeySecp256k1", args{sdk.NewInfiniteGasMeter(), nil, secp256k1.GenPrivKey().PubKey(), params}, types.DefaultSigVerifyCostSecp256k1, false}, + {"PubKeyEd25519", args{sdk.NewInfiniteGasMeter(), nil, ed25519.GenPrivKey().PubKey(), params}, p.SigVerifyCostED25519, true}, + {"PubKeySecp256k1", args{sdk.NewInfiniteGasMeter(), nil, secp256k1.GenPrivKey().PubKey(), params}, p.SigVerifyCostSecp256k1, false}, + {"PubKeySecp256r1", args{sdk.NewInfiniteGasMeter(), nil, skR1.PubKey(), params}, p.SigVerifyCostSecp256r1(), false}, {"Multisig", args{sdk.NewInfiniteGasMeter(), multisignature1, multisigKey1, params}, expectedCost1, false}, {"unknown key", args{sdk.NewInfiniteGasMeter(), nil, nil, params}, 0, true}, } diff --git a/x/auth/types/params.go b/x/auth/types/params.go index 13db369d23..3b9dbe9592 100644 --- a/x/auth/types/params.go +++ b/x/auth/types/params.go @@ -69,6 +69,16 @@ func DefaultParams() Params { } } +// SigVerifyCostSecp256r1 returns gas fee of secp256r1 signature verification. +// Set by benchmarking current implementation: +// BenchmarkSig/secp256k1 4334 277167 ns/op 4128 B/op 79 allocs/op +// BenchmarkSig/secp256r1 10000 108769 ns/op 1672 B/op 33 allocs/op +// Based on the results above secp256k1 is 2.7x is slwer. However we propose to discount it +// because we are we don't compare the cgo implementation of secp256k1, which is faster. +func (p Params) SigVerifyCostSecp256r1() uint64 { + return p.SigVerifyCostSecp256k1 / 2 +} + // String implements the stringer interface. func (p Params) String() string { out, _ := yaml.Marshal(p)