From 515fcc952cd52504ab7d3866a83adb9bf0f8e56b Mon Sep 17 00:00:00 2001
From: Liu-Cheng Xu <xuliuchengxlc@gmail.com>
Date: Sat, 21 Sep 2024 02:27:39 +0800
Subject: [PATCH] Avoid unnecessary state reset of `allowed_requests` when no
 block requests are sent (#5774)

This PR is cherry-picked from
https://github.com/paritytech/polkadot-sdk/pull/5663 so that I can
maintain a smaller polkadot-sdk diff downstream sooner than later.

cc @lexnv @dmitry-markin

---------

Co-authored-by: Alexandru Vasile <60601340+lexnv@users.noreply.github.com>
Co-authored-by: Dmitry Markin <dmitry@markin.tech>
---
 prdoc/pr_5774.prdoc                              | 13 +++++++++++++
 .../network/sync/src/strategy/chain_sync.rs      | 16 +++++++++++++---
 2 files changed, 26 insertions(+), 3 deletions(-)
 create mode 100644 prdoc/pr_5774.prdoc

diff --git a/prdoc/pr_5774.prdoc b/prdoc/pr_5774.prdoc
new file mode 100644
index 000000000000..15aa64f54104
--- /dev/null
+++ b/prdoc/pr_5774.prdoc
@@ -0,0 +1,13 @@
+title: Avoid unnecessary state reset of allowed_requests when no block requests are sent
+
+doc:
+  - audience: Node Dev
+    description: |
+      Previously, the state of `allowed_requests` was always reset to the default
+      even if there were no new block requests. This could cause an edge case
+      because `peer_block_request()` will return early next time when there are no ongoing block requests.
+      This patch fixes it by checking whether block requests are empty before updating the state.
+
+crates:
+  - name: sc-network-sync
+    bump: patch
diff --git a/substrate/client/network/sync/src/strategy/chain_sync.rs b/substrate/client/network/sync/src/strategy/chain_sync.rs
index fd0e3ea1a76c..b0e28d00f64a 100644
--- a/substrate/client/network/sync/src/strategy/chain_sync.rs
+++ b/substrate/client/network/sync/src/strategy/chain_sync.rs
@@ -148,6 +148,7 @@ impl Metrics {
 	}
 }
 
+#[derive(Debug, Clone)]
 enum AllowedRequests {
 	Some(HashSet<PeerId>),
 	All,
@@ -1714,13 +1715,14 @@ where
 		let best_queued = self.best_queued_number;
 		let client = &self.client;
 		let queue_blocks = &self.queue_blocks;
-		let allowed_requests = self.allowed_requests.take();
+		let allowed_requests = self.allowed_requests.clone();
 		let max_parallel = if is_major_syncing { 1 } else { self.max_parallel_downloads };
 		let max_blocks_per_request = self.max_blocks_per_request;
 		let gap_sync = &mut self.gap_sync;
 		let disconnected_peers = &mut self.disconnected_peers;
 		let metrics = self.metrics.as_ref();
-		self.peers
+		let requests = self
+			.peers
 			.iter_mut()
 			.filter_map(move |(&id, peer)| {
 				if !peer.state.is_available() ||
@@ -1819,7 +1821,15 @@ where
 					None
 				}
 			})
-			.collect()
+			.collect::<Vec<_>>();
+
+		// Clear the allowed_requests state when sending new block requests
+		// to prevent multiple inflight block requests from being issued.
+		if !requests.is_empty() {
+			self.allowed_requests.take();
+		}
+
+		requests
 	}
 
 	/// Get a state request scheduled by sync to be sent out (if any).