From 5d6db6ab185dba27fe20c26616488c8b877a8e34 Mon Sep 17 00:00:00 2001 From: Divma Date: Mon, 17 Jul 2023 05:31:53 +0000 Subject: [PATCH] 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 --- .../src/discovery/enr_ext.rs | 19 ++++++++++++------- .../lighthouse_network/src/discovery/mod.rs | 10 ++++++---- .../lighthouse_network/src/types/globals.rs | 5 ++--- lcli/src/generate_bootnode_enr.rs | 14 ++++++-------- 4 files changed, 26 insertions(+), 22 deletions(-) diff --git a/beacon_node/lighthouse_network/src/discovery/enr_ext.rs b/beacon_node/lighthouse_network/src/discovery/enr_ext.rs index 3df7f7c16f3..5ce0c55cacd 100644 --- a/beacon_node/lighthouse_network/src/discovery/enr_ext.rs +++ b/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; + /// 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 { match key { - Keypair::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::Secp256k1(key) => Ok(CombinedKey::from_secp256k1(key)), 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 diff --git a/beacon_node/lighthouse_network/src/discovery/mod.rs b/beacon_node/lighthouse_network/src/discovery/mod.rs index 3ee74ebf018..d4d0baef6b7 100644 --- a/beacon_node/lighthouse_network/src/discovery/mod.rs +++ b/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 { - 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::(&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) -> Enr { let mut builder = EnrBuilder::new("v4"); - let keypair = libp2p::identity::Keypair::generate_secp256k1(); - let enr_key: CombinedKey = CombinedKey::from_libp2p(&keypair).unwrap(); + let keypair = secp256k1::Keypair::generate(); + let enr_key: CombinedKey = CombinedKey::from_secp256k1(&keypair); // set the "attnets" field on our ENR let mut bitfield = BitVector::::new(); diff --git a/beacon_node/lighthouse_network/src/types/globals.rs b/beacon_node/lighthouse_network/src/types/globals.rs index 295616f36ba..97eaaa0051b 100644 --- a/beacon_node/lighthouse_network/src/types/globals.rs +++ b/beacon_node/lighthouse_network/src/types/globals.rs @@ -134,9 +134,8 @@ impl NetworkGlobals { log: &slog::Logger, ) -> NetworkGlobals { use crate::CombinedKeyExt; - let keypair = libp2p::identity::Keypair::generate_secp256k1(); - let enr_key: discv5::enr::CombinedKey = - discv5::enr::CombinedKey::from_libp2p(&keypair).unwrap(); + let keypair = libp2p::identity::secp256k1::Keypair::generate(); + let enr_key: discv5::enr::CombinedKey = discv5::enr::CombinedKey::from_secp256k1(&keypair); let enr = discv5::enr::EnrBuilder::new("v4").build(&enr_key).unwrap(); NetworkGlobals::new( enr, diff --git a/lcli/src/generate_bootnode_enr.rs b/lcli/src/generate_bootnode_enr.rs index 8662a804761..0584cd65496 100644 --- a/lcli/src/generate_bootnode_enr.rs +++ b/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(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 enr_key = CombinedKey::from_libp2p(&local_keypair)?; + let secp256k1_keypair = secp256k1::Keypair::generate(); + 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(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))?;