fix: enr signature changes on restart even if enr content is the same (…

…#952)

portalnet/src/discovery.rs

@@ -3,7 +3,6 @@ use crate::socket;
 use anyhow::anyhow;
 use async_trait::async_trait;
 use bytes::BytesMut;
-use discv5::enr::EnrPublicKey;
 use discv5::{
  enr::{CombinedKey, EnrBuilder, NodeId},
  ConfigBuilder, Discv5, Event, ListenConfig, RequestError, TalkRequest,
@@ -17,6 +16,7 @@ use rlp::RlpStream;
 use serde_json::{json, Value};
 use std::hash::{Hash, Hasher};
 use std::net::Ipv4Addr;
+use std::path::PathBuf;
 use std::str::FromStr;
 use std::{convert::TryFrom, fmt, fs, io, net::SocketAddr, sync::Arc};
 use tokio::sync::mpsc;
@@ -69,7 +69,7 @@ impl fmt::Debug for Discovery {
 }
 
 impl Discovery {
- pub fn new(portal_config: PortalnetConfig) -> Result<Self, String> {
+ pub fn new(portal_config: PortalnetConfig, node_data_dir: PathBuf) -> Result<Self, String> {
  let listen_all_ips = SocketAddr::new(
  "0.0.0.0"
  .parse()
@@ -112,27 +112,28 @@ impl Discovery {
  };
 
  // Check if we have an old version of our Enr and if we do, increase our sequence number
- if let Some(enr_file_location) = portal_config.enr_file_location {
- let trin_enr_file_location = enr_file_location.join(ENR_FILE_NAME);
- if trin_enr_file_location.is_file() {
- let data = fs::read_to_string(trin_enr_file_location.clone())
- .expect("Unable to read Trin Enr from file");
- let old_enr = Enr::from_str(&data).expect("Expected read trin.enr to be valid");
-
- enr.set_seq(old_enr.seq(), &enr_key)
- .expect("Unable to set Enr sequence number");
-
- // If the old Enr's signature is different then the new one
- if !verify_by_signature(&enr, old_enr.signature())
- && !verify_by_signature(&old_enr, enr.signature())
- {
- enr.set_seq(old_enr.seq() + 1, &enr_key)
- .expect("Unable to increase Enr sequence number");
- }
+ let trin_enr_path = node_data_dir.join(ENR_FILE_NAME);
+ if trin_enr_path.is_file() {
+ let data = fs::read_to_string(trin_enr_path.clone())
+ .expect("Unable to read Trin Enr from file");
+ let old_enr = Enr::from_str(&data).expect("Expected to read valid Trin Enr from file");
+ enr.set_seq(old_enr.seq(), &enr_key)
+ .expect("Unable to set Enr sequence number");
+
+ // If the content is different then increase the sequence number
+ if get_enr_rlp_content(&enr) != get_enr_rlp_content(&old_enr) {
+ enr.set_seq(old_enr.seq() + 1, &enr_key)
+ .expect("Unable to increase Enr sequence number");
+ fs::write(trin_enr_path, enr.to_base64())
+ .expect("Unable to update Trin Enr to file");
+ } else {
+ // the content is the same, we don't want to change signatures on restart
+ // so set enr to old one to keep the same signature per sequence number
+ enr = old_enr;
  }
+ } else {
  // Write enr to disk
- fs::write(trin_enr_file_location, enr.to_base64())
- .expect("Unable to write Trin Enr to file");
+ fs::write(trin_enr_path, enr.to_base64()).expect("Unable to write Trin Enr to file");
  }
 
  let listen_config = ListenConfig::Ipv4 {
@@ -425,7 +426,7 @@ impl AsyncUdpSocket<UtpEnr> for Discv5UdpSocket {
 
 // todo: remove this once sigp/enr implements this for enr's
 // we need this because signatures can be different for the same data but still valid
-fn verify_by_signature(enr: &Enr, signature: &[u8]) -> bool {
+fn get_enr_rlp_content(enr: &Enr) -> BytesMut {
  match enr.id() {
  Some(ref id) if id == "v4" => {
  let mut stream = RlpStream::new_with_buffer(BytesMut::with_capacity(300));
@@ -439,10 +440,10 @@ fn verify_by_signature(enr: &Enr, signature: &[u8]) -> bool {
  stream.append_raw(v, 1);
  }
 
- enr.public_key().verify_v4(&stream.out(), signature)
+ stream.out()
  }
  // unsupported identity schemes
- _ => false,
+ _ => BytesMut::with_capacity(0),
  }
 }
 
@@ -463,7 +464,6 @@ mod tests {
  let mut portalnet_config = PortalnetConfig {
  private_key,
  bootnodes: Bootnodes::None,
- enr_file_location: Some(node_data_dir.clone()),
  ..Default::default()
  };
 
@@ -472,15 +472,15 @@ mod tests {
  assert!(!trin_enr_file_location.is_file());
 
  // test trin.enr is made on first run
- let discovery = Discovery::new(portalnet_config.clone()).unwrap();
+ let discovery = Discovery::new(portalnet_config.clone(), node_data_dir.clone()).unwrap();
  let data = fs::read_to_string(trin_enr_file_location.clone()).unwrap();
  let old_enr = Enr::from_str(&data).unwrap();
  assert_eq!(discovery.local_enr(), old_enr);
  assert_eq!(old_enr.seq(), 1);
 
  // test if Enr changes the Enr sequence is increased and if it is written to disk
  portalnet_config.listen_port = 2424;
- let discovery = Discovery::new(portalnet_config.clone()).unwrap();
+ let discovery = Discovery::new(portalnet_config.clone(), node_data_dir.clone()).unwrap();
  assert_ne!(discovery.local_enr(), old_enr);
  let data = fs::read_to_string(trin_enr_file_location.clone()).unwrap();
  let old_enr = Enr::from_str(&data).unwrap();
@@ -489,11 +489,8 @@ mod tests {
  assert_eq!(discovery.local_enr(), old_enr);
 
  // test if the enr isn't changed that it's sequence stays the same
- let discovery = Discovery::new(portalnet_config).unwrap();
- assert!(
- verify_by_signature(&discovery.local_enr(), old_enr.signature())
- && verify_by_signature(&old_enr, discovery.local_enr().signature())
- );
+ let discovery = Discovery::new(portalnet_config, node_data_dir).unwrap();
+ assert_eq!(discovery.local_enr(), old_enr);
  let data = fs::read_to_string(trin_enr_file_location).unwrap();
  let old_enr = Enr::from_str(&data).unwrap();
  assert_eq!(discovery.local_enr().seq(), 2);

portalnet/src/overlay_service.rs

@@ -2695,17 +2695,17 @@ mod tests {
  types::messages::PortalnetConfig,
  };
 
+ use crate::utils::db::setup_temp_dir;
+ use discv5::kbucket::Entry;
+ use ethereum_types::U256;
  use ethportal_api::types::content_key::overlay::IdentityContentKey;
  use ethportal_api::types::distance::XorMetric;
  use ethportal_api::types::enr::generate_random_remote_enr;
- use trin_validation::validator::MockValidator;
-
- use discv5::kbucket::Entry;
- use ethereum_types::U256;
  use serial_test::serial;
  use std::net::SocketAddr;
  use tokio::sync::mpsc::unbounded_channel;
  use tokio_test::{assert_pending, assert_ready, task};
+ use trin_validation::validator::MockValidator;
 
  macro_rules! poll_command_rx {
  ($service:ident) => {
@@ -2719,7 +2719,8 @@ mod tests {
  no_stun: true,
  ..Default::default()
  };
- let discovery = Arc::new(Discovery::new(portal_config).unwrap());
+ let temp_dir = setup_temp_dir().unwrap().into_path();
+ let discovery = Arc::new(Discovery::new(portal_config, temp_dir).unwrap());
 
  let (_utp_talk_req_tx, utp_talk_req_rx) = unbounded_channel();
  let discv5_utp =

portalnet/src/types/messages.rs

@@ -1,4 +1,3 @@
-use std::path::PathBuf;
 use std::{
  convert::{TryFrom, TryInto},
  fmt,
@@ -169,7 +168,6 @@ pub struct PortalnetConfig {
  pub internal_ip: bool,
  pub no_stun: bool,
  pub node_addr_cache_capacity: usize,
- pub enr_file_location: Option<PathBuf>,
 }
 
 impl Default for PortalnetConfig {
@@ -182,7 +180,6 @@ impl Default for PortalnetConfig {
  data_radius: Distance::MAX,
  internal_ip: false,
  no_stun: false,
- enr_file_location: None,
  node_addr_cache_capacity: NODE_ADDR_CACHE_CAPACITY,
  }
  }

portalnet/tests/overlay.rs

@@ -21,6 +21,7 @@ use tokio::{
 use utp_rs::socket::UtpSocket;
 
 use ethportal_api::utils::bytes::hex_encode_upper;
+use portalnet::utils::db::setup_temp_dir;
 
 async fn init_overlay(
  discovery: Arc<Discovery>,
@@ -102,7 +103,8 @@ async fn overlay() {
  external_addr: Some(SocketAddr::new(ip_addr, 8001)),
  ..PortalnetConfig::default()
  };
- let mut discovery_one = Discovery::new(portal_config_one).unwrap();
+ let temp_dir_one = setup_temp_dir().unwrap().into_path();
+ let mut discovery_one = Discovery::new(portal_config_one, temp_dir_one).unwrap();
  let talk_req_rx_one = discovery_one.start().await.unwrap();
  let discovery_one = Arc::new(discovery_one);
  let overlay_one = Arc::new(init_overlay(Arc::clone(&discovery_one), protocol.clone()).await);
@@ -115,7 +117,8 @@ async fn overlay() {
  external_addr: Some(SocketAddr::new(ip_addr, 8002)),
  ..PortalnetConfig::default()
  };
- let mut discovery_two = Discovery::new(portal_config_two).unwrap();
+ let temp_dir_two = setup_temp_dir().unwrap().into_path();
+ let mut discovery_two = Discovery::new(portal_config_two, temp_dir_two).unwrap();
  let talk_req_rx_two = discovery_two.start().await.unwrap();
  let discovery_two = Arc::new(discovery_two);
  let overlay_two = Arc::new(init_overlay(Arc::clone(&discovery_two), protocol.clone()).await);
@@ -128,7 +131,8 @@ async fn overlay() {
  external_addr: Some(SocketAddr::new(ip_addr, 8003)),
  ..PortalnetConfig::default()
  };
- let mut discovery_three = Discovery::new(portal_config_three).unwrap();
+ let temp_dir_three = setup_temp_dir().unwrap().into_path();
+ let mut discovery_three = Discovery::new(portal_config_three, temp_dir_three).unwrap();
  let talk_req_rx_three = discovery_three.start().await.unwrap();
  let discovery_three = Arc::new(discovery_three);
  let overlay_three =
@@ -247,7 +251,8 @@ async fn overlay_event_stream() {
  no_stun: true,
  ..Default::default()
  };
- let discovery = Arc::new(Discovery::new(portal_config).unwrap());
+ let temp_dir = setup_temp_dir().unwrap().into_path();
+ let discovery = Arc::new(Discovery::new(portal_config, temp_dir).unwrap());
  let overlay = init_overlay(discovery, ProtocolId::Beacon).await;
 
  overlay.event_stream().await.unwrap();

rpc/src/rpc_server.rs

@@ -612,6 +612,7 @@ mod tests {
  use crate::builder::RpcModuleSelection;
  use crate::{PortalRpcModule, RpcModuleBuilder};
  use portalnet::discovery::Discovery;
+ use portalnet::utils::db::setup_temp_dir;
  use std::io;
  use std::sync::Arc;
 
@@ -633,7 +634,8 @@ mod tests {
  pub fn test_rpc_builder() -> RpcModuleBuilder {
  let (history_tx, _) = tokio::sync::mpsc::unbounded_channel();
  let (beacon_tx, _) = tokio::sync::mpsc::unbounded_channel();
- let discv5 = Arc::new(Discovery::new(Default::default()).unwrap());
+ let temp_dir = setup_temp_dir().unwrap().into_path();
+ let discv5 = Arc::new(Discovery::new(Default::default(), temp_dir).unwrap());
  RpcModuleBuilder::new(discv5)
  .with_history(history_tx)
  .with_beacon(beacon_tx)

src/lib.rs

@@ -51,12 +51,11 @@ pub async fn run_trin(
  listen_port: trin_config.discovery_port,
  no_stun: trin_config.no_stun,
  bootnodes: trin_config.bootnodes.clone(),
- enr_file_location: Some(node_data_dir.clone()),
  ..Default::default()
  };
 
  // Initialize base discovery protocol
- let mut discovery = Discovery::new(portalnet_config.clone())?;
+ let mut discovery = Discovery::new(portalnet_config.clone(), node_data_dir.clone())?;
  let talk_req_rx = discovery.start().await?;
  let discovery = Arc::new(discovery);
 

utp-testing/src/lib.rs

@@ -12,6 +12,7 @@ use jsonrpsee::proc_macros::rpc;
 use jsonrpsee::server::{Server, ServerHandle};
 use portalnet::discovery::{Discovery, UtpEnr};
 use portalnet::types::messages::{PortalnetConfig, ProtocolId};
+use portalnet::utils::db::setup_temp_dir;
 use std::net::SocketAddr;
 use std::str::FromStr;
 use std::sync::Arc;
@@ -148,7 +149,8 @@ pub async fn run_test_app(
  ..Default::default()
  };
 
- let mut discovery = Discovery::new(config).unwrap();
+ let temp_dir = setup_temp_dir().unwrap().into_path();
+ let mut discovery = Discovery::new(config, temp_dir).unwrap();
  let talk_req_rx = discovery.start().await.unwrap();
  let enr = discovery.local_enr();
  let discovery = Arc::new(discovery);