From 32e917ffb26a5c176691602b8e315b570be37211 Mon Sep 17 00:00:00 2001 From: stormshield-frb <144998884+stormshield-frb@users.noreply.github.com> Date: Tue, 18 Jun 2024 19:59:44 +0200 Subject: [PATCH] fix(kad): correctly handle `NoKnownPeers` error when bootstrap After testing `master`, we encountered a bug due to #4838 when doing automatic or periodic bootstrap if the node has no known peers. Since it failed immediately, I though there was no need to call the `bootstrap_status.on_started` method. But not doing so never resets the periodic timer inside `bootstrap_status` resulting in getting stuck to try to bootstrap every time `poll` is called on `kad::Behaviour`. Pull-Request: #5349. --- protocols/kad/CHANGELOG.md | 2 ++ protocols/kad/src/behaviour.rs | 1 + protocols/kad/src/bootstrap.rs | 18 +++++++++++------- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/protocols/kad/CHANGELOG.md b/protocols/kad/CHANGELOG.md index 1d124fb37a2..5cd537cae9e 100644 --- a/protocols/kad/CHANGELOG.md +++ b/protocols/kad/CHANGELOG.md @@ -19,6 +19,8 @@ See [PR 5317](https://github.com/libp2p/rust-libp2p/pull/5317). - Use `web-time` instead of `instant`. See [PR 5347](https://github.com/libp2p/rust-libp2p/pull/5347). +- Correctly handle the `NoKnownPeers` error on automatic bootstrap. + See [PR 5349](https://github.com/libp2p/rust-libp2p/pull/5349). ## 0.45.3 diff --git a/protocols/kad/src/behaviour.rs b/protocols/kad/src/behaviour.rs index 5d8a8bb0fd8..c9690d2c873 100644 --- a/protocols/kad/src/behaviour.rs +++ b/protocols/kad/src/behaviour.rs @@ -939,6 +939,7 @@ where }; let peers = self.kbuckets.closest_keys(&local_key).collect::>(); if peers.is_empty() { + self.bootstrap_status.reset_timers(); Err(NoKnownPeers()) } else { self.bootstrap_status.on_started(); diff --git a/protocols/kad/src/bootstrap.rs b/protocols/kad/src/bootstrap.rs index bcb9d0cccf3..fd9e3d41be6 100644 --- a/protocols/kad/src/bootstrap.rs +++ b/protocols/kad/src/bootstrap.rs @@ -61,19 +61,23 @@ impl Status { } } + pub(crate) fn reset_timers(&mut self) { + // Canceling the `throttle_timer` if any and resetting the `delay` if any. + self.throttle_timer = None; + + if let Some((interval, delay)) = self.interval_and_delay.as_mut() { + delay.reset(*interval); + } + } + pub(crate) fn on_started(&mut self) { // No periodic or automatic bootstrap will be triggered as long as // `self.current_bootstrap_requests > 0` but the user could still manually // trigger a bootstrap. self.current_bootstrap_requests += 1; - // Canceling the `throttle_timer` if any since a bootstrap request is being triggered right now. - self.throttle_timer = None; - - // Resetting the `delay` if any since a bootstrap request is being triggered right now. - if let Some((interval, delay)) = self.interval_and_delay.as_mut() { - delay.reset(*interval); - } + // Resetting the Status timers since a bootstrap request is being triggered right now. + self.reset_timers(); } pub(crate) fn on_finish(&mut self) {