Skip to content

Commit

Permalink
Fall back to default custody requiremnt if peer ENR is not present.
Browse files Browse the repository at this point in the history
  • Loading branch information
jimmygchen committed Jul 1, 2024
1 parent 4373a28 commit 014c994
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 4 deletions.
20 changes: 19 additions & 1 deletion beacon_node/lighthouse_network/src/peer_manager/peerdb.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use crate::discovery::enr::PEERDAS_CUSTODY_SUBNET_COUNT_ENR_KEY;
use crate::discovery::CombinedKey;
use crate::{metrics, multiaddr::Multiaddr, types::Subnet, Enr, EnrExt, Gossipsub, PeerId};
#[cfg(test)]
use crate::EnrExt;
use crate::{metrics, multiaddr::Multiaddr, types::Subnet, Enr, Gossipsub, PeerId};
use peer_info::{ConnectionDirection, PeerConnectionStatus, PeerInfo};
use rand::seq::SliceRandom;
use score::{PeerAction, ReportSource, Score, ScoreState};
Expand Down Expand Up @@ -703,6 +705,7 @@ impl<E: EthSpec> PeerDB<E> {
}

/// Updates the connection state. MUST ONLY BE USED IN TESTS.
#[cfg(test)]
pub(crate) fn __add_connected_peer_enr_testing_only(
&mut self,
enr: Enr,
Expand All @@ -716,6 +719,21 @@ impl<E: EthSpec> PeerDB<E> {
self.update_connection_state(&peer_id, new_state)
}

/// Updates the connection state. MUST ONLY BE USED IN TESTS.
#[cfg(test)]
pub(crate) fn __add_connected_peer_multiaddr_testing_only(
&mut self,
peer_id: &PeerId,
multiaddr: Multiaddr,
) -> Option<BanOperation> {
let new_state = NewConnectionState::Connected {
enr: None,
seen_address: multiaddr,
direction: ConnectionDirection::Outgoing,
};
self.update_connection_state(&peer_id, new_state)
}

/// The connection state of the peer has been changed. Modify the peer in the db to ensure all
/// variables are in sync with libp2p.
/// Updating the state can lead to a `BanOperation` which needs to be processed via the peer
Expand Down
48 changes: 45 additions & 3 deletions beacon_node/lighthouse_network/src/types/globals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::types::{BackFillState, SyncState};
use crate::EnrExt;
use crate::{Client, Eth2Enr};
use crate::{Enr, GossipTopic, Multiaddr, PeerId};
use discv5::handler::NodeContact;
use itertools::Itertools;
use parking_lot::RwLock;
use std::collections::HashSet;
Expand Down Expand Up @@ -138,12 +139,28 @@ impl<E: EthSpec> NetworkGlobals<E> {
.read()
.connected_peers()
.filter_map(|(peer_id, peer_info)| {
peer_info.enr().and_then(|enr| {
let node_id_and_csc = if let Some(enr) = peer_info.enr() {
let custody_subnet_count = enr.custody_subnet_count::<E>(spec);
Some((enr.node_id(), custody_subnet_count))
} else if let Some(node_contact) = peer_info
.seen_multiaddrs()
.last()
.cloned()
.and_then(|multiaddr| NodeContact::try_from_multiaddr(multiaddr).ok())
{
let node_id = node_contact.node_id();
// TODO(das): Use `custody_subnet_count` from `MetaDataV3` before
// falling back to minimum custody requirement.
Some((node_id, spec.custody_requirement))
} else {
None
};

node_id_and_csc.and_then(|(node_id, custody_subnet_count)| {
// TODO(das): consider caching a map of subnet -> Vec<PeerId> and invalidating
// whenever a peer connected or disconnect event in received
DataColumnSubnetId::compute_custody_columns::<E>(
enr.node_id().raw().into(),
node_id.raw().into(),
custody_subnet_count,
spec,
)
Expand Down Expand Up @@ -197,7 +214,7 @@ mod test {
}

#[test]
fn custody_peers_for_column() {
fn custody_peers_for_column_enr_present() {
let spec = E::default_spec();
let log = logging::test_logger();
let globals = NetworkGlobals::<E>::new_test_globals(vec![], &log);
Expand Down Expand Up @@ -235,4 +252,29 @@ mod test {
);
}
}

// If ENR is not preset, fallback to deriving node_id and use `spec.custody_requirement`.
#[test]
fn custody_peers_for_column_no_enr_use_default() {
let spec = E::default_spec();
let log = logging::test_logger();
let globals = NetworkGlobals::<E>::new_test_globals(vec![], &log);

// Add peer without enr
let peer_id_str = "16Uiu2HAm86zWajwnBFD8uxkRpxhRzeUEf6Brfz2VBxGAaWx9ejyr";
let peer_id = PeerId::from_str(peer_id_str).unwrap();
let multiaddr =
Multiaddr::from_str(&format!("/ip4/0.0.0.0/udp/9000/p2p/{peer_id_str}")).unwrap();

let mut peers_db_write_lock = globals.peers.write();
peers_db_write_lock.__add_connected_peer_multiaddr_testing_only(&peer_id, multiaddr);
drop(peers_db_write_lock);

let custody_subnets = (0..spec.data_column_sidecar_subnet_count)
.filter(|col_index| globals.custody_peers_for_column(*col_index, &spec).len() > 0)
.count();

// The single peer's custody subnet should match custody_requirement.
assert_eq!(custody_subnets, spec.custody_requirement as usize);
}
}

0 comments on commit 014c994

Please sign in to comment.