Merge pull request #527 from getlipa/feature/fail-recovery-on-corrupt…

…ed-monitor
getlipa · Jul 24, 2023 · 8b2e8fc · 8b2e8fc
2 parents 0232f56 + 51e5b9f
commit 8b2e8fc
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 11 deletions.
diff --git a/eel/src/recovery.rs b/eel/src/recovery.rs
@@ -2,7 +2,7 @@ use crate::errors::Result;
 use crate::interfaces::RemoteStorage;
 use crate::key_derivation;
 use crate::keys_manager::init_keys_manager;
-use crate::storage_persister::{has_local_install, StoragePersister};
+use crate::storage_persister::{has_local_install, CorruptedMonitorPolicy, StoragePersister};
 use log::info;
 use perro::{invalid_input, MapToError};
 use std::fs;
@@ -38,8 +38,11 @@ pub fn recover_lightning_node(
     seed_first_half.copy_from_slice(&seed[..32]);
     let keys_manager = Arc::new(init_keys_manager(&seed_first_half)?);
 
-    let remote_channel_monitors =
-        storage.fetch_remote_channel_monitors(&*keys_manager, &*keys_manager)?;
+    let remote_channel_monitors = storage.fetch_remote_channel_monitors(
+        &*keys_manager,
+        &*keys_manager,
+        CorruptedMonitorPolicy::Fail,
+    )?;
     info!(
         "Fetched {} channel monitors from remote storage during recovery procedure",
         remote_channel_monitors.len()

diff --git a/eel/src/storage_persister.rs b/eel/src/storage_persister.rs
@@ -29,7 +29,7 @@ use lightning::util::ser::{ReadableArgs, Writeable};
 use lightning_persister::FilesystemPersister;
 use log::{debug, error, warn};
 use perro::Error::RuntimeError;
-use perro::{invalid_input, permanent_failure, MapToError, ResultTrait};
+use perro::{invalid_input, permanent_failure, runtime_error, MapToError, ResultTrait};
 use std::fs;
 use std::io::{BufReader, Cursor};
 use std::ops::Deref;
@@ -45,6 +45,11 @@ static MANAGER_KEY: &str = "manager";
 static GRAPH_KEY: &str = "network_graph";
 static SCORER_KEY: &str = "scorer";
 
+pub(crate) enum CorruptedMonitorPolicy {
+    Ignore,
+    Fail,
+}
+
 pub(crate) struct StoragePersister {
     storage: Arc<Box<dyn RemoteStorage>>,
     fs_persister: FilesystemPersister,
@@ -98,8 +103,11 @@ impl StoragePersister {
             .map_to_permanent_failure("Failed to read channel monitors from disk")?;
 
         // Fetch remote channel monitors to make sure remote state hasn't advanced
-        let mut remote_channel_monitors =
-            self.fetch_remote_channel_monitors(entropy_source, signer_provider)?;
+        let mut remote_channel_monitors = self.fetch_remote_channel_monitors(
+            entropy_source,
+            signer_provider,
+            CorruptedMonitorPolicy::Ignore,
+        )?;
 
         Self::verify_local_state_is_latest_state::<SP>(
             &mut local_channel_monitors,
@@ -114,6 +122,7 @@ impl StoragePersister {
         &self,
         entropy_source: ES,
         signer_provider: SP,
+        corrupted_monitor_policy: CorruptedMonitorPolicy,
     ) -> Result<
         Vec<(
             BlockHash,
@@ -160,11 +169,12 @@ impl StoragePersister {
                         "Failed to deserialize remote ChannelMonitor `{}`: {}",
                         key, e
                     );
-                    // TODO: Should we return this information to the caller?
-                    //      A corrupt remote ChannelMonitor could be harmless if in this case we
-                    //      load the local ChannelMonitors and the situation gets fixed. If this
-                    //      is a wallet recovery, we will need to load the remote ChannelMonitors,
-                    //      and this channel will be lost.
+                    match corrupted_monitor_policy {
+                        CorruptedMonitorPolicy::Ignore => {}
+                        CorruptedMonitorPolicy::Fail => {
+                            return Err(runtime_error(RuntimeErrorCode::RemoteStorageError, "Failed to deserialize a remote ChannelMonitor. Proceeding could cause this channel to be lost."));
+                        }
+                    }
                 }
             }
         }