Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve ENR updates #5483

Merged
merged 4 commits into from
May 1, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 49 additions & 11 deletions beacon_node/lighthouse_network/src/discovery/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,33 +398,70 @@ impl<TSpec: EthSpec> Discovery<TSpec> {
/// automatically update the external address.
///
/// If the external address needs to be modified, use `update_enr_udp_socket.
pub fn update_enr_tcp_port(&mut self, port: u16) -> Result<(), String> {
///
/// This returns Ok(true) if the ENR was updated, otherwise Ok(false) if nothing was done.
pub fn update_enr_tcp_port(&mut self, port: u16, v6: bool) -> Result<bool, String> {

let enr_field = if v6 {
if self.discv5.external_enr().read().tcp6() == Some(port) {
// The field is already set to the same value, nothing to do
return Ok(false);
}
"tcp6"
} else {
if self.discv5.external_enr().read().tcp4() == Some(port) {
// The field is already set to the same value, nothing to do
return Ok(false);
}
"tcp"
};
AgeManning marked this conversation as resolved.
Show resolved Hide resolved

self.discv5
.enr_insert("tcp", &port)
.enr_insert(enr_field, &port)
.map_err(|e| format!("{:?}", e))?;

// replace the global version
*self.network_globals.local_enr.write() = self.discv5.local_enr();
// persist modified enr to disk
enr::save_enr_to_disk(Path::new(&self.enr_dir), &self.local_enr(), &self.log);
Ok(())
Ok(true)
}

// TODO: Group these functions here once the ENR is shared across discv5 and lighthouse and
// Lighthouse can modify the ENR directly.
// This currently doesn't support ipv6. All of these functions should be removed and
// addressed properly in the following issue.
// https://github.com/sigp/lighthouse/issues/4706
pub fn update_enr_quic_port(&mut self, port: u16) -> Result<(), String> {
pub fn update_enr_quic_port(&mut self, port: u16, v6: bool) -> Result<bool, String> {

let enr_field = if v6 {
if self.discv5.external_enr().read().quic6() == Some(port) {
// The field is already set to the same value, nothing to do
return Ok(false);
}
"quic6"
} else {
if self.discv5.external_enr().read().quic4() == Some(port) {
// The field is already set to the same value, nothing to do
return Ok(false);
}
"quic"
};
let current_field = self.discv5.external_enr().read().quic4();
if current_field == Some(port) {
// The current field is already set, no need to update.
return Ok(false);
}

self.discv5
.enr_insert("quic", &port)
.enr_insert(enr_field, &port)
.map_err(|e| format!("{:?}", e))?;

// replace the global version
*self.network_globals.local_enr.write() = self.discv5.local_enr();
// persist modified enr to disk
enr::save_enr_to_disk(Path::new(&self.enr_dir), &self.local_enr(), &self.log);
Ok(())
Ok(true)
}

/// Updates the local ENR UDP socket.
Expand Down Expand Up @@ -1057,15 +1094,15 @@ impl<TSpec: EthSpec> NetworkBehaviour for Discovery<TSpec> {
return;
}

self.update_enr_tcp_port(port)
self.update_enr_tcp_port(port, false)
}
(Some(Protocol::Udp(port)), Some(Protocol::QuicV1)) => {
if !self.update_ports.quic4 {
debug!(self.log, "Skipping ENR update"; "multiaddr" => ?addr);
return;
}

self.update_enr_quic_port(port)
self.update_enr_quic_port(port, false)
}
_ => {
debug!(self.log, "Encountered unacceptable multiaddr for listening (unsupported transport)"; "addr" => ?addr);
Expand All @@ -1079,15 +1116,15 @@ impl<TSpec: EthSpec> NetworkBehaviour for Discovery<TSpec> {
return;
}

self.update_enr_tcp_port(port)
self.update_enr_tcp_port(port, true)
}
(Some(Protocol::Udp(port)), Some(Protocol::QuicV1)) => {
if !self.update_ports.quic6 {
debug!(self.log, "Skipping ENR update"; "multiaddr" => ?addr);
return;
}

self.update_enr_quic_port(port)
self.update_enr_quic_port(port, true)
}
_ => {
debug!(self.log, "Encountered unacceptable multiaddr for listening (unsupported transport)"; "addr" => ?addr);
Expand All @@ -1103,9 +1140,10 @@ impl<TSpec: EthSpec> NetworkBehaviour for Discovery<TSpec> {
let local_enr: Enr = self.discv5.local_enr();

match attempt_enr_update {
Ok(_) => {
Ok(true) => {
info!(self.log, "Updated local ENR"; "enr" => local_enr.to_base64(), "seq" => local_enr.seq(), "id"=> %local_enr.node_id(), "ip4" => ?local_enr.ip4(), "udp4"=> ?local_enr.udp4(), "tcp4" => ?local_enr.tcp4(), "tcp6" => ?local_enr.tcp6(), "udp6" => ?local_enr.udp6())
}
Ok(false) => {} // Nothing to do, ENR already configured
Err(e) => warn!(self.log, "Failed to update ENR"; "error" => ?e),
}
}
Expand Down
14 changes: 10 additions & 4 deletions beacon_node/lighthouse_network/src/service/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1651,12 +1651,18 @@ impl<AppReqId: ReqId, TSpec: EthSpec> Network<AppReqId, TSpec> {
libp2p::upnp::Event::NewExternalAddr(addr) => {
info!(self.log, "UPnP route established"; "addr" => %addr);
let mut iter = addr.iter();
// Skip Ip address.
iter.next();
let is_ip6 = {
let addr = iter.next();
if let Some(MProtocol::Ip6(_)) = addr {
true
} else {
false
}
};
match iter.next() {
Some(multiaddr::Protocol::Udp(udp_port)) => match iter.next() {
Some(multiaddr::Protocol::QuicV1) => {
if let Err(e) = self.discovery_mut().update_enr_quic_port(udp_port) {
if let Err(e) = self.discovery_mut().update_enr_quic_port(udp_port, is_ip6) {
warn!(self.log, "Failed to update ENR"; "error" => e);
}
}
Expand All @@ -1665,7 +1671,7 @@ impl<AppReqId: ReqId, TSpec: EthSpec> Network<AppReqId, TSpec> {
}
},
Some(multiaddr::Protocol::Tcp(tcp_port)) => {
if let Err(e) = self.discovery_mut().update_enr_tcp_port(tcp_port) {
if let Err(e) = self.discovery_mut().update_enr_tcp_port(tcp_port, is_ip6) {
warn!(self.log, "Failed to update ENR"; "error" => e);
}
}
Expand Down
Loading