From 44cdffaff7af5a8fa33e3c2e120432af95b46af2 Mon Sep 17 00:00:00 2001 From: Jernej Kos Date: Wed, 18 Jan 2023 18:33:42 +0100 Subject: [PATCH] runtime: Simplify the LRU-backed consensus verifier light store --- .changelog/5149.internal.md | 1 + .../src/consensus/tendermint/verifier/mod.rs | 5 +- .../tendermint/verifier/store/lru.rs | 123 ++++++++---------- 3 files changed, 62 insertions(+), 67 deletions(-) create mode 100644 .changelog/5149.internal.md diff --git a/.changelog/5149.internal.md b/.changelog/5149.internal.md new file mode 100644 index 00000000000..fb4c4818a03 --- /dev/null +++ b/.changelog/5149.internal.md @@ -0,0 +1 @@ +runtime: Simplify the LRU-backed consensus verifier light store diff --git a/runtime/src/consensus/tendermint/verifier/mod.rs b/runtime/src/consensus/tendermint/verifier/mod.rs index 3662b30ab8e..ea4e64cef02 100644 --- a/runtime/src/consensus/tendermint/verifier/mod.rs +++ b/runtime/src/consensus/tendermint/verifier/mod.rs @@ -581,7 +581,10 @@ impl Verifier { )?; // Insert all of the trusted blocks into the light store as trusted. - let mut store = Box::new(LruStore::new(1024)); + let mut store = Box::new(LruStore::new( + 1024, + trusted_state.trust_root.height.try_into().unwrap(), + )); for lb in trusted_state.trusted_blocks { store.insert(lb.into(), Status::Trusted); } diff --git a/runtime/src/consensus/tendermint/verifier/store/lru.rs b/runtime/src/consensus/tendermint/verifier/store/lru.rs index 3ecd7da43e2..fc73b4bbf27 100644 --- a/runtime/src/consensus/tendermint/verifier/store/lru.rs +++ b/runtime/src/consensus/tendermint/verifier/store/lru.rs @@ -20,47 +20,33 @@ impl std::fmt::Debug for LruStore { } } -struct SubStore { - lowest_height: Option, - blocks: lru::LruCache, +#[derive(Clone, Debug, PartialEq)] +struct StoreEntry { + light_block: LightBlock, + status: Status, } -impl SubStore { - fn new(capacity: usize) -> Self { +impl StoreEntry { + fn new(light_block: LightBlock, status: Status) -> Self { Self { - lowest_height: None, - blocks: lru::LruCache::new(NonZeroUsize::new(capacity).unwrap()), + light_block, + status, } } } struct Inner { - unverified: SubStore, - verified: SubStore, - trusted: SubStore, - failed: SubStore, -} - -impl Inner { - fn store(&mut self, status: Status) -> &mut SubStore { - match status { - Status::Unverified => &mut self.unverified, - Status::Verified => &mut self.verified, - Status::Trusted => &mut self.trusted, - Status::Failed => &mut self.failed, - } - } + trust_root_height: Height, + blocks: lru::LruCache, } impl LruStore { - /// Create a new, empty, in-memory LRU store of the given capacity. - pub fn new(capacity: usize) -> Self { + /// Create a new, empty, in-memory LRU store of the given capacity and trust root height. + pub fn new(capacity: usize, trust_root_height: Height) -> Self { Self { inner: Mutex::new(Inner { - unverified: SubStore::new(capacity), - verified: SubStore::new(capacity), - trusted: SubStore::new(capacity), - failed: SubStore::new(capacity), + trust_root_height, + blocks: lru::LruCache::new(NonZeroUsize::new(capacity).unwrap()), }), } } @@ -76,36 +62,41 @@ impl LruStore { impl LightStore for LruStore { fn get(&self, height: Height, status: Status) -> Option { - self.inner().store(status).blocks.get(&height).cloned() + self.inner() + .blocks + .get(&height) + .filter(|e| e.status == status) + .map(|e| e.light_block.clone()) } fn insert(&mut self, light_block: LightBlock, status: Status) { - let mut store = self.inner_mut().store(status); - - // Promote lowest to prevent it from being evicted. - let height = light_block.height(); - if let Some(lowest_height) = store.lowest_height { - if height < lowest_height { - store.lowest_height = Some(height); - } else { - store.blocks.promote(&lowest_height); - } - } else { - store.lowest_height = Some(height); - } + let store = self.inner_mut(); + + // Promote trust root to prevent it from being evicted. + store.blocks.promote(&store.trust_root_height); - store.blocks.put(height, light_block); + store + .blocks + .put(light_block.height(), StoreEntry::new(light_block, status)); } fn remove(&mut self, height: Height, status: Status) { - let mut store = self.inner_mut().store(status); + let store = self.inner_mut(); - store.blocks.pop(&height); + // Prevent removal of trust root. + if height == store.trust_root_height { + return; + } - // Check if this is the lowest height being explicitly removed. - if store.lowest_height.map(|lh| height == lh).unwrap_or(false) { - store.lowest_height = store.blocks.iter().map(|(height, _)| *height).min(); + if store + .blocks + .get(&height) + .map(|e| e.status != status) + .unwrap_or(true) + { + return; } + store.blocks.pop(&height); } fn update(&mut self, light_block: &LightBlock, status: Status) { @@ -114,40 +105,40 @@ impl LightStore for LruStore { fn highest(&self, status: Status) -> Option { self.inner() - .store(status) .blocks .iter() + .filter(|(_, e)| e.status == status) .max_by_key(|(&height, _)| height) - .map(|(_, lb)| lb.clone()) + .map(|(_, e)| e.light_block.clone()) } fn highest_before(&self, height: Height, status: Status) -> Option { self.inner() - .store(status) .blocks .iter() + .filter(|(_, e)| e.status == status) .filter(|(h, _)| h <= &&height) .max_by_key(|(&height, _)| height) - .map(|(_, lb)| lb.clone()) + .map(|(_, e)| e.light_block.clone()) } fn lowest(&self, status: Status) -> Option { - let mut inner = self.inner(); - let store = inner.store(status); - - store - .lowest_height - .and_then(|lowest_height| store.blocks.get(&lowest_height).cloned()) + self.inner() + .blocks + .iter() + .filter(|(_, e)| e.status == status) + .min_by_key(|(&height, _)| height) + .map(|(_, e)| e.light_block.clone()) } #[allow(clippy::needless_collect)] fn all(&self, status: Status) -> Box> { let light_blocks: Vec<_> = self .inner() - .store(status) .blocks .iter() - .map(|(_, lb)| lb.clone()) + .filter(|(_, e)| e.status == status) + .map(|(_, e)| e.light_block.clone()) .collect(); Box::new(light_blocks.into_iter()) @@ -179,9 +170,9 @@ mod test { } #[test] - fn test_lowest_height_retained() { + fn test_trust_root_height_retained() { let blocks = generate_blocks(10); - let mut store = LruStore::new(2); // Only storing two blocks. + let mut store = LruStore::new(2, blocks[0].height()); // Only storing two blocks. store.insert(blocks[0].clone(), Status::Trusted); store.insert(blocks[1].clone(), Status::Trusted); store.insert(blocks[2].clone(), Status::Trusted); @@ -196,7 +187,7 @@ mod test { #[test] fn test_basic() { let blocks = generate_blocks(10); - let mut store = LruStore::new(10); + let mut store = LruStore::new(10, blocks[0].height()); for block in &blocks { store.insert(block.clone(), Status::Trusted); @@ -222,15 +213,15 @@ mod test { assert_eq!(highest_before.as_ref(), Some(block)); } - // Test removal of lowest block. + // Test removal of trust root block. store.remove(blocks[0].height(), Status::Trusted); let block_zero = store.get(blocks[0].height(), Status::Trusted); - assert!(block_zero.is_none()); + assert_eq!(block_zero.as_ref(), Some(&blocks[0])); let lowest = store .lowest(Status::Trusted) .expect("there should be a lowest block"); - assert_eq!(lowest, blocks[1]); + assert_eq!(lowest, blocks[0]); } }