Repository: informalsystems/tendermint-rs (language: rust)

Issue:
Consider `ed25519-zebra` crate?
Presently the `tendermint` crate uses the [`ed25519-dalek`](https://github.com/dalek-cryptography/ed25519-dalek) vicariously through the [`signatory-dalek`](https://github.com/iqlusioninc/signatory/tree/develop/signatory-dalek) crate.

There's an alternative I think that's worth considering: [`ed25519-zebra`](https://github.com/ZcashFoundation/ed25519-zebra). Both `ed25519-dalek` and `ed25519-zebra` are based on the underlying [`curve25519-dalek`](https://github.com/dalek-cryptography/curve25519-dalek) library. However, `ed25519-zebra` focuses on Ed25519 usage in consensus-critical (i.e. blockchain) contexts, where [discrepancies in Ed25519 validation have been problematic in the past](https://github.com/zcash/zcash/issues/2872). Also, `ed25519-zebra` is written by `curve25519-dalek` author @hdevalence.

There are a few issues at play here: existing Ed25519 libraries don't properly implement [RFC 8032](https://tools.ietf.org/html/rfc8032), and RFC 8032 doesn't go far enough to remove edge cases in verification. Per @hdevalence, he aims for `ed25519-zebra` to be the first Ed25519 library to actually implement RFC 8032 verification logic correctly.

If you do decide to adopt `ed25519-zebra` it seems like you'd want to ensure the Go side has a matching solution, either by implementing the same verification checks that `ed25519-zebra` uses, or by using `ed25519-zebra` directly. I'm guessing the latter might be problematic as it seems unlikely Tendermint would adopt a Rust-based dependency.

More generally, even if you don't wind up adopting `ed25519-zebra`, I think it would make sense to more precisely specify the Ed25519 verification logic Tendermint uses as part of an overall Tendermint specification, as I assume it's presently "whatever Go does".

This seems like the sort of change it might be good to introduce in a chain upgrade (e.g. Stargate). Namely Tendermint only uses Ed25519 signatures for consensus, so previous chains' Ed25519 signatures are irrelevant (only the ECDSA/secp256k1 signed transactions are persisted across chain upgrades). This means introducing stricter Ed25519 verification logic during a chain upgrade carries no risk (to my knowledge) of existing signatures which previously verified but wouldn't verify under the stricter logic, since it's a brand new chain and therefore has no existing Ed25519 signatures to worry about.

/cc @zmanian


Discussion / hints:
Just to update/clarify this: `ed25519-zebra` is aiming to implement [ZIP 215](https://github.com/zcash/zips/blob/master/zip-0215.rst) in its `2.x` series and legacy Zcash-flavored validation in its `1.x` series.

ZIP 215 defines precise validation criteria for Ed25519 signatures and verification keys to ensure that there are no edge cases in verification and that individual and batch verification produce identical results.  However, while these criteria are internally consistent (and thus suitable for consensus-critical applications), they aren't exactly the same as RFC8032 (which doesn't require conformant implementations to agree on whether signatures are valid) or the Go standard library (which does not conform to RFC8032 because it allows non-canonical point encodings).

However, it would not be difficult to implement ZIP 215 rules in Go and use that instead of the standard library implementation, and it might be preferable for a consensus-critical system like Tendermint to do so.  I'm not sure it's safe for `tendermint-rs` to use *any* Ed25519 implementation other than the existing Go one without carefully replicating its exact behavior (i.e., creating an `ed25519-tendermint` for the same reason that `ed25519-zebra` exists). I am also not sure what the policy of the Go standard library is around changes to Ed25519 validation criteria.  I assume that Go has a strong focus on maintaining compatibility, but if consensus-critical applications are not be top-of-mind, it's possible for breaking changes to be introduced under the guise of "stronger validation" as happened in libsodium.

The tracking issue for ZIP 215 support in `ed25519-zebra` is here: https://github.com/ZcashFoundation/ed25519-zebra/issues/16
Wow thanks for raising this and working on it. 

Is it correct that neither ed25519-dalek nor ed25519-zebra are expected to be compatible with Go's x/crypto/ed25519 ?

And does the ambiguity in RFC 8032 apply even if not doing batch verification? 

And what does ed25519-zebra 1.X target? Is it compatibility with some C++ implementation?

Presumably this is something other blockchain systems are going to be grappling with as well as ed25519 adoption picks up and we'd presumably want to see something like ZIP 215 established as a common standard for libraries to target.

We should probably open a corresponding issue in the Tendermint Go repo and target the next block protocol breaking release which will also include other changes to prepare Tendermint to support signature aggregation (eg. https://github.com/tendermint/tendermint/issues/2840)

> Wow thanks for raising this and working on it.

Thanks, I'm glad it's useful work! I've been working on a cross-ecosystem survey of how different implementations behave on these edge cases using a list of test vectors I created for ZIP 215 and a blog post writeup explaining more of the context and motivation, but I haven't finished it yet.  I'd be happy to share a draft with you if you'd like.
 
> Is it correct that neither ed25519-dalek nor ed25519-zebra are expected to be compatible with Go's x/crypto/ed25519 ?
> 
> And what does ed25519-zebra 1.X target? Is it compatibility with some C++ implementation?

Yes, ed25519-zebra 1.x is expected to be compatible with the specific point release of libsodium originally used in Zcash, and ed25519-zebra 2.x targets the ZIP 215 rules.  In general I think it's not safe to assume that any Ed25519 implementations are compatible unless they specifically advertise compatibility with each other. Even if they are coincidentally compatible at one point, if that compatibility isn't an explicit goal, a future release might be incompatible.  (This is what happened with libsodium, which changed validation criteria in point releases).

> And does the ambiguity in RFC 8032 apply even if not doing batch verification?
> 
> Presumably this is something other blockchain systems are going to be grappling with as well as ed25519 adoption picks up and we'd presumably want to see something like ZIP 215 established as a common standard for libraries to target.

The ambiguity is in the spec, in the sense that conformant implementations are not required to agree on which signatures are valid.  It's possible to select a precise set of rules, but then it's not that an implementation is conformant with RFC 8032 or not, but with some specific choices. 

Basically there are two choices to make:

- Compatibility with batch verification -- do the validation rules allow implementations to perform batch verification, or not? In order to allow this, batch and individual verification must always agree, which means that individual verification MUST use the "cofactored" verification equation rather than the cofactorless one.

- Compatibility with existing signatures accepted by other implementations (like Go's crypto/ed25519) that don't require canonically encoded points.  This is asking whether every signature that was considered valid by an existing implementation is considered valid under the new rules.  This allows using a new implementation in place of an existing one and "retconning" the new validation criteria onto the old signatures.  On the other hand, if the new rules are more strict than the previous ones, it might be possible for someone to insert a signature valid under the old rules and not the new ones, which could cause unexpected breakage. Note that there's no security impact to choosing the less strict validation criteria here, because they only affect signatures created with specially crafted low-order public keys -- the goal is just to have *consistent* criteria to prevent someone from injecting a signature into the chain that could later cause consensus divergence.

Each pair of choices on these decision points gives a different set of validation criteria.  The ZIP 215 rules are obtained by choosing yes to both, in order to allow batch verification and to ease the upgrade process.  

> Presumably this is something other blockchain systems are going to be grappling with as well as ed25519 adoption picks up and we'd presumably want to see something like ZIP 215 established as a common standard for libraries to target.
> 
> We should probably open a corresponding issue in the Tendermint Go repo and target the next block protocol breaking release which will also include other changes to prepare Tendermint to support signature aggregation (eg. [tendermint/tendermint#2840](https://github.com/tendermint/tendermint/issues/2840))

Yes, I'd like it if other projects adopted these rules.  To this end I've started work on a Go implementation of these rules here: https://github.com/hdevalence/ed25519consensus

So far this just has vendored crypto/ed25519 source, but I should have the ZIP 215 rules (& tests) done soon. I'm less familiar with the practical mechanics of "doing Go" (from the software engineering point of view, releases, publishing, modules, api design, etc), so if you know anyone who'd like to help I'd appreciate a second set of eyes on it.


Update: I've added a `VerifyConsensus` function to that repo with an implementation of the ZIP215 rules.
cf https://github.com/golang/go/issues/40478#issuecomment-666043072 with description of the current consensus rules inherited by tendermint from crypto/ed25519

Update to this -- since this issue was filed, Tendermint now uses the ZIP215 rules. I've created https://github.com/penumbra-zone/ed25519-consensus for use in Penumbra.

Fix patch:
diff --git a/.changelog/unreleased/dependencies/1046-ed25519-consensus-dep.md b/.changelog/unreleased/dependencies/1046-ed25519-consensus-dep.md
new file mode 100644
index 000000000..d505cda2b
--- /dev/null
+++ b/.changelog/unreleased/dependencies/1046-ed25519-consensus-dep.md
@@ -0,0 +1,3 @@
+- `[tendermint, tendermint-p2p]` Replaced the `ed25519-dalek` dependency with
+  `ed25519-consensus`
+  ([#1046](https://github.com/informalsystems/tendermint-rs/pull/1046))
diff --git a/config/src/node_key.rs b/config/src/node_key.rs
index c27a710d2..ba66fb2cc 100644
--- a/config/src/node_key.rs
+++ b/config/src/node_key.rs
@@ -36,7 +36,7 @@ impl NodeKey {
     pub fn public_key(&self) -> PublicKey {
         #[allow(unreachable_patterns)]
         match &self.priv_key {
-            PrivateKey::Ed25519(keypair) => keypair.public.into(),
+            PrivateKey::Ed25519(signing_key) => PublicKey::Ed25519(signing_key.verification_key()),
             _ => unreachable!(),
         }
     }
diff --git a/p2p/Cargo.toml b/p2p/Cargo.toml
index a12305d4a..a27c29f72 100644
--- a/p2p/Cargo.toml
+++ b/p2p/Cargo.toml
@@ -1,7 +1,7 @@
 [package]
 name        = "tendermint-p2p"
 version     = "0.28.0"
-edition     = "2018"
+edition     = "2021"
 license     = "Apache-2.0"
 repository  = "https://github.com/informalsystems/tendermint-rs"
 homepage    = "https://tendermint.com"
@@ -28,7 +28,7 @@ amino = ["prost-derive"]
 
 [dependencies]
 chacha20poly1305 = { version = "0.8", default-features = false, features = ["reduced-round"] }
-ed25519-dalek = { version = "1", default-features = false }
+ed25519-consensus = { version = "2", default-features = false }
 eyre = { version = "0.6", default-features = false }
 flume = { version = "0.10.7", default-features = false }
 hkdf = { version = "0.10.0", default-features = false }
@@ -37,7 +37,7 @@ prost = { version = "0.11", default-features = false }
 rand_core = { version = "0.5", default-features = false, features = ["std"] }
 sha2 = { version = "0.9", default-features = false }
 subtle = { version = "2", default-features = false }
-x25519-dalek = { version = "1.1", default-features = false }
+x25519-dalek = { version = "1.1", default-features = false, features = ["u64_backend"] }
 zeroize = { version = "1", default-features = false }
 signature = { version = "1", default-features = false }
 aead = { version = "0.4.1", default-features = false }
diff --git a/p2p/src/error.rs b/p2p/src/error.rs
index a23230c95..053649d46 100644
--- a/p2p/src/error.rs
+++ b/p2p/src/error.rs
@@ -6,7 +6,6 @@
 
 use flex_error::{define_error, DisplayOnly};
 use prost::DecodeError;
-use signature::Error as SignatureError;
 
 define_error! {
     Error {
@@ -40,7 +39,6 @@ define_error! {
             | _ | { "public key missing" },
 
         Signature
-            [ DisplayOnly<SignatureError> ]
             | _ | { "signature error" },
 
         UnsupportedKey
diff --git a/p2p/src/secret_connection.rs b/p2p/src/secret_connection.rs
index 28af8c600..a5f0a335b 100644
--- a/p2p/src/secret_connection.rs
+++ b/p2p/src/secret_connection.rs
@@ -16,7 +16,6 @@ use chacha20poly1305::{
     aead::{generic_array::GenericArray, AeadInPlace, NewAead},
     ChaCha20Poly1305,
 };
-use ed25519_dalek::{self as ed25519, Signer, Verifier};
 use merlin::Transcript;
 use rand_core::OsRng;
 use subtle::ConstantTimeEq;
@@ -61,7 +60,7 @@ pub struct Handshake<S> {
 
 /// `AwaitingEphKey` means we're waiting for the remote ephemeral pubkey.
 pub struct AwaitingEphKey {
-    local_privkey: ed25519::Keypair,
+    local_privkey: ed25519_consensus::SigningKey,
     local_eph_privkey: Option<EphemeralSecret>,
 }
 
@@ -71,7 +70,7 @@ pub struct AwaitingAuthSig {
     kdf: Kdf,
     recv_cipher: ChaCha20Poly1305,
     send_cipher: ChaCha20Poly1305,
-    local_signature: ed25519::Signature,
+    local_signature: ed25519_consensus::Signature,
 }
 
 #[allow(clippy::use_self)]
@@ -79,7 +78,7 @@ impl Handshake<AwaitingEphKey> {
     /// Initiate a handshake.
     #[must_use]
     pub fn new(
-        local_privkey: ed25519::Keypair,
+        local_privkey: ed25519_consensus::SigningKey,
         protocol_version: Version,
     ) -> (Self, EphemeralPublic) {
         // Generate an ephemeral key for perfect forward secrecy.
@@ -151,9 +150,9 @@ impl Handshake<AwaitingEphKey> {
 
         // Sign the challenge bytes for authentication.
         let local_signature = if self.protocol_version.has_transcript() {
-            sign_challenge(&sc_mac, &self.state.local_privkey)?
+            self.state.local_privkey.sign(&sc_mac)
         } else {
-            sign_challenge(&kdf.challenge, &self.state.local_privkey)?
+            self.state.local_privkey.sign(&kdf.challenge)
         };
 
         Ok(Handshake {
@@ -186,22 +185,23 @@ impl Handshake<AwaitingAuthSig> {
 
         let remote_pubkey = match pk_sum {
             proto::crypto::public_key::Sum::Ed25519(ref bytes) => {
-                ed25519::PublicKey::from_bytes(bytes).map_err(Error::signature)
+                ed25519_consensus::VerificationKey::try_from(&bytes[..])
+                    .map_err(|_| Error::signature())
             },
             proto::crypto::public_key::Sum::Secp256k1(_) => Err(Error::unsupported_key()),
         }?;
 
-        let remote_sig =
-            ed25519::Signature::try_from(auth_sig_msg.sig.as_slice()).map_err(Error::signature)?;
+        let remote_sig = ed25519_consensus::Signature::try_from(auth_sig_msg.sig.as_slice())
+            .map_err(|_| Error::signature())?;
 
         if self.protocol_version.has_transcript() {
             remote_pubkey
-                .verify(&self.state.sc_mac, &remote_sig)
-                .map_err(Error::signature)?;
+                .verify(&remote_sig, &self.state.sc_mac)
+                .map_err(|_| Error::signature())?;
         } else {
             remote_pubkey
-                .verify(&self.state.kdf.challenge, &remote_sig)
-                .map_err(Error::signature)?;
+                .verify(&remote_sig, &self.state.kdf.challenge)
+                .map_err(|_| Error::signature())?;
         }
 
         // We've authorized.
@@ -279,7 +279,7 @@ impl<IoHandler: Read + Write + Send + Sync> SecretConnection<IoHandler> {
     /// * if receiving the signature fails
     pub fn new(
         mut io_handler: IoHandler,
-        local_privkey: ed25519::Keypair,
+        local_privkey: ed25519_consensus::SigningKey,
         protocol_version: Version,
     ) -> Result<Self, Error> {
         // Start a handshake process.
@@ -470,20 +470,12 @@ fn share_eph_pubkey<IoHandler: Read + Write + Send + Sync>(
     protocol_version.decode_initial_handshake(&buf)
 }
 
-/// Sign the challenge with the local private key
-fn sign_challenge(
-    challenge: &[u8; 32],
-    local_privkey: &dyn Signer<ed25519::Signature>,
-) -> Result<ed25519::Signature, Error> {
-    local_privkey.try_sign(challenge).map_err(Error::signature)
-}
-
 // TODO(ismail): change from DecodeError to something more generic
 // this can also fail while writing / sending
 fn share_auth_signature<IoHandler: Read + Write + Send + Sync>(
     sc: &mut SecretConnection<IoHandler>,
-    pubkey: &ed25519::PublicKey,
-    local_signature: &ed25519::Signature,
+    pubkey: &ed25519_consensus::VerificationKey,
+    local_signature: &ed25519_consensus::Signature,
 ) -> Result<proto::p2p::AuthSigMessage, Error> {
     let buf = sc
         .protocol_version
diff --git a/p2p/src/secret_connection/amino_types.rs b/p2p/src/secret_connection/amino_types.rs
index 31cceafe9..8a69fa6eb 100644
--- a/p2p/src/secret_connection/amino_types.rs
+++ b/p2p/src/secret_connection/amino_types.rs
@@ -1,8 +1,6 @@
 //! Amino types used by Secret Connection
 
 use core::convert::TryFrom;
-
-use ed25519_dalek as ed25519;
 use prost_derive::Message;
 use tendermint_proto as proto;
 
@@ -24,13 +22,16 @@ pub struct AuthSigMessage {
 }
 
 impl AuthSigMessage {
-    pub fn new(pub_key: &ed25519::PublicKey, sig: &ed25519::Signature) -> Self {
+    pub fn new(
+        pub_key: &ed25519_consensus::VerificationKey,
+        sig: &ed25519_consensus::Signature,
+    ) -> Self {
         let mut pub_key_bytes = Vec::from(PUB_KEY_ED25519_AMINO_PREFIX);
-        pub_key_bytes.extend_from_slice(pub_key.as_ref());
+        pub_key_bytes.extend_from_slice(pub_key.as_bytes());
 
         Self {
             pub_key: pub_key_bytes,
-            sig: sig.as_ref().to_vec(),
+            sig: sig.to_bytes().to_vec(),
         }
     }
 }
diff --git a/p2p/src/secret_connection/protocol.rs b/p2p/src/secret_connection/protocol.rs
index b17f4ca51..d6d8201f8 100644
--- a/p2p/src/secret_connection/protocol.rs
+++ b/p2p/src/secret_connection/protocol.rs
@@ -2,7 +2,6 @@
 
 use std::convert::TryInto;
 
-use ed25519_dalek as ed25519;
 use prost::Message as _;
 use tendermint_proto as proto;
 use x25519_dalek::PublicKey as EphemeralPublic;
@@ -110,8 +109,8 @@ impl Version {
     #[must_use]
     pub fn encode_auth_signature(
         self,
-        pub_key: &ed25519::PublicKey,
-        signature: &ed25519::Signature,
+        pub_key: &ed25519_consensus::VerificationKey,
+        signature: &ed25519_consensus::Signature,
     ) -> Vec<u8> {
         if self.is_protobuf() {
             // Protobuf `AuthSigMessage`
@@ -123,7 +122,7 @@ impl Version {
 
             let msg = proto::p2p::AuthSigMessage {
                 pub_key: Some(pub_key),
-                sig: signature.as_ref().to_vec(),
+                sig: signature.to_bytes().to_vec(),
             };
 
             let mut buf = Vec::new();
@@ -165,8 +164,8 @@ impl Version {
     #[cfg(feature = "amino")]
     fn encode_auth_signature_amino(
         self,
-        pub_key: &ed25519::PublicKey,
-        signature: &ed25519::Signature,
+        pub_key: &ed25519_consensus::VerificationKey,
+        signature: &ed25519_consensus::Signature,
     ) -> Vec<u8> {
         // Legacy Amino encoded `AuthSigMessage`
         let msg = amino_types::AuthSigMessage::new(pub_key, signature);
@@ -181,8 +180,8 @@ impl Version {
     #[cfg(not(feature = "amino"))]
     const fn encode_auth_signature_amino(
         self,
-        _: &ed25519::PublicKey,
-        _: &ed25519::Signature,
+        _: &ed25519_consensus::VerificationKey,
+        _: &ed25519_consensus::Signature,
     ) -> Vec<u8> {
         panic!("attempted to encode auth signature using amino, but 'amino' feature is not present")
     }
diff --git a/p2p/src/secret_connection/public_key.rs b/p2p/src/secret_connection/public_key.rs
index b7abced66..8e64c8dd5 100644
--- a/p2p/src/secret_connection/public_key.rs
+++ b/p2p/src/secret_connection/public_key.rs
@@ -2,7 +2,6 @@
 
 use std::fmt::{self, Display};
 
-use ed25519_dalek as ed25519;
 use sha2::{digest::Digest, Sha256};
 use tendermint::{error::Error, node};
 
@@ -10,7 +9,7 @@ use tendermint::{error::Error, node};
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
 pub enum PublicKey {
     /// Ed25519 Secret Connection Keys
-    Ed25519(ed25519::PublicKey),
+    Ed25519(ed25519_consensus::VerificationKey),
 }
 
 impl PublicKey {
@@ -20,14 +19,14 @@ impl PublicKey {
     ///
     /// * if the bytes given are invalid
     pub fn from_raw_ed25519(bytes: &[u8]) -> Result<Self, Error> {
-        ed25519::PublicKey::from_bytes(bytes)
+        ed25519_consensus::VerificationKey::try_from(bytes)
             .map(Self::Ed25519)
-            .map_err(Error::signature)
+            .map_err(|_| Error::signature())
     }
 
     /// Get Ed25519 public key
     #[must_use]
-    pub const fn ed25519(self) -> Option<ed25519::PublicKey> {
+    pub const fn ed25519(self) -> Option<ed25519_consensus::VerificationKey> {
         match self {
             Self::Ed25519(pk) => Some(pk),
         }
@@ -54,14 +53,14 @@ impl Display for PublicKey {
     }
 }
 
-impl From<&ed25519::Keypair> for PublicKey {
-    fn from(sk: &ed25519::Keypair) -> Self {
-        Self::Ed25519(sk.public)
+impl From<&ed25519_consensus::SigningKey> for PublicKey {
+    fn from(sk: &ed25519_consensus::SigningKey) -> Self {
+        Self::Ed25519(sk.verification_key())
     }
 }
 
-impl From<ed25519::PublicKey> for PublicKey {
-    fn from(pk: ed25519::PublicKey) -> Self {
+impl From<ed25519_consensus::VerificationKey> for PublicKey {
+    fn from(pk: ed25519_consensus::VerificationKey) -> Self {
         Self::Ed25519(pk)
     }
 }
diff --git a/tendermint/Cargo.toml b/tendermint/Cargo.toml
index 0d4d146c7..f7b1c67ee 100644
--- a/tendermint/Cargo.toml
+++ b/tendermint/Cargo.toml
@@ -32,7 +32,7 @@ rustdoc-args = ["--cfg", "docsrs"]
 [dependencies]
 bytes = { version = "1.2", default-features = false, features = ["serde"] }
 ed25519 = { version = "1.3", default-features = false }
-ed25519-dalek = { version = "1", default-features = false, features = ["u64_backend"] }
+ed25519-consensus = { version = "2", default-features = false }
 futures = { version = "0.3", default-features = false }
 num-traits = { version = "0.2", default-features = false }
 once_cell = { version = "1.3", default-features = false }
diff --git a/tendermint/src/account.rs b/tendermint/src/account.rs
index e01d83074..78c1c5b42 100644
--- a/tendermint/src/account.rs
+++ b/tendermint/src/account.rs
@@ -161,7 +161,7 @@ mod tests {
         let id_bytes = Id::from_str(id_hex).expect("expected id_hex to decode properly");
 
         // get id for pubkey
-        let pubkey = Ed25519::from_bytes(pubkey_bytes).unwrap();
+        let pubkey = Ed25519::try_from(&pubkey_bytes[..]).unwrap();
         let id = Id::from(pubkey);
 
         assert_eq!(id_bytes.ct_eq(&id).unwrap_u8(), 1);
diff --git a/tendermint/src/error.rs b/tendermint/src/error.rs
index 0e01e2c9a..0cb41f16f 100644
--- a/tendermint/src/error.rs
+++ b/tendermint/src/error.rs
@@ -209,8 +209,7 @@ define_error! {
             |_| { format_args!("subtle encoding error") },
 
         Signature
-            [ DisplayOnly<signature::Error> ]
-            |_| { format_args!("signature error") },
+            |_| { "signature error" },
 
         TrustThresholdTooLarge
             |_| { "trust threshold is too large (must be <= 1)" },
diff --git a/tendermint/src/private_key.rs b/tendermint/src/private_key.rs
index 057d51d20..0c0989f54 100644
--- a/tendermint/src/private_key.rs
+++ b/tendermint/src/private_key.rs
@@ -1,11 +1,15 @@
 //! Cryptographic private keys
 
-pub use ed25519_dalek::{Keypair as Ed25519, EXPANDED_SECRET_KEY_LENGTH as ED25519_KEYPAIR_SIZE};
+pub use ed25519_consensus::SigningKey as Ed25519;
+
+use crate::prelude::*;
+use crate::public_key::PublicKey;
+use ed25519_consensus::VerificationKey;
 use serde::{de, ser, Deserialize, Serialize};
 use subtle_encoding::{Base64, Encoding};
 use zeroize::Zeroizing;
 
-use crate::{prelude::*, public_key::PublicKey};
+pub const ED25519_KEYPAIR_SIZE: usize = 64;
 
 /// Private keys as parsed from configuration files
 #[derive(Serialize, Deserialize)]
@@ -25,24 +29,28 @@ impl PrivateKey {
     /// Get the public key associated with this private key
     pub fn public_key(&self) -> PublicKey {
         match self {
-            PrivateKey::Ed25519(private_key) => private_key.public.into(),
+            PrivateKey::Ed25519(signing_key) => PublicKey::Ed25519(signing_key.verification_key()),
         }
     }
 
     /// If applicable, borrow the Ed25519 keypair
-    pub fn ed25519_keypair(&self) -> Option<&Ed25519> {
+    pub fn ed25519_signing_key(&self) -> Option<&Ed25519> {
         match self {
-            PrivateKey::Ed25519(keypair) => Some(keypair),
+            PrivateKey::Ed25519(signing_key) => Some(signing_key),
         }
     }
 }
 
 /// Serialize an Ed25519 keypair as Base64
-fn serialize_ed25519_keypair<S>(keypair: &Ed25519, serializer: S) -> Result<S::Ok, S::Error>
+fn serialize_ed25519_keypair<S>(signing_key: &Ed25519, serializer: S) -> Result<S::Ok, S::Error>
 where
     S: ser::Serializer,
 {
-    let keypair_bytes = Zeroizing::new(keypair.to_bytes());
+    // Tendermint uses a serialization format inherited from Go that includes
+    // a cached copy of the public key as the second half.
+    let mut keypair_bytes = Zeroizing::new([0u8; ED25519_KEYPAIR_SIZE]);
+    keypair_bytes[0..32].copy_from_slice(signing_key.as_bytes());
+    keypair_bytes[32..64].copy_from_slice(signing_key.verification_key().as_bytes());
     Zeroizing::new(String::from_utf8(Base64::default().encode(&keypair_bytes[..])).unwrap())
         .serialize(serializer)
 }
@@ -63,5 +71,17 @@ where
         return Err(D::Error::custom("invalid Ed25519 keypair size"));
     }
 
-    Ed25519::from_bytes(&*keypair_bytes).map_err(D::Error::custom)
+    // Tendermint uses a serialization format inherited from Go that includes a
+    // cached copy of the public key as the second half.  This is somewhat
+    // dangerous, since there's no validation that the two parts are consistent
+    // with each other, so we ignore the second half and just check consistency
+    // with the re-derived data.
+    let signing_key = Ed25519::try_from(&keypair_bytes[0..32])
+        .map_err(|_| D::Error::custom("invalid signing key"))?;
+    let verification_key = VerificationKey::from(&signing_key);
+    if &keypair_bytes[32..64] != verification_key.as_bytes() {
+        return Err(D::Error::custom("keypair mismatch"));
+    }
+
+    Ok(signing_key)
 }
diff --git a/tendermint/src/public_key.rs b/tendermint/src/public_key.rs
index bfeebba19..286f005ee 100644
--- a/tendermint/src/public_key.rs
+++ b/tendermint/src/public_key.rs
@@ -1,18 +1,19 @@
 //! Public keys used in Tendermint networks
 
-pub use ed25519_dalek::PublicKey as Ed25519;
+pub use ed25519_consensus::VerificationKey as Ed25519;
 #[cfg(feature = "secp256k1")]
 pub use k256::ecdsa::VerifyingKey as Secp256k1;
 
 mod pub_key_request;
 mod pub_key_response;
-use core::{cmp::Ordering, convert::TryFrom, fmt, ops::Deref, str::FromStr};
 
 pub use pub_key_request::PubKeyRequest;
 pub use pub_key_response::PubKeyResponse;
+
+use core::convert::TryFrom;
+use core::{cmp::Ordering, fmt, ops::Deref, str::FromStr};
 use serde::{de, ser, Deserialize, Deserializer, Serialize};
 use serde_json::Value;
-use signature::Verifier as _;
 use subtle_encoding::{base64, bech32, hex};
 use tendermint_proto::{
     crypto::{public_key::Sum, PublicKey as RawPublicKey},
@@ -21,6 +22,9 @@ use tendermint_proto::{
 
 use crate::{error::Error, prelude::*, signature::Signature};
 
+#[cfg(feature = "secp256k1")]
+use signature::Verifier as _;
+
 // Note:On the golang side this is generic in the sense that it could everything that implements
 // github.com/tendermint/tendermint/crypto.PubKey
 // While this is meant to be used with different key-types, it currently only uses a PubKeyEd25519
@@ -170,7 +174,7 @@ impl PublicKey {
 
     /// From raw Ed25519 public key bytes
     pub fn from_raw_ed25519(bytes: &[u8]) -> Option<PublicKey> {
-        Ed25519::from_bytes(bytes).map(Into::into).ok()
+        Ed25519::try_from(bytes).map(PublicKey::Ed25519).ok()
     }
 
     /// Get Ed25519 public key
@@ -196,16 +200,15 @@ impl PublicKey {
     pub fn verify(&self, msg: &[u8], signature: &Signature) -> Result<(), Error> {
         match self {
             PublicKey::Ed25519(pk) => {
-                match ed25519_dalek::Signature::try_from(signature.as_bytes()) {
-                    Ok(sig) => pk.verify(msg, &sig).map_err(|_| {
+                match ed25519_consensus::Signature::try_from(signature.as_bytes()) {
+                    Ok(sig) => pk.verify(&sig, msg).map_err(|_| {
                         Error::signature_invalid(
                             "Ed25519 signature verification failed".to_string(),
                         )
                     }),
-                    Err(e) => Err(Error::signature_invalid(format!(
-                        "invalid Ed25519 signature: {}",
-                        e
-                    ))),
+                    Err(_) => Err(Error::signature_invalid(
+                        "Could not parse Ed25519 signature".to_string(),
+                    )),
                 }
             },
             #[cfg(feature = "secp256k1")]
@@ -424,7 +427,7 @@ where
     use de::Error;
     let encoded = String::deserialize(deserializer)?;
     let bytes = base64::decode(encoded).map_err(D::Error::custom)?;
-    Ed25519::from_bytes(&bytes).map_err(D::Error::custom)
+    Ed25519::try_from(&bytes[..]).map_err(|_| D::Error::custom("invalid Ed25519 key"))
 }
 
 #[cfg(feature = "secp256k1")]
diff --git a/tendermint/src/signature.rs b/tendermint/src/signature.rs
index 43703c1c3..37d9b104c 100644
--- a/tendermint/src/signature.rs
+++ b/tendermint/src/signature.rs
@@ -86,8 +86,14 @@ impl AsRef<[u8]> for Signature {
 }
 
 impl From<Ed25519Signature> for Signature {
-    fn from(pk: Ed25519Signature) -> Signature {
-        Self(pk.as_ref().to_vec())
+    fn from(sig: Ed25519Signature) -> Signature {
+        Self(sig.as_ref().to_vec())
+    }
+}
+
+impl From<ed25519_consensus::Signature> for Signature {
+    fn from(sig: ed25519_consensus::Signature) -> Signature {
+        Self(sig.to_bytes().to_vec())
     }
 }
 
diff --git a/tendermint/src/validator.rs b/tendermint/src/validator.rs
index ad2993782..a5a610888 100644
--- a/tendermint/src/validator.rs
+++ b/tendermint/src/validator.rs
@@ -273,7 +273,7 @@ impl From<&Info> for SimpleValidator {
     fn from(info: &Info) -> SimpleValidator {
         let sum = match &info.pub_key {
             PublicKey::Ed25519(pk) => Some(tendermint_proto::crypto::public_key::Sum::Ed25519(
-                pk.as_bytes().to_vec(),
+                pk.as_ref().to_vec(),
             )),
             #[cfg(feature = "secp256k1")]
             PublicKey::Secp256k1(pk) => Some(tendermint_proto::crypto::public_key::Sum::Secp256k1(


Test patch:
diff --git a/test/Cargo.toml b/test/Cargo.toml
index 9d9c1668f..e836cb268 100644
--- a/test/Cargo.toml
+++ b/test/Cargo.toml
@@ -14,10 +14,10 @@ authors     = ["Alexander Simmerl <a.simmerl@gmail.com>"]
 test = true
 
 [dev-dependencies]
-ed25519-dalek = { version = "1", default-features = false, features = ["rand"] }
+ed25519-consensus = { version = "2", default-features = false }
 flex-error = { version = "0.4.4", default-features = false }
 flume = { version = "0.10", default-features = false }
-rand_core = { version = "0.5", default-features = false, features = ["std"] }
+rand_core = { version = "0.6", default-features = false, features = ["std"] }
 readwrite = { version = "^0.1.1", default-features = false }
 subtle-encoding = { version = "0.5", default-features = false }
 x25519-dalek = { version = "1.1", default-features = false }
diff --git a/test/src/test/unit/p2p/secret_connection.rs b/test/src/test/unit/p2p/secret_connection.rs
index ddcb551f6..222b73476 100644
--- a/test/src/test/unit/p2p/secret_connection.rs
+++ b/test/src/test/unit/p2p/secret_connection.rs
@@ -4,7 +4,6 @@ use std::{
     thread,
 };
 
-use ed25519_dalek::{self as ed25519};
 use rand_core::OsRng;
 use tendermint_p2p::secret_connection::{sort32, Handshake, SecretConnection, Version};
 use tendermint_proto as proto;
@@ -63,8 +62,8 @@ fn test_read_write_single_message() {
 
 #[test]
 fn test_evil_peer_shares_invalid_eph_key() {
-    let mut csprng = OsRng {};
-    let local_privkey: ed25519::Keypair = ed25519::Keypair::generate(&mut csprng);
+    let csprng = OsRng {};
+    let local_privkey = ed25519_consensus::SigningKey::new(csprng);
     let (mut h, _) = Handshake::new(local_privkey, Version::V0_34);
     let bytes: [u8; 32] = [0; 32];
     let res = h.got_key(EphemeralPublic::from(bytes));
@@ -73,8 +72,8 @@ fn test_evil_peer_shares_invalid_eph_key() {
 
 #[test]
 fn test_evil_peer_shares_invalid_auth_sig() {
-    let mut csprng = OsRng {};
-    let local_privkey: ed25519::Keypair = ed25519::Keypair::generate(&mut csprng);
+    let csprng = OsRng {};
+    let local_privkey = ed25519_consensus::SigningKey::new(csprng);
     let (mut h, _) = Handshake::new(local_privkey, Version::V0_34);
     let res = h.got_key(EphemeralPublic::from(x25519_dalek::X25519_BASEPOINT_BYTES));
     assert!(res.is_ok());
@@ -185,7 +184,7 @@ fn new_peer_conn<IoHandler>(
 where
     IoHandler: std::io::Read + std::io::Write + Send + Sync,
 {
-    let mut csprng = OsRng {};
-    let privkey1: ed25519::Keypair = ed25519::Keypair::generate(&mut csprng);
+    let csprng = OsRng {};
+    let privkey1 = ed25519_consensus::SigningKey::new(csprng);
     SecretConnection::new(io_handler, privkey1, Version::V0_34)
 }
diff --git a/testgen/Cargo.toml b/testgen/Cargo.toml
index 6a295fc53..f47c6798d 100644
--- a/testgen/Cargo.toml
+++ b/testgen/Cargo.toml
@@ -2,7 +2,7 @@
 name        = "tendermint-testgen"
 version     = "0.28.0"
 authors     = ["Informal Systems <hello@informal.systems>"]
-edition     = "2018"
+edition     = "2021"
 readme      = "README.md"
 license     = "Apache-2.0"
 homepage    = "https://www.tendermint.com/"
@@ -19,7 +19,7 @@ description = """
 tendermint = { version = "0.28.0", path = "../tendermint", features = ["clock"] }
 serde = { version = "1", default-features = false, features = ["derive"] }
 serde_json = { version = "1", default-features = false, features = ["std"] }
-ed25519-dalek = { version = "1", default-features = false }
+ed25519-consensus = { version = "2", default-features = false }
 gumdrop = { version = "0.8.0", default-features = false }
 simple-error = { version = "0.2.1", default-features = false }
 tempfile = { version = "3.1.0", default-features = false }
diff --git a/testgen/src/helpers.rs b/testgen/src/helpers.rs
index ce948e8de..63c0a5286 100644
--- a/testgen/src/helpers.rs
+++ b/testgen/src/helpers.rs
@@ -4,11 +4,7 @@ use std::io::{self, Read};
 
 use serde::de::DeserializeOwned;
 use simple_error::*;
-use tendermint::{
-    chain, public_key,
-    signature::{Signature, Verifier},
-    vote, Time,
-};
+use tendermint::{chain, public_key, signature::Signature, vote, Time};
 
 /// A macro that generates a complete setter method from a one-liner with necessary information
 #[macro_export]
@@ -50,10 +46,8 @@ pub fn get_vote_sign_bytes(chain_id: chain::Id, vote: &vote::Vote) -> Vec<u8> {
 }
 
 pub fn verify_signature(verifier: &public_key::Ed25519, msg: &[u8], signature: &Signature) -> bool {
-    use std::convert::TryFrom;
-
-    let sig = ed25519_dalek::Signature::try_from(signature.as_bytes());
-    sig.and_then(|sig| verifier.verify(msg, &sig)).is_ok()
+    let sig = ed25519_consensus::Signature::try_from(signature.as_bytes());
+    sig.and_then(|sig| verifier.verify(&sig, msg)).is_ok()
 }
 
 pub fn get_time(abs: u64) -> Result<Time, SimpleError> {
diff --git a/testgen/src/validator.rs b/testgen/src/validator.rs
index e66272715..5e679b217 100644
--- a/testgen/src/validator.rs
+++ b/testgen/src/validator.rs
@@ -1,6 +1,4 @@
-use std::convert::TryFrom;
-
-use ed25519_dalek::SecretKey as Ed25519SecretKey;
+use ed25519_consensus::SigningKey as Ed25519SigningKey;
 use gumdrop::Options;
 use serde::{Deserialize, Serialize};
 use simple_error::*;
@@ -51,17 +49,17 @@ impl Validator {
             bail!("validator identifier is too long")
         }
         bytes.extend(vec![0u8; 32 - bytes.len()].iter());
-        let secret = require_with!(
-            Ed25519SecretKey::from_bytes(&bytes).ok(),
+        let signing_key = require_with!(
+            Ed25519SigningKey::try_from(&bytes[..]).ok(),
             "failed to construct a seed from validator identifier"
         );
-        let public = public_key::Ed25519::from(&secret);
-        Ok(private_key::Ed25519 { secret, public })
+        Ok(signing_key)
     }
 
     /// Get public key for this validator companion.
     pub fn get_public_key(&self) -> Result<public_key::Ed25519, SimpleError> {
-        self.get_private_key().map(|keypair| keypair.public)
+        self.get_private_key()
+            .map(|secret_key| secret_key.verification_key())
     }
 }
 
@@ -108,10 +106,10 @@ impl Generator<validator::Info> for Validator {
     }
 
     fn generate(&self) -> Result<validator::Info, SimpleError> {
-        let keypair = self.get_private_key()?;
+        let verification_key = self.get_private_key()?.verification_key();
         let info = validator::Info {
-            address: account::Id::from(keypair.public),
-            pub_key: PublicKey::from(keypair.public),
+            address: account::Id::from(verification_key),
+            pub_key: PublicKey::from(verification_key),
             power: vote::Power::try_from(self.voting_power.unwrap_or(0)).unwrap(),
             name: None,
             proposer_priority: validator::ProposerPriority::from(
diff --git a/testgen/src/vote.rs b/testgen/src/vote.rs
index 7eee51ed9..2e136578d 100644
--- a/testgen/src/vote.rs
+++ b/testgen/src/vote.rs
@@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize};
 use simple_error::*;
 use tendermint::{
     block::{self, parts::Header as PartSetHeader},
-    signature::{Ed25519Signature, Signature, Signer},
+    signature::{Ed25519Signature, Signature},
     vote,
     vote::ValidatorIndex,
 };
@@ -89,7 +89,7 @@ impl Generator<vote::Vote> for Vote {
             None => bail!("failed to generate vote: header is missing"),
             Some(h) => h,
         };
-        let signer = validator.get_private_key()?;
+        let signing_key = validator.get_private_key()?;
         let block_validator = validator.generate()?;
         let block_header = header.generate()?;
         let block_id = if self.nil.is_some() {
@@ -137,7 +137,7 @@ impl Generator<vote::Vote> for Vote {
         };
 
         let sign_bytes = get_vote_sign_bytes(block_header.chain_id, &vote);
-        vote.signature = Some(signer.sign(sign_bytes.as_slice()).into());
+        vote.signature = Some(signing_key.sign(sign_bytes.as_slice()).into());
 
         Ok(vote)
     }