gc for view < decided view

Author: imabdulbasit

crates/espresso/node/src/context.rs

@@ -2,7 +2,6 @@ use std::{
     fmt::{Debug, Display},
     future::Future,
     marker::PhantomData,
-    num::NonZeroU64,
     sync::Arc,
     time::{Duration, Instant},
 };
@@ -126,7 +125,6 @@ where
         event_consumer: impl PersistenceEventConsumer + 'static,
         proposal_fetcher_cfg: ProposalFetcherConfig,
         bootstrap_epoch_catchup_timeout: Duration,
-        new_protocol_consensus_gc_interval: NonZeroU64,
     ) -> anyhow::Result<Self> {
         let config = &network_config.config;
         let pub_key = validator_config.public_key;
@@ -221,7 +219,6 @@ where
             .stake_table_capacity(stake_table_capacity)
             .timeout_duration(Duration::from_secs(10))
             .storage(Arc::clone(&persistence))
-            .garbage_collection_interval(new_protocol_consensus_gc_interval.get())
             .make();
 
         let legacy_event_rx = handle.event_stream_known_impl().deactivate();

crates/espresso/node/src/lib.rs

@@ -19,7 +19,7 @@ pub mod state_cert;
 pub mod state_signature;
 pub mod util;
 
-use std::{fmt::Debug, marker::PhantomData, num::NonZeroU64, sync::Arc, time::Duration};
+use std::{fmt::Debug, marker::PhantomData, sync::Arc, time::Duration};
 
 use alloy::primitives::U256;
 use anyhow::Context;
@@ -133,8 +133,6 @@ pub struct NetworkParams {
     /// Per-step timeout for the startup stake-table catchup walk
     /// (`bootstrap_epoch_window`).
     pub bootstrap_epoch_catchup_timeout: Duration,
-    /// Number of blocks between new-protocol consensus garbage collection passes.
-    pub new_protocol_consensus_gc_interval: NonZeroU64,
     /// The address to advertise as our public API's URL
     pub public_api_url: Option<Url>,
     /// Cliquenet network address.
@@ -830,7 +828,6 @@ where
         event_consumer,
         proposal_fetcher_config,
         network_params.bootstrap_epoch_catchup_timeout,
-        network_params.new_protocol_consensus_gc_interval,
     )
     .await?;
 
@@ -1660,7 +1657,6 @@ pub mod testing {
                 event_consumer,
                 Default::default(),
                 Duration::from_secs(2),
-                NonZeroU64::new(100).unwrap(),
             )
             .await
             .unwrap()

crates/espresso/node/src/options.rs

@@ -6,7 +6,6 @@ use std::{
     collections::HashSet,
     fmt::{self, Formatter},
     iter::once,
-    num::NonZeroU64,
     path::PathBuf,
     time::Duration,
 };
@@ -341,18 +340,6 @@ pub struct Options {
     #[clap(long, env = "ESPRESSO_NODE_BOOTSTRAP_EPOCH_CATCHUP_TIMEOUT", default_value = "30s", value_parser = parse_duration)]
     pub bootstrap_epoch_catchup_timeout: Duration,
 
-    /// Number of blocks between new-protocol garbage collection passes.
-    ///
-    /// Controls how often the new-protocol coordinator triggers garbage
-    /// collection of decided views/epochs across consensus state, VID,
-    /// vote collectors, and storage.
-    #[clap(
-        long,
-        env = "ESPRESSO_NODE_NEW_PROTOCOL_CONSENSUS_GC_INTERVAL",
-        default_value = "100"
-    )]
-    pub new_protocol_consensus_gc_interval: NonZeroU64,
-
     #[clap(flatten)]
     pub logging: logging::Config,
 
