Skip to content

Commit

Permalink
Network: correctly update own peer contact
Browse files Browse the repository at this point in the history
  • Loading branch information
styppo committed Jan 22, 2025
1 parent 09e6a5d commit a84c105
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 39 deletions.
8 changes: 3 additions & 5 deletions network-libp2p/src/discovery/behaviour.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ impl Behaviour {
#[cfg(feature = "kad")] verifier: Arc<dyn ValidatorRecordVerifier>,
) -> Self {
let house_keeping_timer = interval(config.house_keeping_interval);
peer_contact_book.write().update_own_contact(&keypair);
peer_contact_book.write().refresh_own_contact();

// Report our own known addresses as candidates to the swarm
let mut events = VecDeque::new();
Expand All @@ -151,9 +151,7 @@ impl Behaviour {

/// Adds addresses into our own contact within the peer contact book
pub fn add_own_addresses(&self, addresses: Vec<Multiaddr>) {
self.peer_contact_book
.write()
.add_own_addresses(addresses, &self.keypair)
self.peer_contact_book.write().add_own_addresses(addresses)
}

/// Returns whether an address in `Multiaddr` format is a dialable websocket address
Expand Down Expand Up @@ -238,7 +236,7 @@ impl NetworkBehaviour for Behaviour {
Poll::Ready(Some(_)) => {
trace!("Doing house-keeping in peer address book");
let mut peer_address_book = self.peer_contact_book.write();
peer_address_book.update_own_contact(&self.keypair);
peer_address_book.refresh_own_contact();
peer_address_book.house_keeping();
}
Poll::Ready(None) => unreachable!(),
Expand Down
47 changes: 25 additions & 22 deletions network-libp2p/src/discovery/peer_contacts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -429,10 +429,12 @@ pub struct PeerContactBook {
own_peer_contact: PeerContactInfo,
/// Own Peer ID (also present in `own_peer_contact`)
own_peer_id: PeerId,
/// Identity keypair for this node.
key_pair: Keypair,
/// Contact information for other peers in the network indexed by their
/// peer ID.
peer_contacts: HashMap<PeerId, Arc<PeerContactInfo>>,
/// Reverse map when we
/// Map from validator address to peer ids.
validator_peer_ids: HashMap<Address, HashSet<PeerId>>,
/// Only return secure websocket addresses.
/// With this flag non secure websocket addresses will be stored (to still have a valid signature of the peer contact)
Expand Down Expand Up @@ -468,15 +470,19 @@ impl PeerContactBook {

/// Creates a new `PeerContactBook` given our own peer contact information.
pub fn new(
own_peer_contact: SignedPeerContact,
mut own_peer_contact: PeerContact,
key_pair: Keypair,
only_secure_addresses: bool,
allow_loopback_addresses: bool,
memory_transport: bool,
) -> Self {
let own_peer_id = own_peer_contact.inner.peer_id();
let own_peer_id = own_peer_contact.peer_id();
own_peer_contact.set_current_time();

Self {
own_peer_contact: own_peer_contact.into(),
own_peer_contact: own_peer_contact.sign(&key_pair).into(),
own_peer_id,
key_pair,
peer_contacts: HashMap::new(),
only_secure_addresses,
allow_loopback_addresses,
Expand Down Expand Up @@ -703,37 +709,33 @@ impl PeerContactBook {
}

/// Adds a set of addresses to the list of addresses known for our own contact.
pub fn add_own_addresses<I: IntoIterator<Item = Multiaddr>>(
&mut self,
addresses: I,
keypair: &Keypair,
) {
pub fn add_own_addresses<I: IntoIterator<Item = Multiaddr>>(&mut self, addresses: I) {
let mut contact = self.own_peer_contact.contact.inner.clone();
let addresses = addresses.into_iter().collect::<Vec<Multiaddr>>();
trace!(?addresses, "Adding own addresses");
contact.add_addresses(addresses);
self.own_peer_contact = PeerContactInfo::from(contact.sign(keypair));
self.update_own_contact(contact);
}

/// Removes a set of addresses from the list of addresses known for our own.
pub fn remove_own_addresses<I: IntoIterator<Item = Multiaddr>>(
&mut self,
addresses: I,
keypair: &Keypair,
) {
pub fn remove_own_addresses<I: IntoIterator<Item = Multiaddr>>(&mut self, addresses: I) {
let mut contact = self.own_peer_contact.contact.inner.clone();
let addresses = addresses.into_iter().collect::<Vec<Multiaddr>>();
contact.remove_addresses(addresses);
self.own_peer_contact = PeerContactInfo::from(contact.sign(keypair));
self.update_own_contact(contact);
}

/// Updates the timestamp of our own contact
pub fn update_own_contact(&mut self, keypair: &Keypair) {
// Not really optimal to clone here, but *shrugs*
let mut contact = self.own_peer_contact.contact.inner.clone();
/// Updates the timestamp of our own contact.
pub fn refresh_own_contact(&mut self) {
let contact = self.own_peer_contact.contact.inner.clone();
self.update_own_contact(contact);
}

// Update timestamp
fn update_own_contact(&mut self, mut contact: PeerContact) {
// Update timestamp.
contact.set_current_time();

// Update validator info.
contact.validator_info = self.validator_record_signing.as_ref().and_then(|callback| {
let tagged_signed = (callback)(contact.peer_id(), contact.timestamp);
Some(ValidatorInfo {
Expand All @@ -742,7 +744,7 @@ impl PeerContactBook {
})
});

Check warning on line 745 in network-libp2p/src/discovery/peer_contacts.rs

View workflow job for this annotation

GitHub Actions / Clippy Report

using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)`

warning: using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)` --> network-libp2p/src/discovery/peer_contacts.rs:739:34 | 739 | contact.validator_info = self.validator_record_signing.as_ref().and_then(|callback| { | __________________________________^ 740 | | let tagged_signed = (callback)(contact.peer_id(), contact.timestamp); 741 | | Some(ValidatorInfo { 742 | | validator_address: tagged_signed.record.validator_address.clone(), 743 | | signature: tagged_signed.signature.clone(), 744 | | }) 745 | | }); | |__________^ | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#bind_instead_of_map = note: `#[warn(clippy::bind_instead_of_map)]` on by default help: use `map` instead | 739 ~ contact.validator_info = self.validator_record_signing.as_ref().map(|callback| { 740 | let tagged_signed = (callback)(contact.peer_id(), contact.timestamp); 741 ~ ValidatorInfo { 742 + validator_address: tagged_signed.record.validator_address.clone(), 743 + signature: tagged_signed.signature.clone(), 744 + } |

self.own_peer_contact = PeerContactInfo::from(contact.sign(keypair));
self.own_peer_contact = PeerContactInfo::from(contact.sign(&self.key_pair));
}

/// Gets our own contact information
Expand Down Expand Up @@ -856,6 +858,7 @@ impl PeerContactBook {
+ 'static,
) {
self.validator_record_signing = Some(Box::new(callback));
self.refresh_own_contact();
}
}

Expand Down
3 changes: 2 additions & 1 deletion network-libp2p/src/network.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ impl Network {
// TODO: persist to disk
let own_peer_contact = config.peer_contact.clone();
let contacts = Arc::new(RwLock::new(PeerContactBook::new(
own_peer_contact.sign(&config.keypair),
own_peer_contact,
config.keypair.clone(),
config.only_secure_ws_connections,
config.allow_loopback_addresses,
config.memory_transport,
Expand Down
21 changes: 10 additions & 11 deletions network-libp2p/tests/discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,11 @@ impl TestNode {
.unwrap()
.as_secs(),
)
.expect("PeerContact must be creatable")
.sign(&keypair);
.expect("PeerContact must be creatable");

let peer_contact_book = Arc::new(RwLock::new(PeerContactBook::new(
peer_contact,
keypair.clone(),
false,
true,
true,
Expand Down Expand Up @@ -127,6 +127,11 @@ impl TestNode {
}

fn random_peer_contact(n: usize, services: Services) -> SignedPeerContact {
let (keypair, peer_contact) = random_peer_key_and_contact(n, services);
peer_contact.sign(&keypair)
}

fn random_peer_key_and_contact(n: usize, services: Services) -> (Keypair, PeerContact) {
let keypair = Keypair::generate_ed25519();

let peer_contact = PeerContact::new(
Expand All @@ -144,9 +149,8 @@ fn random_peer_contact(n: usize, services: Services) -> SignedPeerContact {
)
.expect("PeerContact must be creatable");

peer_contact.sign(&keypair)
(keypair, peer_contact)
}

fn test_peers_in_contact_book(
peer_contact_book: &PeerContactBook,
peer_contacts: &[SignedPeerContact],
Expand Down Expand Up @@ -257,18 +261,13 @@ pub async fn test_dialing_peer_from_contacts() {

#[test]
fn test_housekeeping() {
let mut peer_contact_book = PeerContactBook::new(
random_peer_contact(1, Services::FULL_BLOCKS),
false,
true,
true,
);
let (keypair, peer_contact) = random_peer_key_and_contact(1, Services::FULL_BLOCKS);
let mut peer_contact_book = PeerContactBook::new(peer_contact, keypair, false, true, true);

let fresh_contact = random_peer_contact(1, Services::FULL_BLOCKS);

let old_contact = {
let keypair = Keypair::generate_ed25519();

let peer_contact = PeerContact::new(
Some("/dns/test_old.local/tcp/443/wss".parse().unwrap()),
keypair.public(),
Expand Down

0 comments on commit a84c105

Please sign in to comment.