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

feat: add support for ed25519 tx signature verification #23283

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i
* (baseapp) [#20291](https://github.com/cosmos/cosmos-sdk/pull/20291) Simulate nested messages.
* (client/keys) [#21829](https://github.com/cosmos/cosmos-sdk/pull/21829) Add support for importing hex key using standard input.
* (x/auth/ante) [#23128](https://github.com/cosmos/cosmos-sdk/pull/23128) Allow custom verifyIsOnCurve when validate tx for public key like ethsecp256k1.
* (x/auth/ante) [#23283](https://github.com/cosmos/cosmos-sdk/pull/23283) Allow ed25519 transaction signatures.

### Improvements

Expand Down
36 changes: 34 additions & 2 deletions crypto/keys/ed25519/ed25519.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"io"

"filippo.io/edwards25519"
"github.com/cometbft/cometbft/crypto"
"github.com/cometbft/cometbft/crypto/tmhash"
"github.com/hdevalence/ed25519consensus"
Expand All @@ -18,7 +19,7 @@ import (
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
)

//-------------------------------------
// -------------------------------------

const (
PrivKeyName = "tendermint/PrivKeyEd25519"
Expand Down Expand Up @@ -153,7 +154,7 @@ func GenPrivKeyFromSecret(secret []byte) *PrivKey {
return &PrivKey{Key: ed25519.NewKeyFromSeed(seed)}
}

//-------------------------------------
// -------------------------------------

var (
_ cryptotypes.PubKey = &PubKey{}
Expand Down Expand Up @@ -230,3 +231,34 @@ func (pubKey PubKey) MarshalAminoJSON() ([]byte, error) {
func (pubKey *PubKey) UnmarshalAminoJSON(bz []byte) error {
return pubKey.UnmarshalAmino(bz)
}

// identityPoint is the “neutral element” in the ed25519 group, where
// point addition with identityPoint leaves the other point unchanged.
// It corresponds to coordinates (0, 1) in Edwards form and is not a valid public key
var identityPoint = edwards25519.NewIdentityPoint()

// IsOnCurve checks that a 32B ed25519 public key is on the curve.
// The check fails for ed25519 identity points
func (pubKey *PubKey) IsOnCurve() bool {
// Make sure the public key is exactly 32B
if len(pubKey.Key) != ed25519.PublicKeySize {
// Invalid key size
return false
}

// Make sure the public key bytes decodes into an ed25519 point
point, err := new(edwards25519.Point).SetBytes(pubKey.Key)
if err != nil || point == nil {
// Not a valid point on the curve
return false
}

// Make sure the public key is not the identity point (all zeroes)
if point.Equal(identityPoint) == 1 {
// Public key is the identity point (useless)
return false
}

// Public key is a valid point on the ed25519 curve
return true
}
39 changes: 39 additions & 0 deletions crypto/keys/ed25519/ed25519_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ package ed25519_test

import (
stded25519 "crypto/ed25519"
"crypto/rand"
"encoding/base64"
"testing"

"filippo.io/edwards25519"
"github.com/cometbft/cometbft/crypto"
tmed25519 "github.com/cometbft/cometbft/crypto/ed25519"
"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -254,3 +256,40 @@ func TestMarshalJSON(t *testing.T) {
require.NoError(err)
require.True(pk2.Equals(pk))
}

func TestPubKeyOnCurve(t *testing.T) {
t.Parallel()

t.Run("invalid public key size", func(t *testing.T) {
t.Parallel()

key := &ed25519.PubKey{
Key: make(stded25519.PublicKey, ed25519.PubKeySize+1),
}

assert.False(t, key.IsOnCurve())
})

t.Run("identity point", func(t *testing.T) {
t.Parallel()

key := &ed25519.PubKey{
Key: stded25519.PublicKey(edwards25519.NewIdentityPoint().Bytes()),
}

assert.False(t, key.IsOnCurve())
})

t.Run("valid public key", func(t *testing.T) {
t.Parallel()

publicKey, _, err := stded25519.GenerateKey(rand.Reader)
require.NoError(t, err)

key := &ed25519.PubKey{
Key: publicKey,
}

assert.True(t, key.IsOnCurve())
})
}
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ require (
cosmossdk.io/x/bank v0.0.0-00010101000000-000000000000
cosmossdk.io/x/staking v0.0.0-00010101000000-000000000000
cosmossdk.io/x/tx v1.0.0
filippo.io/edwards25519 v1.1.0
github.com/99designs/keyring v1.2.2
github.com/bgentry/speakeasy v0.2.0
github.com/cometbft/cometbft v1.0.0
Expand Down Expand Up @@ -61,7 +62,6 @@ require (
require (
buf.build/gen/go/cometbft/cometbft/protocolbuffers/go v1.36.1-20241120201313-68e42a58b301.1 // indirect
buf.build/gen/go/cosmos/gogo-proto/protocolbuffers/go v1.36.1-20240130113600-88ef6483f90f.1 // indirect
filippo.io/edwards25519 v1.1.0 // indirect
github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4 // indirect
github.com/DataDog/datadog-go v4.8.3+incompatible // indirect
github.com/DataDog/zstd v1.5.6 // indirect
Expand Down
37 changes: 15 additions & 22 deletions x/auth/ante/sigverify.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,24 +69,22 @@ type AccountAbstractionKeeper interface {
//
// CONTRACT: Tx must implement SigVerifiableTx interface
type SigVerificationDecorator struct {
ak AccountKeeper
aaKeeper AccountAbstractionKeeper
signModeHandler *txsigning.HandlerMap
sigGasConsumer SignatureVerificationGasConsumer
extraVerifyIsOnCurve func(pubKey cryptotypes.PubKey) (bool, error)
ak AccountKeeper
aaKeeper AccountAbstractionKeeper
signModeHandler *txsigning.HandlerMap
sigGasConsumer SignatureVerificationGasConsumer
}

func NewSigVerificationDecorator(ak AccountKeeper, signModeHandler *txsigning.HandlerMap, sigGasConsumer SignatureVerificationGasConsumer, aaKeeper AccountAbstractionKeeper) SigVerificationDecorator {
return NewSigVerificationDecoratorWithVerifyOnCurve(ak, signModeHandler, sigGasConsumer, aaKeeper, nil)
return NewSigVerificationDecoratorWithVerifyOnCurve(ak, signModeHandler, sigGasConsumer, aaKeeper)
}

func NewSigVerificationDecoratorWithVerifyOnCurve(ak AccountKeeper, signModeHandler *txsigning.HandlerMap, sigGasConsumer SignatureVerificationGasConsumer, aaKeeper AccountAbstractionKeeper, verifyFn func(pubKey cryptotypes.PubKey) (bool, error)) SigVerificationDecorator {
func NewSigVerificationDecoratorWithVerifyOnCurve(ak AccountKeeper, signModeHandler *txsigning.HandlerMap, sigGasConsumer SignatureVerificationGasConsumer, aaKeeper AccountAbstractionKeeper) SigVerificationDecorator {
return SigVerificationDecorator{
aaKeeper: aaKeeper,
ak: ak,
signModeHandler: signModeHandler,
sigGasConsumer: sigGasConsumer,
extraVerifyIsOnCurve: verifyFn,
aaKeeper: aaKeeper,
ak: ak,
signModeHandler: signModeHandler,
sigGasConsumer: sigGasConsumer,
}
}

Expand All @@ -112,18 +110,16 @@ func OnlyLegacyAminoSigners(sigData signing.SignatureData) bool {
}

func (svd SigVerificationDecorator) VerifyIsOnCurve(pubKey cryptotypes.PubKey) error {
if svd.extraVerifyIsOnCurve != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please revert this change. It is always nil if you use NewSigVerificationDecorator but if you use the NewSigVerificationDecoratorWithVerifyOnCurve you can specify one.
This is a feature that just got added (#23128) :D

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, didn't realize it's relatively new 🤦‍♂️

I just saw that it's always nil, so I dropped it.

Reverted:

e69102c

handled, err := svd.extraVerifyIsOnCurve(pubKey)
if handled {
return err
}
}
// when simulating pubKey.Key will always be nil
if pubKey.Bytes() == nil {
return nil
}

switch typedPubKey := pubKey.(type) {
case *ed25519.PubKey:
if !typedPubKey.IsOnCurve() {
return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "ed25519 key is not on curve")
}
case *secp256k1.PubKey:
pubKeyObject, err := secp256k1dcrd.ParsePubKey(typedPubKey.Bytes())
if err != nil {
Expand Down Expand Up @@ -534,10 +530,7 @@ func DefaultSigVerificationGasConsumer(meter gas.Meter, sig signing.SignatureV2,

switch pubkey := pubkey.(type) {
case *ed25519.PubKey:
if err := meter.Consume(params.SigVerifyCostED25519, "ante verify: ed25519"); err != nil {
return err
}
return errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "ED25519 public keys are unsupported")
return meter.Consume(params.SigVerifyCostED25519, "ante verify: ed25519")

case *secp256k1.PubKey:
return meter.Consume(params.SigVerifyCostSecp256k1, "ante verify: secp256k1")
Expand Down
2 changes: 1 addition & 1 deletion x/auth/ante/sigverify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func TestConsumeSignatureVerificationGas(t *testing.T) {
args{nil, ed25519.GenPrivKey().PubKey(), params, func(mm *gastestutil.MockMeter) {
mm.EXPECT().Consume(p.SigVerifyCostED25519, "ante verify: ed25519").Times(1)
}},
true,
false,
},
{
"PubKeySecp256k1",
Expand Down
Loading