Skip to content

Commit

Permalink
Cleanup unreachable code in lcli::generate_bootnode_enr and some te…
Browse files Browse the repository at this point in the history
…sts (sigp#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
  • Loading branch information
divagant-martian committed Jul 17, 2023
1 parent a25ec16 commit 4435a22
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 22 deletions.
19 changes: 12 additions & 7 deletions beacon_node/lighthouse_network/src/discovery/enr_ext.rs
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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) => {
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])
Expand All @@ -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
Expand Down
10 changes: 6 additions & 4 deletions beacon_node/lighthouse_network/src/discovery/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand All @@ -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(
Expand All @@ -1138,6 +1139,7 @@ mod tests {
false,
&log,
);
let keypair = Keypair::Secp256k1(keypair);
Discovery::new(&keypair, &config, Arc::new(globals), &log)
.await
.unwrap()
Expand Down Expand Up @@ -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 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::<ssz_types::typenum::U64>::new();
Expand Down
5 changes: 2 additions & 3 deletions beacon_node/lighthouse_network/src/types/globals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 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,
Expand Down
14 changes: 6 additions & 8 deletions lcli/src/generate_bootnode_enr.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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 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,
Expand All @@ -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))?;
Expand Down

0 comments on commit 4435a22

Please sign in to comment.