From 3f6f964d93ec5d336d9249dee318ac2e3bccfe5e Mon Sep 17 00:00:00 2001 From: Gonza Montiel Date: Tue, 24 Mar 2026 12:16:45 +0100 Subject: [PATCH] Add slash retry coverage and weight updates. Cover the stored-era retry flow in tests and benchmarks, and update pallet and runtime weights for the new governance retry path. --- .../src/benchmarking.rs | 74 +++-- .../external-validator-slashes/src/tests.rs | 272 +++++++++++++++++- .../external-validator-slashes/src/weights.rs | 10 + .../pallet_external_validator_slashes.rs | 3 + .../pallet_external_validator_slashes.rs | 3 + .../pallet_external_validator_slashes.rs | 3 + 6 files changed, 330 insertions(+), 35 deletions(-) diff --git a/operator/pallets/external-validator-slashes/src/benchmarking.rs b/operator/pallets/external-validator-slashes/src/benchmarking.rs index 7e8d7a00..5575c914 100644 --- a/operator/pallets/external-validator-slashes/src/benchmarking.rs +++ b/operator/pallets/external-validator-slashes/src/benchmarking.rs @@ -35,20 +35,24 @@ const MAX_SLASHES: u32 = 1000; mod benchmarks { use super::*; + fn dummy_slash(slash_id: T::SlashId) -> Slash { + let dummy = || T::AccountId::decode(&mut TrailingZeroInput::zeroes()).unwrap(); + Slash { + validator: dummy(), + reporters: vec![], + slash_id, + percentage: Perbill::from_percent(1), + confirmed: false, + offence_kind: OffenceKind::LivenessOffence, + } + } + #[benchmark] fn cancel_deferred_slash(s: Linear<1, MAX_SLASHES>) -> Result<(), BenchmarkError> { let mut existing_slashes = Vec::new(); let era = T::EraIndexProvider::active_era().index; - let dummy = || T::AccountId::decode(&mut TrailingZeroInput::zeroes()).unwrap(); for _ in 0..MAX_SLASHES { - existing_slashes.push(Slash { - validator: dummy(), - reporters: vec![], - slash_id: One::one(), - percentage: Perbill::from_percent(1), - confirmed: false, - offence_kind: OffenceKind::LivenessOffence, - }); + existing_slashes.push(dummy_slash::(One::one())); } Slashes::::insert( era.saturating_add(T::SlashDeferDuration::get()) @@ -102,35 +106,55 @@ mod benchmarks { #[benchmark] fn process_slashes_queue(s: Linear<1, 200>) -> Result<(), BenchmarkError> { - let mut queue = VecDeque::new(); - let dummy = || T::AccountId::decode(&mut TrailingZeroInput::zeroes()).unwrap(); + let first_batch = (0..s) + .map(|_| dummy_slash::(One::one())) + .collect::>(); + let second_batch = vec![dummy_slash::(One::one())]; - for _ in 0..(s + 1) { - queue.push_back(Slash { - validator: dummy(), - reporters: vec![], - slash_id: One::one(), - percentage: Perbill::from_percent(1), - confirmed: false, - offence_kind: OffenceKind::LivenessOffence, - }); - } - - UnreportedSlashesQueue::::set(queue); + assert!(ExternalValidatorSlashes::::unsent_queue_push(( + 1, + first_batch + ))); + assert!(ExternalValidatorSlashes::::unsent_queue_push(( + 2, + second_batch + ))); let processed; #[block] { - processed = Pallet::::process_slashes_queue(s).unwrap(); + processed = match Pallet::::process_slashes_queue() { + crate::ProcessSlashesQueueOutcome::Sent(count) => count, + crate::ProcessSlashesQueueOutcome::Empty + | crate::ProcessSlashesQueueOutcome::Requeued(_) => { + return Err(BenchmarkError::Stop("unexpected slashes queue outcome")) + } + }; } - assert_eq!(UnreportedSlashesQueue::::get().len(), 1); + assert_eq!(ExternalValidatorSlashes::::unsent_queue_len(), 1); assert_eq!(processed, s); Ok(()) } + #[benchmark] + fn retry_unsent_slash_era() -> Result<(), BenchmarkError> { + let batch = vec![dummy_slash::(One::one())]; + assert!(ExternalValidatorSlashes::::unsent_queue_push((1, batch))); + + let origin = + T::GovernanceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; + + #[extrinsic_call] + _(origin as T::RuntimeOrigin, 1u32); + + assert!(ExternalValidatorSlashes::::unsent_queue_is_empty()); + + Ok(()) + } + #[benchmark] fn set_slashing_mode() -> Result<(), BenchmarkError> { #[extrinsic_call] diff --git a/operator/pallets/external-validator-slashes/src/tests.rs b/operator/pallets/external-validator-slashes/src/tests.rs index 126485b8..350af20f 100644 --- a/operator/pallets/external-validator-slashes/src/tests.rs +++ b/operator/pallets/external-validator-slashes/src/tests.rs @@ -28,6 +28,40 @@ use { sp_staking::offence::ReportOffence, }; +fn queued_slash_ids() -> Vec { + let mut queued = Vec::new(); + let mut slot = UnsentSlashHead::::get(); + let tail = UnsentSlashTail::::get(); + + while slot != tail { + if let Some((_, batch)) = UnsentSlashBatch::::get(slot) { + queued.extend(batch.into_iter().map(|slash| slash.slash_id)); + } + slot = (slot + 1) % UNSENT_QUEUE_CAPACITY; + } + + queued +} + +fn queued_batch_eras() -> Vec { + let mut queued = Vec::new(); + let mut slot = UnsentSlashHead::::get(); + let tail = UnsentSlashTail::::get(); + + while slot != tail { + if let Some((era, _)) = UnsentSlashBatch::::get(slot) { + queued.push(era); + } + slot = (slot + 1) % UNSENT_QUEUE_CAPACITY; + } + + queued +} + +fn unsent_queue_len() -> u32 { + ExternalValidatorSlashes::unsent_queue_len() +} + #[test] fn root_can_inject_manual_offence() { new_test_ext().execute_with(|| { @@ -574,14 +608,228 @@ fn test_on_offence_defer_period_0_messages_get_queued() { assert_eq!(Slashes::::get(get_slashing_era(1)).len(), 25); start_era(2, 2, 2); - assert_eq!(UnreportedSlashesQueue::::get().len(), 25); + assert_eq!(unsent_queue_len(), 2); + assert_eq!(queued_batch_eras(), vec![2, 2]); // this triggers on_initialize run_block(); - assert_eq!(UnreportedSlashesQueue::::get().len(), 5); + assert_eq!(unsent_queue_len(), 1); + assert_eq!(queued_slash_ids(), (20..25).collect::>()); run_block(); - assert_eq!(UnreportedSlashesQueue::::get().len(), 0); + assert!(ExternalValidatorSlashes::unsent_queue_is_empty()); + }); +} + +#[test] +fn failed_slashes_batch_is_moved_to_back_of_queue() { + new_test_ext().execute_with(|| { + crate::mock::DeferPeriodGetter::with_defer_period(0); + MockOkOutboundQueue::set_should_fail(true); + + start_era(0, 0, 0); + start_era(1, 1, 1); + + for i in 0..25 { + PendingOffenceKind::::insert(0, 3 + i, OffenceKind::LivenessOffence); + Pallet::::on_offence( + &[OffenceDetails { + offender: (3 + i, ()), + reporters: vec![], + }], + &[Perbill::from_percent(75)], + 0, + ); + } + + start_era(2, 2, 2); + assert_eq!(queued_slash_ids(), (0..25).collect::>()); + assert_eq!(queued_batch_eras(), vec![2, 2]); + + run_block(); + + assert_eq!(unsent_queue_len(), 2); + assert_eq!( + queued_slash_ids(), + (20..25).chain(0..20).collect::>() + ); + System::assert_has_event(RuntimeEvent::ExternalValidatorSlashes( + crate::Event::SlashesMessageSendFailed { era: 2, count: 20 }, + )); + }); +} + +#[test] +fn failed_slashes_batch_retries_after_send_is_reenabled() { + new_test_ext().execute_with(|| { + crate::mock::DeferPeriodGetter::with_defer_period(0); + MockOkOutboundQueue::set_should_fail(true); + + start_era(0, 0, 0); + start_era(1, 1, 1); + + for i in 0..25 { + PendingOffenceKind::::insert(0, 3 + i, OffenceKind::LivenessOffence); + Pallet::::on_offence( + &[OffenceDetails { + offender: (3 + i, ()), + reporters: vec![], + }], + &[Perbill::from_percent(75)], + 0, + ); + } + + start_era(2, 2, 2); + run_block(); + assert_eq!( + queued_slash_ids(), + (20..25).chain(0..20).collect::>() + ); + + start_era(3, 3, 3); + MockOkOutboundQueue::set_should_fail(false); + + run_block(); + assert_eq!(unsent_queue_len(), 1); + assert_eq!(queued_slash_ids(), (0..20).collect::>()); + assert_eq!(MockOkOutboundQueue::last_sent_slashes().len(), 5); + assert_eq!(MockOkOutboundQueue::last_built_era(), Some(2)); + System::assert_has_event(RuntimeEvent::ExternalValidatorSlashes( + crate::Event::SlashesMessageSent { + message_id: Default::default(), + }, + )); + + run_block(); + assert!(ExternalValidatorSlashes::unsent_queue_is_empty()); + }); +} + +#[test] +fn retry_extrinsic_succeeds_for_matching_era() { + new_test_ext().execute_with(|| { + crate::mock::DeferPeriodGetter::with_defer_period(0); + + start_era(0, 0, 0); + start_era(1, 1, 1); + + for i in 0..25 { + PendingOffenceKind::::insert(0, 3 + i, OffenceKind::LivenessOffence); + Pallet::::on_offence( + &[OffenceDetails { + offender: (3 + i, ()), + reporters: vec![], + }], + &[Perbill::from_percent(75)], + 0, + ); + } + + start_era(2, 2, 2); + start_era(5, 5, 5); + + assert_ok!(ExternalValidatorSlashes::retry_unsent_slash_era( + RuntimeOrigin::root(), + 2, + )); + + assert_eq!(unsent_queue_len(), 1); + assert_eq!(queued_slash_ids(), (20..25).collect::>()); + assert_eq!(MockOkOutboundQueue::last_built_era(), Some(2)); + }); +} + +#[test] +fn retry_extrinsic_errors_when_era_not_queued() { + new_test_ext().execute_with(|| { + assert_noop!( + ExternalValidatorSlashes::retry_unsent_slash_era(RuntimeOrigin::root(), 2), + Error::::EraNotInUnsentQueue + ); + }); +} + +#[test] +fn retry_extrinsic_requires_root() { + new_test_ext().execute_with(|| { + assert_noop!( + ExternalValidatorSlashes::retry_unsent_slash_era(RuntimeOrigin::signed(1), 2), + sp_runtime::DispatchError::BadOrigin + ); + }); +} + +#[test] +fn retry_extrinsic_preserves_failed_batch_when_send_still_fails() { + new_test_ext().execute_with(|| { + crate::mock::DeferPeriodGetter::with_defer_period(0); + MockOkOutboundQueue::set_should_fail(true); + + start_era(0, 0, 0); + start_era(1, 1, 1); + + for i in 0..25 { + PendingOffenceKind::::insert(0, 3 + i, OffenceKind::LivenessOffence); + Pallet::::on_offence( + &[OffenceDetails { + offender: (3 + i, ()), + reporters: vec![], + }], + &[Perbill::from_percent(75)], + 0, + ); + } + + start_era(2, 2, 2); + let before = queued_slash_ids(); + + assert_noop!( + ExternalValidatorSlashes::retry_unsent_slash_era(RuntimeOrigin::root(), 2), + Error::::MessageSendFailed + ); + + assert_eq!(queued_slash_ids(), before); + assert_eq!(unsent_queue_len(), 2); + }); +} + +#[test] +fn unsent_queue_full_emits_event() { + new_test_ext().execute_with(|| { + crate::mock::DeferPeriodGetter::with_defer_period(0); + + for i in 0..63u32 { + let slash = Slash { + validator: 1000 + i as u64, + reporters: vec![], + slash_id: i, + percentage: Perbill::from_percent(1), + confirmed: true, + offence_kind: OffenceKind::LivenessOffence, + }; + assert!(ExternalValidatorSlashes::unsent_queue_push(( + 1, + vec![slash] + ))); + } + + Slashes::::insert( + 2, + vec![Slash { + validator: 5000u64, + reporters: vec![], + slash_id: 999, + percentage: Perbill::from_percent(10), + confirmed: true, + offence_kind: OffenceKind::LivenessOffence, + }], + ); + + start_era(2, 2, 2); + + assert_eq!(unsent_queue_len(), 63); + assert_eq!(Slashes::::get(2).len(), 1); }); } @@ -628,14 +876,13 @@ fn test_on_offence_defer_period_0_messages_get_queued_across_eras() { } assert_eq!(Slashes::::get(get_slashing_era(1)).len(), 25); start_era(2, 2, 2); - assert_eq!(UnreportedSlashesQueue::::get().len(), 25); + assert_eq!(unsent_queue_len(), 2); // this triggers on_initialize run_block(); - assert_eq!(UnreportedSlashesQueue::::get().len(), 5); + assert_eq!(unsent_queue_len(), 1); + assert_eq!(queued_slash_ids(), (20..25).collect::>()); - // We have 5 non-dispatched, which should accumulate - // We shoulld have 30 after we initialie era 3 for i in 0..25 { PendingOffenceKind::::insert(2, 3 + i, OffenceKind::LivenessOffence); Pallet::::on_offence( @@ -651,15 +898,20 @@ fn test_on_offence_defer_period_0_messages_get_queued_across_eras() { } start_era(3, 3, 3); - assert_eq!(UnreportedSlashesQueue::::get().len(), 30); + assert_eq!(unsent_queue_len(), 3); + assert_eq!(queued_batch_eras(), vec![2, 3, 3]); // this triggers on_initialize run_block(); - assert_eq!(UnreportedSlashesQueue::::get().len(), 10); + assert_eq!(unsent_queue_len(), 2); + assert_eq!(queued_batch_eras(), vec![3, 3]); // this triggers on_initialize run_block(); - assert_eq!(UnreportedSlashesQueue::::get().len(), 0); + assert_eq!(unsent_queue_len(), 1); + + run_block(); + assert!(ExternalValidatorSlashes::unsent_queue_is_empty()); }); } diff --git a/operator/pallets/external-validator-slashes/src/weights.rs b/operator/pallets/external-validator-slashes/src/weights.rs index 011374bd..22971e79 100644 --- a/operator/pallets/external-validator-slashes/src/weights.rs +++ b/operator/pallets/external-validator-slashes/src/weights.rs @@ -57,6 +57,7 @@ pub trait WeightInfo { fn force_inject_slash() -> Weight; fn root_test_send_msg_to_eth() -> Weight; fn process_slashes_queue(s: u32, ) -> Weight; + fn retry_unsent_slash_era() -> Weight; fn set_slashing_mode() -> Weight; } @@ -136,6 +137,11 @@ impl WeightInfo for SubstrateWeight { .saturating_add(Weight::from_parts(0, 42).saturating_mul(s.into())) } + fn retry_unsent_slash_era() -> Weight { + // Same as the success path for one queued batch. + Self::process_slashes_queue(10) + } + fn set_slashing_mode() -> Weight { Weight::from_parts(7_402_000, 3601) .saturating_add(T::DbWeight::get().reads(1_u64)) @@ -221,6 +227,10 @@ impl WeightInfo for () { .saturating_add(Weight::from_parts(0, 42).saturating_mul(s.into())) } + fn retry_unsent_slash_era() -> Weight { + Self::process_slashes_queue(10) + } + fn set_slashing_mode() -> Weight { Weight::from_parts(7_402_000, 3601) .saturating_add(RocksDbWeight::get().reads(1_u64)) diff --git a/operator/runtime/mainnet/src/weights/pallet_external_validator_slashes.rs b/operator/runtime/mainnet/src/weights/pallet_external_validator_slashes.rs index deb5261f..bdb90ce5 100644 --- a/operator/runtime/mainnet/src/weights/pallet_external_validator_slashes.rs +++ b/operator/runtime/mainnet/src/weights/pallet_external_validator_slashes.rs @@ -113,6 +113,9 @@ impl pallet_external_validator_slashes::WeightInfo for .saturating_add(T::DbWeight::get().writes(4_u64)) .saturating_add(Weight::from_parts(0, 38).saturating_mul(s.into())) } + fn retry_unsent_slash_era() -> Weight { + Self::process_slashes_queue(10) + } /// Storage: `ExternalValidatorsSlashes::SlashingMode` (r:0 w:1) /// Proof: `ExternalValidatorsSlashes::SlashingMode` (`max_values`: Some(1), `max_size`: Some(1), added: 496, mode: `MaxEncodedLen`) fn set_slashing_mode() -> Weight { diff --git a/operator/runtime/stagenet/src/weights/pallet_external_validator_slashes.rs b/operator/runtime/stagenet/src/weights/pallet_external_validator_slashes.rs index cd06db35..279498d8 100644 --- a/operator/runtime/stagenet/src/weights/pallet_external_validator_slashes.rs +++ b/operator/runtime/stagenet/src/weights/pallet_external_validator_slashes.rs @@ -113,6 +113,9 @@ impl pallet_external_validator_slashes::WeightInfo for .saturating_add(T::DbWeight::get().writes(4_u64)) .saturating_add(Weight::from_parts(0, 38).saturating_mul(s.into())) } + fn retry_unsent_slash_era() -> Weight { + Self::process_slashes_queue(10) + } /// Storage: `ExternalValidatorsSlashes::SlashingMode` (r:0 w:1) /// Proof: `ExternalValidatorsSlashes::SlashingMode` (`max_values`: Some(1), `max_size`: Some(1), added: 496, mode: `MaxEncodedLen`) fn set_slashing_mode() -> Weight { diff --git a/operator/runtime/testnet/src/weights/pallet_external_validator_slashes.rs b/operator/runtime/testnet/src/weights/pallet_external_validator_slashes.rs index 85e6f04f..f40fc12f 100644 --- a/operator/runtime/testnet/src/weights/pallet_external_validator_slashes.rs +++ b/operator/runtime/testnet/src/weights/pallet_external_validator_slashes.rs @@ -113,6 +113,9 @@ impl pallet_external_validator_slashes::WeightInfo for .saturating_add(T::DbWeight::get().writes(4_u64)) .saturating_add(Weight::from_parts(0, 38).saturating_mul(s.into())) } + fn retry_unsent_slash_era() -> Weight { + Self::process_slashes_queue(10) + } /// Storage: `ExternalValidatorsSlashes::SlashingMode` (r:0 w:1) /// Proof: `ExternalValidatorsSlashes::SlashingMode` (`max_values`: Some(1), `max_size`: Some(1), added: 496, mode: `MaxEncodedLen`) fn set_slashing_mode() -> Weight {