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);