Merge branch 'tomas/fst-epoch-start-height' (#1993)

* origin/tomas/fst-epoch-start-height:
  changelog: add #1993
  test/core/storage: fix pred epochs & heights test
  test/app/ledger: fix merkle tree pruning test
  apps/shell/finalize_block: remove first epoch workaround
  core/storage/epochs: Change pred `Epochs` default to be empty
  core/storage: add `BlockHeight::first` and `sentinel`
  core/storage: set the initial height from genesis
  core/storage: change the default start height of first epoch to 1

.changelog/unreleased/bug-fixes/1993-fst-epoch-start-height.md

@@ -0,0 +1,2 @@
+- Fix the start block height of the first epoch.
+  ([\#1993](https://github.com/anoma/namada/pull/1993))

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

@@ -696,11 +696,8 @@ where
             .pred_epochs
             .first_block_heights[last_epoch.0 as usize]
             .0;
-        let num_blocks_in_last_epoch = if first_block_of_last_epoch == 0 {
-            self.wl_storage.storage.block.height.0 - 1
-        } else {
-            self.wl_storage.storage.block.height.0 - first_block_of_last_epoch
-        };
+        let num_blocks_in_last_epoch =
+            self.wl_storage.storage.block.height.0 - first_block_of_last_epoch;
 
         let staking_token = staking_token_address(&self.wl_storage);
 

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

@@ -581,8 +581,9 @@ mod tests {
             None,
             Some(5),
         );
+        let new_epoch_start = BlockHeight(1);
         storage
-            .begin_block(BlockHash::default(), BlockHeight(1))
+            .begin_block(BlockHash::default(), new_epoch_start)
             .expect("begin_block failed");
 
         let key = ibc_key("key").unwrap();
@@ -592,12 +593,13 @@ mod tests {
             .expect("write failed");
 
         storage.block.epoch = storage.block.epoch.next();
-        storage.block.pred_epochs.new_epoch(BlockHeight(1));
+        storage.block.pred_epochs.new_epoch(new_epoch_start);
         let batch = PersistentStorage::batch();
         storage.commit_block(batch).expect("commit failed");
 
+        let new_epoch_start = BlockHeight(6);
         storage
-            .begin_block(BlockHash::default(), BlockHeight(6))
+            .begin_block(BlockHash::default(), new_epoch_start)
             .expect("begin_block failed");
 
         let key = ibc_key("key2").unwrap();
@@ -607,18 +609,19 @@ mod tests {
             .expect("write failed");
 
         storage.block.epoch = storage.block.epoch.next();
-        storage.block.pred_epochs.new_epoch(BlockHeight(6));
+        storage.block.pred_epochs.new_epoch(new_epoch_start);
         let batch = PersistentStorage::batch();
         storage.commit_block(batch).expect("commit failed");
 
         let result = storage.get_merkle_tree(1.into(), Some(StoreType::Ibc));
         assert!(result.is_ok(), "The tree at Height 1 should be restored");
 
+        let new_epoch_start = BlockHeight(11);
         storage
-            .begin_block(BlockHash::default(), BlockHeight(11))
+            .begin_block(BlockHash::default(), new_epoch_start)
             .expect("begin_block failed");
         storage.block.epoch = storage.block.epoch.next();
-        storage.block.pred_epochs.new_epoch(BlockHeight(11));
+        storage.block.pred_epochs.new_epoch(new_epoch_start);
         let batch = PersistentStorage::batch();
         storage.commit_block(batch).expect("commit failed");
 

core/src/ledger/storage/mod.rs

@@ -136,8 +136,8 @@ pub struct BlockStorage<H: StorageHasher> {
     pub hash: BlockHash,
     /// From the start of `FinalizeBlock` until the end of `Commit`, this is
     /// height of the block that is going to be committed. Otherwise, it is the
-    /// height of the most recently committed block, or `BlockHeight(0)` if no
-    /// block has been committed yet.
+    /// height of the most recently committed block, or `BlockHeight::sentinel`
+    /// (0) if no block has been committed yet.
     pub height: BlockHeight,
     /// From the start of `FinalizeBlock` until the end of `Commit`, this is
     /// height of the block that is going to be committed. Otherwise it is the
@@ -1012,6 +1012,9 @@ where
         } = parameters.epoch_duration;
         self.next_epoch_min_start_height = initial_height + min_num_of_blocks;
         self.next_epoch_min_start_time = genesis_time + min_duration;
+        self.block.pred_epochs = Epochs {
+            first_block_heights: vec![initial_height],
+        };
         self.update_epoch_in_merkle_tree()
     }
 
@@ -1394,6 +1397,12 @@ mod tests {
                 minimum_gas_price: BTreeMap::default(),
             };
             parameters.init_storage(&mut wl_storage).unwrap();
+            // Initialize pred_epochs to the current height
+            wl_storage
+                .storage
+                .block
+                .pred_epochs
+                .new_epoch(wl_storage.storage.block.height);
 
             let epoch_before = wl_storage.storage.last_epoch;
             assert_eq!(epoch_before, wl_storage.storage.block.epoch);

core/src/types/storage.rs

@@ -162,9 +162,9 @@ impl BlockResults {
     }
 }
 
-/// Height of a block, i.e. the level.
+/// Height of a block, i.e. the level. The `default` is the
+/// [`BlockHeight::sentinel`] value, which doesn't correspond to any block.
 #[derive(
-    Default,
     Clone,
     Copy,
     BorshSerialize,
@@ -181,6 +181,12 @@ impl BlockResults {
 )]
 pub struct BlockHeight(pub u64);
 
+impl Default for BlockHeight {
+    fn default() -> Self {
+        Self::sentinel()
+    }
+}
+
 impl Display for BlockHeight {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         write!(f, "{}", self.0)
@@ -288,6 +294,17 @@ impl TryFrom<i64> for BlockHeight {
     }
 }
 impl BlockHeight {
+    /// The first block height 1.
+    pub const fn first() -> Self {
+        Self(1)
+    }
+
+    /// A sentinel value block height 0 may be used before any block is
+    /// committed or in queries to read from the latest committed block.
+    pub const fn sentinel() -> Self {
+        Self(0)
+    }
+
     /// Get the height of the next block
     pub fn next_height(&self) -> BlockHeight {
         BlockHeight(self.0 + 1)
@@ -1175,6 +1192,7 @@ impl Mul for Epoch {
 #[derive(
     Clone,
     Debug,
+    Default,
     PartialEq,
     Eq,
     PartialOrd,
@@ -1189,16 +1207,6 @@ pub struct Epochs {
     pub first_block_heights: Vec<BlockHeight>,
 }
 
-impl Default for Epochs {
-    /// Initialize predecessor epochs, assuming starting on the epoch 0 and
-    /// block height 0.
-    fn default() -> Self {
-        Self {
-            first_block_heights: vec![BlockHeight::default()],
-        }
-    }
-}
-
 impl Epochs {
     /// Record start of a new epoch at the given block height
     pub fn new_epoch(&mut self, block_height: BlockHeight) {
@@ -1641,11 +1649,13 @@ mod tests {
 
     #[test]
     fn test_predecessor_epochs_and_heights() {
-        let mut epochs = Epochs::default();
+        let mut epochs = Epochs {
+            first_block_heights: vec![BlockHeight::first()],
+        };
         println!("epochs {:#?}", epochs);
         assert_eq!(
             epochs.get_start_height_of_epoch(Epoch(0)),
-            Some(BlockHeight(0))
+            Some(BlockHeight(1))
         );
         assert_eq!(epochs.get_epoch(BlockHeight(0)), Some(Epoch(0)));
 
@@ -1657,14 +1667,15 @@ mod tests {
             Some(BlockHeight(10))
         );
         assert_eq!(epochs.get_epoch(BlockHeight(0)), Some(Epoch(0)));
+        assert_eq!(epochs.get_epoch_start_height(BlockHeight(0)), None);
         assert_eq!(
-            epochs.get_epoch_start_height(BlockHeight(0)),
-            Some(BlockHeight(0))
+            epochs.get_epoch_start_height(BlockHeight(1)),
+            Some(BlockHeight(1))
         );
         assert_eq!(epochs.get_epoch(BlockHeight(9)), Some(Epoch(0)));
         assert_eq!(
             epochs.get_epoch_start_height(BlockHeight(9)),
-            Some(BlockHeight(0))
+            Some(BlockHeight(1))
         );
         assert_eq!(epochs.get_epoch(BlockHeight(10)), Some(Epoch(1)));
         assert_eq!(

ethereum_bridge/src/test_utils.rs

@@ -228,6 +228,12 @@ pub fn init_storage_with_validators(
             .write(&protocol_pk_key(validator), protocol_key)
             .expect("Test failed");
     }
+    // Initialize pred_epochs to the current height
+    wl_storage
+        .storage
+        .block
+        .pred_epochs
+        .new_epoch(wl_storage.storage.block.height);
     wl_storage.commit_block().expect("Test failed");
     wl_storage.storage.block.height += 1;