Cleanup unreachable code in lcli::generate_bootnode_enr and some tests (#4485)

## Issue Addressed
n/a Noticed this while working on something else

## Proposed Changes
- leverage the appropriate types to avoid a bunch of `unwrap` and errors

## Additional Info
n/a

beacon_node/lighthouse_network/src/discovery/enr_ext.rs

@@ -1,7 +1,10 @@
 //! ENR extension trait to support libp2p integration.
 use crate::{Enr, Multiaddr, PeerId};
 use discv5::enr::{CombinedKey, CombinedPublicKey};
-use libp2p::core::{identity::Keypair, identity::PublicKey, multiaddr::Protocol};
+use libp2p::{
+    core::{identity::Keypair, identity::PublicKey, multiaddr::Protocol},
+    identity::secp256k1,
+};
 use tiny_keccak::{Hasher, Keccak};
 
 /// Extend ENR for libp2p types.
@@ -36,6 +39,8 @@ pub trait CombinedKeyPublicExt {
 pub trait CombinedKeyExt {
     /// Converts a libp2p key into an ENR combined key.
     fn from_libp2p(key: &libp2p::core::identity::Keypair) -> Result<CombinedKey, &'static str>;
+    /// Converts a [`secp256k1::Keypair`] into and Enr [`CombinedKey`].
+    fn from_secp256k1(key: &secp256k1::Keypair) -> CombinedKey;
 }
 
 impl EnrExt for Enr {
@@ -220,12 +225,7 @@ impl CombinedKeyPublicExt for CombinedPublicKey {
 impl CombinedKeyExt for CombinedKey {
     fn from_libp2p(key: &libp2p::core::identity::Keypair) -> Result<CombinedKey, &'static str> {
         match key {
-            Keypair::Secp256k1(key) => {
+            Keypair::Secp256k1(key) => Ok(CombinedKey::from_secp256k1(key)),
-                let secret =
-                    discv5::enr::k256::ecdsa::SigningKey::from_slice(&key.secret().to_bytes())
-                        .expect("libp2p key must be valid");
-                Ok(CombinedKey::Secp256k1(secret))
-            }
             Keypair::Ed25519(key) => {
                 let ed_keypair = discv5::enr::ed25519_dalek::SigningKey::from_bytes(
                     &(key.encode()[..32])
@@ -237,6 +237,11 @@ impl CombinedKeyExt for CombinedKey {
             Keypair::Ecdsa(_) => Err("Ecdsa keypairs not supported"),
         }
     }
+    fn from_secp256k1(key: &secp256k1::Keypair) -> Self {
+        let secret = discv5::enr::k256::ecdsa::SigningKey::from_slice(&key.secret().to_bytes())
+            .expect("libp2p key must be valid");
+        CombinedKey::Secp256k1(secret)
+    }
 }
 
 // helper function to convert a peer_id to a node_id. This is only possible for secp256k1/ed25519 libp2p

beacon_node/lighthouse_network/src/discovery/mod.rs

@@ -1101,6 +1101,7 @@ mod tests {
     use super::*;
     use crate::rpc::methods::{MetaData, MetaDataV2};
     use enr::EnrBuilder;
+    use libp2p::identity::secp256k1;
     use slog::{o, Drain};
     use types::{BitVector, MinimalEthSpec, SubnetId};
 
@@ -1119,10 +1120,10 @@ mod tests {
     }
 
     async fn build_discovery() -> Discovery<E> {
-        let keypair = libp2p::identity::Keypair::generate_secp256k1();
+        let keypair = secp256k1::Keypair::generate();
         let mut config = NetworkConfig::default();
         config.set_listening_addr(crate::ListenAddress::unused_v4_ports());
-        let enr_key: CombinedKey = CombinedKey::from_libp2p(&keypair).unwrap();
+        let enr_key: CombinedKey = CombinedKey::from_secp256k1(&keypair);
         let enr: Enr = build_enr::<E>(&enr_key, &config, &EnrForkId::default()).unwrap();
         let log = build_log(slog::Level::Debug, false);
         let globals = NetworkGlobals::new(
@@ -1138,6 +1139,7 @@ mod tests {
             false,
             &log,
         );
+        let keypair = Keypair::Secp256k1(keypair);
         Discovery::new(&keypair, &config, Arc::new(globals), &log)
             .await
             .unwrap()
@@ -1184,8 +1186,8 @@ mod tests {
 
     fn make_enr(subnet_ids: Vec<usize>) -> Enr {
         let mut builder = EnrBuilder::new("v4");
-        let keypair = libp2p::identity::Keypair::generate_secp256k1();
+        let keypair = secp256k1::Keypair::generate();
-        let enr_key: CombinedKey = CombinedKey::from_libp2p(&keypair).unwrap();
+        let enr_key: CombinedKey = CombinedKey::from_secp256k1(&keypair);
 
         // set the "attnets" field on our ENR
         let mut bitfield = BitVector::<ssz_types::typenum::U64>::new();

beacon_node/lighthouse_network/src/types/globals.rs

@@ -134,9 +134,8 @@ impl<TSpec: EthSpec> NetworkGlobals<TSpec> {
         log: &slog::Logger,
     ) -> NetworkGlobals<TSpec> {
         use crate::CombinedKeyExt;
-        let keypair = libp2p::identity::Keypair::generate_secp256k1();
+        let keypair = libp2p::identity::secp256k1::Keypair::generate();
-        let enr_key: discv5::enr::CombinedKey =
+        let enr_key: discv5::enr::CombinedKey = discv5::enr::CombinedKey::from_secp256k1(&keypair);
-            discv5::enr::CombinedKey::from_libp2p(&keypair).unwrap();
         let enr = discv5::enr::EnrBuilder::new("v4").build(&enr_key).unwrap();
         NetworkGlobals::new(
             enr,

lcli/src/generate_bootnode_enr.rs

@@ -1,6 +1,7 @@
 use clap::ArgMatches;
 use lighthouse_network::{
-    discovery::{build_enr, CombinedKey, CombinedKeyExt, Keypair, ENR_FILENAME},
+    discovery::{build_enr, CombinedKey, CombinedKeyExt, ENR_FILENAME},
+    libp2p::identity::secp256k1,
     NetworkConfig, NETWORK_KEY_FILENAME,
 };
 use std::fs::File;
@@ -29,8 +30,8 @@ pub fn run<T: EthSpec>(matches: &ArgMatches) -> Result<(), String> {
     config.enr_udp4_port = Some(udp_port);
     config.enr_tcp6_port = Some(tcp_port);
 
-    let local_keypair = Keypair::generate_secp256k1();
+    let secp256k1_keypair = secp256k1::Keypair::generate();
-    let enr_key = CombinedKey::from_libp2p(&local_keypair)?;
+    let enr_key = CombinedKey::from_secp256k1(&secp256k1_keypair);
     let enr_fork_id = EnrForkId {
         fork_digest: ChainSpec::compute_fork_digest(genesis_fork_version, Hash256::zero()),
         next_fork_version: genesis_fork_version,
@@ -47,13 +48,10 @@ pub fn run<T: EthSpec>(matches: &ArgMatches) -> Result<(), String> {
         .write_all(enr.to_base64().as_bytes())
         .map_err(|e| format!("Unable to write ENR to {}: {:?}", ENR_FILENAME, e))?;
 
-    let secret_bytes = match local_keypair {
-        Keypair::Secp256k1(key) => key.secret().to_bytes(),
-        _ => return Err("Key is not a secp256k1 key".into()),
-    };
-
     let mut key_file = File::create(output_dir.join(NETWORK_KEY_FILENAME))
         .map_err(|e| format!("Unable to create {}: {:?}", NETWORK_KEY_FILENAME, e))?;
+
+    let secret_bytes = secp256k1_keypair.secret().to_bytes();
     key_file
         .write_all(&secret_bytes)
         .map_err(|e| format!("Unable to write key to {}: {:?}", NETWORK_KEY_FILENAME, e))?;