Skip to content

Commit

Permalink
Merge branch 'brent/fix-epoch-trimming' (#1898)
Browse files Browse the repository at this point in the history 
* brent/fix-epoch-trimming:
  fixup! remove unnecessary `Epochs` methods
  changelog: #1898
  remove unnecessary `Epochs` methods
  fix tests
  remove trimming of `pred_epochs`
  • Loading branch information
@brentstone
brentstone committed Sep 21, 2023
2 parents 25cb5fd + 4a83bbe commit 56902c3
Show file tree
Hide file tree
Showing 7 changed files with 96 additions and 113 deletions.
3 changes: 3 additions & 0 deletions .changelog/unreleased/bug-fixes/1898-fix-epoch-trimming.md
View file
Original file line number Diff line number Diff line change
@@ -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))
2 changes: 1 addition & 1 deletion apps/src/lib/node/ledger/shell/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
13 changes: 5 additions & 8 deletions apps/src/lib/node/ledger/storage/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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");

Expand All @@ -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");

Expand All @@ -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");

Expand Down
9 changes: 2 additions & 7 deletions core/src/ledger/storage/wl_storage.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
174 changes: 79 additions & 95 deletions core/src/types/storage.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<BlockHeight>,
Expand All @@ -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<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);
Expand All @@ -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,
Expand All @@ -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<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.
Expand Down Expand Up @@ -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)),
Expand Down Expand Up @@ -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)));
Expand All @@ -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!(
Expand All @@ -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}"
);
}
}

Expand Down
2 changes: 1 addition & 1 deletion ethereum_bridge/src/protocol/transactions/validator_set_update/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion proof_of_stake/src/pos_queries.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<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.
Expand Down

0 comments on commit 56902c3

Please sign in to comment.