diff --git a/core/src/bank_forks.rs b/core/src/bank_forks.rs index c15625b9d8..6a1114d92a 100644 --- a/core/src/bank_forks.rs +++ b/core/src/bank_forks.rs @@ -35,7 +35,8 @@ impl BankForks { pub fn ancestors(&self) -> HashMap> { let mut ancestors = HashMap::new(); for bank in self.banks.values() { - let set = bank.parents().into_iter().map(|b| b.slot()).collect(); + let mut set: HashSet = bank.ancestors.keys().cloned().collect(); + set.remove(&bank.slot()); ancestors.insert(bank.slot(), set); } ancestors @@ -46,9 +47,11 @@ impl BankForks { let mut descendants = HashMap::new(); for bank in self.banks.values() { let _ = descendants.entry(bank.slot()).or_insert(HashSet::new()); - for parent in bank.parents() { + let mut set: HashSet = bank.ancestors.keys().cloned().collect(); + set.remove(&bank.slot()); + for parent in set { descendants - .entry(parent.slot()) + .entry(parent) .or_insert(HashSet::new()) .insert(bank.slot()); } @@ -116,8 +119,9 @@ impl BankForks { } fn prune_non_root(&mut self, root: u64) { + let descendants = self.descendants(); self.banks - .retain(|slot, bank| *slot >= root || bank.is_in_subtree_of(root)) + .retain(|slot, _| descendants[&root].contains(slot)) } } diff --git a/runtime/src/accounts_index.rs b/runtime/src/accounts_index.rs index fae7fc1055..90428c3e5d 100644 --- a/runtime/src/accounts_index.rs +++ b/runtime/src/accounts_index.rs @@ -66,9 +66,11 @@ impl AccountsIndex { self.roots.contains(&fork) } pub fn add_root(&mut self, fork: Fork) { - if fork > self.last_root { - self.last_root = fork; - } + assert!( + (self.last_root == 0 && fork == 0) || (fork > self.last_root), + "new roots must be increasing" + ); + self.last_root = fork; self.roots.insert(fork); } /// Remove the fork when the storage for the fork is freed @@ -156,15 +158,22 @@ mod tests { fn test_max_last_root() { let mut index = AccountsIndex::::default(); index.add_root(1); - index.add_root(0); assert_eq!(index.last_root, 1); } + #[test] + #[should_panic] + fn test_max_last_root_old() { + let mut index = AccountsIndex::::default(); + index.add_root(1); + index.add_root(0); + } + #[test] fn test_cleanup_first() { let mut index = AccountsIndex::::default(); - index.add_root(1); index.add_root(0); + index.add_root(1); index.cleanup_dead_fork(0); assert!(index.is_root(1)); assert!(!index.is_root(0)); @@ -174,8 +183,8 @@ mod tests { fn test_cleanup_last() { //this behavior might be undefined, clean up should only occur on older forks let mut index = AccountsIndex::::default(); - index.add_root(1); index.add_root(0); + index.add_root(1); index.cleanup_dead_fork(1); assert!(!index.is_root(1)); assert!(index.is_root(0)); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 4e770481d2..e8c6443637 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -124,7 +124,7 @@ pub struct Bank { parent: RwLock>>, /// The set of parents including this bank - ancestors: HashMap, + pub ancestors: HashMap, /// Hash of this Bank's state. Only meaningful after freezing. hash: RwLock, @@ -293,7 +293,7 @@ impl Bank { *self.parent.write().unwrap() = None; let squash_accounts_start = Instant::now(); - for p in &parents { + for p in parents.iter().rev() { // root forks cannot be purged self.accounts.add_root(p.slot()); } @@ -1026,24 +1026,6 @@ impl Bank { // Register a bogus executable account, which will be loaded and ignored. self.register_native_instruction_processor("", &program_id); } - - pub fn is_in_subtree_of(&self, parent: u64) -> bool { - if self.slot() == parent { - return true; - } - let mut next_parent = self.parent(); - - while let Some(p) = next_parent { - if p.slot() == parent { - return true; - } else if p.slot() < parent { - return false; - } - next_parent = p.parent(); - } - - false - } } impl Drop for Bank { @@ -1841,30 +1823,6 @@ mod tests { assert_eq!(bank.is_votable(), true); } - #[test] - fn test_is_in_subtree_of() { - let (genesis_block, _) = GenesisBlock::new(1); - let parent = Arc::new(Bank::new(&genesis_block)); - // Bank 1 - let bank = Arc::new(new_from_parent(&parent)); - // Bank 2 - let bank2 = new_from_parent(&bank); - // Bank 5 - let bank5 = Bank::new_from_parent(&bank, &Pubkey::default(), 5); - - // Parents of bank 2: 0 -> 1 -> 2 - assert!(bank2.is_in_subtree_of(0)); - assert!(bank2.is_in_subtree_of(1)); - assert!(bank2.is_in_subtree_of(2)); - assert!(!bank2.is_in_subtree_of(3)); - - // Parents of bank 5: 0 -> 1 -> 5 - assert!(bank5.is_in_subtree_of(0)); - assert!(bank5.is_in_subtree_of(1)); - assert!(!bank5.is_in_subtree_of(2)); - assert!(!bank5.is_in_subtree_of(4)); - } - #[test] fn test_bank_inherit_tx_count() { let (genesis_block, mint_keypair) = GenesisBlock::new(500);