fix(chain): do not return error on get_gc_stop_height (#3144)

Currently when we do not have enough blocks for garbage collection, `get_gc_stop_height` will return a `DBNotFoundErr` and the caller handles the error. This caused an incorrect handling of error in `process_block` and we will reject valid blocks if we do not have enough data for garbage collection, which can happen after a state sync. This PR changes `get_gc_stop_height` to return genesis height when we do not have enough data for garbage collection and therefore we don't need to worry about error handling at call site.

Test plan
---------
* `test_process_block_after_state_sync`

Author: bowenwang1996

chain/chain/src/chain.rs

@@ -514,14 +514,7 @@ impl Chain {
     ) -> Result<(), Error> {
         let head = self.store.head()?;
         let tail = self.store.tail()?;
-        let gc_stop_height = match self.runtime_adapter.get_gc_stop_height(&head.last_block_hash) {
-            Ok(height) => height,
-            Err(e) => match e.kind() {
-                // We don't have enough data to garbage collect. Do nothing in this case.
-                ErrorKind::DBNotFoundErr(_) => return Ok(()),
-                _ => return Err(e),
-            },
-        };
+        let gc_stop_height = self.runtime_adapter.get_gc_stop_height(&head.last_block_hash);
 
         if gc_stop_height > head.height {
             return Err(ErrorKind::GCError(
@@ -2891,7 +2884,7 @@ impl<'a> ChainUpdate<'a> {
         }
 
         // Do not accept old forks
-        if prev_height < self.runtime_adapter.get_gc_stop_height(&head.last_block_hash)? {
+        if prev_height < self.runtime_adapter.get_gc_stop_height(&head.last_block_hash) {
             return Err(ErrorKind::InvalidBlockHeight(prev_height).into());
         }
 

chain/chain/src/test_utils.rs

@@ -854,10 +854,13 @@ impl RuntimeAdapter for KeyValueRuntime {
         }
     }
 
-    fn get_gc_stop_height(&self, block_hash: &CryptoHash) -> Result<BlockHeight, Error> {
-        let block_height =
-            self.get_block_header(block_hash)?.map(|h| h.height()).unwrap_or_default();
-        Ok(block_height.saturating_sub(NUM_EPOCHS_TO_KEEP_STORE_DATA * self.epoch_length))
+    fn get_gc_stop_height(&self, block_hash: &CryptoHash) -> BlockHeight {
+        let block_height = self
+            .get_block_header(block_hash)
+            .unwrap_or_default()
+            .map(|h| h.height())
+            .unwrap_or_default();
+        block_height.saturating_sub(NUM_EPOCHS_TO_KEEP_STORE_DATA * self.epoch_length)
     }
 
     fn epoch_exists(&self, _epoch_id: &EpochId) -> bool {

chain/chain/src/types.rs

@@ -406,7 +406,7 @@ pub trait RuntimeAdapter: Send + Sync {
     fn get_epoch_start_height(&self, block_hash: &CryptoHash) -> Result<BlockHeight, Error>;
 
     /// Get the block height for which garbage collection should not go over
-    fn get_gc_stop_height(&self, block_hash: &CryptoHash) -> Result<BlockHeight, Error>;
+    fn get_gc_stop_height(&self, block_hash: &CryptoHash) -> BlockHeight;
 
     /// Check if epoch exists.
     fn epoch_exists(&self, epoch_id: &EpochId) -> bool;

chain/client/tests/process_blocks.rs

@@ -1238,16 +1238,52 @@ fn test_gc_after_state_sync() {
         assert!(env.clients[1].chain.runtime_adapter.get_epoch_start_height(&block_hash).is_ok());
     }
     env.clients[1].chain.reset_data_pre_state_sync(sync_hash).unwrap();
-    assert!(matches!(
-        env.clients[1].runtime_adapter.get_gc_stop_height(&sync_hash).unwrap_err().kind(),
-        ErrorKind::DBNotFoundErr(_)
-    ));
+    assert_eq!(env.clients[1].runtime_adapter.get_gc_stop_height(&sync_hash), 0);
     // mimic what we do in possible_targets
     assert!(env.clients[1].runtime_adapter.get_epoch_id_from_prev_block(&prev_block_hash).is_ok());
     let tries = env.clients[1].runtime_adapter.get_tries();
     assert!(env.clients[1].chain.clear_data(tries, 2).is_ok());
 }
 
+#[test]
+fn test_process_block_after_state_sync() {
+    let epoch_length = 1024;
+    let mut genesis = Genesis::test(vec!["test0", "test1"], 1);
+    genesis.config.epoch_length = epoch_length;
+    let runtimes: Vec<Arc<dyn RuntimeAdapter>> = vec![Arc::new(neard::NightshadeRuntime::new(
+        Path::new("."),
+        create_test_store(),
+        Arc::new(genesis.clone()),
+        vec![],
+        vec![],
+    ))];
+    let mut chain_genesis = ChainGenesis::test();
+    chain_genesis.epoch_length = epoch_length;
+    let mut env = TestEnv::new_with_runtime(chain_genesis, 1, 1, runtimes);
+    for i in 1..epoch_length * 4 + 2 {
+        env.produce_block(0, i);
+    }
+    let sync_height = epoch_length * 4 + 1;
+    let sync_block = env.clients[0].chain.get_block_by_height(sync_height).unwrap().clone();
+    let sync_hash = *sync_block.hash();
+    let chunk_extra = env.clients[0].chain.get_chunk_extra(&sync_hash, 0).unwrap().clone();
+    let state_part =
+        env.clients[0].runtime_adapter.obtain_state_part(0, &chunk_extra.state_root, 0, 1).unwrap();
+    // reset cache
+    for i in epoch_length * 3 - 1..sync_height - 1 {
+        let block_hash = *env.clients[0].chain.get_block_by_height(i).unwrap().hash();
+        assert!(env.clients[0].chain.runtime_adapter.get_epoch_start_height(&block_hash).is_ok());
+    }
+    env.clients[0].chain.reset_data_pre_state_sync(sync_hash).unwrap();
+    env.clients[0]
+        .runtime_adapter
+        .confirm_state(0, &chunk_extra.state_root, &vec![state_part])
+        .unwrap();
+    let block = env.clients[0].produce_block(sync_height + 1).unwrap().unwrap();
+    let (_, res) = env.clients[0].process_block(block, Provenance::PRODUCED);
+    assert!(res.is_ok());
+}
+
 #[test]
 fn test_gc_fork_tail() {
     let epoch_length = 101;
@@ -1289,8 +1325,7 @@ fn test_gc_fork_tail() {
     }
     let head = env.clients[1].chain.head().unwrap();
     assert!(
-        env.clients[1].runtime_adapter.get_gc_stop_height(&head.last_block_hash).unwrap()
-            > epoch_length
+        env.clients[1].runtime_adapter.get_gc_stop_height(&head.last_block_hash) > epoch_length
     );
     assert_eq!(env.clients[1].chain.store().fork_tail().unwrap(), 3);
 }

neard/src/runtime.rs

@@ -873,22 +873,36 @@ impl RuntimeAdapter for NightshadeRuntime {
         epoch_manager.get_epoch_start_height(block_hash).map_err(Error::from)
     }
 
-    fn get_gc_stop_height(&self, block_hash: &CryptoHash) -> Result<BlockHeight, Error> {
-        let mut epoch_manager = self.epoch_manager.as_ref().write().expect(POISONED_LOCK_ERR);
-        // an epoch must have a first block.
-        let epoch_first_block = epoch_manager.get_block_info(block_hash)?.epoch_first_block;
-        let epoch_first_block_info = epoch_manager.get_block_info(&epoch_first_block)?;
-        // maintain pointers to avoid cloning.
-        let mut last_block_in_prev_epoch = epoch_first_block_info.prev_hash;
-        let mut epoch_start_height = epoch_first_block_info.height;
-        for _ in 0..NUM_EPOCHS_TO_KEEP_STORE_DATA - 1 {
-            let epoch_first_block =
-                epoch_manager.get_block_info(&last_block_in_prev_epoch)?.epoch_first_block;
-            let epoch_first_block_info = epoch_manager.get_block_info(&epoch_first_block)?;
-            epoch_start_height = epoch_first_block_info.height;
-            last_block_in_prev_epoch = epoch_first_block_info.prev_hash;
+    fn get_gc_stop_height(&self, block_hash: &CryptoHash) -> BlockHeight {
+        let genesis_height = self.genesis_config.genesis_height;
+        macro_rules! unwrap_result_or_return {
+            ($obj: expr) => {
+                match $obj {
+                    Ok(value) => value,
+                    Err(_) => {
+                        return genesis_height;
+                    }
+                }
+            };
         }
-        Ok(epoch_start_height)
+        let get_gc_stop_height_inner = || -> Result<BlockHeight, Error> {
+            let mut epoch_manager = self.epoch_manager.as_ref().write().expect(POISONED_LOCK_ERR);
+            // an epoch must have a first block.
+            let epoch_first_block = epoch_manager.get_block_info(block_hash)?.epoch_first_block;
+            let epoch_first_block_info = epoch_manager.get_block_info(&epoch_first_block)?;
+            // maintain pointers to avoid cloning.
+            let mut last_block_in_prev_epoch = epoch_first_block_info.prev_hash;
+            let mut epoch_start_height = epoch_first_block_info.height;
+            for _ in 0..NUM_EPOCHS_TO_KEEP_STORE_DATA - 1 {
+                let epoch_first_block =
+                    epoch_manager.get_block_info(&last_block_in_prev_epoch)?.epoch_first_block;
+                let epoch_first_block_info = epoch_manager.get_block_info(&epoch_first_block)?;
+                epoch_start_height = epoch_first_block_info.height;
+                last_block_in_prev_epoch = epoch_first_block_info.prev_hash;
+            }
+            Ok(epoch_start_height)
+        };
+        unwrap_result_or_return!(get_gc_stop_height_inner())
     }
 
     fn epoch_exists(&self, epoch_id: &EpochId) -> bool {