Remove witness from peers when request times out during fork detection

informalsystems · Jun 12, 2020 · 56bc022 · 56bc022
1 parent 7a32855
commit 56bc022
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 13 deletions.
diff --git a/light-client/src/components/io.rs b/light-client/src/components/io.rs
@@ -20,8 +20,8 @@ pub enum IoError {
     IoError(#[from] rpc::Error),
 
     /// The request timed out.
-    #[error("request timed out")]
-    Timeout,
+    #[error("request to peer {0} timed out")]
+    Timeout(PeerId),
 }
 
 /// Interface for fetching light blocks from a full node, typically via the RPC client.
@@ -91,6 +91,7 @@ impl ProdIo {
                     _ => rpc_client.commit(height).await,
                 }
             },
+            peer,
             self.timeout,
         )?;
 
@@ -102,7 +103,11 @@ impl ProdIo {
 
     #[pre(self.peer_map.contains_key(&peer))]
     fn fetch_validator_set(&self, peer: PeerId, height: Height) -> Result<TMValidatorSet, IoError> {
-        let res = block_on(self.rpc_client_for(peer).validators(height), self.timeout)?;
+        let res = block_on(
+            self.rpc_client_for(peer).validators(height),
+            peer,
+            self.timeout,
+        )?;
 
         match res {
             Ok(response) => Ok(TMValidatorSet::new(response.validators)),
@@ -118,7 +123,11 @@ impl ProdIo {
     }
 }
 
-fn block_on<F: std::future::Future>(f: F, timeout: Option<Duration>) -> Result<F::Output, IoError> {
+fn block_on<F: std::future::Future>(
+    f: F,
+    peer: PeerId,
+    timeout: Option<Duration>,
+) -> Result<F::Output, IoError> {
     let mut rt = tokio::runtime::Builder::new()
         .basic_scheduler()
         .enable_all()
@@ -127,7 +136,7 @@ fn block_on<F: std::future::Future>(f: F, timeout: Option<Duration>) -> Result<F
 
     if let Some(timeout) = timeout {
         rt.block_on(async { tokio::time::timeout(timeout, f).await })
-            .map_err(|_| IoError::Timeout)
+            .map_err(|_| IoError::Timeout(peer))
     } else {
         Ok(rt.block_on(f))
     }

diff --git a/light-client/src/fork_detector.rs b/light-client/src/fork_detector.rs
@@ -64,8 +64,7 @@ impl ForkDetector for ProdForkDetector {
 
             let witness_block = witness
                 .light_client
-                .get_or_fetch_block(light_block.height(), &mut state)
-                .unwrap(); // FIXME: unwrap
+                .get_or_fetch_block(light_block.height(), &mut state)?;
 
             let witness_hash = self.header_hasher.hash(&witness_block.signed_header.header);
 

diff --git a/light-client/src/supervisor.rs b/light-client/src/supervisor.rs
@@ -1,10 +1,11 @@
-use crate::callback::Callback;
-use crate::fork_detector::{Fork, ForkDetection, ForkDetector};
-use crate::peer_list::PeerList;
 use crate::{
     bail,
+    callback::Callback,
+    components::io::IoError,
     errors::{Error, ErrorKind},
+    fork_detector::{Fork, ForkDetection, ForkDetector},
     light_client::LightClient,
+    peer_list::PeerList,
     state::State,
     store::VerifiedStatus,
     types::{Height, LightBlock},
@@ -162,11 +163,19 @@ impl Supervisor {
 
         let result =
             self.fork_detector
-                .detect_forks(light_block, &trusted_state, self.peers.witnesses())?;
+                .detect_forks(light_block, &trusted_state, self.peers.witnesses());
 
         match result {
-            ForkDetection::Detected(forks) => Ok(Some(forks)),
-            ForkDetection::NotDetected => Ok(None),
+            Ok(ForkDetection::Detected(forks)) => Ok(Some(forks)),
+            Ok(ForkDetection::NotDetected) => Ok(None),
+            Err(e) => match e.kind() {
+                // TODO: Clean this up
+                ErrorKind::Io(IoError::Timeout(peer)) => {
+                    self.peers.remove_witness(peer);
+                    Err(e)
+                }
+                _ => Err(e),
+            },
         }
     }