diff --git a/core/src/sdp/mod.rs b/core/src/sdp/mod.rs index 823e7343b..f9905e2ee 100644 --- a/core/src/sdp/mod.rs +++ b/core/src/sdp/mod.rs @@ -43,9 +43,6 @@ pub struct MinStake { pub struct ServiceParameters { /// Maximum epochs during which an activity message must be sent. pub inactivity_period: InactivityPeriod, - /// Epochs after which a declaration can be safely deleted by Garbage - /// Collection - pub retention_period: NumberOfEpochs, // Epoch number at which this parameter was set pub epoch: Epoch, } diff --git a/deployment/ceremony/genesis/devnet/deployment-template.yaml b/deployment/ceremony/genesis/devnet/deployment-template.yaml index 7ae0a7c15..4dec5f89a 100644 --- a/deployment/ceremony/genesis/devnet/deployment-template.yaml +++ b/deployment/ceremony/genesis/devnet/deployment-template.yaml @@ -31,7 +31,6 @@ cryptarchia: service_params: BN: inactivity_period: 2 - retention_period: 1 epoch: 0 min_stake: threshold: 1 @@ -55,4 +54,3 @@ time: slot_duration: '1.000000000' mempool: pubsub_topic: /logos-blockchain-ENV_PLACEHOLDER-VERSION_PLACEHOLDER/mempool/1.0.0 - diff --git a/deployment/ceremony/genesis/standalone/deployment-template.yaml b/deployment/ceremony/genesis/standalone/deployment-template.yaml index 2b37f4b79..d903633e0 100644 --- a/deployment/ceremony/genesis/standalone/deployment-template.yaml +++ b/deployment/ceremony/genesis/standalone/deployment-template.yaml @@ -31,7 +31,6 @@ cryptarchia: service_params: BN: inactivity_period: 2 - retention_period: 1 epoch: 0 min_stake: threshold: 1 diff --git a/deployment/ceremony/genesis/testnet/deployment-template.yaml b/deployment/ceremony/genesis/testnet/deployment-template.yaml index 7ae0a7c15..cc589186b 100644 --- a/deployment/ceremony/genesis/testnet/deployment-template.yaml +++ b/deployment/ceremony/genesis/testnet/deployment-template.yaml @@ -31,7 +31,6 @@ cryptarchia: service_params: BN: inactivity_period: 2 - retention_period: 1 epoch: 0 min_stake: threshold: 1 diff --git a/deployment/cfgsync/deployment-settings.yaml b/deployment/cfgsync/deployment-settings.yaml index 65b72dd41..d2a521982 100644 --- a/deployment/cfgsync/deployment-settings.yaml +++ b/deployment/cfgsync/deployment-settings.yaml @@ -31,7 +31,6 @@ cryptarchia: service_params: BN: inactivity_period: 2 - retention_period: 1 epoch: 0 min_stake: threshold: 1 diff --git a/ledger/src/config.rs b/ledger/src/config.rs index 2e91c34a6..75d5fe2ed 100644 --- a/ledger/src/config.rs +++ b/ledger/src/config.rs @@ -128,7 +128,6 @@ mod tests { ServiceType::BlendNetwork, ServiceParameters { inactivity_period: 2.try_into().unwrap(), - retention_period: 1.into(), epoch: 0.into(), }, )] @@ -181,7 +180,6 @@ mod tests { ServiceType::BlendNetwork, ServiceParameters { inactivity_period: 2.try_into().unwrap(), - retention_period: 1.into(), epoch: 0.into(), }, )] @@ -243,7 +241,6 @@ mod tests { ServiceType::BlendNetwork, ServiceParameters { inactivity_period: 2.try_into().unwrap(), - retention_period: 1.into(), epoch: 0.into(), }, )] diff --git a/ledger/src/cryptarchia/mod.rs b/ledger/src/cryptarchia/mod.rs index 3eed48aef..a4502a1c7 100644 --- a/ledger/src/cryptarchia/mod.rs +++ b/ledger/src/cryptarchia/mod.rs @@ -913,7 +913,6 @@ pub mod tests { ServiceType::BlendNetwork, ServiceParameters { inactivity_period: 2.try_into().unwrap(), - retention_period: 1.into(), epoch: 0.into(), }, ); @@ -1316,8 +1315,8 @@ pub mod tests { } /// A declaration that lapses past `inactivity_period` but has not yet - /// been garbage-collected must be filtered out of the `EpochState` - /// snapshot built at a later epoch. + /// been withdrawn must be filtered out of the `EpochState` snapshot + /// built at a later epoch. #[test] fn epoch_state_snapshot_excludes_inactive_declaration() { let leader_utxo = utxo(); @@ -1339,10 +1338,10 @@ pub mod tests { sdp_utxo_key, ); - // Advance to epoch 4 (one-by-one). + // Advance to epoch 5 (one-by-one). // With inactivity_period=2, the declaration goes inactive at epoch 5. - // With retention_period=1, GC fires at epoch 6. - // So, at epoch 5, the declaration is inactive yet still present in the ledger. + // It shouldn't be in the snapshot, but should still exist in the live SDP + // ledger because the user has not yet witdrawn it. let mut ledger = ledger0.clone(); let mut head = head0; for epoch in 1..=5u64 { @@ -1358,7 +1357,7 @@ pub mod tests { .sdp .get_declaration(&declare.id()) .is_some(), - "declaration must still be in the live SDP ledger before GC removes it" + "declaration must still be in the live SDP ledger because it is not yet withdrawn" ); assert!( declaration_in_snapshot(&ledger, &head, &declare.id()).is_none(), diff --git a/ledger/src/mantle/sdp/mod.rs b/ledger/src/mantle/sdp/mod.rs index 9dd31a462..45ebc5bd8 100644 --- a/ledger/src/mantle/sdp/mod.rs +++ b/ledger/src/mantle/sdp/mod.rs @@ -27,7 +27,6 @@ use lb_core::{ use lb_cryptarchia_engine::Epoch; use lb_key_management_system_keys::keys::{Ed25519Signature, ZkSignature}; use rewards::{Error as RewardsError, Rewards}; -use tracing::warn; use crate::{EpochState, UtxoTree, mantle::sdp::rewards::blend}; @@ -44,7 +43,7 @@ impl Service { last_epoch_state: &EpochState, epoch_state: &EpochState, locked_notes: &mut LockedNotes, - config: &ServiceParameters, + config: ServiceParameters, rewards_params: &ServiceRewardsParameters, ) -> (Self, Vec, Vec) { match self { @@ -173,26 +172,22 @@ impl ServiceState { last_epoch_state: &EpochState, epoch_state: &EpochState, locked_notes: &mut LockedNotes, - service_params: &ServiceParameters, + service_params: ServiceParameters, rewards_params: &R::Params, ) -> (Self, Vec, Vec) { let mut reward_utxos = Vec::new(); let mut events = Vec::new(); if last_epoch_state.epoch() < epoch_state.epoch() { - // Unlock notes from withdrawn declarations if possible events.extend( - self.unlock_notes_from_withdrawn_declarations(locked_notes, epoch_state.epoch()), + self.unlock_and_remove_withdrawn_declarations(locked_notes, epoch_state.epoch()), ); - // Garbage collect declarations - self.gc_declarations(epoch_state.epoch(), service_params); - // Update and distribute rewards (self.rewards, reward_utxos) = self.rewards.update_epoch( last_epoch_state, epoch_state, - service_params, + &service_params, rewards_params, ); } @@ -200,75 +195,50 @@ impl ServiceState { (self, reward_utxos, events) } - /// Unlock notes from withdrawn declarations whose withdrawn epoch has been - /// reached. + /// For every withdrawn declaration whose `withdrawn` epoch has been + /// reached, unlock the locked note and remove the declaration from the + /// set. /// /// Returns one [`HeaderEvent::SdpNoteUnlocked`] event per unlocked note. - fn unlock_notes_from_withdrawn_declarations( - &self, + fn unlock_and_remove_withdrawn_declarations( + &mut self, locked_notes: &mut LockedNotes, epoch: Epoch, ) -> Vec { let mut events = Vec::new(); - self.declarations.iter().for_each(|(declaration_id, declaration)| { - if let Some(withdraw_at) = declaration.withdraw_at - && epoch >= withdraw_at - && locked_notes - .is_locked_for_service(&declaration.locked_note_id, &declaration.service_type) - { - locked_notes - .unlock(declaration.service_type, &declaration.locked_note_id) - .expect("unlocking note from withdrawn declaraion must be successful if it hasn't been unlocked yet"); - events.push( - HeaderEvent::SdpNoteUnlocked { - note_id: declaration.locked_note_id, - service_type: declaration.service_type, - declaration_id: *declaration_id, - } - ); - } - }); - events - } - /// Garbage collect declarations that have been withdrawn or inactive, - /// if the retention period has passed. - fn gc_declarations(&mut self, epoch: Epoch, service_params: &ServiceParameters) { - let expired: Vec = self + // Collect IDs to remove first, and remove them in a second pass. + // `rpds` doesn't support `retain`, and we can't remove entries while iterating + // over them. + let to_remove: Vec = self .declarations .iter() - .filter(|(_id, declaration)| Self::is_expired(declaration, epoch, service_params)) - .map(|(id, declaration)| { - warn!( - ?declaration, - ?epoch, - ?service_params, - "removing an expired declaration" - ); - *id + .filter_map(|(id, declaration)| { + if epoch < declaration.withdraw_at? { + return None; + } + if locked_notes + .is_locked_for_service(&declaration.locked_note_id, &declaration.service_type) + { + locked_notes + .unlock(declaration.service_type, &declaration.locked_note_id) + .expect("unlocking note from withdrawn declaration must be successful if it hasn't been unlocked yet"); + events.push( + HeaderEvent::SdpNoteUnlocked { + note_id: declaration.locked_note_id, + service_type: declaration.service_type, + declaration_id: *id, + } + ); + } + Some(*id) }) .collect(); - for id in &expired { + for id in &to_remove { self.declarations.remove_mut(id); } - } - /// Returns true if the declaration has been withdrawn or inactive, - /// and if the retention period has passed. - fn is_expired( - declaration: &Declaration, - current_epoch: Epoch, - config: &ServiceParameters, - ) -> bool { - let withdrawn = declaration.withdraw_at.is_some_and(|withdraw_at| { - withdraw_at.strict_add(config.retention_period) < current_epoch - }); - let inactive = declaration - .active - .strict_add(config.inactivity_period.into_inner()) - .strict_add(config.retention_period) - < current_epoch; - withdrawn || inactive + events } fn add_income(&mut self, income: Value) { @@ -283,7 +253,7 @@ impl ServiceState { /// Returns true if the declaration is active at `current_epoch`: /// an activity message has been accepted within `inactivity_period` epochs, /// and its withdrawal (if any) has not yet taken effect. -fn is_active(declaration: &Declaration, current_epoch: Epoch, config: &ServiceParameters) -> bool { +fn is_active(declaration: &Declaration, current_epoch: Epoch, config: ServiceParameters) -> bool { declaration .active .strict_add(config.inactivity_period.into_inner()) @@ -382,7 +352,7 @@ impl SdpLedger { last_epoch_state, epoch_state, &mut locked_notes, - service_params, + *service_params, &config.service_rewards_params, ); all_reward_utxos.extend(reward_utxos); @@ -603,7 +573,7 @@ impl SdpLedger { let entries: HashMap = service .declarations() .iter() - .filter(|(_, declaration)| is_active(declaration, epoch, params)) + .filter(|(_, declaration)| is_active(declaration, epoch, *params)) .map(|(declaration_id, declaration)| (*declaration_id, declaration.clone())) .collect(); if entries.is_empty() { @@ -800,16 +770,24 @@ mod tests { } } + fn epoch_snapshot_contains( + decl_id: &DeclarationId, + epoch: Epoch, + ledger: &SdpLedger, + config: &Config, + ) -> bool { + ledger + .active_declarations(epoch, &config.service_params) + .for_service(&ServiceType::BlendNetwork) + .is_some_and(|m| m.contains_key(decl_id)) + } + /// `active_declarations` must drop entries that have gone inactive (i.e., - /// `active + inactivity_period < snapshot_epoch`) even if they have not - /// been garbage-collected yet. + /// `active + inactivity_period < snapshot_epoch`). #[test] fn active_declarations_filters_out_inactive() { - // Long retention so GC never runs in the window we test, short - // inactivity so the declaration goes inactive quickly. let config = setup(ServiceParameters { inactivity_period: 2.try_into().unwrap(), - retention_period: 100.into(), epoch: 0.into(), }); @@ -845,9 +823,8 @@ mod tests { ) .unwrap(); - // Advance to epoch 6 without an activity message; GC won't fire - // (retention=100), but the declaration is inactive past epoch 5 - // (active=3, inactivity=2 -> 3+2 < 6). + // Advance to epoch 6 without an activity message. The declaration is + // inactive past epoch 5 (active=3, inactivity=2 -> 3+2 < 6). let mut ledger = ledger; for epoch in 2..=6 { let new_epoch_state = next_epoch_state(epoch.into(), &ledger, &config); @@ -857,15 +834,17 @@ mod tests { last_epoch_state = new_epoch_state; } - // The declaration is still present in the live ledger (no GC) + // The declaration is still present in the live ledger ... assert!(ledger.get_declaration(&declaration_id).is_some()); - // but active_declarations at epoch 6 must filter it out. + // ... but active_declarations at epoch 6 must filter it out. assert!( - ledger - .active_declarations(6.into(), &config.service_params) - .for_service(&ServiceType::BlendNetwork) - .is_none_or(|m| !m.contains_key(&declaration_id)), - "inactive declaration must be excluded from the active-declarations snapshot" + !epoch_snapshot_contains(&declaration_id, 6.into(), &ledger, &config), + "inactive declaration must be excluded from the epoch-6 active snapshot" + ); + // whereas active_declarations at epoch 5 must include it. + assert!( + epoch_snapshot_contains(&declaration_id, 5.into(), &ledger, &config), + "declaration must be included in the epoch-5 active snapshot" ); } @@ -876,11 +855,10 @@ mod tests { fn active_declarations_includes_genesis_at_epochs_0_and_1() { let config = setup(ServiceParameters { inactivity_period: 2.try_into().unwrap(), - retention_period: 1.into(), epoch: 0.into(), }); - // Build an SDP ledger with an declaration at epoch 0. + // Build an SDP ledger with a declaration at epoch 0. let ledger = dummy_sdp_ledger(0.into(), &config); let (_utxo_sk, utxo) = utxo_with_sk(); let signing_key = create_signing_key(); @@ -909,10 +887,7 @@ mod tests { // At epoch 0 and 1, the declaration must be included in the active set. for epoch in [0u32, 1] { assert!( - ledger - .active_declarations(epoch.into(), &config.service_params) - .for_service(&ServiceType::BlendNetwork) - .is_some_and(|m| m.contains_key(&declaration_id)), + epoch_snapshot_contains(&declaration_id, epoch.into(), &ledger, &config), "genesis declaration must be active at epoch {epoch}" ); } @@ -923,11 +898,10 @@ mod tests { /// the declaration is still present in the live SDP ledger. #[test] fn active_declarations_filters_out_withdrawn_at_effective_epoch() { - // Long inactivity/retention so the only filter that fires in this test + // Long inactivity epoch so the only filter that fires in this test // is the withdrawn-effective-epoch check. let config = setup(ServiceParameters { inactivity_period: 100.try_into().unwrap(), - retention_period: 100.into(), epoch: 0.into(), }); @@ -986,10 +960,7 @@ mod tests { // include the declaration. for epoch in 0..withdraw_at.into_inner() { assert!( - ledger - .active_declarations(epoch.into(), &config.service_params) - .for_service(&ServiceType::BlendNetwork) - .is_some_and(|m| m.contains_key(&declaration_id)), + epoch_snapshot_contains(&declaration_id, epoch.into(), &ledger, &config), "withdrawn-but-not-yet-effective declaration must be active at epoch {epoch}" ); } @@ -997,24 +968,19 @@ mod tests { // Snapshot at `withdrawn_epoch` (and beyond) must exclude it. for epoch in withdraw_at.into_inner()..=withdraw_at.into_inner() + 2 { assert!( - ledger - .active_declarations(epoch.into(), &config.service_params) - .for_service(&ServiceType::BlendNetwork) - .is_none_or(|m| !m.contains_key(&declaration_id)), + !epoch_snapshot_contains(&declaration_id, epoch.into(), &ledger, &config), "withdrawn declaration must be excluded from the snapshot at epoch {epoch}" ); } } - /// A provider that hasn't submit a new active message during - /// `inactivity_period + retention_period` epochs must be removed. + /// A provider's `active` field is refreshed when it submits an activity + /// message, and the declaration persists across epochs regardless of how + /// long it has been inactive. #[test] - fn gc_inactive_declaration() { + fn active_message_refreshes_declaration() { let config = setup(ServiceParameters { - // Set inactivity/retention periods very short to check that - // declaration is NOT removed before an activity message is submitted. inactivity_period: 2.try_into().unwrap(), - retention_period: 1.into(), epoch: 0.into(), }); @@ -1061,7 +1027,10 @@ mod tests { (ledger, _) = ledger.try_apply_header(&config, &epoch3, &epoch4).unwrap(); // Check that the declaration is still present. let declarations = ledger.get_declarations(ServiceType::BlendNetwork).unwrap(); - assert!(declarations.contains_key(&declaration_id)); + assert_eq!( + declarations.get(&declaration_id).unwrap().active, + Epoch::new(3) + ); // Submit an activity message at epoch 4 let active_op = SDPActiveOp { @@ -1075,46 +1044,43 @@ mod tests { ))), }; let mut ledger = apply_active_with_dummies(ledger, &active_op, zk_key, &config).unwrap(); - let declaration = ledger.get_declarations(ServiceType::BlendNetwork).unwrap(); + let declarations = ledger.get_declarations(ServiceType::BlendNetwork).unwrap(); assert_eq!( - declaration.get(&declaration_id).unwrap().active, + declarations.get(&declaration_id).unwrap().active, Epoch::new(4) // epoch when the activity message is submitted/accepted ); - // Move forward to the epoch 7. The declaration should be still present - // because the activity message was accepted at epoch 4. + // Move forward to the epoch 7 where declaration will become inactive + // (active=4, inactivity=2 -> 4+2 < 7). let epoch5 = next_epoch_state(5.into(), &ledger, &config); (ledger, _) = ledger.try_apply_header(&config, &epoch4, &epoch5).unwrap(); let epoch6 = next_epoch_state(6.into(), &ledger, &config); (ledger, _) = ledger.try_apply_header(&config, &epoch5, &epoch6).unwrap(); let epoch7 = next_epoch_state(7.into(), &ledger, &config); (ledger, _) = ledger.try_apply_header(&config, &epoch6, &epoch7).unwrap(); + // Nevertheless, the declaration should be still present because no withdraw + // message was submitted. let declarations = ledger.get_declarations(ServiceType::BlendNetwork).unwrap(); - assert!(declarations.contains_key(&declaration_id)); - - // Before moving to epoch 8 where declaration will be removed, - // applying another header within the same epoch 7 must be a no-op - // (GC and unlock are gated to epoch transitions only). - let ledger_before = ledger.clone(); - (ledger, _) = ledger.try_apply_header(&config, &epoch7, &epoch7).unwrap(); assert_eq!( - ledger, ledger_before, - "within-epoch try_apply_header must not change ledger state" + declarations.get(&declaration_id).unwrap().active, + Epoch::new(4) // not changed + ); + // but active_declarations at epoch 7 must filter it out. + assert!( + !epoch_snapshot_contains(&declaration_id, 7.into(), &ledger, &config), + "inactive declaration must be excluded from the epoch-7 active snapshot" + ); + // whereas active_declarations at epoch 6 must include it. + assert!( + epoch_snapshot_contains(&declaration_id, 6.into(), &ledger, &config), + "declaration must be included in the epoch-6 active snapshot" ); - - // Move forward to epoch 8 where declaration should be removed - // because no activity message has been submitted since epoch 4 - let epoch8 = next_epoch_state(8.into(), &ledger, &config); - (ledger, _) = ledger.try_apply_header(&config, &epoch7, &epoch8).unwrap(); - let declarations = ledger.get_declarations(ServiceType::BlendNetwork).unwrap(); - assert!(!declarations.contains_key(&declaration_id)); } #[test] fn rewards_distributed_to_active_provider() { let config = setup(ServiceParameters { inactivity_period: 2.try_into().unwrap(), - retention_period: 100.into(), epoch: 0.into(), }); @@ -1256,11 +1222,7 @@ mod tests { #[test] fn test_withdraw_provider() { let config = setup(ServiceParameters { - // inactivity/retention periods should be long enough - // for this test to avoid the declaration being removed due to - // inacitivity before we can test the withdraw logic. inactivity_period: 20.try_into().unwrap(), - retention_period: 20.into(), epoch: 0.into(), }); @@ -1300,18 +1262,17 @@ mod tests { let sdp_ledger = apply_withdraw_with_dummies(sdp_ledger, withdraw_op, utxo_sk, zk_key, &config).unwrap(); - let withdrawn_epoch = sdp_ledger.get_declaration(&declaration_id) - .expect("declaration must still exist even after withdrawal because GC shouldn't remove it immediately") + let withdraw_epoch = sdp_ledger + .get_declaration(&declaration_id) + .expect("declaration must still exist until the withdrawn epoch is reached") .withdraw_at .expect("withdraw_at must be set after withdraw tx is accepted"); - // Move forward epochs until withdrawn_epoch is reached, - // and check that the note has been unlocked. The unlock event - // must fire exactly once — on the epoch reaching `withdrawn_epoch`, - // and never earlier. + // Move forward to the epoch just before the withdrawn epoch. + // The declaration must still be present and the note still locked. let mut sdp_ledger = sdp_ledger; let mut last_epoch_state = epoch0; - for epoch in 1..=withdrawn_epoch.into_inner() { + for epoch in 1..withdraw_epoch.into_inner() { let new_epoch_state = next_epoch_state(epoch.into(), &sdp_ledger, &config); let events; (sdp_ledger, HeaderEffect { events, .. }) = sdp_ledger @@ -1326,56 +1287,46 @@ mod tests { (*unlocked_note == note_id && *service_type == service_a && *id == declaration_id) .then_some(event) }); - if epoch == withdrawn_epoch.into_inner() { - assert_eq!(unlock_events.count(), 1); - } else { - assert_eq!(unlock_events.count(), 0); - } + assert_eq!(unlock_events.count(), 0); last_epoch_state = new_epoch_state; } assert!( sdp_ledger.get_declaration(&declaration_id).is_some(), - "declaration must still exist because GC shouldn't remove it until snapshot_finalization + retention_period has passed" + "declaration must still exist before the withdrawn epoch is reached" + ); + assert!( + sdp_ledger + .locked_notes() + .is_locked_for_service(&declare_op.locked_note_id, &ServiceType::BlendNetwork), + "the provider's note must still be locked before the withdrawn epoch is reached" + ); + + // Move forward to the withdrawn epoch. The declaration must be removed + // and the note must be unlocked. + let new_epoch_state = next_epoch_state(withdraw_epoch, &sdp_ledger, &config); + let events; + (sdp_ledger, HeaderEffect { events, .. }) = sdp_ledger + .try_apply_header(&config, &last_epoch_state, &new_epoch_state) + .unwrap(); + let unlock_events = events.into_iter().filter_map(|event| { + let HeaderEvent::SdpNoteUnlocked { + note_id: unlocked_note, + service_type, + declaration_id: id, + } = &event; + (*unlocked_note == note_id && *service_type == service_a && *id == declaration_id) + .then_some(event) + }); + assert_eq!(unlock_events.count(), 1); + assert!( + sdp_ledger.get_declaration(&declaration_id).is_none(), + "declaration must be removed at the withdrawn epoch" ); assert!( !sdp_ledger .locked_notes() .is_locked_for_service(&declare_op.locked_note_id, &ServiceType::BlendNetwork), - "the provider's note must be unlocked once withdrawn_epoch is reached" - ); - - // Move forward epochs just before the `snapshot_finalization + - // retention_period` has elapsed, and check that the declaration hasn't - // been removed yet (boundary check). - let retention_period = config - .service_params - .get(&ServiceType::BlendNetwork) - .unwrap() - .retention_period; - let target_epoch = withdrawn_epoch - .strict_add(retention_period) - .strict_add(Epoch::new(1)); - for epoch in (withdrawn_epoch.into_inner() + 1)..target_epoch.into_inner() { - let new_epoch_state = next_epoch_state(epoch.into(), &sdp_ledger, &config); - (sdp_ledger, _) = sdp_ledger - .try_apply_header(&config, &last_epoch_state, &new_epoch_state) - .unwrap(); - last_epoch_state = new_epoch_state; - } - assert!( - sdp_ledger.get_declaration(&declaration_id).is_some(), - "declaration must still exist because GC shouldn't remove it until snapshot_finalization + retention_period has passed" - ); - - // Move forward one more epoch. Now, `snapshot_finalization + retention_period` - // has passed. Check that the declaration has been removed. - let new_epoch_state = next_epoch_state(target_epoch, &sdp_ledger, &config); - (sdp_ledger, _) = sdp_ledger - .try_apply_header(&config, &last_epoch_state, &new_epoch_state) - .unwrap(); - assert!( - sdp_ledger.get_declaration(&declaration_id).is_none(), - "declaration should have been removed" + "the provider's note must be unlocked at the withdrawn epoch" ); } } diff --git a/ledger/src/mantle/sdp/rewards/test_utils.rs b/ledger/src/mantle/sdp/rewards/test_utils.rs index 4b05088f0..78fcf8c3b 100644 --- a/ledger/src/mantle/sdp/rewards/test_utils.rs +++ b/ledger/src/mantle/sdp/rewards/test_utils.rs @@ -74,7 +74,6 @@ pub fn create_provider_id(byte: u8) -> ProviderId { pub fn create_service_parameters() -> ServiceParameters { ServiceParameters { inactivity_period: 2.try_into().unwrap(), - retention_period: 1.into(), epoch: 0.into(), } } diff --git a/nodes/node/binary/src/config/cryptarchia/deployment.rs b/nodes/node/binary/src/config/cryptarchia/deployment.rs index 00de98135..c9474d8a1 100644 --- a/nodes/node/binary/src/config/cryptarchia/deployment.rs +++ b/nodes/node/binary/src/config/cryptarchia/deployment.rs @@ -4,7 +4,7 @@ use std::collections::HashMap; use lb_chain_service::Epoch; use lb_core::{ block::genesis::GenesisBlock, - sdp::{InactivityPeriod, MinStake, NumberOfEpochs, ServiceType}, + sdp::{InactivityPeriod, MinStake, ServiceType}, }; use lb_cryptarchia_engine::{ Config as ConsensusConfig, average_slots_for_blocks, base_period_length, time::epoch_length, @@ -88,6 +88,5 @@ pub struct SdpConfig { #[derive(Serialize, Deserialize, Debug, Clone)] pub struct ServiceParameters { pub inactivity_period: InactivityPeriod, - pub retention_period: NumberOfEpochs, pub epoch: Epoch, } diff --git a/nodes/node/binary/src/config/cryptarchia/mod.rs b/nodes/node/binary/src/config/cryptarchia/mod.rs index 3f34ff5ae..e7bd22815 100644 --- a/nodes/node/binary/src/config/cryptarchia/mod.rs +++ b/nodes/node/binary/src/config/cryptarchia/mod.rs @@ -61,7 +61,6 @@ impl ServiceConfig { service_type, ServiceParameters { inactivity_period: service_params.inactivity_period, - retention_period: service_params.retention_period, epoch: service_params.epoch, }, ) diff --git a/nodes/node/binary/src/config/deployment/settings.yaml b/nodes/node/binary/src/config/deployment/settings.yaml index e6952bbdd..44f5fb205 100644 --- a/nodes/node/binary/src/config/deployment/settings.yaml +++ b/nodes/node/binary/src/config/deployment/settings.yaml @@ -31,7 +31,6 @@ cryptarchia: service_params: BN: inactivity_period: 2 - retention_period: 1 epoch: 0 min_stake: threshold: 1 diff --git a/nodes/node/standalone-deployment-config.yaml b/nodes/node/standalone-deployment-config.yaml index 7d307b849..9d0dd410d 100644 --- a/nodes/node/standalone-deployment-config.yaml +++ b/nodes/node/standalone-deployment-config.yaml @@ -31,7 +31,6 @@ cryptarchia: service_params: BN: inactivity_period: 2 - retention_period: 1 epoch: 0 min_stake: threshold: 1 diff --git a/services/chain/chain-leader/src/leadership.rs b/services/chain/chain-leader/src/leadership.rs index 18eb9989d..578aa119f 100644 --- a/services/chain/chain-leader/src/leadership.rs +++ b/services/chain/chain-leader/src/leadership.rs @@ -662,7 +662,6 @@ mod pol_tests { ServiceType::BlendNetwork, ServiceParameters { inactivity_period: 20.try_into().unwrap(), - retention_period: 100.into(), epoch: 0.into(), }, )] diff --git a/services/chain/chain-network/src/bootstrap/ibd.rs b/services/chain/chain-network/src/bootstrap/ibd.rs index f216e7ea1..7ed0c1593 100644 --- a/services/chain/chain-network/src/bootstrap/ibd.rs +++ b/services/chain/chain-network/src/bootstrap/ibd.rs @@ -1148,7 +1148,6 @@ mod tests { ServiceType::BlendNetwork, ServiceParameters { inactivity_period: 20.try_into().unwrap(), - retention_period: 100.into(), epoch: 0.into(), }, )] diff --git a/services/chain/chain-service/src/states.rs b/services/chain/chain-service/src/states.rs index 8615730da..66081949a 100644 --- a/services/chain/chain-service/src/states.rs +++ b/services/chain/chain-service/src/states.rs @@ -148,7 +148,6 @@ mod tests { ServiceType::BlendNetwork, ServiceParameters { inactivity_period: 20.try_into().unwrap(), - retention_period: 100.into(), epoch: 0.into(), }, )] @@ -281,7 +280,6 @@ mod tests { ServiceType::BlendNetwork, ServiceParameters { inactivity_period: 20.try_into().unwrap(), - retention_period: 100.into(), epoch: 0.into(), }, )] diff --git a/services/chain/chain-service/src/tests/mod.rs b/services/chain/chain-service/src/tests/mod.rs index cc248c6d8..e4668e210 100644 --- a/services/chain/chain-service/src/tests/mod.rs +++ b/services/chain/chain-service/src/tests/mod.rs @@ -344,7 +344,6 @@ pub fn ledger_config(security_param: NonZero) -> lb_ledger::Config { lb_core::sdp::ServiceType::BlendNetwork, ServiceParameters { inactivity_period: 2.try_into().unwrap(), - retention_period: 1.into(), epoch: 0.into(), }, ); diff --git a/tests/src/tests/mantle/sdp/activity.rs b/tests/src/tests/mantle/sdp/activity.rs index ab0108206..ba811ede7 100644 --- a/tests/src/tests/mantle/sdp/activity.rs +++ b/tests/src/tests/mantle/sdp/activity.rs @@ -29,11 +29,11 @@ const NODE_COUNT: usize = 2; /// End-to-end test for blend SDP activity proofs: /// /// 1. Spawn two validators with blend declarations in the genesis transaction. -/// 2. Wait long enough that declarations would be removed if no activity -/// message was submitted during `inactivity_period + retention_period` -/// epochs. -/// 3. Verify that both declarations are still present, proving that the nodes -/// automatically submitted valid activity messages that the ledger accepted. +/// 2. Wait past `inactivity_period` so that any activity messages produced by +/// the nodes have to refresh the `active` field on the declarations. +/// 3. Verify that each declaration's `active` epoch has advanced past its +/// initial value, proving that the nodes automatically submitted valid +/// activity messages that the ledger accepted. #[tokio::test] async fn sdp_blend_activity() { let slots_per_epoch = Arc::new(AtomicU64::new(0)); @@ -67,23 +67,22 @@ async fn sdp_blend_activity() { declarations.len() ); - // Wait past the point where declarations would be removed if no activity - // proofs were submitted. + // Wait past `inactivity_period` so any activity messages produced by the + // nodes have to refresh the `active` field on the declarations. let initial_active_epoch = declarations.values().next().unwrap().active; - let survival_epochs = initial_active_epoch + let target_epoch = initial_active_epoch .strict_add(INACTIVITY_PERIOD) - .strict_add(RETENTION_PERIOD) .strict_add(Epoch::new(2)); // +1 margin - let survival_slots = Slot::new(u64::from(u32::from(survival_epochs)) * slots_per_epoch); + let target_slot = Slot::new(u64::from(u32::from(target_epoch)) * slots_per_epoch); wait_for_nodes_tip_slot( &[&node0.client, &node1.client], - survival_slots, + target_slot, Duration::from_secs(500), ) .await; - // Declarations must still be present — this proves that activity messages were - // submitted/accepted, keeping the declarations alive. + // Each declaration's `active` epoch must have advanced past its initial + // value, proving activity messages were submitted and accepted. let declarations_after = wait_for_declarations(&node0.client, Duration::from_secs(30)).await; // Check if at least one declaration is still present because blocks may have @@ -105,7 +104,6 @@ async fn sdp_blend_activity() { } const INACTIVITY_PERIOD: NumberOfEpochs = NumberOfEpochs::new(2); -const RETENTION_PERIOD: NumberOfEpochs = NumberOfEpochs::new(1); fn test_config(mut config: RunConfig, slots_per_epoch: &AtomicU64) -> RunConfig { config.deployment.time.slot_duration = Duration::from_secs(1); @@ -127,8 +125,8 @@ fn test_config(mut config: RunConfig, slots_per_epoch: &AtomicU64) -> RunConfig Ordering::Relaxed, ); - // Set small inactivity/retention periods so that declarations are removed - // quickly if no activity proofs are submitted. + // Set a small inactivity period so the inactivity window is short enough + // for the test to observe `active` being refreshed quickly. let blend_params = config .deployment .cryptarchia @@ -137,7 +135,6 @@ fn test_config(mut config: RunConfig, slots_per_epoch: &AtomicU64) -> RunConfig .get_mut(&ServiceType::BlendNetwork) .expect("blend network params should exist"); blend_params.inactivity_period = INACTIVITY_PERIOD.try_into().unwrap(); - blend_params.retention_period = RETENTION_PERIOD; // Shorten Blend delay to speed up the test config diff --git a/tests/src/tests/mantle/sdp/ops.rs b/tests/src/tests/mantle/sdp/ops.rs index af2b5335e..79bc99530 100644 --- a/tests/src/tests/mantle/sdp/ops.rs +++ b/tests/src/tests/mantle/sdp/ops.rs @@ -19,10 +19,7 @@ use lb_core::{ tx::{GasPrices, MantleTxGasContext}, tx_builder::MantleTxBuilder, }, - sdp::{ - Declaration, DeclarationMessage, Locator, NumberOfEpochs, ProviderId, ServiceType, - WithdrawMessage, - }, + sdp::{Declaration, DeclarationMessage, Locator, ProviderId, ServiceType, WithdrawMessage}, }; use lb_key_management_system_service::keys::{Ed25519Key, Ed25519Signature, ZkKey}; use lb_node::config::{ @@ -48,13 +45,11 @@ use num_bigint::BigUint; use testing_framework_core::scenario::{DynError, StartNodeOptions}; use tokio::time::{sleep, timeout}; -const RETENTION_PERIOD: NumberOfEpochs = NumberOfEpochs::new(1); - /// High-level SDP flow covered by this E2E: /// - submit a `Declare` transaction backed by an unused genesis note and wait /// for inclusion; -/// - submit a `Withdraw` transaction, wait for the finalization delay and the -/// retention period to pass, and check that the declaration disappears. +/// - submit a `Withdraw` transaction, wait for the finalization delay and check +/// that the declaration disappears. /// /// Note: Activity testing requires the blend service to generate real proofs, /// which happens automatically for nodes that are declared as blend providers. @@ -192,20 +187,19 @@ async fn sdp_ops_e2e() { let withdraw_epoch = get_declaration(&node0, &provider_id) .await .expect("API must succeed") - .expect("declaration must still exist even after withdrawal because GC shouldn't remove it immediately") + .expect("declaration must still exist until the snapshot finalization delay has passed") .withdraw_at .expect("withdraw_at must be set after withdraw tx is accepted"); - // Wait for the snapshot finalization delay and the retention period to pass. + // Wait for the snapshot finalization delay to pass. At the `withdrawn` + // epoch the locked note is unlocked and the declaration is removed. wait_for_tip_slot( &node0, - (u64::from((withdraw_epoch.strict_add(RETENTION_PERIOD).strict_add(Epoch::new(1))).into_inner()) - * slots_per_epoch) - .into(), + (u64::from(withdraw_epoch.strict_add(Epoch::new(1)).into_inner()) * slots_per_epoch).into(), Duration::from_mins(3), ) .await - .expect("timed out to wait until the snapshot finalization delay and the retention period pass after withdraw"); + .expect("timed out to wait until the snapshot finalization delay passes after withdraw"); // Check that the declaration has been removed assert!( @@ -449,7 +443,6 @@ fn patch_sdp_manual_cluster_config(mut config: RunConfig) -> RunConfig { .get_mut(&ServiceType::BlendNetwork) .expect("blend network params should exist"); service_params.inactivity_period = 10.try_into().unwrap(); - service_params.retention_period = RETENTION_PERIOD; config.deployment.blend.common.num_blend_layers = 1.try_into().unwrap(); config.deployment.blend.common.minimum_network_size = MinimumNetworkSize::try_new(2).unwrap(); diff --git a/tools/config/src/deployment.rs b/tools/config/src/deployment.rs index 0a984e92c..23a8749cd 100644 --- a/tools/config/src/deployment.rs +++ b/tools/config/src/deployment.rs @@ -54,7 +54,6 @@ const EPOCH_PERIOD_NONCE_BUFFER: u8 = 3; const EPOCH_PERIOD_NONCE_STABILIZATION: u8 = 4; const SDP_INACTIVITY_PERIOD: NumberOfEpochs = NumberOfEpochs::new(2); -const SDP_RETENTION_PERIOD: NumberOfEpochs = NumberOfEpochs::new(1); const SDP_EPOCH: Epoch = Epoch::new(0); const MIN_STAKE_THRESHOLD: u64 = 1; const MIN_STAKE_TIMESTAMP: u64 = 0; @@ -132,7 +131,6 @@ pub fn e2e_deployment_settings_with_genesis_block( ServiceType::BlendNetwork, ServiceParameters { inactivity_period: SDP_INACTIVITY_PERIOD.try_into().unwrap(), - retention_period: SDP_RETENTION_PERIOD, epoch: SDP_EPOCH, }, )] diff --git a/wallet/src/lib.rs b/wallet/src/lib.rs index c661e9304..8f96d3e88 100644 --- a/wallet/src/lib.rs +++ b/wallet/src/lib.rs @@ -1512,7 +1512,6 @@ mod tests { ServiceType::BlendNetwork, ServiceParameters { inactivity_period: 20.try_into().unwrap(), - retention_period: 100.into(), epoch: 0.into(), }, )]