Merge pull request #1124 from mintlayer/latch_ibd_flag

Latch `is_initial_block_download_finished` flag

Author: TheQuantumPhysicist

chainstate/src/detail/mod.rs

@@ -76,6 +76,7 @@ use common::{
 use logging::log;
 use utils::{
     eventhandler::{EventHandler, EventsController},
+    set_flag::SetFlag,
     tap_error_log::LogError,
 };
 use utxo::UtxosDB;
@@ -103,7 +104,7 @@ pub struct Chainstate<S, V> {
     custom_orphan_error_hook: Option<Arc<OrphanErrorHandler>>,
     events_controller: EventsController<ChainstateEvent>,
     time_getter: TimeGetter,
-    is_initial_block_download_finished: bool,
+    is_initial_block_download_finished: SetFlag,
 }
 
 #[derive(Copy, Clone, Eq, Debug, PartialEq)]
@@ -189,6 +190,8 @@ impl<S: BlockchainStorage, V: TransactionVerificationStrategy> Chainstate<S, V>
             chainstate.check_genesis().map_err(crate::ChainstateError::from)?;
         }
 
+        chainstate.update_initial_block_download_flag()?;
+
         Ok(chainstate)
     }
 
@@ -210,7 +213,7 @@ impl<S: BlockchainStorage, V: TransactionVerificationStrategy> Chainstate<S, V>
             custom_orphan_error_hook,
             events_controller: EventsController::new(),
             time_getter,
-            is_initial_block_download_finished: false,
+            is_initial_block_download_finished: SetFlag::new(),
         }
     }
 
@@ -621,7 +624,8 @@ impl<S: BlockchainStorage, V: TransactionVerificationStrategy> Chainstate<S, V>
                 bi.block_timestamp(),
             );
 
-            self.is_initial_block_download_finished = self.is_fresh_block(&bi.block_timestamp());
+            self.update_initial_block_download_flag()
+                .map_err(BlockError::BestBlockIdQueryError)?;
         }
 
         Ok(result)
@@ -744,28 +748,38 @@ impl<S: BlockchainStorage, V: TransactionVerificationStrategy> Chainstate<S, V>
         &self.events_controller
     }
 
