From aa0299bcb12225d3bf9eefed9272e1a20545f7fd Mon Sep 17 00:00:00 2001 From: raychu86 <14917648+raychu86@users.noreply.github.com> Date: Sun, 3 Mar 2024 18:45:25 -0800 Subject: [PATCH 1/3] Perform GC on initialization to ensure our gc_round is correct --- node/bft/src/helpers/storage.rs | 2 ++ node/bft/src/sync/mod.rs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/node/bft/src/helpers/storage.rs b/node/bft/src/helpers/storage.rs index 90911ab594..f10efd0d1a 100644 --- a/node/bft/src/helpers/storage.rs +++ b/node/bft/src/helpers/storage.rs @@ -114,6 +114,8 @@ impl Storage { })); // Update the storage to the current round. storage.update_current_round(current_round); + // Perform GC on the current round. + storage.garbage_collect_certificates(current_round); // Return the storage. storage } diff --git a/node/bft/src/sync/mod.rs b/node/bft/src/sync/mod.rs index 555bdb5a48..153e6556b3 100644 --- a/node/bft/src/sync/mod.rs +++ b/node/bft/src/sync/mod.rs @@ -212,6 +212,8 @@ impl Sync { self.storage.sync_height_with_block(latest_block.height()); // Sync the round with the block. self.storage.sync_round_with_block(latest_block.round()); + // Perform GC on the latest block round. + self.storage.garbage_collect_certificates(latest_block.round()); // Iterate over the blocks. for block in &blocks { // If the block authority is a subdag, then sync the batch certificates with the block. From 0aeccc0d1d55f23dd24130dd50dee9cc49239e5f Mon Sep 17 00:00:00 2001 From: raychu86 <14917648+raychu86@users.noreply.github.com> Date: Sun, 3 Mar 2024 19:40:10 -0800 Subject: [PATCH 2/3] Add test for gc on storage init --- node/bft/src/worker.rs | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/node/bft/src/worker.rs b/node/bft/src/worker.rs index bfe975f608..428f9e378f 100644 --- a/node/bft/src/worker.rs +++ b/node/bft/src/worker.rs @@ -486,6 +486,8 @@ mod tests { type CurrentNetwork = snarkvm::prelude::MainnetV0; + const ITERATIONS: usize = 100; + mock! { Gateway {} #[async_trait] @@ -868,6 +870,34 @@ mod tests { assert!(!worker.pending.contains(transmission_id)); assert!(worker.ready.contains(transmission_id)); } + + #[tokio::test] + async fn test_storage_gc_on_initialization() { + let rng = &mut TestRng::default(); + + for _ in 0..ITERATIONS { + // Mock the ledger round. + let max_gc_rounds = rng.gen_range(50..=100); + let latest_ledger_round = rng.gen_range((max_gc_rounds + 1)..1000); + let expected_gc_round = latest_ledger_round - max_gc_rounds; + + // Sample a committee. + let committee = + snarkvm::ledger::committee::test_helpers::sample_committee_for_round(latest_ledger_round, rng); + + // Setup the mock gateway and ledger. + let mut mock_ledger = MockLedger::default(); + mock_ledger.expect_current_committee().returning(move || Ok(committee.clone())); + + let ledger: Arc> = Arc::new(mock_ledger); + // Initialize the storage. + let storage = + Storage::::new(ledger.clone(), Arc::new(BFTMemoryService::new()), max_gc_rounds); + + // Ensure that the storage GC round is correct. + assert_eq!(storage.gc_round(), expected_gc_round); + } + } } #[cfg(test)] From f54d3bb5a0c82bc74677feea48dbc3e29356a71b Mon Sep 17 00:00:00 2001 From: raychu86 <14917648+raychu86@users.noreply.github.com> Date: Sun, 3 Mar 2024 20:13:49 -0800 Subject: [PATCH 3/3] Add test to ensure BFT performs GC on commit --- node/bft/src/bft.rs | 67 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/node/bft/src/bft.rs b/node/bft/src/bft.rs index 0ca528f96f..7667863b0b 100644 --- a/node/bft/src/bft.rs +++ b/node/bft/src/bft.rs @@ -1226,4 +1226,71 @@ mod tests { assert_eq!(result.unwrap_err().to_string(), error_msg); Ok(()) } + + #[tokio::test] + #[tracing_test::traced_test] + async fn test_bft_gc_on_commit() -> Result<()> { + let rng = &mut TestRng::default(); + + // Initialize the round parameters. + let max_gc_rounds = 1; + let committee_round = 0; + let commit_round = 2; + let current_round = commit_round + 1; + + // Sample the certificates. + let (_, certificates) = snarkvm::ledger::narwhal::batch_certificate::test_helpers::sample_batch_certificate_with_previous_certificates( + current_round, + rng, + ); + + // Initialize the committee. + let committee = snarkvm::ledger::committee::test_helpers::sample_committee_for_round_and_members( + committee_round, + vec![ + certificates[0].author(), + certificates[1].author(), + certificates[2].author(), + certificates[3].author(), + ], + rng, + ); + + // Initialize the ledger. + let ledger = Arc::new(MockLedgerService::new(committee.clone())); + + // Initialize the storage. + let transmissions = Arc::new(BFTMemoryService::new()); + let storage = Storage::new(ledger.clone(), transmissions, max_gc_rounds); + // Insert the certificates into the storage. + for certificate in certificates.iter() { + storage.testing_only_insert_certificate_testing_only(certificate.clone()); + } + + // Get the leader certificate. + let leader = committee.get_leader(commit_round).unwrap(); + let leader_certificate = storage.get_certificate_for_round_with_author(commit_round, leader).unwrap(); + + // Initialize the BFT. + let account = Account::new(rng)?; + let bft = BFT::new(account, storage.clone(), ledger, None, &[], None)?; + // Insert a mock DAG in the BFT. + *bft.dag.write() = crate::helpers::dag::test_helpers::mock_dag_with_modified_last_committed_round(commit_round); + + // Ensure that the `gc_round` has not been updated yet. + assert_eq!(bft.storage().gc_round(), committee_round.saturating_sub(max_gc_rounds)); + + // Insert the certificates into the BFT. + for certificate in certificates { + assert!(bft.update_dag::(certificate).await.is_ok()); + } + + // Commit the leader certificate. + bft.commit_leader_certificate::(leader_certificate).await.unwrap(); + + // Ensure that the `gc_round` has been updated. + assert_eq!(bft.storage().gc_round(), commit_round - max_gc_rounds); + + Ok(()) + } }