Fix the trimming of the record of the first block height of each epoch

.changelog/unreleased/bug-fixes/1898-fix-epoch-trimming.md

@@ -0,0 +1,3 @@
+- Keep a record of the first block heights of every epoch in the chain's history
+  instead of trimming to only keep this data for a certain number of epochs in
+  the past. ([\#1898](https://github.com/anoma/namada/pull/1898))

apps/src/lib/node/ledger/shell/mod.rs

@@ -2075,7 +2075,7 @@ mod test_utils {
             .storage
             .block
             .pred_epochs
-            .new_epoch(BlockHeight(1), 1000);
+            .new_epoch(BlockHeight(1));
         update_allowed_conversions(&mut shell.wl_storage)
             .expect("update conversions failed");
         shell.wl_storage.commit_block().expect("commit failed");

apps/src/lib/node/ledger/storage/mod.rs

@@ -141,7 +141,7 @@ mod tests {
             .write(&key, value_bytes.clone())
             .expect("write failed");
         storage.block.epoch = storage.block.epoch.next();
-        storage.block.pred_epochs.new_epoch(BlockHeight(100), 1000);
+        storage.block.pred_epochs.new_epoch(BlockHeight(100));
         // make wl_storage to update conversion for a new epoch
         let mut wl_storage = WlStorage::new(WriteLog::default(), storage);
         update_allowed_conversions(&mut wl_storage)
@@ -420,10 +420,7 @@ mod tests {
                 if storage.block.height.0 % 5 == 0 {
                     // new epoch every 5 heights
                     storage.block.epoch = storage.block.epoch.next();
-                    storage
-                        .block
-                        .pred_epochs
-                        .new_epoch(storage.block.height, 1000);
+                    storage.block.pred_epochs.new_epoch(storage.block.height);
                 }
                 storage.commit_block(batch)?;
                 let hash = BlockHash::default();
@@ -512,7 +509,7 @@ mod tests {
             .expect("write failed");
 
         storage.block.epoch = storage.block.epoch.next();
-        storage.block.pred_epochs.new_epoch(BlockHeight(1), 1000);
+        storage.block.pred_epochs.new_epoch(BlockHeight(1));
         let batch = PersistentStorage::batch();
         storage.commit_block(batch).expect("commit failed");
 
@@ -527,7 +524,7 @@ mod tests {
             .expect("write failed");
 
         storage.block.epoch = storage.block.epoch.next();
-        storage.block.pred_epochs.new_epoch(BlockHeight(6), 1000);
+        storage.block.pred_epochs.new_epoch(BlockHeight(6));
         let batch = PersistentStorage::batch();
         storage.commit_block(batch).expect("commit failed");
 
@@ -538,7 +535,7 @@ mod tests {
             .begin_block(BlockHash::default(), BlockHeight(11))
             .expect("begin_block failed");
         storage.block.epoch = storage.block.epoch.next();
-        storage.block.pred_epochs.new_epoch(BlockHeight(11), 1000);
+        storage.block.pred_epochs.new_epoch(BlockHeight(11));
         let batch = PersistentStorage::batch();
         storage.commit_block(batch).expect("commit failed");
 

core/src/ledger/storage/wl_storage.rs

@@ -215,13 +215,8 @@ where
             self.storage.next_epoch_min_start_height =
                 height + min_num_of_blocks;
             self.storage.next_epoch_min_start_time = time + min_duration;
-            // TODO put this into PoS parameters and pass it to tendermint
-            // `consensus_params` on `InitChain` and `EndBlock`
-            let evidence_max_age_num_blocks: u64 = 100000;
-            self.storage
-                .block
-                .pred_epochs
-                .new_epoch(height, evidence_max_age_num_blocks);
+
+            self.storage.block.pred_epochs.new_epoch(height);
             tracing::info!("Began a new epoch {}", self.storage.block.epoch);
         }
         Ok(new_epoch)

core/src/types/storage.rs

@@ -1149,8 +1149,6 @@ impl Mul for Epoch {
     BorshDeserialize,
 )]
 pub struct Epochs {
-    /// The oldest epoch we can look-up.
-    first_known_epoch: Epoch,
     /// The block heights of the first block of each known epoch.
     /// Invariant: the values must be sorted in ascending order.
     pub first_block_heights: Vec<BlockHeight>,
@@ -1161,48 +1159,25 @@ impl Default for Epochs {
     /// block height 0.
     fn default() -> Self {
         Self {
-            first_known_epoch: Epoch::default(),
             first_block_heights: vec![BlockHeight::default()],
         }
     }
 }
 
 impl Epochs {
-    /// Record start of a new epoch at the given block height and trim any
-    /// epochs that ended more than `max_age_num_blocks` ago.
-    pub fn new_epoch(
-        &mut self,
-        block_height: BlockHeight,
-        max_age_num_blocks: u64,
-    ) {
-        let min_block_height_to_keep = (block_height.0 + 1)
-            .checked_sub(max_age_num_blocks)
-            .unwrap_or_default();
-        // trim off any epochs whose last block is before the limit
-        while let Some((_first_known_epoch_height, rest)) =
-            self.first_block_heights.split_first()
-        {
-            if let Some(second_known_epoch_height) = rest.first() {
-                if second_known_epoch_height.0 < min_block_height_to_keep {
-                    self.first_known_epoch = self.first_known_epoch.next();
-                    self.first_block_heights = rest.to_vec();
-                    continue;
-                }
-            }
-            break;
-        }
+    /// Record start of a new epoch at the given block height
+    pub fn new_epoch(&mut self, block_height: BlockHeight) {
         self.first_block_heights.push(block_height);
     }
 
-    /// Look-up the epoch of a given block height.
+    /// Look up the epoch of a given block height. If the given height is
+    /// greater than the current height, the current epoch will be returned even
+    /// though an epoch for a future block cannot be determined.
     pub fn get_epoch(&self, block_height: BlockHeight) -> Option<Epoch> {
-        if let Some((first_known_epoch_height, rest)) =
+        if let Some((_first_known_epoch_height, rest)) =
             self.first_block_heights.split_first()
         {
-            if block_height < *first_known_epoch_height {
-                return None;
-            }
-            let mut epoch = self.first_known_epoch;
+            let mut epoch = Epoch::default();
             for next_block_height in rest {
                 if block_height < *next_block_height {
                     return Some(epoch);
@@ -1215,7 +1190,7 @@ impl Epochs {
         None
     }
 
-    /// Look-up the starting block height of an epoch at or before a given
+    /// Look up the starting block height of an epoch at or before a given
     /// height.
     pub fn get_epoch_start_height(
         &self,
@@ -1229,32 +1204,15 @@ impl Epochs {
         None
     }
 
-    /// Look-up the starting block height of the given epoch
+    /// Look up the starting block height of the given epoch
     pub fn get_start_height_of_epoch(
         &self,
         epoch: Epoch,
     ) -> Option<BlockHeight> {
-        if epoch < self.first_known_epoch {
+        if epoch.0 > self.first_block_heights.len() as u64 {
             return None;
         }
-
-        let mut cur_epoch = self.first_known_epoch;
-        for height in &self.first_block_heights {
-            if epoch == cur_epoch {
-                return Some(*height);
-            } else {
-                cur_epoch = cur_epoch.next();
-            }
-        }
-        None
-    }
-
-    /// Look-up the block height of a given epoch.
-    pub fn get_height(&self, epoch: Epoch) -> Option<BlockHeight> {
-        // the given epoch should be greater than or equal to the
-        // first known epoch
-        let index = epoch.0.checked_sub(self.first_known_epoch.0)? as usize;
-        self.first_block_heights.get(index).copied()
+        self.first_block_heights.get(epoch.0 as usize).copied()
     }
 
     /// Return all starting block heights for each successive Epoch.
@@ -1657,14 +1615,19 @@ mod tests {
     fn test_predecessor_epochs_and_heights() {
         let mut epochs = Epochs::default();
         println!("epochs {:#?}", epochs);
-        assert_eq!(epochs.get_height(Epoch(0)), Some(BlockHeight(0)));
+        assert_eq!(
+            epochs.get_start_height_of_epoch(Epoch(0)),
+            Some(BlockHeight(0))
+        );
         assert_eq!(epochs.get_epoch(BlockHeight(0)), Some(Epoch(0)));
-        let mut max_age_num_blocks = 100;
 
         // epoch 1
-        epochs.new_epoch(BlockHeight(10), max_age_num_blocks);
+        epochs.new_epoch(BlockHeight(10));
         println!("epochs {:#?}", epochs);
-        assert_eq!(epochs.get_height(Epoch(1)), Some(BlockHeight(10)));
+        assert_eq!(
+            epochs.get_start_height_of_epoch(Epoch(1)),
+            Some(BlockHeight(10))
+        );
         assert_eq!(epochs.get_epoch(BlockHeight(0)), Some(Epoch(0)));
         assert_eq!(
             epochs.get_epoch_start_height(BlockHeight(0)),
@@ -1692,9 +1655,12 @@ mod tests {
         );
 
         // epoch 2
-        epochs.new_epoch(BlockHeight(20), max_age_num_blocks);
+        epochs.new_epoch(BlockHeight(20));
         println!("epochs {:#?}", epochs);
-        assert_eq!(epochs.get_height(Epoch(2)), Some(BlockHeight(20)));
+        assert_eq!(
+            epochs.get_start_height_of_epoch(Epoch(2)),
+            Some(BlockHeight(20))
+        );
         assert_eq!(epochs.get_epoch(BlockHeight(0)), Some(Epoch(0)));
         assert_eq!(epochs.get_epoch(BlockHeight(9)), Some(Epoch(0)));
         assert_eq!(epochs.get_epoch(BlockHeight(10)), Some(Epoch(1)));
@@ -1714,14 +1680,17 @@ mod tests {
             Some(BlockHeight(20))
         );
 
-        // epoch 3, epoch 0 and 1 should be trimmed
-        epochs.new_epoch(BlockHeight(200), max_age_num_blocks);
+        // epoch 3
+        epochs.new_epoch(BlockHeight(200));
         println!("epochs {:#?}", epochs);
-        assert_eq!(epochs.get_height(Epoch(3)), Some(BlockHeight(200)));
-        assert_eq!(epochs.get_epoch(BlockHeight(0)), None);
-        assert_eq!(epochs.get_epoch(BlockHeight(9)), None);
-        assert_eq!(epochs.get_epoch(BlockHeight(10)), None);
-        assert_eq!(epochs.get_epoch(BlockHeight(11)), None);
+        assert_eq!(
+            epochs.get_start_height_of_epoch(Epoch(3)),
+            Some(BlockHeight(200))
+        );
+        assert_eq!(epochs.get_epoch(BlockHeight(0)), Some(Epoch(0)));
+        assert_eq!(epochs.get_epoch(BlockHeight(9)), Some(Epoch(0)));
+        assert_eq!(epochs.get_epoch(BlockHeight(10)), Some(Epoch(1)));
+        assert_eq!(epochs.get_epoch(BlockHeight(11)), Some(Epoch(1)));
         assert_eq!(epochs.get_epoch(BlockHeight(20)), Some(Epoch(2)));
         assert_eq!(epochs.get_epoch(BlockHeight(100)), Some(Epoch(2)));
         assert_eq!(
@@ -1734,62 +1703,77 @@ mod tests {
             Some(BlockHeight(200))
         );
 
-        // increase the limit
-        max_age_num_blocks = 200;
-
         // epoch 4
-        epochs.new_epoch(BlockHeight(300), max_age_num_blocks);
+        epochs.new_epoch(BlockHeight(300));
         println!("epochs {:#?}", epochs);
-        assert_eq!(epochs.get_height(Epoch(4)), Some(BlockHeight(300)));
+        assert_eq!(
+            epochs.get_start_height_of_epoch(Epoch(4)),
+            Some(BlockHeight(300))
+        );
         assert_eq!(epochs.get_epoch(BlockHeight(20)), Some(Epoch(2)));
         assert_eq!(epochs.get_epoch(BlockHeight(100)), Some(Epoch(2)));
         assert_eq!(epochs.get_epoch(BlockHeight(200)), Some(Epoch(3)));
         assert_eq!(epochs.get_epoch(BlockHeight(300)), Some(Epoch(4)));
 
-        // epoch 5, epoch 2 should be trimmed
-        epochs.new_epoch(BlockHeight(499), max_age_num_blocks);
+        // epoch 5
+        epochs.new_epoch(BlockHeight(499));
         println!("epochs {:#?}", epochs);
-        assert_eq!(epochs.get_height(Epoch(5)), Some(BlockHeight(499)));
-        assert_eq!(epochs.get_epoch(BlockHeight(20)), None);
-        assert_eq!(epochs.get_epoch(BlockHeight(100)), None);
+        assert_eq!(
+            epochs.get_start_height_of_epoch(Epoch(5)),
+            Some(BlockHeight(499))
+        );
+        assert_eq!(epochs.get_epoch(BlockHeight(20)), Some(Epoch(2)));
+        assert_eq!(epochs.get_epoch(BlockHeight(100)), Some(Epoch(2)));
         assert_eq!(epochs.get_epoch(BlockHeight(200)), Some(Epoch(3)));
         assert_eq!(epochs.get_epoch(BlockHeight(300)), Some(Epoch(4)));
         assert_eq!(epochs.get_epoch(BlockHeight(499)), Some(Epoch(5)));
 
-        // epoch 6, epoch 3 should be trimmed
-        epochs.new_epoch(BlockHeight(500), max_age_num_blocks);
+        // epoch 6
+        epochs.new_epoch(BlockHeight(500));
         println!("epochs {:#?}", epochs);
-        assert_eq!(epochs.get_height(Epoch(6)), Some(BlockHeight(500)));
-        assert_eq!(epochs.get_epoch(BlockHeight(200)), None);
+        assert_eq!(
+            epochs.get_start_height_of_epoch(Epoch(6)),
+            Some(BlockHeight(500))
+        );
+        assert_eq!(epochs.get_epoch(BlockHeight(200)), Some(Epoch(3)));
         assert_eq!(epochs.get_epoch(BlockHeight(300)), Some(Epoch(4)));
         assert_eq!(epochs.get_epoch(BlockHeight(499)), Some(Epoch(5)));
         assert_eq!(epochs.get_epoch(BlockHeight(500)), Some(Epoch(6)));
 
-        // decrease the limit
-        max_age_num_blocks = 50;
-
-        // epoch 7, epoch 4 and 5 should be trimmed
-        epochs.new_epoch(BlockHeight(550), max_age_num_blocks);
+        // epoch 7
+        epochs.new_epoch(BlockHeight(550));
         println!("epochs {:#?}", epochs);
-        assert_eq!(epochs.get_height(Epoch(7)), Some(BlockHeight(550)));
-        assert_eq!(epochs.get_epoch(BlockHeight(300)), None);
-        assert_eq!(epochs.get_epoch(BlockHeight(499)), None);
+        assert_eq!(
+            epochs.get_start_height_of_epoch(Epoch(7)),
+            Some(BlockHeight(550))
+        );
+        assert_eq!(epochs.get_epoch(BlockHeight(300)), Some(Epoch(4)));
+        assert_eq!(epochs.get_epoch(BlockHeight(499)), Some(Epoch(5)));
         assert_eq!(epochs.get_epoch(BlockHeight(500)), Some(Epoch(6)));
         assert_eq!(epochs.get_epoch(BlockHeight(550)), Some(Epoch(7)));
 
-        // epoch 8, epoch 6 should be trimmed
-        epochs.new_epoch(BlockHeight(600), max_age_num_blocks);
+        // epoch 8
+        epochs.new_epoch(BlockHeight(600));
         println!("epochs {:#?}", epochs);
-        assert_eq!(epochs.get_height(Epoch(7)), Some(BlockHeight(550)));
-        assert_eq!(epochs.get_height(Epoch(8)), Some(BlockHeight(600)));
-        assert_eq!(epochs.get_epoch(BlockHeight(500)), None);
+        assert_eq!(
+            epochs.get_start_height_of_epoch(Epoch(7)),
+            Some(BlockHeight(550))
+        );
+        assert_eq!(
+            epochs.get_start_height_of_epoch(Epoch(8)),
+            Some(BlockHeight(600))
+        );
+        assert_eq!(epochs.get_epoch(BlockHeight(500)), Some(Epoch(6)));
         assert_eq!(epochs.get_epoch(BlockHeight(550)), Some(Epoch(7)));
         assert_eq!(epochs.get_epoch(BlockHeight(600)), Some(Epoch(8)));
 
         // try to fetch height values out of range
         // at this point, the min known epoch is 7
-        for e in [1, 2, 3, 4, 5, 6, 9, 10, 11, 12] {
-            assert!(epochs.get_height(Epoch(e)).is_none(), "Epoch: {e}");
+        for e in [9, 10, 11, 12] {
+            assert!(
+                epochs.get_start_height_of_epoch(Epoch(e)).is_none(),
+                "Epoch: {e}"
+            );
         }
     }
 

ethereum_bridge/src/protocol/transactions/validator_set_update/mod.rs

@@ -56,7 +56,7 @@ where
         .storage
         .block
         .pred_epochs
-        .get_height(signing_epoch)
+        .get_start_height_of_epoch(signing_epoch)
         // NOTE: The only way this can fail is if validator set updates do not
         // reach a `seen` state before the relevant epoch data is purged from
         // Namada. In most scenarios, we should reach a complete proof before

proof_of_stake/src/pos_queries.rs

@@ -275,7 +275,11 @@ where
     /// been purged from Namada, or if it is not available yet.
     #[inline]
     pub fn get_height(self, epoch: Epoch) -> Option<BlockHeight> {
-        self.wl_storage.storage.block.pred_epochs.get_height(epoch)
+        self.wl_storage
+            .storage
+            .block
+            .pred_epochs
+            .get_start_height_of_epoch(epoch)
     }
 
     /// Retrieves the [`BlockHeight`] that is currently being decided.