rework: only consider duplicate proofs from root + 1

anza-xyz · Jul 9, 2024 · cd3f151 · cd3f151
1 parent ee5bf30
commit cd3f151
Show file tree

Hide file tree

Showing 4 changed files with 8 additions and 39 deletions.
diff --git a/core/src/consensus.rs b/core/src/consensus.rs
@@ -3243,10 +3243,6 @@ pub mod test {
         assert!(!blockstore.is_root(1));
         assert!(!blockstore.is_root(3));
         assert!(!blockstore.is_root(4));
-        for i in 0..4 {
-            blockstore.store_duplicate_slot(i, vec![], vec![]).unwrap();
-            assert!(blockstore.has_duplicate_shreds_in_slot(i));
-        }
 
         let mut tower = Tower::default();
         tower.vote_state.root_slot = Some(4);
@@ -3258,13 +3254,9 @@ pub mod test {
         .unwrap();
 
         assert!(!blockstore.is_root(0));
-        assert!(blockstore.has_duplicate_shreds_in_slot(0));
         assert!(blockstore.is_root(1));
-        assert!(!blockstore.has_duplicate_shreds_in_slot(1));
         assert!(!blockstore.is_root(3));
-        assert!(blockstore.has_duplicate_shreds_in_slot(3));
         assert!(blockstore.is_root(4));
-        assert!(!blockstore.has_duplicate_shreds_in_slot(4));
     }
 
     #[test]

diff --git a/core/src/replay_stage.rs b/core/src/replay_stage.rs
@@ -1390,15 +1390,21 @@ impl ReplayStage {
     ) -> (ProgressMap, HeaviestSubtreeForkChoice) {
         let (root_bank, frozen_banks, duplicate_slot_hashes) = {
             let bank_forks = bank_forks.read().unwrap();
+            let root_bank = bank_forks.root_bank();
             let duplicate_slots = blockstore
-                .duplicate_slots_iterator(bank_forks.root_bank().slot())
+                // It is important that the root bank is not marked as duplicate on initialization.
+                // Although this bank could contain a duplicate proof, the fact that it was rooted
+                // either during a previous run or artificially means that we should ignore any
+                // duplicate proofs for the root slot, thus we start consuming duplicate proofs
+                // from the root slot + 1
+                .duplicate_slots_iterator(root_bank.slot().saturating_add(1))
                 .unwrap();
             let duplicate_slot_hashes = duplicate_slots.filter_map(|slot| {
                 let bank = bank_forks.get(slot)?;
                 Some((slot, bank.hash()))
             });
             (
-                bank_forks.root_bank(),
+                root_bank,
                 bank_forks.frozen_banks().values().cloned().collect(),
                 duplicate_slot_hashes.collect::<Vec<(Slot, Hash)>>(),
             )

diff --git a/ledger/src/blockstore.rs b/ledger/src/blockstore.rs
@@ -3864,8 +3864,6 @@ impl Blockstore {
         with_hash: bool,
     ) -> Result<()> {
         self.set_roots(slots.iter().map(|(slot, _hash)| slot))?;
-        // Remove any old duplicate proofs
-        self.remove_slot_duplicate_proofs(slots.iter().map(|(slot, _hash)| *slot))?;
         if with_hash {
             self.set_duplicate_confirmed_slots_and_hashes(
                 slots
@@ -3897,20 +3895,6 @@ impl Blockstore {
         self.duplicate_slots_cf.delete(slot)
     }
 
-    fn remove_slot_duplicate_proofs<I>(&self, slots: I) -> Result<()>
-    where
-        I: Iterator<Item = Slot>,
-    {
-        let mut write_batch = self.db.batch()?;
-
-        for slot in slots {
-            write_batch.delete::<cf::DuplicateSlots>(slot)?;
-        }
-
-        self.db.write(write_batch)?;
-        Ok(())
-    }
-
     pub fn get_first_duplicate_proof(&self) -> Option<(Slot, DuplicateSlotProof)> {
         let mut iter = self
             .db

diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs
@@ -3990,14 +3990,6 @@ pub mod tests {
             accounts_db_test_hash_calculation: true,
             ..ProcessOptions::default()
         };
-
-        // Mark all blocks 1 - 6 as duplicate
-        for slot in 1..6 {
-            blockstore
-                .store_duplicate_slot(slot, vec![], vec![])
-                .unwrap();
-        }
-
         let recyclers = VerifyRecyclers::default();
         let replay_tx_thread_pool = create_thread_pool(1);
         process_bank_0(
@@ -4071,11 +4063,6 @@ pub mod tests {
 
         // Check that bank forks has the correct banks
         verify_fork_infos(&bank_forks);
-
-        // Verify that the duplicate information has been cleared and that fork choice can be initialized
-        for slot in 1..6 {
-            assert!(!blockstore.has_duplicate_shreds_in_slot(slot));
-        }
     }
 
     #[test]