feat: ✨ Bound execution of slashable providers loop in Proofs Dealer'…

…s `on_poll`

pallets/file-system/src/mock.rs

@@ -383,6 +383,7 @@ impl pallet_proofs_dealer::Config for Test {
     type BlockFullnessPeriod = ConstU64<10>;
     type BlockFullnessHeadroom = BlockFullnessHeadroom;
     type MinNotFullBlocksRatio = MinNotFullBlocksRatio;
+    type MaxSlashableProvidersPerTick = ConstU32<100>;
 }
 
 /// Structure to mock a verifier that returns `true` when `proof` is not empty

pallets/proofs-dealer/src/lib.rs

@@ -217,11 +217,22 @@ pub mod pallet {
         /// If less than this percentage of blocks are not full, the networks is considered to be presumably
         /// under a spam attack.
         /// This can also be thought of as the maximum ratio of misbehaving collators tolerated. For example,
-        /// if this is set to `Perbill::from_percent(50)`, then if more than half of the last `BlockFullnessPeriod`
+        /// if this is set to `Perbill::from_percent(50)`, then if more than half of the last [`Config::BlockFullnessPeriod`]
         /// blocks are not full, then one of those blocks surely was produced by an honest collator, meaning
-        /// that there was at least one truly _not_ full block in the last `BlockFullnessPeriod` blocks.
+        /// that there was at least one truly _not_ full block in the last [`Config::BlockFullnessPeriod`] blocks.
         #[pallet::constant]
         type MinNotFullBlocksRatio: Get<Perbill>;
+
+        /// The maximum number of Providers that can be slashed per tick.
+        ///
+        /// Providers are marked as slashable if they are found in the [`TickToProvidersDeadlines`] StorageMap
+        /// for the current challenges tick. It is expected that most of the times, there will be little to
+        /// no Providers in the [`TickToProvidersDeadlines`] StorageMap for the current challenges tick. That
+        /// is because Providers are expected to submit proofs in time. However, in the extreme scenario where
+        /// a large number of Providers are missing the proof submissions, this configuration is used to keep
+        /// the execution of the `on_poll` hook bounded.
+        #[pallet::constant]
+        type MaxSlashableProvidersPerTick: Get<u32>;
     }
 
     #[pallet::pallet]
