This repository has been archived by the owner on Jan 22, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
pull Checkpoints out of #2289 #2457
Closed
Closed
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev
Previous commit
100% coverage, added invert()
- Loading branch information
commit 2461aebc82b14a8c169d50be2c8dbf3127ac626c
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,7 @@ | |
//! | ||
//! `latest` forks is a set of all the forks with no children. | ||
//! | ||
//! A trunk is the latest fork that is a parent all the `latest` forks. If | ||
//! A trunk is the most recent fork that is a parent to all the `latest` forks. If | ||
//! consensus works correctly, then latest should be pruned such that only one | ||
//! trunk exists within N links. | ||
|
||
|
@@ -28,6 +28,14 @@ impl<T> Checkpoints<T> { | |
fork, parent | ||
); | ||
} | ||
|
||
if self.checkpoints.get(&fork).is_some() { | ||
panic!( | ||
"fork: {}, parent: {} error: fork is already checkpointed", | ||
fork, parent | ||
); | ||
} | ||
|
||
self.latest.remove(&parent); | ||
self.latest.insert(fork); | ||
self.checkpoints.insert(fork, (data, parent)); | ||
|
@@ -46,6 +54,19 @@ impl<T> Checkpoints<T> { | |
} | ||
rv | ||
} | ||
|
||
/// returns the trunks, and an inverse dag | ||
pub fn invert(&self) -> (HashSet<u64>, HashMap<u64, HashSet<u64>>) { | ||
let mut trunks = HashSet::new(); | ||
let mut idag = HashMap::new(); | ||
for (k, (_, v)) in &self.checkpoints { | ||
trunks.remove(k); | ||
trunks.insert(*v); | ||
idag.entry(*v).or_insert(HashSet::new()).insert(*k); | ||
} | ||
(trunks, idag) | ||
} | ||
|
||
/// given a maximum depth, find the highest parent shared by all | ||
/// forks | ||
pub fn trunk(&self, num: usize) -> Option<u64> { | ||
|
@@ -91,12 +112,14 @@ impl<T> Checkpoints<T> { | |
} | ||
trunk = Some(trytrunk); | ||
|
||
// all chains are now truncated before the trytrunk entry | ||
// all chains are now truncated before the trytrunk entry, | ||
// none should be empty, implies a structure like | ||
// 3->1 | ||
// 1 | ||
// which we prevent in store() | ||
for chain in chains.iter() { | ||
if chain.is_empty() { | ||
// outer loop needs all chains to have at least one item | ||
// (because of last().unwrap() above) | ||
break 'outer; | ||
panic!("parent forks should never appear in latest"); | ||
} | ||
} | ||
} | ||
|
@@ -159,6 +182,10 @@ mod test { | |
|
||
let points = checkpoints.collect(std::usize::MAX, 10); | ||
assert_eq!(points.len(), 10); | ||
for (i, (parent, data)) in points.iter().enumerate() { | ||
assert_eq!(**data, 10 - i); | ||
assert_eq!(*parent, 10 - i as u64); | ||
} | ||
|
||
let points = checkpoints.collect(0, 10); | ||
assert_eq!(points.len(), 0); | ||
|
@@ -169,11 +196,6 @@ mod test { | |
let points = checkpoints.collect(std::usize::MAX, 11); | ||
assert_eq!(points.len(), 0); | ||
|
||
for (i, (parent, data)) in points.iter().enumerate() { | ||
assert_eq!(**data, 10 - i); | ||
assert_eq!(*parent, 10 - i as u64); | ||
} | ||
|
||
// only one chain, so tip of trunk is latest | ||
assert_eq!(checkpoints.trunk(11), Some(10)); | ||
|
||
|
@@ -199,7 +221,7 @@ mod test { | |
assert_eq!(checkpoints.trunk(10), Some(1)); | ||
assert_eq!(checkpoints.trunk(11), Some(1)); | ||
|
||
// higher trunk | ||
// test higher trunk | ||
let mut checkpoints: Checkpoints<usize> = Default::default(); | ||
|
||
// 3->2->1 | ||
|
@@ -217,6 +239,64 @@ mod test { | |
vec![(4u64, &0), (2u64, &0), (1u64, &0)] | ||
); | ||
assert_eq!(checkpoints.trunk(std::usize::MAX), Some(2)); | ||
// assert_eq!(checkpoints.collect(std::usize::MAX, 4); | ||
} | ||
|
||
#[test] | ||
fn test_checkpoints_invert() { | ||
// test higher trunk | ||
let mut checkpoints: Checkpoints<usize> = Default::default(); | ||
|
||
// 3->2->1 | ||
// 4->2->1 | ||
checkpoints.store(1, 0, 0); | ||
checkpoints.store(2, 0, 1); | ||
checkpoints.store(3, 0, 2); | ||
checkpoints.store(4, 0, 2); | ||
|
||
let _trunks: Vec<_> = checkpoints.invert().0.into_iter().collect(); | ||
// TODO: returns [0, 2] ?? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @aeyakovenko is this the expected behavior? |
||
|
||
// 30->20->10 | ||
// 40->20->10 | ||
checkpoints.store(10, 0, 0); | ||
checkpoints.store(20, 0, 10); | ||
checkpoints.store(30, 0, 20); | ||
checkpoints.store(40, 0, 20); | ||
|
||
let _trunks: Vec<_> = checkpoints.invert().0.into_iter().collect(); | ||
// TODO: returns [0, 2, 10, 20, 1] ?? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @aeyakovenko is this the expected behavior? I don't understand how 1 is now part of trunks |
||
} | ||
|
||
#[test] | ||
#[should_panic] | ||
fn test_checkpoints_latest_is_a_parent() { | ||
// trunk is actually live... | ||
let mut checkpoints: Checkpoints<usize> = Default::default(); | ||
|
||
// 2->1 | ||
// 1 | ||
checkpoints.store(1, 0, 0); | ||
checkpoints.store(3, 0, 1); | ||
checkpoints.store(1, 0, 0); // <== panic... | ||
} | ||
#[test] | ||
#[should_panic] | ||
fn test_checkpoints_latest_is_a_parent_corrupt() { | ||
// trunk is actually live... | ||
let mut checkpoints: Checkpoints<usize> = Default::default(); | ||
|
||
// 2->1 | ||
// 1 | ||
checkpoints.store(1, 0, 0); | ||
checkpoints.store(3, 0, 1); | ||
|
||
// same as: | ||
// checkpoints.store(1, 0, 0); // <== would corrupt the checkpoints... | ||
checkpoints.latest.remove(&0); | ||
checkpoints.latest.insert(1); | ||
checkpoints.checkpoints.insert(1, (0, 0)); | ||
|
||
checkpoints.trunk(std::usize::MAX); // panic | ||
} | ||
|
||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not a method that does
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and then? probably what you've written is more along the lines of what you want for bank_state?