diff --git a/src/bitcoin/d/mod.rs b/src/bitcoin/d/mod.rs index 0827deb5..e56d15f5 100644 --- a/src/bitcoin/d/mod.rs +++ b/src/bitcoin/d/mod.rs @@ -42,6 +42,7 @@ pub enum BitcoindError { InvalidVersion(u64), NetworkMismatch(String /*config*/, String /*bitcoind*/), MissingDescriptor, + AlreadyRescanning, } impl BitcoindError { @@ -113,6 +114,9 @@ impl std::fmt::Display for BitcoindError { BitcoindError::MissingDescriptor => { write!(f, "The watchonly wallet loaded on bitcoind does not have the main descriptor imported.") } + BitcoindError::AlreadyRescanning => { + write!(f, "A rescan is already ongoing for the watchonly wallet.") + } } } } @@ -780,10 +784,15 @@ impl BitcoinD { Ok(()) } - pub fn start_rescan(&self, desc: &MultipathDescriptor, timestamp: u32) { + pub fn start_rescan( + &self, + desc: &MultipathDescriptor, + timestamp: u32, + ) -> Result<(), BitcoindError> { // The wallet must not be already rescanning - // FIXME: don't assert, propagate an error instead. - assert!(self.rescan_progress().is_none()); + if self.rescan_progress().is_some() { + return Err(BitcoindError::AlreadyRescanning); + } // Re-import the receive and change descriptors to the watchonly wallet for the purpose of // rescanning. @@ -815,7 +824,7 @@ impl BitcoinD { // error and bitcoind will keep the new timestamps for the descriptors as if it had // successfully rescanned them. match self.make_noreply_request("importdescriptors", ¶ms!(Json::Array(descriptors))) { - Ok(()) => {} + Ok(()) => Ok(()), Err(e) => { // We checked at the beginning it was not already rescanning. If we get an error // because of that it's just a spurious error from calling it multiple times in the @@ -823,12 +832,9 @@ impl BitcoinD { if !e.to_string().contains("is currently rescanning.") { // However, if it's not an error because we triggered it twice due to a // flickering connection fail as it shouldn't happen. - // TODO: propagate the error back to the RPC command. There are too many - // reasons this could fail that we shouldn't be crashing for that. - panic!( - "Unexpected error when starting rescan on bitcoind wallet: {}", - e - ); + Err(e) + } else { + Ok(()) } } } diff --git a/src/bitcoin/mod.rs b/src/bitcoin/mod.rs index a6531f34..ae771dcc 100644 --- a/src/bitcoin/mod.rs +++ b/src/bitcoin/mod.rs @@ -76,7 +76,11 @@ pub trait BitcoinInterface: Send { /// Trigger a rescan of the block chain for transactions related to this descriptor since /// the given date. - fn start_rescan(&self, desc: &descriptors::MultipathDescriptor, timestamp: u32); + fn start_rescan( + &self, + desc: &descriptors::MultipathDescriptor, + timestamp: u32, + ) -> Result<(), String>; /// Rescan progress percentage. Between 0 and 1. fn rescan_progress(&self) -> Option; @@ -283,9 +287,14 @@ impl BitcoinInterface for d::BitcoinD { } } - fn start_rescan(&self, desc: &descriptors::MultipathDescriptor, timestamp: u32) { + fn start_rescan( + &self, + desc: &descriptors::MultipathDescriptor, + timestamp: u32, + ) -> Result<(), String> { // FIXME: in theory i think this could potentially fail to actually start the rescan. - self.start_rescan(desc, timestamp); + self.start_rescan(desc, timestamp) + .map_err(|e| e.to_string()) } fn rescan_progress(&self) -> Option { @@ -357,7 +366,11 @@ impl BitcoinInterface for sync::Arc> self.lock().unwrap().broadcast_tx(tx) } - fn start_rescan(&self, desc: &descriptors::MultipathDescriptor, timestamp: u32) { + fn start_rescan( + &self, + desc: &descriptors::MultipathDescriptor, + timestamp: u32, + ) -> Result<(), String> { self.lock().unwrap().start_rescan(desc, timestamp) } diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 48762a64..05e60e53 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -61,6 +61,8 @@ pub enum CommandError { TxBroadcast(String), AlreadyRescanning, InsaneRescanTimestamp(u32), + /// An error that might occur in the racy rescan triggering logic. + RescanTrigger(String), } impl fmt::Display for CommandError { @@ -92,6 +94,7 @@ impl fmt::Display for CommandError { "There is already a rescan ongoing. Please wait for it to complete first." ), Self::InsaneRescanTimestamp(t) => write!(f, "Insane timestamp '{}'.", t), + Self::RescanTrigger(s) => write!(f, "Error while starting rescan: '{}'", s), } } } @@ -521,7 +524,8 @@ impl DaemonControl { // rescanning. This could make us crash with the bitcoind backend if someone triggered a // rescan of the wallet just after we checked above and did now. self.bitcoin - .start_rescan(&self.config.main_descriptor, timestamp); + .start_rescan(&self.config.main_descriptor, timestamp) + .map_err(CommandError::RescanTrigger)?; db_conn.set_rescan(timestamp); Ok(()) diff --git a/src/jsonrpc/mod.rs b/src/jsonrpc/mod.rs index 595dce33..c0258d9d 100644 --- a/src/jsonrpc/mod.rs +++ b/src/jsonrpc/mod.rs @@ -164,7 +164,8 @@ impl From for Error { | commands::CommandError::AlreadyRescanning => { Error::new(ErrorCode::InvalidParams, e.to_string()) } - commands::CommandError::SanityCheckFailure(_) => { + commands::CommandError::SanityCheckFailure(_) + | commands::CommandError::RescanTrigger(..) => { Error::new(ErrorCode::InternalError, e.to_string()) } commands::CommandError::TxBroadcast(_) => { diff --git a/src/testutils.rs b/src/testutils.rs index 3607e25c..7768cae9 100644 --- a/src/testutils.rs +++ b/src/testutils.rs @@ -75,7 +75,7 @@ impl BitcoinInterface for DummyBitcoind { todo!() } - fn start_rescan(&self, _: &descriptors::MultipathDescriptor, _: u32) { + fn start_rescan(&self, _: &descriptors::MultipathDescriptor, _: u32) -> Result<(), String> { todo!() }