Pause sync when EE is offline (#3428)

## Issue Addressed

#3032

## Proposed Changes

Pause sync when ee is offline. Changes include three main parts:
- Online/offline notification system
- Pause sync
- Resume sync

#### Online/offline notification system
- The engine state is now guarded behind a new struct `State` that ensures every change is correctly notified. Notifications are only sent if the state changes. The new `State` is behind a `RwLock` (as before) as the synchronization mechanism.
- The actual notification channel is a [tokio::sync::watch](https://docs.rs/tokio/latest/tokio/sync/watch/index.html) which ensures only the last value is in the receiver channel. This way we don't need to worry about message order etc.
- Sync waits for state changes concurrently with normal messages.

#### Pause Sync
Sync has four components, pausing is done differently in each:
- **Block lookups**: Disabled while in this state. We drop current requests and don't search for new blocks. Block lookups are infrequent and I don't think it's worth the extra logic of keeping these and delaying processing. If we later see that this is required, we can add it.
- **Parent lookups**: Disabled while in this state. We drop current requests and don't search for new parents. Parent lookups are even less frequent and I don't think it's worth the extra logic of keeping these and delaying processing. If we later see that this is required, we can add it.
- **Range**: Chains don't send batches for processing to the beacon processor. This is easily done by guarding the channel to the beacon processor and giving it access only if the ee is responsive. I find this the simplest and most powerful approach since we don't need to deal with new sync states and chain segments that are added while the ee is offline will follow the same logic without needing to synchronize a shared state among those. Another advantage of passive pause vs active pause is that we can still keep track of active advertised chain segments so that on resume we don't need to re-evaluate all our peers.
- **Backfill**: Not affected by ee states, we don't pause.

#### Resume Sync
- **Block lookups**: Enabled again.
- **Parent lookups**: Enabled again.
- **Range**: Active resume. Since the only real pause range does is not sending batches for processing, resume makes all chains that are holding read-for-processing batches send them.
- **Backfill**: Not affected by ee states, no need to resume.

## Additional Info

**QUESTION**: Originally I made this to notify and change on synced state, but @pawanjay176 on talks with @paulhauner concluded we only need to check online/offline states. The upcheck function mentions extra checks to have a very up to date sync status to aid the networking stack. However, the only need the networking stack would have is this one. I added a TODO to review if the extra check can be removed

Next gen of #3094

Will work best with #3439 

Co-authored-by: Pawan Dhananjay <pawandhananjay@gmail.com>

Author: divagant-martian

Cargo.lock

@@ -59,6 +59,7 @@ pub use block_verification::{BlockError, ExecutionPayloadError, GossipVerifiedBl
 pub use canonical_head::{CachedHead, CanonicalHead, CanonicalHeadRwLock};
 pub use eth1_chain::{Eth1Chain, Eth1ChainBackend};
 pub use events::ServerSentEventHandler;
+pub use execution_layer::EngineState;
 pub use fork_choice::{ExecutionStatus, ForkchoiceUpdateParameters};
 pub use metrics::scrape_for_metrics;
 pub use parking_lot;

beacon_node/beacon_chain/src/lib.rs

@@ -43,4 +43,5 @@ fork_choice = { path = "../../consensus/fork_choice" }
 mev-build-rs = {git = "https://github.com/ralexstokes/mev-rs", tag = "v0.2.0"}
 ethereum-consensus = {git = "https://github.com/ralexstokes/ethereum-consensus"}
 ssz-rs = {git = "https://github.com/ralexstokes/ssz-rs"}
+tokio-stream = { version = "0.1.9", features = [ "sync" ] }
 strum = "0.24.0"

beacon_node/execution_layer/Cargo.toml

@@ -9,7 +9,8 @@ use slog::{debug, error, info, Logger};
 use std::future::Future;
 use std::sync::Arc;
 use task_executor::TaskExecutor;
-use tokio::sync::{Mutex, RwLock};
+use tokio::sync::{watch, Mutex, RwLock};
+use tokio_stream::wrappers::WatchStream;
 use types::{Address, ExecutionBlockHash, Hash256};
 
 /// The number of payload IDs that will be stored for each `Engine`.
@@ -18,14 +19,74 @@ use types::{Address, ExecutionBlockHash, Hash256};
 const PAYLOAD_ID_LRU_CACHE_SIZE: usize = 512;
 
 /// Stores the remembered state of a engine.
-#[derive(Copy, Clone, PartialEq, Debug)]
-enum EngineState {
+#[derive(Copy, Clone, PartialEq, Debug, Eq, Default)]
+enum EngineStateInternal {
     Synced,
+    #[default]
     Offline,
     Syncing,
     AuthFailed,
 }
 
+/// A subset of the engine state to inform other services if the engine is online or offline.
+#[derive(Debug, Clone, PartialEq, Eq, Copy)]
+pub enum EngineState {
+    Online,
+    Offline,
+}
+
+impl From<EngineStateInternal> for EngineState {
+    fn from(state: EngineStateInternal) -> Self {
+        match state {
+            EngineStateInternal::Synced | EngineStateInternal::Syncing => EngineState::Online,
+            EngineStateInternal::Offline | EngineStateInternal::AuthFailed => EngineState::Offline,
+        }
+    }
+}
+
+/// Wrapper structure that ensures changes to the engine state are correctly reported to watchers.
+struct State {
+    /// The actual engine state.
+    state: EngineStateInternal,
+    /// Notifier to watch the engine state.
+    notifier: watch::Sender<EngineState>,
+}
+
+impl std::ops::Deref for State {
+    type Target = EngineStateInternal;
+
+    fn deref(&self) -> &Self::Target {
+        &self.state
+    }
+}
+
+impl Default for State {
+    fn default() -> Self {
+        let state = EngineStateInternal::default();
+        let (notifier, _receiver) = watch::channel(state.into());
+        State { state, notifier }
+    }
+}
+
+impl State {
+    // Updates the state and notifies all watchers if the state has changed.
+    pub fn update(&mut self, new_state: EngineStateInternal) {
+        self.state = new_state;
+        self.notifier.send_if_modified(|last_state| {
+            let changed = *last_state != new_state.into(); // notify conditionally
+            *last_state = new_state.into(); // update the state unconditionally
+            changed
+        });
+    }
+
+    /// Gives access to a channel containing whether the last state is online.
+    ///
+    /// This can be called several times.
+    pub fn watch(&self) -> WatchStream<EngineState> {
+        self.notifier.subscribe().into()
+    }
+}
+
 #[derive(Copy, Clone, PartialEq, Debug)]
 pub struct ForkChoiceState {
     pub head_block_hash: ExecutionBlockHash,
@@ -53,10 +114,10 @@ pub enum EngineError {
 pub struct Engine {
     pub api: HttpJsonRpc,
     payload_id_cache: Mutex<LruCache<PayloadIdCacheKey, PayloadId>>,
-    state: RwLock<EngineState>,
-    pub latest_forkchoice_state: RwLock<Option<ForkChoiceState>>,
-    pub executor: TaskExecutor,
-    pub log: Logger,
+    state: RwLock<State>,
+    latest_forkchoice_state: RwLock<Option<ForkChoiceState>>,
+    executor: TaskExecutor,
+    log: Logger,
 }
 
 impl Engine {
@@ -65,13 +126,20 @@ impl Engine {
         Self {
             api,
             payload_id_cache: Mutex::new(LruCache::new(PAYLOAD_ID_LRU_CACHE_SIZE)),
-            state: RwLock::new(EngineState::Offline),
+            state: Default::default(),
             latest_forkchoice_state: Default::default(),
             executor,
             log: log.clone(),
         }
     }
 
+    /// Gives access to a channel containing the last engine state.
+    ///
+    /// This can be called several times.
+    pub async fn watch_state(&self) -> WatchStream<EngineState> {
+        self.state.read().await.watch()
+    }
+
     pub async fn get_payload_id(
         &self,
         head_block_hash: ExecutionBlockHash,
@@ -165,17 +233,16 @@ impl Engine {
 
     /// Returns `true` if the engine has a "synced" status.
     pub async fn is_synced(&self) -> bool {
-        *self.state.read().await == EngineState::Synced
+        **self.state.read().await == EngineStateInternal::Synced
     }
 
     /// Run the `EngineApi::upcheck` function if the node's last known state is not synced. This
     /// might be used to recover the node if offline.
     pub async fn upcheck(&self) {
-        let state: EngineState = match self.api.upcheck().await {
+        let state: EngineStateInternal = match self.api.upcheck().await {
             Ok(()) => {
                 let mut state = self.state.write().await;
-
-                if *state != EngineState::Synced {
+                if **state != EngineStateInternal::Synced {
                     info!(
                         self.log,
                         "Execution engine online";
@@ -189,14 +256,13 @@ impl Engine {
                         "Execution engine online";
                     );
                 }
-
-                *state = EngineState::Synced;
-                *state
+                state.update(EngineStateInternal::Synced);
+                **state
             }
             Err(EngineApiError::IsSyncing) => {
                 let mut state = self.state.write().await;
-                *state = EngineState::Syncing;
-                *state
+                state.update(EngineStateInternal::Syncing);
+                **state
             }
             Err(EngineApiError::Auth(err)) => {
                 error!(
@@ -206,8 +272,8 @@ impl Engine {
                 );
 
                 let mut state = self.state.write().await;
-                *state = EngineState::AuthFailed;
-                *state
+                state.update(EngineStateInternal::AuthFailed);
+                **state
             }
             Err(e) => {
                 error!(
@@ -217,8 +283,8 @@ impl Engine {
                 );
 
                 let mut state = self.state.write().await;
-                *state = EngineState::Offline;
-                *state
+                state.update(EngineStateInternal::Offline);
+                **state
             }
         };
 
@@ -244,12 +310,10 @@ impl Engine {
             Ok(result) => {
                 // Take a clone *without* holding the read-lock since the `upcheck` function will
                 // take a write-lock.
-                let state: EngineState = *self.state.read().await;
+                let state: EngineStateInternal = **self.state.read().await;
 
-                // If this request just returned successfully but we don't think this node is
-                // synced, check to see if it just became synced. This helps to ensure that the
-                // networking stack can get fast feedback about a synced engine.
-                if state != EngineState::Synced {
+                // Keep an up to date engine state.
+                if state != EngineStateInternal::Synced {
                     // Spawn the upcheck in another task to avoid slowing down this request.
                     let inner_self = self.clone();
                     self.executor.spawn(
@@ -293,3 +357,22 @@ impl PayloadIdCacheKey {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use tokio_stream::StreamExt;
+
+    #[tokio::test]
+    async fn test_state_notifier() {
+        let mut state = State::default();
+        let initial_state: EngineState = state.state.into();
+        assert_eq!(initial_state, EngineState::Offline);
+        state.update(EngineStateInternal::Synced);
+
+        // a watcher that arrives after the first update.
+        let mut watcher = state.watch();
+        let new_state = watcher.next().await.expect("Last state is always present");
+        assert_eq!(new_state, EngineState::Online);
+    }
+}

beacon_node/execution_layer/src/engines.rs

@@ -10,8 +10,8 @@ use builder_client::BuilderHttpClient;
 use engine_api::Error as ApiError;
 pub use engine_api::*;
 pub use engine_api::{http, http::deposit_methods, http::HttpJsonRpc};
-pub use engines::ForkChoiceState;
 use engines::{Engine, EngineError};
+pub use engines::{EngineState, ForkChoiceState};
 use fork_choice::ForkchoiceUpdateParameters;
 use lru::LruCache;
 use payload_status::process_payload_status;
@@ -31,6 +31,7 @@ use tokio::{
     sync::{Mutex, MutexGuard, RwLock},
     time::sleep,
 };
+use tokio_stream::wrappers::WatchStream;
 use types::{
     BlindedPayload, BlockType, ChainSpec, Epoch, ExecPayload, ExecutionBlockHash, ForkName,
     ProposerPreparationData, PublicKeyBytes, SignedBeaconBlock, Slot,
@@ -286,6 +287,13 @@ impl<T: EthSpec> ExecutionLayer<T> {
         self.inner.execution_blocks.lock().await
     }
 
+    /// Gives access to a channel containing if the last engine state is online or not.
+    ///
+    /// This can be called several times.
+    pub async fn get_responsiveness_watch(&self) -> WatchStream<EngineState> {
+        self.engine().watch_state().await
+    }
+
     /// Note: this function returns a mutex guard, be careful to avoid deadlocks.
     async fn proposer_preparation_data(
         &self,