From 8251fce08974d593f0aedcc794182fbb9e495fd3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bastian=20K=C3=B6cher?= <bkchr@users.noreply.github.com>
Date: Fri, 5 Aug 2022 19:53:56 +0200
Subject: [PATCH] system_syncState: Always return highest block (#11979)

Before `highestBlock` was an optional that was omitted when it was `None`. We recently changed the
way the `highestBlock` is determined, this resulted in having this value in 99.99% of the time being
`None` when the node is syncing blocks at the tip. Now we always return a block for `highestBlock`.
If sync doesn't return us any best seen block, we return our own local best block as `highestBlock`.
This should mainly reflect the same behavior to before we changed the way the best seen block is determined.
---
 client/rpc-api/src/system/helpers.rs | 12 ++++++------
 client/rpc/src/system/tests.rs       |  7 ++-----
 client/service/src/lib.rs            |  6 ++++--
 3 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/client/rpc-api/src/system/helpers.rs b/client/rpc-api/src/system/helpers.rs
index 4561fccc1e81b..7ddb3f813c249 100644
--- a/client/rpc-api/src/system/helpers.rs
+++ b/client/rpc-api/src/system/helpers.rs
@@ -88,10 +88,10 @@ pub struct SyncState<Number> {
 	pub starting_block: Number,
 	/// Height of the current best block of the node.
 	pub current_block: Number,
-	/// Height of the highest block learned from the network. Missing if no block is known yet.
-	#[serde(default = "Default::default", skip_serializing_if = "Option::is_none")]
-	pub highest_block: Option<Number>,
+	/// Height of the highest block in the network.
+	pub highest_block: Number,
 }
+
 #[cfg(test)]
 mod tests {
 	use super::*;
@@ -129,7 +129,7 @@ mod tests {
 			::serde_json::to_string(&SyncState {
 				starting_block: 12u32,
 				current_block: 50u32,
-				highest_block: Some(128u32),
+				highest_block: 128u32,
 			})
 			.unwrap(),
 			r#"{"startingBlock":12,"currentBlock":50,"highestBlock":128}"#,
@@ -139,10 +139,10 @@ mod tests {
 			::serde_json::to_string(&SyncState {
 				starting_block: 12u32,
 				current_block: 50u32,
-				highest_block: None,
+				highest_block: 50u32,
 			})
 			.unwrap(),
-			r#"{"startingBlock":12,"currentBlock":50}"#,
+			r#"{"startingBlock":12,"currentBlock":50,"highestBlock":50}"#,
 		);
 	}
 }
diff --git a/client/rpc/src/system/tests.rs b/client/rpc/src/system/tests.rs
index 77acdf8418ccc..facad7a0b347a 100644
--- a/client/rpc/src/system/tests.rs
+++ b/client/rpc/src/system/tests.rs
@@ -123,7 +123,7 @@ fn api<T: Into<Option<Status>>>(sync: T) -> RpcModule<System<Block>> {
 					let _ = sender.send(SyncState {
 						starting_block: 1,
 						current_block: 2,
-						highest_block: Some(3),
+						highest_block: 3,
 					});
 				},
 			};
@@ -297,10 +297,7 @@ async fn system_node_roles() {
 async fn system_sync_state() {
 	let sync_state: SyncState<i32> =
 		api(None).call("system_syncState", EmptyParams::new()).await.unwrap();
-	assert_eq!(
-		sync_state,
-		SyncState { starting_block: 1, current_block: 2, highest_block: Some(3) }
-	);
+	assert_eq!(sync_state, SyncState { starting_block: 1, current_block: 2, highest_block: 3 });
 }
 
 #[tokio::test]
diff --git a/client/service/src/lib.rs b/client/service/src/lib.rs
index 98bcb17174157..1de45e5527eec 100644
--- a/client/service/src/lib.rs
+++ b/client/service/src/lib.rs
@@ -264,10 +264,12 @@ async fn build_network_future<
 					sc_rpc::system::Request::SyncState(sender) => {
 						use sc_rpc::system::SyncState;
 
+						let best_number = client.info().best_number;
+
 						let _ = sender.send(SyncState {
 							starting_block,
-							current_block: client.info().best_number,
-							highest_block: network.best_seen_block(),
+							current_block: best_number,
+							highest_block: network.best_seen_block().unwrap_or(best_number),
 						});
 					}
 				}