diff --git a/.changelog/unreleased/bug-fixes/1898-fix-epoch-trimming.md b/.changelog/unreleased/bug-fixes/1898-fix-epoch-trimming.md new file mode 100644 index 0000000000..4f920cc456 --- /dev/null +++ b/.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)) \ No newline at end of file diff --git a/apps/src/lib/node/ledger/shell/mod.rs b/apps/src/lib/node/ledger/shell/mod.rs index 5044b55c9d..a1c17fe450 100644 --- a/apps/src/lib/node/ledger/shell/mod.rs +++ b/apps/src/lib/node/ledger/shell/mod.rs @@ -2045,7 +2045,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"); diff --git a/apps/src/lib/node/ledger/storage/mod.rs b/apps/src/lib/node/ledger/storage/mod.rs index ce77d7487f..659f561cce 100644 --- a/apps/src/lib/node/ledger/storage/mod.rs +++ b/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"); diff --git a/core/src/ledger/storage/wl_storage.rs b/core/src/ledger/storage/wl_storage.rs index a8543625c8..87107a35c9 100644 --- a/core/src/ledger/storage/wl_storage.rs +++ b/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) diff --git a/core/src/types/storage.rs b/core/src/types/storage.rs index 4aae00b42e..ad0c14f499 100644 --- a/core/src/types/storage.rs +++ b/core/src/types/storage.rs @@ -1157,8 +1157,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, @@ -1169,48 +1167,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 { - 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); @@ -1223,7 +1198,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, @@ -1237,32 +1212,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 { - 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 { - // 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. @@ -1658,14 +1616,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)), @@ -1693,9 +1656,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))); @@ -1715,14 +1681,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!( @@ -1735,62 +1704,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}" + ); } } diff --git a/ethereum_bridge/src/protocol/transactions/validator_set_update/mod.rs b/ethereum_bridge/src/protocol/transactions/validator_set_update/mod.rs index 8457b82c4f..137fdddf28 100644 --- a/ethereum_bridge/src/protocol/transactions/validator_set_update/mod.rs +++ b/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 diff --git a/proof_of_stake/src/pos_queries.rs b/proof_of_stake/src/pos_queries.rs index ce2e25cf39..190548570b 100644 --- a/proof_of_stake/src/pos_queries.rs +++ b/proof_of_stake/src/pos_queries.rs @@ -279,7 +279,11 @@ where /// been purged from Namada, or if it is not available yet. #[inline] pub fn get_height(self, epoch: Epoch) -> Option { - 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.