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.

Sign up for GitHub

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

Jump to bottom

Improve ENR updates #5483

Merged
merged 4 commits into from
May 1, 2024
Merged

Improve ENR updates #5483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1b770a3
Improve ENR updates
AgeManning Mar 26, 2024
364bb6e
forever fmt
AgeManning Mar 28, 2024
2dee89d
Appease my old friend clippy
AgeManning Mar 28, 2024
68af60d
Merge network unstable
AgeManning May 1, 2024
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
58 changes: 47 additions & 11 deletions beacon_node/lighthouse_network/src/discovery/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,33 +398,68 @@ impl<E: EthSpec> Discovery<E> {
/// 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 +1092,15 @@ impl<E: EthSpec> NetworkBehaviour for Discovery<E> {
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 +1114,15 @@ impl<E: EthSpec> NetworkBehaviour for Discovery<E> {
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 +1138,10 @@ impl<E: EthSpec> NetworkBehaviour for Discovery<E> {
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
12 changes: 8 additions & 4 deletions beacon_node/lighthouse_network/src/service/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1681,12 +1681,16 @@ impl<AppReqId: ReqId, E: EthSpec> Network<AppReqId, E> {
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();
matches!(addr, Some(MProtocol::Ip6(_)))
};
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 @@ -1695,7 +1699,7 @@ impl<AppReqId: ReqId, E: EthSpec> Network<AppReqId, E> {
}
},
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