KBucketsTable.applied_pending is confusing

[nazar-pc]

I was looking into #5626 and noticed that all iterators require exclusive (mutable) access to the whole KBucketsTable, moreover, its buckets are being modified during iteration over k-buckets, which is extremely confusing and feels like a completely arbitrary place to do this:

rust-libp2p/protocols/kad/src/kbucket.rs

Lines 463 to 501 in 417968e

	impl<TTarget, TKey, TVal, TMap, TOut> Iterator for ClosestIter<'_, TTarget, TKey, TVal, TMap, TOut>
	where
	TTarget: AsRef<KeyBytes>,
	TKey: Clone + AsRef<KeyBytes>,
	TVal: Clone,
	TMap: Fn(&KBucket<TKey, TVal>) -> Vec<TOut>,
	TOut: AsRef<KeyBytes>,
	{
	type Item = TOut;
	fn next(&mut self) -> Option<Self::Item> {
	loop {
	match &mut self.iter {
	Some(iter) => match iter.next() {
	Some(k) => return Some(k),
	None => self.iter = None,
	},
	None => {
	if let Some(i) = self.buckets_iter.next() {
	let bucket = &mut self.table.buckets[i.get()];
	if let Some(applied) = bucket.apply_pending() {
	self.table.applied_pending.push_back(applied)
	}
	let mut v = (self.fmap)(bucket);
	v.sort_by(|a, b| {
	self.target
	.as_ref()
	.distance(a.as_ref())
	.cmp(&self.target.as_ref().distance(b.as_ref()))
	});
	self.iter = Some(v.into_iter());
	} else {
	return None;
	}
	}
	}
	}
	}
	}

Specifically this part:

rust-libp2p/protocols/kad/src/kbucket.rs

Lines 482 to 485 in 417968e

	let bucket = &mut self.table.buckets[i.get()];
	if let Some(applied) = bucket.apply_pending() {
	self.table.applied_pending.push_back(applied)
	}

I did a quick look around, but wasn't able to easily fix this. Someone with better knowledge of the codebase should look into it. It is very hard to maintain the project when code so convoluted.

[guillaumemichel]

I just had a look at the code, and I also find it very convoluted. It can definitely be simplified, but it will take a non negligible amount of refactor.