feat: add support for ed25519 tx signature verification (backport #23283) (#23606)

Co-authored-by: Miloš Živković <[email protected]>
Co-authored-by: Julien Robert <[email protected]>

Author: 3 people

CHANGELOG.md

@@ -40,6 +40,10 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 Every module contains its own CHANGELOG.md. Please refer to the module you are interested in.
 
+### Features
+
+* (x/auth/ante) [#23283](https://github.com/cosmos/cosmos-sdk/pull/23283) Allow ed25519 transaction signatures.
+
 ### Bug Fixes
 
 * (codec) [#23504](https://github.com/cosmos/cosmos-sdk/pull/23504) Provide `*codec.LegacyAmino` alongside `registry.AminoRegistrar` by default in depinject provider.

crypto/keys/ed25519/ed25519.go

@@ -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"
@@ -18,7 +19,7 @@ import (
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 )
 
-//-------------------------------------
+// -------------------------------------
 
 const (
 	PrivKeyName = "tendermint/PrivKeyEd25519"
@@ -153,7 +154,7 @@ func GenPrivKeyFromSecret(secret []byte) *PrivKey {
 	return &PrivKey{Key: ed25519.NewKeyFromSeed(seed)}
 }
 
-//-------------------------------------
+// -------------------------------------
 
 var (
 	_ cryptotypes.PubKey   = &PubKey{}
@@ -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
+}

crypto/keys/ed25519/ed25519_test.go

@@ -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"
@@ -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())
+	})
+}

docs/learn/beginner/03-accounts.md

@@ -53,10 +53,10 @@ The Cosmos SDK supports the following digital key schemes for creating digital s
 * `tm-ed25519`, as implemented in the [Cosmos SDK `crypto/keys/ed25519` package](https://github.com/cosmos/cosmos-sdk/blob/v0.52.0-beta.2/crypto/keys/ed25519/ed25519.go). This scheme is supported only for the consensus validation.
 
 |              | Address length in bytes | Public key length in bytes | Used for transaction authentication | Used for consensus (cometbft) |
-| :----------: | :---------------------: | :------------------------: | :---------------------------------: | :-----------------------------: |
+| :----------: |:-----------------------:| :------------------------: |:-----------------------------------:| :-----------------------------: |
 | `secp256k1`  |           20            |             33             |                 yes                 |               no                |
 | `secp256r1`  |           32            |             33             |                 yes                 |               no                |
-| `tm-ed25519` |     -- not used --      |             32             |                 no                  |               yes               |
+| `tm-ed25519` |           20            |             32             |                 yes                 |               yes               |
 
 ## Addresses
 

go.mod

@@ -64,9 +64,10 @@ require (
 	sigs.k8s.io/yaml v1.4.0
 )
 
+require filippo.io/edwards25519 v1.1.0
+
 require (
 	buf.build/gen/go/cosmos/gogo-proto/protocolbuffers/go v1.36.4-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

x/auth/ante/sigverify.go

@@ -125,6 +125,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 {
@@ -535,10 +539,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")

x/auth/ante/sigverify_test.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/mock/gomock"
 
@@ -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",
@@ -390,23 +391,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
 
@@ -423,16 +422,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)
 		})
 	}
 }