@@ -378,6 +389,21 @@ pub mod pallet {
     #[pallet::getter(fn not_full_blocks_count)]
     pub type NotFullBlocksCount<T: Config> = StorageValue<_, BlockNumberFor<T>, ValueQuery>;
 
+    /// The tick to check and see if Providers failed to submit proofs before their deadline.
+    ///
+    /// In a normal situation, this should always be equal to [`ChallengesTicker`].
+    /// However, in the unlikely scenario where a large number of Providers fail to submit proofs (larger
+    /// than [`Config::MaxSlashableProvidersPerTick`]), and all of them had the same deadline, not all of
+    /// them will be marked as slashable. Only the first [`Config::MaxSlashableProvidersPerTick`] will be.
+    /// In that case, this stored tick will lag behind [`ChallengesTicker`].
+    ///
+    /// It is expected that this tick should catch up to [`ChallengesTicker`], as blocks with less
+    /// slashable Providers follow.
+    #[pallet::storage]
+    #[pallet::getter(fn last_tick_checked_for_missing_proofs)]
+    pub type TickToCheckedForSlashableProviders<T: Config> =
+        StorageValue<_, BlockNumberFor<T>, ValueQuery>;
+
     // Pallets use events to inform users when important changes are made.
     // https://docs.substrate.io/v3/runtime/events-and-errors
     #[pallet::event]

pallets/proofs-dealer/src/mock.rs

@@ -300,6 +300,7 @@ impl crate::Config for Test {
     type BlockFullnessPeriod = ConstU64<10>;
     type BlockFullnessHeadroom = BlockFullnessHeadroom;
     type MinNotFullBlocksRatio = MinNotFullBlocksRatio;
+    type MaxSlashableProvidersPerTick = ConstU32<100>;
 }
 
 /// Structure to mock a verifier that returns `true` when `proof` is not empty

pallets/proofs-dealer/src/types.rs

@@ -157,3 +157,6 @@ pub type BlockFullnessHeadroomFor<T> = <T as crate::Config>::BlockFullnessHeadro
 
 /// Syntactic sugar for MinNotFullBlocksRatio type used in the ProofsDealer pallet.
 pub type MinNotFullBlocksRatioFor<T> = <T as crate::Config>::MinNotFullBlocksRatio;
+
+/// Syntactic sugar for MaxSlashableProvidersPerTick type used in the ProofsDealer pallet.
+pub type MaxSlashableProvidersPerTickFor<T> = <T as crate::Config>::MaxSlashableProvidersPerTick;

pallets/proofs-dealer/src/utils.rs

@@ -32,14 +32,16 @@ use crate::{
         ChallengeTicksToleranceFor, ChallengesFeeFor, ChallengesQueueLengthFor,
         CheckpointChallengePeriodFor, ForestVerifierFor, ForestVerifierProofFor, KeyFor,
         KeyVerifierFor, KeyVerifierProofFor, MaxCustomChallengesPerBlockFor,
-        MaxSubmittersPerTickFor, MinChallengePeriodFor, Proof, ProviderIdFor, ProvidersPalletFor,
-        RandomChallengesPerBlockFor, RandomnessOutputFor, RandomnessProviderFor,
-        StakeToChallengePeriodFor, TargetTicksStorageOfSubmittersFor, TreasuryAccountFor,
+        MaxSlashableProvidersPerTickFor, MaxSubmittersPerTickFor, MinChallengePeriodFor, Proof,
+        ProviderIdFor, ProvidersPalletFor, RandomChallengesPerBlockFor, RandomnessOutputFor,
+        RandomnessProviderFor, StakeToChallengePeriodFor, TargetTicksStorageOfSubmittersFor,
+        TreasuryAccountFor,
     },
     ChallengesQueue, ChallengesTicker, ChallengesTickerPaused, Error, Event, LastCheckpointTick,
     LastDeletedTick, LastTickProviderSubmittedAProofFor, NotFullBlocksCount, Pallet,
     PastBlocksWeight, PriorityChallengesQueue, SlashableProviders, TickToChallengesSeed,
-    TickToCheckpointChallenges, TickToProvidersDeadlines, ValidProofSubmittersLastTicks,
+    TickToCheckedForSlashableProviders, TickToCheckpointChallenges, TickToProvidersDeadlines,
+    ValidProofSubmittersLastTicks,
 };
 
 macro_rules! expect_or_err {
@@ -439,7 +441,8 @@ where
 
         let last_checkpoint_tick = LastCheckpointTick::<T>::get();
 
-        // Count last checkpoint challenges tick challenges
+        // Count last checkpoint challenges tick challenges. This is to consider if slashable Providers should
+        // have responses to checkpoint challenges, and slash them for the corresponding number of missed challenges.
         let checkpoint_challenges_count =
             TickToCheckpointChallenges::<T>::get(last_checkpoint_tick)
                 .unwrap_or_else(||
@@ -457,104 +460,133 @@ where
         }
         weight.consume(T::DbWeight::get().reads_writes(2, 0));
 
-        // If there are providers left in `TickToProvidersDeadlines` for this tick,
-        // they are marked as slashable.
+        // If there are Providers left in `TickToProvidersDeadlines` for `TickToCheckedForSlashableProviders`,
+        // they will be marked as slashable.
+        let mut tick_to_check_for_slashable_providers =
+            TickToCheckedForSlashableProviders::<T>::get();
         let mut slashable_providers =
-            TickToProvidersDeadlines::<T>::drain_prefix(challenges_ticker);
-        while let Some((provider, _)) = slashable_providers.next() {
-            // One read for every provider in the prefix, and one write as we're consuming and deleting the entry.
-            weight.consume(T::DbWeight::get().reads_writes(1, 1));
-
-            // Accrue number of failed proof submission for this slashable provider.
-            // Add custom checkpoint challenges if the provider needed to respond to them.
-            SlashableProviders::<T>::mutate(provider, |slashable| {
-                let mut accrued = slashable.unwrap_or(0);
-
-                let last_tick_provider_submitted_proof =
-                    match LastTickProviderSubmittedAProofFor::<T>::get(provider) {
-                        Some(tick) => tick,
-                        None => {
-                            Self::deposit_event(Event::NoRecordOfLastSubmittedProof { provider });
-
-                            #[cfg(test)]
-                            unreachable!(
-                                "Provider should have a last tick it submitted a proof for."
-                            );
-
-                            #[allow(unreachable_code)]
-                            {
-                                // If the Provider has no record of the last tick it submitted a proof for,
-                                // we set it to the current challenges ticker, so they will not be slashed.
-                                challenges_ticker
+            TickToProvidersDeadlines::<T>::drain_prefix(tick_to_check_for_slashable_providers);
+
+        // This loop is expected to run for a low number of iterations, given that normally, there should
+        // be little to no Providers in the `TickToProvidersDeadlines` StorageMap for the `TickToCheckedForSlashableProviders`.
+        // However, in the extreme scenario where a large number of Providers are missing the proof submissions,
+        // this is bounded by the `MaxSlashableProvidersPerTick` configuration.
+        let max_slashable_providers = MaxSlashableProvidersPerTickFor::<T>::get();
+        let mut slashable_providers_count = 0;
+        while tick_to_check_for_slashable_providers <= challenges_ticker
+            && slashable_providers_count < max_slashable_providers
+        {
+            // If there are Providers left in `TickToProvidersDeadlines` for `TickToCheckedForSlashableProviders`,
+            // they are marked as slashable.
+            if let Some((provider, _)) = slashable_providers.next() {
+                // One read for every provider in the prefix, and one write as we're consuming and deleting the entry.
+                weight.consume(T::DbWeight::get().reads_writes(1, 1));
+
+                // Accrue number of failed proof submission for this slashable provider.
+                // Add custom checkpoint challenges if the provider needed to respond to them.
+                SlashableProviders::<T>::mutate(provider, |slashable| {
+                    let mut accrued = slashable.unwrap_or(0);
+
+                    let last_tick_provider_submitted_proof =
+                        match LastTickProviderSubmittedAProofFor::<T>::get(provider) {
+                            Some(tick) => tick,
+                            None => {
+                                Self::deposit_event(Event::NoRecordOfLastSubmittedProof {
+                                    provider,
+                                });
+
+                                #[cfg(test)]
+                                unreachable!(
+                                    "Provider should have a last tick it submitted a proof for."
+                                );
+
+                                #[allow(unreachable_code)]
+                                {
+                                    // If the Provider has no record of the last tick it submitted a proof for,
+                                    // we set it to the current challenges ticker, so they will not be slashed.
+                                    challenges_ticker
+                                }
                             }
-                        }
-                    };
-                weight.consume(T::DbWeight::get().reads_writes(1, 0));
-
-                let challenge_ticker_provider_should_have_responded_to =
-                    challenges_ticker.saturating_sub(T::ChallengeTicksTolerance::get());
-
-                if checkpoint_challenges_count != 0
-                    && last_tick_provider_submitted_proof <= last_checkpoint_tick
-                    && last_checkpoint_tick < challenge_ticker_provider_should_have_responded_to
-                {
-                    accrued = accrued.saturating_add(checkpoint_challenges_count as u32);
-                }
+                        };
+                    weight.consume(T::DbWeight::get().reads_writes(1, 0));
 
-                accrued = accrued.saturating_add(RandomChallengesPerBlockFor::<T>::get());
+                    let challenge_ticker_provider_should_have_responded_to =
+                        challenges_ticker.saturating_sub(T::ChallengeTicksTolerance::get());
 
-                *slashable = Some(accrued);
-            });
+                    if checkpoint_challenges_count != 0
+                        && last_tick_provider_submitted_proof <= last_checkpoint_tick
+                        && last_checkpoint_tick < challenge_ticker_provider_should_have_responded_to
+                    {
+                        accrued = accrued.saturating_add(checkpoint_challenges_count as u32);
+                    }
 
-            weight.consume(T::DbWeight::get().reads_writes(0, 1));
+                    accrued = accrued.saturating_add(RandomChallengesPerBlockFor::<T>::get());
 
-            // Get the stake for this Provider, to know its challenge period.
-            // If a submitter is a registered Provider, it must have a stake, so there shouldn't be an error.
-            let stake = match ProvidersPalletFor::<T>::get_stake(provider) {
-                Some(stake) => stake,
-                // But to avoid panics, in the odd case of a Provider not being registered, we
-                // arbitrarily set the stake to be that which would result in `CheckpointChallengePeriod` ticks of challenge period.
-                None => {
-                    weight.consume(T::DbWeight::get().reads_writes(1, 0));
-                    let checkpoint_challenge_period =
-                        CheckpointChallengePeriodFor::<T>::get().saturated_into::<u32>();
-                    StakeToChallengePeriodFor::<T>::get() * checkpoint_challenge_period.into()
-                }
-            };
-            weight.consume(T::DbWeight::get().reads_writes(1, 0));
+                    *slashable = Some(accrued);
+                });
 
-            // Calculate the next challenge deadline for this Provider.
-            // At this point, we are processing all providers who have reached their deadline (i.e. tolerance ticks after the tick they should provide a proof for):
-            // challenge_ticker = last_tick_provider_should_have_submitted_a_proof_for + ChallengeTicksTolerance
-            //
-            // By definition, the next deadline should be tolerance ticks after the next tick they should submit proof for (i.e. one period after the last tick they should have submitted a proof for):
-            // next_challenge_deadline = last_tick_provider_should_have_submitted_a_proof_for + provider_period + ChallengeTicksTolerance
-            //
-            // Therefore, the next deadline is one period from now:
-            // next_challenge_deadline = challenge_ticker + provider_period
-            let next_challenge_deadline =
-                challenges_ticker.saturating_add(Self::stake_to_challenge_period(stake));
-
-            // Update this Provider's next challenge deadline.
-            TickToProvidersDeadlines::<T>::set(next_challenge_deadline, provider, Some(()));
+                weight.consume(T::DbWeight::get().reads_writes(0, 1));
+
+                // Get the stake for this Provider, to know its challenge period.
+                // If a submitter is a registered Provider, it must have a stake, so there shouldn't be an error.
+                let stake = match ProvidersPalletFor::<T>::get_stake(provider) {
+                    Some(stake) => stake,
+                    // But to avoid panics, in the odd case of a Provider not being registered, we
+                    // arbitrarily set the stake to be that which would result in `CheckpointChallengePeriod` ticks of challenge period.
+                    None => {
+                        weight.consume(T::DbWeight::get().reads_writes(1, 0));
+                        let checkpoint_challenge_period =
+                            CheckpointChallengePeriodFor::<T>::get().saturated_into::<u32>();
+                        StakeToChallengePeriodFor::<T>::get() * checkpoint_challenge_period.into()
+                    }
+                };
+                weight.consume(T::DbWeight::get().reads_writes(1, 0));
 
-            weight.consume(T::DbWeight::get().reads_writes(0, 1));
+                // Calculate the next challenge deadline for this Provider.
+                // At this point, we are processing all providers who have reached their deadline (i.e. tolerance ticks after the tick they should provide a proof for):
+                // challenge_ticker = last_tick_provider_should_have_submitted_a_proof_for + ChallengeTicksTolerance
+                //
+                // By definition, the next deadline should be tolerance ticks after the next tick they should submit proof for (i.e. one period after the last tick they should have submitted a proof for):
+                // next_challenge_deadline = last_tick_provider_should_have_submitted_a_proof_for + provider_period + ChallengeTicksTolerance
+                //
+                // Therefore, the next deadline is one period from now:
+                // next_challenge_deadline = challenge_ticker + provider_period
+                let next_challenge_deadline =
+                    challenges_ticker.saturating_add(Self::stake_to_challenge_period(stake));
+
+                // Update this Provider's next challenge deadline.
+                TickToProvidersDeadlines::<T>::set(next_challenge_deadline, provider, Some(()));
+
+                weight.consume(T::DbWeight::get().reads_writes(0, 1));
+
+                // Calculate the tick for which the Provider should have submitted a proof.
+                let last_interval_tick =
+                    challenges_ticker.saturating_sub(T::ChallengeTicksTolerance::get());
+                weight.consume(T::DbWeight::get().reads_writes(1, 0));
 
-            // Calculate the tick for which the Provider should have submitted a proof.
-            let last_interval_tick =
-                challenges_ticker.saturating_sub(T::ChallengeTicksTolerance::get());
-            weight.consume(T::DbWeight::get().reads_writes(1, 0));
+                // Update this Provider's last interval tick for the next challenge.
+                LastTickProviderSubmittedAProofFor::<T>::set(provider, Some(last_interval_tick));
+                weight.consume(T::DbWeight::get().reads_writes(0, 1));
 
-            // Update this Provider's last interval tick for the next challenge.
-            LastTickProviderSubmittedAProofFor::<T>::set(provider, Some(last_interval_tick));
-            weight.consume(T::DbWeight::get().reads_writes(0, 1));
+                // Emit slashable provider event.
+                Self::deposit_event(Event::SlashableProvider {
+                    provider,
+                    next_challenge_deadline,
+                });
 
-            // Emit slashable provider event.
-            Self::deposit_event(Event::SlashableProvider {
-                provider,
-                next_challenge_deadline,
-            });
+                // Increment the number of slashable providers.
+                slashable_providers_count += 1;
+            } else {
+                // If there are no more Providers left in `TickToProvidersDeadlines` for `TickToCheckedForSlashableProviders`,
+                // we increment `TickToCheckedForSlashableProviders` to the next tick. If in doing so, `TickToCheckedForSlashableProviders`
+                // goes beyond `ChallengesTicker`, this loop will exit, leaving everything ready for the next tick.
+                tick_to_check_for_slashable_providers =
+                    tick_to_check_for_slashable_providers.saturating_add(One::one());
+            }
         }
+
+        // Update `TickToCheckedForSlashableProviders` to the value resulting from the last iteration of the loop.
+        TickToCheckedForSlashableProviders::<T>::set(tick_to_check_for_slashable_providers);
     }
 
     /// Check if the network is presumably under a spam attack.

pallets/providers/src/mock.rs

@@ -146,6 +146,7 @@ impl pallet_proofs_dealer::Config for Test {
     type BlockFullnessPeriod = ConstU64<10>;
     type BlockFullnessHeadroom = BlockFullnessHeadroom;
     type MinNotFullBlocksRatio = MinNotFullBlocksRatio;
+    type MaxSlashableProvidersPerTick = ConstU32<100>;
 }
 
 // Converter from the Balance type to the BlockNumber type for math.