-    pub fn is_initial_block_download(&self) -> Result<bool, PropertyQueryError> {
-        if self.is_initial_block_download_finished {
-            return Ok(false);
+    pub fn is_initial_block_download(&self) -> bool {
+        !self.is_initial_block_download_finished.test()
+    }
+
+    /// Returns true if the given block timestamp is newer than `ChainstateConfig::max_tip_age`.
+    fn is_fresh_block(&self, time: &BlockTimestamp) -> bool {
+        let now = self.time_getter.get_time();
+        time.as_duration_since_epoch() + self.chainstate_config.max_tip_age.clone().into() > now
+    }
+
+    /// Update `is_initial_block_download_finished` when tip changes (can only be set once)
+    fn update_initial_block_download_flag(&mut self) -> Result<(), PropertyQueryError> {
+        if self.is_initial_block_download_finished.test() {
+            return Ok(());
         }
 
         // TODO: Add a check for importing and reindex.
 
         // TODO: Add a check for the chain trust.
 
         let tip_timestamp = match self.query()?.get_best_block_header() {
-            Ok(h) => Ok(h.timestamp()),
+            Ok(h) => h.timestamp(),
             // There is only the genesis block, so the initial block download isn't finished yet.
-            Err(PropertyQueryError::GenesisHeaderRequested) => return Ok(true),
-            Err(e) => Err(e),
-        }?;
-        Ok(!self.is_fresh_block(&tip_timestamp))
-    }
+            Err(PropertyQueryError::GenesisHeaderRequested) => return Ok(()),
+            Err(e) => return Err(e),
+        };
 
-    /// Returns true if the given block timestamp is newer than `ChainstateConfig::max_tip_age`.
-    fn is_fresh_block(&self, time: &BlockTimestamp) -> bool {
-        let now = self.time_getter.get_time();
-        time.as_duration_since_epoch() + self.chainstate_config.max_tip_age.clone().into() > now
+        if self.is_fresh_block(&tip_timestamp) {
+            self.is_initial_block_download_finished.set();
+        }
+
+        Ok(())
     }
 
     fn check_legitimate_orphan(

chainstate/src/interface/chainstate_interface.rs

@@ -190,7 +190,7 @@ pub trait ChainstateInterface: Send {
     fn utxo(&self, outpoint: &UtxoOutPoint) -> Result<Option<Utxo>, ChainstateError>;
 
     /// Returns true if the initial block download isn't finished yet.
-    fn is_initial_block_download(&self) -> Result<bool, ChainstateError>;
+    fn is_initial_block_download(&self) -> bool;
 
     /// Check whether stake pool with given ID exists.
     fn stake_pool_exists(&self, pool_id: PoolId) -> Result<bool, ChainstateError>;

chainstate/src/interface/chainstate_interface_impl.rs

@@ -477,8 +477,8 @@ impl<S: BlockchainStorage, V: TransactionVerificationStrategy> ChainstateInterfa
             .map_err(|e| ChainstateError::FailedToReadProperty(e.into()))
     }
 
-    fn is_initial_block_download(&self) -> Result<bool, ChainstateError> {
-        self.chainstate.is_initial_block_download().map_err(ChainstateError::from)
+    fn is_initial_block_download(&self) -> bool {
+        self.chainstate.is_initial_block_download()
     }
 
     fn stake_pool_exists(&self, pool_id: PoolId) -> Result<bool, ChainstateError> {
@@ -558,7 +558,7 @@ impl<S: BlockchainStorage, V: TransactionVerificationStrategy> ChainstateInterfa
 
         let median_time = self.calculate_median_time_past(&best_block_id)?;
 
-        let is_initial_block_download = self.is_initial_block_download()?;
+        let is_initial_block_download = self.is_initial_block_download();
 
         Ok(ChainInfo {
             best_block_height,

chainstate/src/interface/chainstate_interface_impl_delegation.rs

@@ -280,7 +280,7 @@ where
         self.deref().utxo(outpoint)
     }
 
-    fn is_initial_block_download(&self) -> Result<bool, ChainstateError> {
+    fn is_initial_block_download(&self) -> bool {
         self.deref().is_initial_block_download()
     }
 

chainstate/test-suite/src/tests/syncing_tests.rs

@@ -470,7 +470,7 @@ fn initial_block_download(#[case] seed: Seed) {
             .build();
 
         // Only genesis block, so is_initial_block_download should return true.
-        assert!(tf.chainstate.is_initial_block_download().unwrap());
+        assert!(tf.chainstate.is_initial_block_download());
 
         // Create a block with an "old" timestamp.
         let now = tf.current_time();
@@ -479,15 +479,26 @@ fn initial_block_download(#[case] seed: Seed) {
             .with_timestamp(BlockTimestamp::from_duration_since_epoch(now))
             .build_and_process()
             .unwrap();
-        assert!(tf.chainstate.is_initial_block_download().unwrap());
+        assert!(tf.chainstate.is_initial_block_download());
 
         // Create a block with fresh timestamp.
         tf.make_block_builder().build_and_process().unwrap();
-        assert!(!tf.chainstate.is_initial_block_download().unwrap());
+        assert!(!tf.chainstate.is_initial_block_download());
 
         // Add one more block.
         tf.make_block_builder().build_and_process().unwrap();
-        assert!(!tf.chainstate.is_initial_block_download().unwrap());
+        assert!(!tf.chainstate.is_initial_block_download());
+
+        // Check that receiving an "old" block does not revert `is_initial_block_download` back
+        tf.progress_time_seconds_since_epoch(5);
+        let now = tf.current_time();
+        let block = tf
+            .make_block_builder()
+            .with_timestamp(BlockTimestamp::from_duration_since_epoch(now))
+            .build();
+        tf.progress_time_seconds_since_epoch(10);
+        tf.process_block(block, BlockSource::Local).unwrap();
+        assert!(!tf.chainstate.is_initial_block_download());
     });
 }
 

mocks/src/chainstate.rs

@@ -140,7 +140,7 @@ mockall::mock! {
             include_orphans: bool,
         ) -> Result<(), ChainstateError>;
         fn utxo(&self, outpoint: &UtxoOutPoint) -> Result<Option<Utxo>, ChainstateError>;
-        fn is_initial_block_download(&self) -> Result<bool, ChainstateError>;
+        fn is_initial_block_download(&self) -> bool;
         fn stake_pool_exists(&self, pool_id: PoolId) -> Result<bool, ChainstateError>;
         fn get_stake_pool_balance(&self, pool_id: PoolId) -> Result<Option<Amount>, ChainstateError>;
         fn get_stake_pool_data(&self, pool_id: PoolId) -> Result<Option<PoolData>, ChainstateError>;

p2p/src/sync/mod.rs

@@ -137,7 +137,7 @@ where
                 .await
                 // This shouldn't fail unless the chainstate subsystem is down which shouldn't
                 // happen since subsystems are shutdown in reverse order.
-                .expect("Chainstate call failed")?,
+                .expect("Chainstate call failed"),
         );
 
         let mut tx_processed_receiver = subscribe_to_tx_processed(&self.mempool_handle).await?;
@@ -215,7 +215,7 @@ where
     /// Announces the header of a new block to peers.
     async fn handle_new_tip(&mut self, block_id: Id<Block>) -> Result<()> {
         let is_initial_block_download = if self.is_initial_block_download.load() {
-            let is_ibd = self.chainstate_handle.call(|c| c.is_initial_block_download()).await??;
+            let is_ibd = self.chainstate_handle.call(|c| c.is_initial_block_download()).await?;
             self.is_initial_block_download.store(is_ibd);
             is_ibd
         } else {