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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
9 changes: 5 additions & 4 deletions x/auth/ante/sigverify.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ func (svd SigVerificationDecorator) VerifyIsOnCurve(pubKey cryptotypes.PubKey) e
}

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 +538,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
35 changes: 13 additions & 22 deletions x/auth/ante/sigverify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/mock/gomock"

Expand Down Expand Up @@ -74,7 +75,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 Expand Up @@ -401,23 +402,21 @@ func TestAnteHandlerChecks(t *testing.T) {
anteHandler := sdk.ChainAnteDecorators(sigVerificationDecorator)

type testCase struct {
name string
privs []cryptotypes.PrivKey
msg sdk.Msg
accNums []uint64
accSeqs []uint64
shouldErr bool
supported bool
name string
privs []cryptotypes.PrivKey
msg sdk.Msg
accNums []uint64
accSeqs []uint64
}

// Secp256r1 keys that are not on curve will fail before even doing any operation i.e when trying to get the pubkey
testCases := []testCase{
{"secp256k1_onCurve", []cryptotypes.PrivKey{priv1}, msgs[0], []uint64{accs[0].GetAccountNumber()}, []uint64{0}, false, true},
{"secp256r1_onCurve", []cryptotypes.PrivKey{priv2}, msgs[1], []uint64{accs[1].GetAccountNumber()}, []uint64{0}, false, true},
{"ed255619", []cryptotypes.PrivKey{priv3}, msgs[2], []uint64{accs[2].GetAccountNumber()}, []uint64{2}, true, false},
{"secp256k1_onCurve", []cryptotypes.PrivKey{priv1}, msgs[0], []uint64{accs[0].GetAccountNumber()}, []uint64{0}},
{"secp256r1_onCurve", []cryptotypes.PrivKey{priv2}, msgs[1], []uint64{accs[1].GetAccountNumber()}, []uint64{0}},
{"ed255619", []cryptotypes.PrivKey{priv3}, msgs[2], []uint64{accs[2].GetAccountNumber()}, []uint64{2}},
}

for i, tc := range testCases {
for _, tc := range testCases {
t.Run(fmt.Sprintf("%s key", tc.name), func(t *testing.T) {
suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder() // Create new txBuilder for each test

Expand All @@ -434,16 +433,8 @@ func TestAnteHandlerChecks(t *testing.T) {

byteCtx := suite.ctx.WithTxBytes(txBytes)
_, err = anteHandler(byteCtx, tx, true)
if tc.shouldErr {
require.NotNil(t, err, "TestCase %d: %s did not error as expected", i, tc.name)
if tc.supported {
require.ErrorContains(t, err, "not on curve")
} else {
require.ErrorContains(t, err, "unsupported key type")
}
} else {
require.Nil(t, err, "TestCase %d: %s errored unexpectedly. Err: %v", i, tc.name, err)
}

assert.NoError(t, err)
})
}
}
Loading