From 8ac56ec1ca7913f1c95de7994bf1de01c33b0e71 Mon Sep 17 00:00:00 2001 From: nanocryk <6422796+nanocryk@users.noreply.github.com> Date: Wed, 8 Jan 2025 15:26:21 +0100 Subject: [PATCH 1/3] simplify slash process queue and process a page every block --- pallets/external-validator-slashes/src/lib.rs | 135 +++++++++--------- .../external-validator-slashes/src/mock.rs | 3 +- solo-chains/runtime/dancelight/src/lib.rs | 1 + 3 files changed, 70 insertions(+), 69 deletions(-) diff --git a/pallets/external-validator-slashes/src/lib.rs b/pallets/external-validator-slashes/src/lib.rs index da660b8ac..603768429 100644 --- a/pallets/external-validator-slashes/src/lib.rs +++ b/pallets/external-validator-slashes/src/lib.rs @@ -43,6 +43,7 @@ use { offence::{OffenceDetails, OnOffenceHandler}, EraIndex, SessionIndex, }, + sp_std::collections::vec_deque::VecDeque, sp_std::vec, sp_std::vec::Vec, tp_traits::{EraIndexProvider, InvulnerablesProvider, OnEraStart}, @@ -138,6 +139,9 @@ pub mod pallet { /// Provider to retrieve the current block timestamp. type TimestampProvider: Get; + /// How many queued slashes are being processed per block. + type QueuedSlashesProcessedPerBlock: Get; + /// The weight information of this pallet. type WeightInfo: WeightInfo; } @@ -197,8 +201,8 @@ pub mod pallet { #[pallet::storage] #[pallet::unbounded] #[pallet::getter(fn unreported_slashes)] - pub type UnreportedSlashes = - StorageValue<_, Vec>, ValueQuery>; + pub type UnreportedSlashesQueue = + StorageValue<_, VecDeque>, ValueQuery>; #[pallet::call] impl Pallet { @@ -339,6 +343,19 @@ pub mod pallet { Ok(()) } } + + #[pallet::hooks] + impl Hooks> for Pallet { + fn on_initialize(_n: BlockNumberFor) -> Weight { + let weight = Weight::zero(); + + Self::process_slashes_queue_page(); + + // TODO: Weight + + weight + } + } } /// This is intended to be used with `FilterHistoricalOffences`. @@ -494,87 +511,69 @@ impl OnEraStart for Pallet { } }); - Self::confirm_unconfirmed_slashes(era_index); + Self::add_era_slashes_to_queue(era_index); + + // we can process one queue page immediatly + Self::process_slashes_queue_page(); } } impl Pallet { - /// Apply previously-unapplied slashes on the beginning of a new era, after a delay. - /// In this case, we also send (or schedule for sending) slashes to ethereum - fn confirm_unconfirmed_slashes(active_era: EraIndex) { - const SLASH_PAGE_SIZE: usize = 20; + fn add_era_slashes_to_queue(active_era: EraIndex) { + let mut slashes: VecDeque<_> = Slashes::::take(&active_era).into(); - Slashes::::mutate(&active_era, |era_slashes| { - let unreported_slashes = UnreportedSlashes::::get(); + UnreportedSlashesQueue::::mutate(|queue| queue.append(&mut slashes)); + } - let free_slashing_space = SLASH_PAGE_SIZE.saturating_sub(unreported_slashes.len()); + fn process_slashes_queue_page() { + let mut slashes_to_send: Vec<_> = vec![]; + let era_index = T::EraIndexProvider::active_era().index; - let mut slashes_to_send: Vec<_> = vec![]; + // prepare up to QueuedSlashesProcessedPerBlock slashes to be sent + for _ in 0..(T::QueuedSlashesProcessedPerBlock::get() as usize) { + let Some(slash) = UnreportedSlashesQueue::::mutate(VecDeque::pop_front) else { + // no more slashes to process in the queue + break; + }; - for unreported_slash in unreported_slashes.iter() { - // TODO: check if validator.clone().encode() matches with the actual account bytes. - slashes_to_send.push(( - unreported_slash.validator.clone().encode(), - unreported_slash.percentage.deconstruct(), - )); - } + // TODO: check if validator.clone().encode() matches with the actual account bytes. + slashes_to_send.push(( + slash.validator.clone().encode(), + slash.percentage.deconstruct(), + )); + } - // TODO: optimize code logic - if era_slashes.len() > free_slashing_space { - let limit = era_slashes.len().saturating_div(free_slashing_space); + if slashes_to_send.is_empty() { + return; + } - let (slashes_to_include_send, slashes_to_unreport) = era_slashes.split_at(limit); + // Build command with slashes. + let command = Command::ReportSlashes { + // TODO: change this + timestamp: T::TimestampProvider::get(), + era_index, + slashes: slashes_to_send, + }; - for slash_to_include in slashes_to_include_send.iter() { - slashes_to_send.push(( - slash_to_include.validator.clone().encode(), - slash_to_include.percentage.deconstruct(), - )); - } - //print!("Unreported slashes appending {:?}", slashes_to_unreport); + let channel_id: ChannelId = snowbridge_core::PRIMARY_GOVERNANCE_CHANNEL; - UnreportedSlashes::::mutate(|unreported_slashes| { - unreported_slashes.append(&mut slashes_to_unreport.to_vec()); - }); - } else { - for slash in era_slashes { - slash.confirmed = true; - slashes_to_send.push(( - slash.validator.clone().encode(), - slash.percentage.deconstruct(), - )); + let outbound_message = Message { + id: None, + channel_id, + command, + }; + + // Validate and deliver the message + match T::ValidateMessage::validate(&outbound_message) { + Ok((ticket, _fee)) => { + if let Err(err) = T::OutboundQueue::deliver(ticket) { + log::error!(target: "ext_validators_slashes", "OutboundQueue delivery of message failed. {err:?}"); } } - - if slashes_to_send.len() > 0 { - let command = Command::ReportSlashes { - // TODO: change this - timestamp: T::TimestampProvider::get(), - era_index: active_era, - slashes: slashes_to_send, - }; - - let channel_id: ChannelId = snowbridge_core::PRIMARY_GOVERNANCE_CHANNEL; - - let outbound_message = Message { - id: None, - channel_id, - command, - }; - - // Validate and deliver the message - match T::ValidateMessage::validate(&outbound_message) { - Ok((ticket, _fee)) => { - if let Err(err) = T::OutboundQueue::deliver(ticket) { - log::error!(target: "ext_validators_slashes", "OutboundQueue delivery of message failed. {err:?}"); - } - } - Err(err) => { - log::error!(target: "ext_validators_slashes", "OutboundQueue validation of message failed. {err:?}"); - } - }; + Err(err) => { + log::error!(target: "ext_validators_slashes", "OutboundQueue validation of message failed. {err:?}"); } - }); + }; } } diff --git a/pallets/external-validator-slashes/src/mock.rs b/pallets/external-validator-slashes/src/mock.rs index 75e708917..115dd5324 100644 --- a/pallets/external-validator-slashes/src/mock.rs +++ b/pallets/external-validator-slashes/src/mock.rs @@ -18,7 +18,7 @@ use { crate as external_validator_slashes, frame_support::{ parameter_types, - traits::{ConstU16, ConstU64, Get, OnInitialize, OnFinalize}, + traits::{ConstU16, ConstU32, ConstU64, Get}, }, frame_system as system, snowbridge_core::outbound::{SendError, SendMessageFeeProvider}, @@ -255,6 +255,7 @@ impl external_validator_slashes::Config for Test { type ValidateMessage = (); type OutboundQueue = MockOkOutboundQueue; type TimestampProvider = TimestampProvider; + type QueuedSlashesProcessedPerBlock = ConstU32<20>; type WeightInfo = (); } diff --git a/solo-chains/runtime/dancelight/src/lib.rs b/solo-chains/runtime/dancelight/src/lib.rs index fc881780e..e8e467c4b 100644 --- a/solo-chains/runtime/dancelight/src/lib.rs +++ b/solo-chains/runtime/dancelight/src/lib.rs @@ -1385,6 +1385,7 @@ impl pallet_external_validator_slashes::Config for Runtime { type ValidateMessage = tp_bridge::MessageValidator; type OutboundQueue = tp_bridge::CustomSendMessage; type TimestampProvider = TimestampProvider; + type QueuedSlashesProcessedPerBlock = ConstU32<20>; type WeightInfo = weights::pallet_external_validator_slashes::SubstrateWeight; } From 3da7015fd9800e224efc2efd6ff38b8e057b1438 Mon Sep 17 00:00:00 2001 From: nanocryk <6422796+nanocryk@users.noreply.github.com> Date: Wed, 8 Jan 2025 15:38:54 +0100 Subject: [PATCH 2/3] on process page in on_initialize --- pallets/external-validator-slashes/src/lib.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/pallets/external-validator-slashes/src/lib.rs b/pallets/external-validator-slashes/src/lib.rs index 603768429..603bc4bb2 100644 --- a/pallets/external-validator-slashes/src/lib.rs +++ b/pallets/external-validator-slashes/src/lib.rs @@ -512,9 +512,6 @@ impl OnEraStart for Pallet { }); Self::add_era_slashes_to_queue(era_index); - - // we can process one queue page immediatly - Self::process_slashes_queue_page(); } } From 7afc8ee9774e4374ecdcbd9c2b9360521af95e36 Mon Sep 17 00:00:00 2001 From: nanocryk <6422796+nanocryk@users.noreply.github.com> Date: Wed, 8 Jan 2025 15:53:28 +0100 Subject: [PATCH 3/3] fix one test --- pallets/external-validator-slashes/src/mock.rs | 2 +- pallets/external-validator-slashes/src/tests.rs | 17 ++++++++--------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/pallets/external-validator-slashes/src/mock.rs b/pallets/external-validator-slashes/src/mock.rs index 115dd5324..d6d5f75ce 100644 --- a/pallets/external-validator-slashes/src/mock.rs +++ b/pallets/external-validator-slashes/src/mock.rs @@ -18,7 +18,7 @@ use { crate as external_validator_slashes, frame_support::{ parameter_types, - traits::{ConstU16, ConstU32, ConstU64, Get}, + traits::{ConstU16, ConstU32, ConstU64, Get, Hooks}, }, frame_system as system, snowbridge_core::outbound::{SendError, SendMessageFeeProvider}, diff --git a/pallets/external-validator-slashes/src/tests.rs b/pallets/external-validator-slashes/src/tests.rs index 30dc68a03..be756ef94 100644 --- a/pallets/external-validator-slashes/src/tests.rs +++ b/pallets/external-validator-slashes/src/tests.rs @@ -272,7 +272,6 @@ fn test_on_offence_defer_period_0() { }] ); start_era(2, 2); - // One on-initialize should be needed to dispatch messages roll_one_block(); assert_eq!(sent_ethereum_message_nonce(), 1); @@ -300,16 +299,16 @@ fn test_on_offence_defer_period_0_messages_get_queued() { assert_eq!(Slashes::::get(get_slashing_era(1)).len(), 25); start_era(2, 2); - assert_eq!(UnreportedSlashes::::get().len(), 25); + assert_eq!(UnreportedSlashesQueue::::get().len(), 25); // this triggers on_initialize roll_one_block(); assert_eq!(sent_ethereum_message_nonce(), 1); - assert_eq!(UnreportedSlashes::::get().len(), 5); + assert_eq!(UnreportedSlashesQueue::::get().len(), 5); roll_one_block(); assert_eq!(sent_ethereum_message_nonce(), 2); - assert_eq!(UnreportedSlashes::::get().len(), 0); + assert_eq!(UnreportedSlashesQueue::::get().len(), 0); }); } @@ -333,12 +332,12 @@ 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); - assert_eq!(UnreportedSlashes::::get().len(), 25); + assert_eq!(UnreportedSlashesQueue::::get().len(), 25); // this triggers on_initialize roll_one_block(); assert_eq!(sent_ethereum_message_nonce(), 1); - assert_eq!(UnreportedSlashes::::get().len(), 5); + assert_eq!(UnreportedSlashesQueue::::get().len(), 5); // We have 5 non-dispatched, which should accumulate // We shoulld have 30 after we initialie era 3 @@ -356,16 +355,16 @@ fn test_on_offence_defer_period_0_messages_get_queued_across_eras() { } start_era(3, 3); - assert_eq!(UnreportedSlashes::::get().len(), 30); + assert_eq!(UnreportedSlashesQueue::::get().len(), 30); // this triggers on_initialize roll_one_block(); - assert_eq!(UnreportedSlashes::::get().len(), 10); + assert_eq!(UnreportedSlashesQueue::::get().len(), 10); assert_eq!(sent_ethereum_message_nonce(), 2); // this triggers on_initialize roll_one_block(); - assert_eq!(UnreportedSlashes::::get().len(), 0); + assert_eq!(UnreportedSlashesQueue::::get().len(), 0); assert_eq!(sent_ethereum_message_nonce(), 3); });