p2p/address-book: enable workspace lints (#286)

* address-book: enable workspace lints

* fix

* fixes
This commit is contained in:
hinto-janai 2024-09-18 18:18:31 -04:00 committed by GitHub
parent 2afc0e8373
commit a1267619ef
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 52 additions and 55 deletions

1
Cargo.lock generated
View file

@ -504,7 +504,6 @@ dependencies = [
"cuprate-p2p-core", "cuprate-p2p-core",
"cuprate-pruning", "cuprate-pruning",
"cuprate-test-utils", "cuprate-test-utils",
"cuprate-wire",
"futures", "futures",
"indexmap", "indexmap",
"rand", "rand",

View file

@ -8,7 +8,6 @@ authors = ["Boog900"]
[dependencies] [dependencies]
cuprate-pruning = { path = "../../pruning" } cuprate-pruning = { path = "../../pruning" }
cuprate-wire = { path= "../../net/wire" }
cuprate-p2p-core = { path = "../p2p-core" } cuprate-p2p-core = { path = "../p2p-core" }
tower = { workspace = true, features = ["util"] } tower = { workspace = true, features = ["util"] }
@ -29,3 +28,6 @@ borsh = { workspace = true, features = ["derive", "std"]}
cuprate-test-utils = {path = "../../test-utils"} cuprate-test-utils = {path = "../../test-utils"}
tokio = { workspace = true, features = ["rt-multi-thread", "macros"]} tokio = { workspace = true, features = ["rt-multi-thread", "macros"]}
[lints]
workspace = true

View file

@ -36,7 +36,7 @@ use crate::{
mod tests; mod tests;
/// An entry in the connected list. /// An entry in the connected list.
pub struct ConnectionPeerEntry<Z: NetworkZone> { pub(crate) struct ConnectionPeerEntry<Z: NetworkZone> {
addr: Option<Z::Addr>, addr: Option<Z::Addr>,
id: u64, id: u64,
handle: ConnectionHandle, handle: ConnectionHandle,
@ -109,14 +109,14 @@ impl<Z: BorshNetworkZone> AddressBook<Z> {
match handle.poll_unpin(cx) { match handle.poll_unpin(cx) {
Poll::Pending => return, Poll::Pending => return,
Poll::Ready(Ok(Err(e))) => { Poll::Ready(Ok(Err(e))) => {
tracing::error!("Could not save peer list to disk, got error: {}", e) tracing::error!("Could not save peer list to disk, got error: {e}");
} }
Poll::Ready(Err(e)) => { Poll::Ready(Err(e)) => {
if e.is_panic() { if e.is_panic() {
panic::resume_unwind(e.into_panic()) panic::resume_unwind(e.into_panic())
} }
} }
_ => (), Poll::Ready(_) => (),
} }
} }
// the task is finished. // the task is finished.
@ -144,6 +144,7 @@ impl<Z: BorshNetworkZone> AddressBook<Z> {
let mut internal_addr_disconnected = Vec::new(); let mut internal_addr_disconnected = Vec::new();
let mut addrs_to_ban = Vec::new(); let mut addrs_to_ban = Vec::new();
#[expect(clippy::iter_over_hash_type, reason = "ordering doesn't matter here")]
for (internal_addr, peer) in &mut self.connected_peers { for (internal_addr, peer) in &mut self.connected_peers {
if let Some(time) = peer.handle.check_should_ban() { if let Some(time) = peer.handle.check_should_ban() {
match internal_addr { match internal_addr {
@ -158,7 +159,7 @@ impl<Z: BorshNetworkZone> AddressBook<Z> {
} }
} }
for (addr, time) in addrs_to_ban.into_iter() { for (addr, time) in addrs_to_ban {
self.ban_peer(addr, time); self.ban_peer(addr, time);
} }
@ -172,12 +173,7 @@ impl<Z: BorshNetworkZone> AddressBook<Z> {
.remove(&addr); .remove(&addr);
// If the amount of peers with this ban id is 0 remove the whole set. // If the amount of peers with this ban id is 0 remove the whole set.
if self if self.connected_peers_ban_id[&addr.ban_id()].is_empty() {
.connected_peers_ban_id
.get(&addr.ban_id())
.unwrap()
.is_empty()
{
self.connected_peers_ban_id.remove(&addr.ban_id()); self.connected_peers_ban_id.remove(&addr.ban_id());
} }
// remove the peer from the anchor list. // remove the peer from the anchor list.
@ -188,7 +184,7 @@ impl<Z: BorshNetworkZone> AddressBook<Z> {
fn ban_peer(&mut self, addr: Z::Addr, time: Duration) { fn ban_peer(&mut self, addr: Z::Addr, time: Duration) {
if self.banned_peers.contains_key(&addr.ban_id()) { if self.banned_peers.contains_key(&addr.ban_id()) {
tracing::error!("Tried to ban peer twice, this shouldn't happen.") tracing::error!("Tried to ban peer twice, this shouldn't happen.");
} }
if let Some(connected_peers_with_ban_id) = self.connected_peers_ban_id.get(&addr.ban_id()) { if let Some(connected_peers_with_ban_id) = self.connected_peers_ban_id.get(&addr.ban_id()) {
@ -242,10 +238,10 @@ impl<Z: BorshNetworkZone> AddressBook<Z> {
peer_list.retain_mut(|peer| { peer_list.retain_mut(|peer| {
peer.adr.make_canonical(); peer.adr.make_canonical();
if !peer.adr.should_add_to_peer_list() { if peer.adr.should_add_to_peer_list() {
false
} else {
!self.is_peer_banned(&peer.adr) !self.is_peer_banned(&peer.adr)
} else {
false
} }
// TODO: check rpc/ p2p ports not the same // TODO: check rpc/ p2p ports not the same
}); });
@ -391,7 +387,7 @@ impl<Z: BorshNetworkZone> Service<AddressBookRequest<Z>> for AddressBook<Z> {
rpc_credits_per_hash, rpc_credits_per_hash,
}, },
) )
.map(|_| AddressBookResponse::Ok), .map(|()| AddressBookResponse::Ok),
AddressBookRequest::IncomingPeerList(peer_list) => { AddressBookRequest::IncomingPeerList(peer_list) => {
self.handle_incoming_peer_list(peer_list); self.handle_incoming_peer_list(peer_list);
Ok(AddressBookResponse::Ok) Ok(AddressBookResponse::Ok)

View file

@ -109,7 +109,7 @@ async fn add_new_peer_already_connected() {
}, },
), ),
Err(AddressBookError::PeerAlreadyConnected) Err(AddressBookError::PeerAlreadyConnected)
) );
} }
#[tokio::test] #[tokio::test]
@ -143,5 +143,5 @@ async fn banned_peer_removed_from_peer_lists() {
.unwrap() .unwrap()
.into_inner(), .into_inner(),
TestNetZoneAddr(1) TestNetZoneAddr(1)
) );
} }

View file

@ -7,31 +7,31 @@ use cuprate_p2p_core::{services::ZoneSpecificPeerListEntryBase, NetZoneAddress,
use cuprate_pruning::{PruningSeed, CRYPTONOTE_MAX_BLOCK_HEIGHT}; use cuprate_pruning::{PruningSeed, CRYPTONOTE_MAX_BLOCK_HEIGHT};
#[cfg(test)] #[cfg(test)]
pub mod tests; pub(crate) mod tests;
/// A Peer list in the address book. /// A Peer list in the address book.
/// ///
/// This could either be the white list or gray list. /// This could either be the white list or gray list.
#[derive(Debug)] #[derive(Debug)]
pub struct PeerList<Z: NetworkZone> { pub(crate) struct PeerList<Z: NetworkZone> {
/// The peers with their peer data. /// The peers with their peer data.
pub peers: IndexMap<Z::Addr, ZoneSpecificPeerListEntryBase<Z::Addr>>, pub peers: IndexMap<Z::Addr, ZoneSpecificPeerListEntryBase<Z::Addr>>,
/// An index of Pruning seed to address, so can quickly grab peers with the blocks /// An index of Pruning seed to address, so can quickly grab peers with the blocks
/// we want. /// we want.
/// ///
/// Pruning seeds are sorted by first their log_stripes and then their stripe. /// Pruning seeds are sorted by first their `log_stripes` and then their stripe.
/// This means the first peers in this list will store more blocks than peers /// This means the first peers in this list will store more blocks than peers
/// later on. So when we need a peer with a certain block we look at the peers /// later on. So when we need a peer with a certain block we look at the peers
/// storing more blocks first then work our way to the peers storing less. /// storing more blocks first then work our way to the peers storing less.
/// ///
pruning_seeds: BTreeMap<PruningSeed, Vec<Z::Addr>>, pruning_seeds: BTreeMap<PruningSeed, Vec<Z::Addr>>,
/// A hashmap linking ban_ids to addresses. /// A hashmap linking `ban_ids` to addresses.
ban_ids: HashMap<<Z::Addr as NetZoneAddress>::BanID, Vec<Z::Addr>>, ban_ids: HashMap<<Z::Addr as NetZoneAddress>::BanID, Vec<Z::Addr>>,
} }
impl<Z: NetworkZone> PeerList<Z> { impl<Z: NetworkZone> PeerList<Z> {
/// Creates a new peer list. /// Creates a new peer list.
pub fn new(list: Vec<ZoneSpecificPeerListEntryBase<Z::Addr>>) -> PeerList<Z> { pub(crate) fn new(list: Vec<ZoneSpecificPeerListEntryBase<Z::Addr>>) -> Self {
let mut peers = IndexMap::with_capacity(list.len()); let mut peers = IndexMap::with_capacity(list.len());
let mut pruning_seeds = BTreeMap::new(); let mut pruning_seeds = BTreeMap::new();
let mut ban_ids = HashMap::with_capacity(list.len()); let mut ban_ids = HashMap::with_capacity(list.len());
@ -49,7 +49,7 @@ impl<Z: NetworkZone> PeerList<Z> {
peers.insert(peer.adr, peer); peers.insert(peer.adr, peer);
} }
PeerList { Self {
peers, peers,
pruning_seeds, pruning_seeds,
ban_ids, ban_ids,
@ -57,21 +57,20 @@ impl<Z: NetworkZone> PeerList<Z> {
} }
/// Gets the length of the peer list /// Gets the length of the peer list
pub fn len(&self) -> usize { pub(crate) fn len(&self) -> usize {
self.peers.len() self.peers.len()
} }
/// Adds a new peer to the peer list /// Adds a new peer to the peer list
pub fn add_new_peer(&mut self, peer: ZoneSpecificPeerListEntryBase<Z::Addr>) { pub(crate) fn add_new_peer(&mut self, peer: ZoneSpecificPeerListEntryBase<Z::Addr>) {
if self.peers.insert(peer.adr, peer).is_none() { if self.peers.insert(peer.adr, peer).is_none() {
// It's more clear with this #[expect(clippy::unwrap_or_default, reason = "It's more clear with this")]
#[allow(clippy::unwrap_or_default)]
self.pruning_seeds self.pruning_seeds
.entry(peer.pruning_seed) .entry(peer.pruning_seed)
.or_insert_with(Vec::new) .or_insert_with(Vec::new)
.push(peer.adr); .push(peer.adr);
#[allow(clippy::unwrap_or_default)] #[expect(clippy::unwrap_or_default)]
self.ban_ids self.ban_ids
.entry(peer.adr.ban_id()) .entry(peer.adr.ban_id())
.or_insert_with(Vec::new) .or_insert_with(Vec::new)
@ -85,7 +84,7 @@ impl<Z: NetworkZone> PeerList<Z> {
/// list. /// list.
/// ///
/// The given peer will be removed from the peer list. /// The given peer will be removed from the peer list.
pub fn take_random_peer<R: Rng>( pub(crate) fn take_random_peer<R: Rng>(
&mut self, &mut self,
r: &mut R, r: &mut R,
block_needed: Option<usize>, block_needed: Option<usize>,
@ -127,7 +126,7 @@ impl<Z: NetworkZone> PeerList<Z> {
None None
} }
pub fn get_random_peers<R: Rng>( pub(crate) fn get_random_peers<R: Rng>(
&self, &self,
r: &mut R, r: &mut R,
len: usize, len: usize,
@ -142,7 +141,7 @@ impl<Z: NetworkZone> PeerList<Z> {
} }
/// Returns a mutable reference to a peer. /// Returns a mutable reference to a peer.
pub fn get_peer_mut( pub(crate) fn get_peer_mut(
&mut self, &mut self,
peer: &Z::Addr, peer: &Z::Addr,
) -> Option<&mut ZoneSpecificPeerListEntryBase<Z::Addr>> { ) -> Option<&mut ZoneSpecificPeerListEntryBase<Z::Addr>> {
@ -150,7 +149,7 @@ impl<Z: NetworkZone> PeerList<Z> {
} }
/// Returns true if the list contains this peer. /// Returns true if the list contains this peer.
pub fn contains_peer(&self, peer: &Z::Addr) -> bool { pub(crate) fn contains_peer(&self, peer: &Z::Addr) -> bool {
self.peers.contains_key(peer) self.peers.contains_key(peer)
} }
@ -189,11 +188,11 @@ impl<Z: NetworkZone> PeerList<Z> {
/// MUST NOT BE USED ALONE /// MUST NOT BE USED ALONE
fn remove_peer_from_all_idxs(&mut self, peer: &ZoneSpecificPeerListEntryBase<Z::Addr>) { fn remove_peer_from_all_idxs(&mut self, peer: &ZoneSpecificPeerListEntryBase<Z::Addr>) {
self.remove_peer_pruning_idx(peer); self.remove_peer_pruning_idx(peer);
self.remove_peer_ban_idx(peer) self.remove_peer_ban_idx(peer);
} }
/// Removes a peer from the peer list /// Removes a peer from the peer list
pub fn remove_peer( pub(crate) fn remove_peer(
&mut self, &mut self,
peer: &Z::Addr, peer: &Z::Addr,
) -> Option<ZoneSpecificPeerListEntryBase<Z::Addr>> { ) -> Option<ZoneSpecificPeerListEntryBase<Z::Addr>> {
@ -203,7 +202,7 @@ impl<Z: NetworkZone> PeerList<Z> {
} }
/// Removes all peers with a specific ban id. /// Removes all peers with a specific ban id.
pub fn remove_peers_with_ban_id(&mut self, ban_id: &<Z::Addr as NetZoneAddress>::BanID) { pub(crate) fn remove_peers_with_ban_id(&mut self, ban_id: &<Z::Addr as NetZoneAddress>::BanID) {
let Some(addresses) = self.ban_ids.get(ban_id) else { let Some(addresses) = self.ban_ids.get(ban_id) else {
// No peers to ban // No peers to ban
return; return;
@ -217,8 +216,8 @@ impl<Z: NetworkZone> PeerList<Z> {
/// Tries to reduce the peer list to `new_len`. /// Tries to reduce the peer list to `new_len`.
/// ///
/// This function could keep the list bigger than `new_len` if `must_keep_peers`s length /// This function could keep the list bigger than `new_len` if `must_keep_peers`s length
/// is larger than new_len, in that case we will remove as much as we can. /// is larger than `new_len`, in that case we will remove as much as we can.
pub fn reduce_list(&mut self, must_keep_peers: &HashSet<Z::Addr>, new_len: usize) { pub(crate) fn reduce_list(&mut self, must_keep_peers: &HashSet<Z::Addr>, new_len: usize) {
if new_len >= self.len() { if new_len >= self.len() {
return; return;
} }

View file

@ -14,7 +14,7 @@ fn make_fake_peer(
) -> ZoneSpecificPeerListEntryBase<TestNetZoneAddr> { ) -> ZoneSpecificPeerListEntryBase<TestNetZoneAddr> {
ZoneSpecificPeerListEntryBase { ZoneSpecificPeerListEntryBase {
adr: TestNetZoneAddr(id), adr: TestNetZoneAddr(id),
id: id as u64, id: u64::from(id),
last_seen: 0, last_seen: 0,
pruning_seed: PruningSeed::decompress(pruning_seed.unwrap_or(0)).unwrap(), pruning_seed: PruningSeed::decompress(pruning_seed.unwrap_or(0)).unwrap(),
rpc_port: 0, rpc_port: 0,
@ -22,14 +22,14 @@ fn make_fake_peer(
} }
} }
pub fn make_fake_peer_list( pub(crate) fn make_fake_peer_list(
start_idx: u32, start_idx: u32,
numb_o_peers: u32, numb_o_peers: u32,
) -> PeerList<TestNetZone<true, true, true>> { ) -> PeerList<TestNetZone<true, true, true>> {
let mut peer_list = Vec::with_capacity(numb_o_peers as usize); let mut peer_list = Vec::with_capacity(numb_o_peers as usize);
for idx in start_idx..(start_idx + numb_o_peers) { for idx in start_idx..(start_idx + numb_o_peers) {
peer_list.push(make_fake_peer(idx, None)) peer_list.push(make_fake_peer(idx, None));
} }
PeerList::new(peer_list) PeerList::new(peer_list)
@ -50,7 +50,7 @@ fn make_fake_peer_list_with_random_pruning_seeds(
} else { } else {
r.gen_range(384..=391) r.gen_range(384..=391)
}), }),
)) ));
} }
PeerList::new(peer_list) PeerList::new(peer_list)
} }
@ -70,7 +70,7 @@ fn peer_list_reduce_length() {
#[test] #[test]
fn peer_list_reduce_length_with_peers_we_need() { fn peer_list_reduce_length_with_peers_we_need() {
let mut peer_list = make_fake_peer_list(0, 500); let mut peer_list = make_fake_peer_list(0, 500);
let must_keep_peers = HashSet::from_iter(peer_list.peers.keys().copied()); let must_keep_peers = peer_list.peers.keys().copied().collect::<HashSet<_>>();
let target_len = 49; let target_len = 49;
@ -92,7 +92,7 @@ fn peer_list_remove_specific_peer() {
let peers = peer_list.peers; let peers = peer_list.peers;
for (_, addrs) in pruning_idxs { for (_, addrs) in pruning_idxs {
addrs.iter().for_each(|adr| assert_ne!(adr, &peer.adr)) addrs.iter().for_each(|adr| assert_ne!(adr, &peer.adr));
} }
assert!(!peers.contains_key(&peer.adr)); assert!(!peers.contains_key(&peer.adr));
@ -104,13 +104,13 @@ fn peer_list_pruning_idxs_are_correct() {
let mut total_len = 0; let mut total_len = 0;
for (seed, list) in peer_list.pruning_seeds { for (seed, list) in peer_list.pruning_seeds {
for peer in list.iter() { for peer in &list {
assert_eq!(peer_list.peers.get(peer).unwrap().pruning_seed, seed); assert_eq!(peer_list.peers.get(peer).unwrap().pruning_seed, seed);
total_len += 1; total_len += 1;
} }
} }
assert_eq!(total_len, peer_list.peers.len()) assert_eq!(total_len, peer_list.peers.len());
} }
#[test] #[test]
@ -122,11 +122,7 @@ fn peer_list_add_new_peer() {
assert_eq!(peer_list.len(), 11); assert_eq!(peer_list.len(), 11);
assert_eq!(peer_list.peers.get(&new_peer.adr), Some(&new_peer)); assert_eq!(peer_list.peers.get(&new_peer.adr), Some(&new_peer));
assert!(peer_list assert!(peer_list.pruning_seeds[&new_peer.pruning_seed].contains(&new_peer.adr));
.pruning_seeds
.get(&new_peer.pruning_seed)
.unwrap()
.contains(&new_peer.adr));
} }
#[test] #[test]
@ -164,7 +160,7 @@ fn peer_list_get_peer_with_block() {
assert!(peer assert!(peer
.pruning_seed .pruning_seed
.get_next_unpruned_block(1, 1_000_000) .get_next_unpruned_block(1, 1_000_000)
.is_ok()) .is_ok());
} }
#[test] #[test]

View file

@ -1,3 +1,8 @@
#![expect(
single_use_lifetimes,
reason = "false positive on generated derive code on `SerPeerDataV1`"
)]
use std::fs; use std::fs;
use borsh::{from_slice, to_vec, BorshDeserialize, BorshSerialize}; use borsh::{from_slice, to_vec, BorshDeserialize, BorshSerialize};
@ -21,7 +26,7 @@ struct DeserPeerDataV1<A: NetZoneAddress> {
gray_list: Vec<ZoneSpecificPeerListEntryBase<A>>, gray_list: Vec<ZoneSpecificPeerListEntryBase<A>>,
} }
pub fn save_peers_to_disk<Z: BorshNetworkZone>( pub(crate) fn save_peers_to_disk<Z: BorshNetworkZone>(
cfg: &AddressBookConfig, cfg: &AddressBookConfig,
white_list: &PeerList<Z>, white_list: &PeerList<Z>,
gray_list: &PeerList<Z>, gray_list: &PeerList<Z>,
@ -38,7 +43,7 @@ pub fn save_peers_to_disk<Z: BorshNetworkZone>(
spawn_blocking(move || fs::write(&file, &data)) spawn_blocking(move || fs::write(&file, &data))
} }
pub async fn read_peers_from_disk<Z: BorshNetworkZone>( pub(crate) async fn read_peers_from_disk<Z: BorshNetworkZone>(
cfg: &AddressBookConfig, cfg: &AddressBookConfig,
) -> Result< ) -> Result<
( (