@@ -691,7 +678,6 @@ pub struct PublicNodeConfig {
     pub catchup_base_timeout: Duration,
     pub local_catchup_timeout: Duration,
     pub bootstrap_epoch_catchup_timeout: Duration,
-    pub new_protocol_consensus_gc_interval: NonZeroU64,
     pub catchup_backoff: BackoffParams,
     pub proposal_fetcher: ProposalFetcherConfig,
     pub libp2p: Libp2pTuning,
@@ -1031,7 +1017,6 @@ impl PublicNodeConfig {
             catchup_base_timeout: opt.catchup_base_timeout,
             local_catchup_timeout: opt.local_catchup_timeout,
             bootstrap_epoch_catchup_timeout: opt.bootstrap_epoch_catchup_timeout,
-            new_protocol_consensus_gc_interval: opt.new_protocol_consensus_gc_interval,
             catchup_backoff: opt.catchup_backoff,
             proposal_fetcher: opt.proposal_fetcher_config,
             libp2p: Libp2pTuning::from(opt),

crates/espresso/node/src/run.rs

@@ -103,7 +103,6 @@ where
         catchup_base_timeout: opt.catchup_base_timeout,
         local_catchup_timeout: opt.local_catchup_timeout,
         bootstrap_epoch_catchup_timeout: opt.bootstrap_epoch_catchup_timeout,
-        new_protocol_consensus_gc_interval: opt.new_protocol_consensus_gc_interval,
         libp2p_history_gossip: opt.libp2p_history_gossip,
         libp2p_history_length: opt.libp2p_history_length,
         libp2p_max_ihave_length: opt.libp2p_max_ihave_length,

crates/hotshot/new-protocol/bench/src/node.rs

@@ -131,7 +131,6 @@ async fn build_coordinator(
         upgrade_lock.clone(),
         genesis_leaf.clone(),
         epoch_height,
-        100,
     );
 
     let vote1_collector = VoteCollector::new(membership.clone(), upgrade_lock.clone());

crates/hotshot/new-protocol/src/consensus.rs

@@ -199,7 +199,6 @@ pub struct Consensus<T: NodeType> {
     node_id: KeyPrefix,
     upgrade_lock: UpgradeLock<T>,
 
-    garbage_collection_interval: BlockNumber,
     pub(crate) epoch_height: BlockNumber,
 }
 
@@ -232,7 +231,6 @@ impl<T: NodeType> Consensus<T> {
         upgrade_lock: UpgradeLock<T>,
         genesis_leaf: Leaf2<T>,
         epoch_height: B,
-        garbage_collection_interval: B,
     ) -> Self
     where
         B: Into<BlockNumber>,
@@ -270,7 +268,6 @@ impl<T: NodeType> Consensus<T> {
             state_certs: BTreeMap::new(),
             upgrade_lock,
             vid_shares: BTreeMap::new(),
-            garbage_collection_interval: garbage_collection_interval.into(),
             epoch_height: epoch_height.into(),
         }
     }
@@ -1180,10 +1177,8 @@ impl<T: NodeType> Consensus<T> {
         }
         let new_decided_view = max(self.last_decided_view, leaf.view_number());
         let last_decided_leaf = leaf.clone();
-        let mut gc = None;
-        if leaf.block_header().block_number() % *self.garbage_collection_interval == 0 {
-            gc = Some((leaf.view_number(), leaf.justify_qc().epoch()));
-        }
+        // garbage collect everything strictly below the newly decided view.
+        let gc = Some((leaf.view_number(), leaf.justify_qc().epoch()));
         let mut decided = vec![leaf];
         let mut vid_shares = vec![self.signed_vid_share(view)];
 
@@ -1201,11 +1196,6 @@ impl<T: NodeType> Consensus<T> {
             if let Some(payload) = self.blocks.get(&parent_view) {
                 leaf.fill_block_payload_unchecked(payload.clone());
             }
-            if gc.is_none()
-                && leaf.block_header().block_number() % *self.garbage_collection_interval == 0
-            {
-                gc = Some((leaf.view_number(), leaf.justify_qc().epoch()));
-            }
             vid_shares.push(self.signed_vid_share(parent_view));
             decided.push(leaf);
             parent_view = proposal.justify_qc.view_number();

crates/hotshot/new-protocol/src/coordinator.rs

@@ -118,7 +118,6 @@ where
         stake_table_capacity: usize,
         timeout_duration: Duration,
         storage: S,
-        garbage_collection_interval: u64,
     ) -> Self {
         let mut consensus = Consensus::new(
             membership_coordinator.clone(),
@@ -129,7 +128,6 @@ where
             upgrade_lock.clone(),
             initializer.anchor_leaf.clone(),
             initializer.epoch_height,
-            garbage_collection_interval,
         );
 
         let genesis_cert1 = initializer.high_qc.clone();

crates/hotshot/new-protocol/src/tests/common/coordinator_builder.rs

@@ -73,7 +73,6 @@ pub async fn build_test_coordinator<N: Network<TestTypes>>(
         upgrade_lock.clone(),
         genesis_leaf.clone(),
         epoch_height,
-        100,
     );
 
     let vid_disperser = VidDisperser::new(membership.clone());

crates/hotshot/new-protocol/src/tests/common/harness.rs

@@ -88,7 +88,6 @@ impl TestHarness {
             upgrade_lock.clone(),
             genesis_leaf.clone(),
             epoch_height,
-            100,
         );
 
         let vid_disperse_task = VidDisperser::new(membership.clone());

crates/hotshot/new-protocol/src/tests/common/utils.rs

@@ -877,7 +877,6 @@ impl ConsensusHarness {
             test_upgrade_lock(),
             genesis_leaf,
             epoch_height,
-            100,
         );
         Self {
             consensus,