Remove backwards compatibility for el_offline and is_optimstic (#…

…6168) * Remove Option around is_optimistic and el_offline
sigp · Jul 25, 2024 · a3f44c6 · a3f44c6
1 parent b4a7560
commit a3f44c6
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 20 deletions.
diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs
@@ -2889,8 +2889,8 @@ pub fn serve<T: BeaconChainTypes>(
 
                             let syncing_data = api_types::SyncingData {
                                 is_syncing: !network_globals.sync_state.read().is_synced(),
-                                is_optimistic: Some(is_optimistic),
-                                el_offline: Some(el_offline),
+                                is_optimistic,
+                                el_offline,
                                 head_slot,
                                 sync_distance,
                             };

diff --git a/beacon_node/http_api/tests/status_tests.rs b/beacon_node/http_api/tests/status_tests.rs
@@ -56,17 +56,17 @@ async fn el_syncing_then_synced() {
     mock_el.el.upcheck().await;
 
     let api_response = tester.client.get_node_syncing().await.unwrap().data;
-    assert_eq!(api_response.el_offline, Some(false));
-    assert_eq!(api_response.is_optimistic, Some(false));
+    assert_eq!(api_response.el_offline, false);
+    assert_eq!(api_response.is_optimistic, false);
     assert_eq!(api_response.is_syncing, false);
 
     // EL synced
     mock_el.server.set_syncing_response(Ok(false));
     mock_el.el.upcheck().await;
 
     let api_response = tester.client.get_node_syncing().await.unwrap().data;
-    assert_eq!(api_response.el_offline, Some(false));
-    assert_eq!(api_response.is_optimistic, Some(false));
+    assert_eq!(api_response.el_offline, false);
+    assert_eq!(api_response.is_optimistic, false);
     assert_eq!(api_response.is_syncing, false);
 }
 
@@ -84,8 +84,8 @@ async fn el_offline() {
     mock_el.el.upcheck().await;
 
     let api_response = tester.client.get_node_syncing().await.unwrap().data;
-    assert_eq!(api_response.el_offline, Some(true));
-    assert_eq!(api_response.is_optimistic, Some(false));
+    assert_eq!(api_response.el_offline, true);
+    assert_eq!(api_response.is_optimistic, false);
     assert_eq!(api_response.is_syncing, false);
 }
 
@@ -127,8 +127,8 @@ async fn el_error_on_new_payload() {
 
     // The EL should now be *offline* according to the API.
     let api_response = tester.client.get_node_syncing().await.unwrap().data;
-    assert_eq!(api_response.el_offline, Some(true));
-    assert_eq!(api_response.is_optimistic, Some(false));
+    assert_eq!(api_response.el_offline, true);
+    assert_eq!(api_response.is_optimistic, false);
     assert_eq!(api_response.is_syncing, false);
 
     // Processing a block successfully should remove the status.
@@ -143,8 +143,8 @@ async fn el_error_on_new_payload() {
     harness.process_block_result((block, blobs)).await.unwrap();
 
     let api_response = tester.client.get_node_syncing().await.unwrap().data;
-    assert_eq!(api_response.el_offline, Some(false));
-    assert_eq!(api_response.is_optimistic, Some(false));
+    assert_eq!(api_response.el_offline, false);
+    assert_eq!(api_response.is_optimistic, false);
     assert_eq!(api_response.is_syncing, false);
 }
 

diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs
@@ -2159,9 +2159,9 @@ impl ApiTester {
 
         let expected = SyncingData {
             is_syncing: false,
-            is_optimistic: Some(false),
+            is_optimistic: false,
             // these tests run without the Bellatrix fork enabled
-            el_offline: Some(true),
+            el_offline: true,
             head_slot,
             sync_distance,
         };

diff --git a/common/eth2/src/types.rs b/common/eth2/src/types.rs
@@ -599,8 +599,8 @@ pub struct VersionData {
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
 pub struct SyncingData {
     pub is_syncing: bool,
-    pub is_optimistic: Option<bool>,
-    pub el_offline: Option<bool>,
+    pub is_optimistic: bool,
+    pub el_offline: bool,
     pub head_slot: Slot,
     pub sync_distance: Slot,
 }

diff --git a/validator_client/src/check_synced.rs b/validator_client/src/check_synced.rs
@@ -36,10 +36,8 @@ pub async fn check_synced<T: SlotClock>(
         }
     };
 
-    // Default EL status to "online" for backwards-compatibility with BNs that don't include it.
-    let el_offline = resp.data.el_offline.unwrap_or(false);
     let bn_is_synced = !resp.data.is_syncing || (resp.data.sync_distance.as_u64() < SYNC_TOLERANCE);
-    let is_synced = bn_is_synced && !el_offline;
+    let is_synced = bn_is_synced && !resp.data.el_offline;
 
     if let Some(log) = log_opt {
         if !is_synced {
@@ -55,7 +53,7 @@ pub async fn check_synced<T: SlotClock>(
                 "sync_distance" => resp.data.sync_distance.as_u64(),
                 "head_slot" => resp.data.head_slot.as_u64(),
                 "endpoint" => %beacon_node,
-                "el_offline" => el_offline,
+                "el_offline" => resp.data.el_offline,
             );
         }