diff --git a/pallets/collator-assignment/src/lib.rs b/pallets/collator-assignment/src/lib.rs index fa63c6757..4b294892b 100644 --- a/pallets/collator-assignment/src/lib.rs +++ b/pallets/collator-assignment/src/lib.rs @@ -54,9 +54,9 @@ use { }, sp_std::{collections::btree_set::BTreeSet, fmt::Debug, prelude::*, vec}, tp_traits::{ - CollatorAssignmentHook, CollatorAssignmentTip, GetContainerChainAuthor, - GetHostConfiguration, GetSessionContainerChains, ParaId, RemoveInvulnerables, - RemoveParaIdsWithNoCredits, ShouldRotateAllCollators, Slot, + CollatorAssignmentTip, GetContainerChainAuthor, GetHostConfiguration, + GetSessionContainerChains, ParaId, RemoveInvulnerables, RemoveParaIdsWithNoCredits, + ShouldRotateAllCollators, Slot, }, }; pub use {dp_collator_assignment::AssignedCollators, pallet::*}; @@ -105,8 +105,10 @@ pub mod pallet { type ShouldRotateAllCollators: ShouldRotateAllCollators; type GetRandomnessForNextBlock: GetRandomnessForNextBlock>; type RemoveInvulnerables: RemoveInvulnerables; - type RemoveParaIdsWithNoCredits: RemoveParaIdsWithNoCredits; - type CollatorAssignmentHook: CollatorAssignmentHook>; + type RemoveParaIdsWithNoCredits: RemoveParaIdsWithNoCredits< + BalanceOf, + Self::AccountId, + >; type Currency: Currency; type CollatorAssignmentTip: CollatorAssignmentTip>; type ForceEmptyOrchestrator: Get; @@ -309,13 +311,13 @@ pub mod pallet { old_assigned.container_chains.keys().cloned().collect(); // Remove the containerChains that do not have enough credits for block production - T::RemoveParaIdsWithNoCredits::remove_para_ids_with_no_credits( + T::RemoveParaIdsWithNoCredits::pre_assignment_remove_para_ids_with_no_credits( &mut container_chain_ids, &old_assigned_para_ids, ); // TODO: parathreads should be treated a bit differently, they don't need to have the same amount of credits // as parathreads because they will not be producing blocks on every slot. - T::RemoveParaIdsWithNoCredits::remove_para_ids_with_no_credits( + T::RemoveParaIdsWithNoCredits::pre_assignment_remove_para_ids_with_no_credits( &mut parathreads, &old_assigned_para_ids, ); @@ -467,51 +469,11 @@ pub mod pallet { // TODO: this probably is asking for a refactor // only apply the onCollatorAssignedHook if sufficient collators - for para_id in &container_chain_ids { - if !new_assigned - .container_chains - .get(para_id) - .unwrap_or(&vec![]) - .is_empty() - { - if let Err(e) = T::CollatorAssignmentHook::on_collators_assigned( - *para_id, - maybe_tip.as_ref(), - false, - ) { - // On error remove para from assignment - log::warn!( - "CollatorAssignmentHook error! Removing para {} from assignment: {:?}", - u32::from(*para_id), - e - ); - new_assigned.container_chains.remove(para_id); - } - } - } - - for para_id in ¶threads { - if !new_assigned - .container_chains - .get(para_id) - .unwrap_or(&vec![]) - .is_empty() - { - if let Err(e) = T::CollatorAssignmentHook::on_collators_assigned( - *para_id, - maybe_tip.as_ref(), - true, - ) { - // On error remove para from assignment - log::warn!( - "CollatorAssignmentHook error! Removing para {} from assignment: {:?}", - u32::from(*para_id), - e - ); - new_assigned.container_chains.remove(para_id); - } - } - } + T::RemoveParaIdsWithNoCredits::post_assignment_remove_para_ids_with_no_credits( + &old_assigned_para_ids, + &mut new_assigned.container_chains, + &maybe_tip, + ); Self::store_collator_fullness( &new_assigned, diff --git a/pallets/collator-assignment/src/mock.rs b/pallets/collator-assignment/src/mock.rs index 9059b4443..b6ef06af0 100644 --- a/pallets/collator-assignment/src/mock.rs +++ b/pallets/collator-assignment/src/mock.rs @@ -416,14 +416,23 @@ impl RemoveInvulnerables for RemoveAccountIdsAbove100 { /// Any ParaId >= 5000 will be considered to not have enough credits pub struct RemoveParaIdsAbove5000; -impl RemoveParaIdsWithNoCredits for RemoveParaIdsAbove5000 { - fn remove_para_ids_with_no_credits( +impl RemoveParaIdsWithNoCredits for RemoveParaIdsAbove5000 { + fn pre_assignment_remove_para_ids_with_no_credits( para_ids: &mut Vec, - _currently_assigned: &BTreeSet, + _old_assigned: &BTreeSet, ) { para_ids.retain(|para_id| *para_id <= ParaId::from(5000)); } + fn post_assignment_remove_para_ids_with_no_credits( + _current_assigned: &BTreeSet, + new_assigned: &mut BTreeMap>, + _maybe_tip: &Option, + ) -> Weight { + new_assigned.retain(|para_id, _| *para_id <= ParaId::from(5000)); + Weight::zero() + } + #[cfg(feature = "runtime-benchmarks")] fn make_valid_para_ids(_para_ids: &[ParaId]) {} } diff --git a/pallets/collator-assignment/src/tests.rs b/pallets/collator-assignment/src/tests.rs index 8010ce5ad..e7dd4aa2f 100644 --- a/pallets/collator-assignment/src/tests.rs +++ b/pallets/collator-assignment/src/tests.rs @@ -1344,64 +1344,6 @@ fn assign_collators_prioritizing_tip() { }); } -#[test] -fn on_collators_assigned_hook_failure_removes_para_from_assignment() { - new_test_ext().execute_with(|| { - run_to_block(1); - - MockData::mutate(|m| { - m.collators_per_container = 2; - m.collators_per_parathread = 2; - m.min_orchestrator_chain_collators = 5; - m.max_orchestrator_chain_collators = 5; - - m.collators = vec![1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]; - m.container_chains = vec![1001, 1002, 1003, 1004]; - m.assignment_hook_errors = false; - }); - run_to_block(11); - - assert_eq!( - assigned_collators(), - BTreeMap::from_iter(vec![ - (1, 1000), - (2, 1000), - (3, 1000), - (4, 1000), - (5, 1000), - (6, 1001), - (7, 1001), - (8, 1002), - (9, 1002), - (10, 1003), - (11, 1003), - ]), - ); - - // Para 1001 will fail on_assignment_hook - MockData::mutate(|m| { - m.assignment_hook_errors = true; - }); - - run_to_block(21); - - assert_eq!( - assigned_collators(), - BTreeMap::from_iter(vec![ - (1, 1000), - (2, 1000), - (3, 1000), - (4, 1000), - (5, 1000), - (8, 1002), - (9, 1002), - (10, 1003), - (11, 1003), - ]), - ); - }); -} - #[test] fn assign_collators_truncates_before_shuffling() { // Check that if there are more collators than needed, we only assign the first collators diff --git a/pallets/services-payment/src/lib.rs b/pallets/services-payment/src/lib.rs index 0c4cb4333..65da9e6c7 100644 --- a/pallets/services-payment/src/lib.rs +++ b/pallets/services-payment/src/lib.rs @@ -422,6 +422,27 @@ pub mod pallet { }); } + pub fn charge_tip(para_id: &ParaId, tip: &BalanceOf) -> Result<(), DispatchError> { + // Only charge the tip to the paras that had a max tip set + // (aka were willing to tip for being assigned a collator) + if MaxTip::::get(para_id).is_some() { + let tip_imbalance = T::Currency::withdraw( + &Self::parachain_tank(*para_id), + *tip, + WithdrawReasons::TIP, + ExistenceRequirement::KeepAlive, + )?; + + Self::deposit_event(Event::::CollatorAssignmentTipCollected { + para_id: *para_id, + payer: Self::parachain_tank(*para_id), + tip: *tip, + }); + T::OnChargeForCollatorAssignmentTip::on_unbalanced(tip_imbalance); + } + Ok(()) + } + pub fn free_block_production_credits(para_id: ParaId) -> Option> { BlockProductionCredits::::get(para_id) } diff --git a/primitives/traits/src/lib.rs b/primitives/traits/src/lib.rs index d9430cc91..810f5fa04 100644 --- a/primitives/traits/src/lib.rs +++ b/primitives/traits/src/lib.rs @@ -44,7 +44,7 @@ use { traits::{CheckedAdd, CheckedMul}, ArithmeticError, DispatchResult, Perbill, }, - sp_std::{collections::btree_set::BTreeSet, vec::Vec}, + sp_std::{collections::btree_map::BTreeMap, collections::btree_set::BTreeSet, vec::Vec}, }; // Separate import as rustfmt wrongly change it to `sp_std::vec::self`, which is the module instead @@ -271,26 +271,39 @@ impl RemoveInvulnerables for () { /// Helper trait for pallet_collator_assignment to be able to not assign collators to container chains with no credits /// in pallet_services_payment -pub trait RemoveParaIdsWithNoCredits { +pub trait RemoveParaIdsWithNoCredits { /// Remove para ids with not enough credits. The resulting order will affect priority: the first para id in the list /// will be the first one to get collators. - fn remove_para_ids_with_no_credits( + fn pre_assignment_remove_para_ids_with_no_credits( para_ids: &mut Vec, - currently_assigned: &BTreeSet, + old_assigned: &BTreeSet, ); + fn post_assignment_remove_para_ids_with_no_credits( + current_assigned: &BTreeSet, + new_assigned: &mut BTreeMap>, + maybe_tip: &Option, + ) -> Weight; /// Make those para ids valid by giving them enough credits, for benchmarking. #[cfg(feature = "runtime-benchmarks")] fn make_valid_para_ids(para_ids: &[ParaId]); } -impl RemoveParaIdsWithNoCredits for () { - fn remove_para_ids_with_no_credits( +impl RemoveParaIdsWithNoCredits for () { + fn pre_assignment_remove_para_ids_with_no_credits( _para_ids: &mut Vec, _currently_assigned: &BTreeSet, ) { } + fn post_assignment_remove_para_ids_with_no_credits( + _current_assigned: &BTreeSet, + _new_assigned: &mut BTreeMap>, + _maybe_tip: &Option, + ) -> Weight { + Default::default() + } + #[cfg(feature = "runtime-benchmarks")] fn make_valid_para_ids(_para_ids: &[ParaId]) {} } diff --git a/runtime/dancebox/src/lib.rs b/runtime/dancebox/src/lib.rs index 24ad0f42d..aff4a4c66 100644 --- a/runtime/dancebox/src/lib.rs +++ b/runtime/dancebox/src/lib.rs @@ -24,18 +24,22 @@ include!(concat!(env!("OUT_DIR"), "/wasm_binary.rs")); pub mod xcm_config; +use frame_support::storage::{with_storage_layer, with_transaction}; +use frame_support::traits::{ExistenceRequirement, WithdrawReasons}; use polkadot_runtime_common::SlowAdjustingFeeUpdate; #[cfg(feature = "std")] use sp_version::NativeVersion; #[cfg(any(feature = "std", test))] pub use sp_runtime::BuildStorage; +use sp_runtime::{DispatchError, TransactionOutcome}; pub mod weights; #[cfg(test)] mod tests; +use pallet_services_payment::BalanceOf; use { cumulus_pallet_parachain_system::{ RelayChainStateProof, RelayNumberMonotonicallyIncreases, RelaychainDataProvider, @@ -105,6 +109,7 @@ use { transaction_validity::{TransactionSource, TransactionValidity}, AccountId32, ApplyExtrinsicResult, }, + sp_std::collections::btree_map::BTreeMap, sp_std::{collections::btree_set::BTreeSet, marker::PhantomData, prelude::*}, sp_version::RuntimeVersion, staging_xcm::{ @@ -811,54 +816,127 @@ impl RemoveInvulnerables for RemoveInvulnerablesImpl { pub struct RemoveParaIdsWithNoCreditsImpl; -impl RemoveParaIdsWithNoCredits for RemoveParaIdsWithNoCreditsImpl { - fn remove_para_ids_with_no_credits( - para_ids: &mut Vec, +impl RemoveParaIdsWithNoCreditsImpl { + fn charge_para_ids_internal( + blocks_per_session: tp_traits::BlockNumber, + para_id: ParaId, currently_assigned: &BTreeSet, - ) { - let blocks_per_session = Period::get(); - - para_ids.retain(|para_id| { - // If the para has been assigned collators for this session it must have enough block credits - // for the current and the next session. - let block_credits_needed = if currently_assigned.contains(para_id) { - blocks_per_session * 2 + maybe_tip: &Option>, + ) -> Result { + use frame_support::traits::Currency; + type ServicePaymentCurrency = ::Currency; + + // Check if the container chain has enough credits for a session assignments + let maybe_assignment_imbalance = + if pallet_services_payment::Pallet::::burn_collator_assignment_free_credit_for_para(¶_id).is_err() { + let (amount_to_charge, _weight) = + ::ProvideCollatorAssignmentCost::collator_assignment_cost(¶_id); + Some(>::withdraw( + &pallet_services_payment::Pallet::::parachain_tank(para_id), + amount_to_charge, + WithdrawReasons::FEE, + ExistenceRequirement::KeepAlive, + )?) } else { - blocks_per_session + None }; - // Check if the container chain has enough credits for producing blocks - let free_block_credits = pallet_services_payment::BlockProductionCredits::::get(para_id) - .unwrap_or_default(); - - // Check if the container chain has enough credits for a session assignments - let free_session_credits = pallet_services_payment::CollatorAssignmentCredits::::get(para_id) - .unwrap_or_default(); - - // If para's max tip is set it should have enough to pay for one assignment with tip - let max_tip = pallet_services_payment::MaxTip::::get(para_id).unwrap_or_default() ; - - // Return if we can survive with free credits - if free_block_credits >= block_credits_needed && free_session_credits >= 1 { - // Max tip should always be checked, as it can be withdrawn even if free credits were used - return Balances::can_withdraw(&pallet_services_payment::Pallet::::parachain_tank(*para_id), max_tip).into_result(true).is_ok() + if let Some(tip) = maybe_tip { + if let Err(e) = pallet_services_payment::Pallet::::charge_tip(¶_id, tip) { + // Return assignment imbalance to tank on error + if let Some(assignment_imbalance) = maybe_assignment_imbalance { + ::Currency::resolve_creating( + &pallet_services_payment::Pallet::::parachain_tank(para_id), + assignment_imbalance, + ); + } + return Err(e); } + } - let remaining_block_credits = block_credits_needed.saturating_sub(free_block_credits); - let remaining_session_credits = 1u32.saturating_sub(free_session_credits); + if let Some(assignment_imbalance) = maybe_assignment_imbalance { + ::OnChargeForCollatorAssignment::on_unbalanced(assignment_imbalance); + } - let (block_production_costs, _) = ::ProvideBlockProductionCost::block_cost(para_id); - let (collator_assignment_costs, _) = ::ProvideCollatorAssignmentCost::collator_assignment_cost(para_id); - // let's check if we can withdraw - let remaining_block_credits_to_pay = u128::from(remaining_block_credits).saturating_mul(block_production_costs); - let remaining_session_credits_to_pay = u128::from(remaining_session_credits).saturating_mul(collator_assignment_costs); + // If the para has been assigned collators for this session it must have enough block credits + // for the current and the next session. + let block_credits_needed = if currently_assigned.contains(¶_id) { + blocks_per_session * 2 + } else { + blocks_per_session + }; + // Check if the container chain has enough credits for producing blocks + let free_block_credits = + pallet_services_payment::BlockProductionCredits::::get(para_id) + .unwrap_or_default(); + let remaining_block_credits = block_credits_needed.saturating_sub(free_block_credits); + let (block_production_costs, _) = + ::ProvideBlockProductionCost::block_cost( + ¶_id, + ); + // Check if we can withdraw + let remaining_block_credits_to_pay = + u128::from(remaining_block_credits).saturating_mul(block_production_costs); + let remaining_to_pay = remaining_block_credits_to_pay; + // This should take into account whether we tank goes below ED + // The true refers to keepAlive + Balances::can_withdraw( + &pallet_services_payment::Pallet::::parachain_tank(para_id), + remaining_to_pay, + ) + .into_result(true)?; + // TODO: Have proper weight + Ok(Weight::zero()) + } +} - let remaining_to_pay = remaining_block_credits_to_pay.saturating_add(remaining_session_credits_to_pay).saturating_add(max_tip); +impl RemoveParaIdsWithNoCredits, AC> for RemoveParaIdsWithNoCreditsImpl { + fn pre_assignment_remove_para_ids_with_no_credits( + para_ids: &mut Vec, + currently_assigned: &BTreeSet, + ) { + let blocks_per_session = Period::get(); + para_ids.retain(|para_id| { + with_transaction(|| { + let max_tip = + pallet_services_payment::MaxTip::::get(para_id).unwrap_or_default(); + TransactionOutcome::Rollback(Self::charge_para_ids_internal( + blocks_per_session, + *para_id, + currently_assigned, + &Some(max_tip), + )) + }) + .is_ok() + }); + } - // This should take into account whether we tank goes below ED - // The true refers to keepAlive - Balances::can_withdraw(&pallet_services_payment::Pallet::::parachain_tank(*para_id), remaining_to_pay).into_result(true).is_ok() + fn post_assignment_remove_para_ids_with_no_credits( + current_assigned: &BTreeSet, + new_assigned: &mut BTreeMap>, + maybe_tip: &Option>, + ) -> Weight { + let blocks_per_session = Period::get(); + let mut total_weight = Weight::zero(); + new_assigned.retain(|¶_id, collators| { + // Short-circuit in case collators are empty + if collators.is_empty() { + return true; + } + with_storage_layer(|| { + Self::charge_para_ids_internal( + blocks_per_session, + para_id, + current_assigned, + maybe_tip, + ) + }) + .inspect(|weight| { + total_weight += *weight; + }) + .is_ok() }); + total_weight } /// Make those para ids valid by giving them enough credits, for benchmarking. @@ -897,7 +975,6 @@ impl pallet_collator_assignment::Config for Runtime { type GetRandomnessForNextBlock = BabeGetRandomnessForNextBlock; type RemoveInvulnerables = RemoveInvulnerablesImpl; type RemoveParaIdsWithNoCredits = RemoveParaIdsWithNoCreditsImpl; - type CollatorAssignmentHook = ServicesPayment; type CollatorAssignmentTip = ServicesPayment; type Currency = Balances; type ForceEmptyOrchestrator = ConstBool; diff --git a/runtime/flashbox/src/lib.rs b/runtime/flashbox/src/lib.rs index 702f3f01f..4a6488938 100644 --- a/runtime/flashbox/src/lib.rs +++ b/runtime/flashbox/src/lib.rs @@ -22,6 +22,8 @@ #[cfg(feature = "std")] include!(concat!(env!("OUT_DIR"), "/wasm_binary.rs")); +use frame_support::storage::{with_storage_layer, with_transaction}; +use frame_support::traits::{ExistenceRequirement, WithdrawReasons}; #[cfg(feature = "std")] use sp_version::NativeVersion; use { @@ -31,12 +33,14 @@ use { #[cfg(any(feature = "std", test))] pub use sp_runtime::BuildStorage; +use sp_runtime::{DispatchError, TransactionOutcome}; pub mod weights; #[cfg(test)] mod tests; +use pallet_services_payment::BalanceOf; use { cumulus_pallet_parachain_system::RelayNumberMonotonicallyIncreases, cumulus_primitives_core::{relay_chain::SessionIndex, BodyId, ParaId}, @@ -94,6 +98,7 @@ use { transaction_validity::{TransactionSource, TransactionValidity}, AccountId32, ApplyExtrinsicResult, RuntimeDebug, }, + sp_std::collections::btree_map::BTreeMap, sp_std::{collections::btree_set::BTreeSet, marker::PhantomData, prelude::*}, sp_version::RuntimeVersion, tp_traits::{ @@ -650,54 +655,127 @@ impl RemoveInvulnerables for RemoveInvulnerablesImpl { pub struct RemoveParaIdsWithNoCreditsImpl; -impl RemoveParaIdsWithNoCredits for RemoveParaIdsWithNoCreditsImpl { - fn remove_para_ids_with_no_credits( - para_ids: &mut Vec, +impl RemoveParaIdsWithNoCreditsImpl { + fn charge_para_ids_internal( + blocks_per_session: tp_traits::BlockNumber, + para_id: ParaId, currently_assigned: &BTreeSet, - ) { - let blocks_per_session = Period::get(); - - para_ids.retain(|para_id| { - // If the para has been assigned collators for this session it must have enough block credits - // for the current and the next session. - let block_credits_needed = if currently_assigned.contains(para_id) { - blocks_per_session * 2 + maybe_tip: &Option>, + ) -> Result { + use frame_support::traits::Currency; + type ServicePaymentCurrency = ::Currency; + + // Check if the container chain has enough credits for a session assignments + let maybe_assignment_imbalance = + if pallet_services_payment::Pallet::::burn_collator_assignment_free_credit_for_para(¶_id).is_err() { + let (amount_to_charge, _weight) = + ::ProvideCollatorAssignmentCost::collator_assignment_cost(¶_id); + Some(>::withdraw( + &pallet_services_payment::Pallet::::parachain_tank(para_id), + amount_to_charge, + WithdrawReasons::FEE, + ExistenceRequirement::KeepAlive, + )?) } else { - blocks_per_session + None }; - // Check if the container chain has enough credits for producing blocks - let free_block_credits = pallet_services_payment::BlockProductionCredits::::get(para_id) - .unwrap_or_default(); - - // Check if the container chain has enough credits for a session assignments - let free_session_credits = pallet_services_payment::CollatorAssignmentCredits::::get(para_id) - .unwrap_or_default(); - - // If para's max tip is set it should have enough to pay for one assignment with tip - let max_tip = pallet_services_payment::MaxTip::::get(para_id).unwrap_or_default() ; - - // Return if we can survive with free credits - if free_block_credits >= block_credits_needed && free_session_credits >= 1 { - // Max tip should always be checked, as it can be withdrawn even if free credits were used - return Balances::can_withdraw(&pallet_services_payment::Pallet::::parachain_tank(*para_id), max_tip).into_result(true).is_ok() + if let Some(tip) = maybe_tip { + if let Err(e) = pallet_services_payment::Pallet::::charge_tip(¶_id, tip) { + // Return assignment imbalance to tank on error + if let Some(assignment_imbalance) = maybe_assignment_imbalance { + ::Currency::resolve_creating( + &pallet_services_payment::Pallet::::parachain_tank(para_id), + assignment_imbalance, + ); + } + return Err(e); } + } - let remaining_block_credits = block_credits_needed.saturating_sub(free_block_credits); - let remaining_session_credits = 1u32.saturating_sub(free_session_credits); + if let Some(assignment_imbalance) = maybe_assignment_imbalance { + ::OnChargeForCollatorAssignment::on_unbalanced(assignment_imbalance); + } - let (block_production_costs, _) = ::ProvideBlockProductionCost::block_cost(para_id); - let (collator_assignment_costs, _) = ::ProvideCollatorAssignmentCost::collator_assignment_cost(para_id); - // let's check if we can withdraw - let remaining_block_credits_to_pay = u128::from(remaining_block_credits).saturating_mul(block_production_costs); - let remaining_session_credits_to_pay = u128::from(remaining_session_credits).saturating_mul(collator_assignment_costs); + // If the para has been assigned collators for this session it must have enough block credits + // for the current and the next session. + let block_credits_needed = if currently_assigned.contains(¶_id) { + blocks_per_session * 2 + } else { + blocks_per_session + }; + // Check if the container chain has enough credits for producing blocks + let free_block_credits = + pallet_services_payment::BlockProductionCredits::::get(para_id) + .unwrap_or_default(); + let remaining_block_credits = block_credits_needed.saturating_sub(free_block_credits); + let (block_production_costs, _) = + ::ProvideBlockProductionCost::block_cost( + ¶_id, + ); + // Check if we can withdraw + let remaining_block_credits_to_pay = + u128::from(remaining_block_credits).saturating_mul(block_production_costs); + let remaining_to_pay = remaining_block_credits_to_pay; + // This should take into account whether we tank goes below ED + // The true refers to keepAlive + Balances::can_withdraw( + &pallet_services_payment::Pallet::::parachain_tank(para_id), + remaining_to_pay, + ) + .into_result(true)?; + // TODO: Have proper weight + Ok(Weight::zero()) + } +} - let remaining_to_pay = remaining_block_credits_to_pay.saturating_add(remaining_session_credits_to_pay).saturating_add(max_tip); +impl RemoveParaIdsWithNoCredits, AC> for RemoveParaIdsWithNoCreditsImpl { + fn pre_assignment_remove_para_ids_with_no_credits( + para_ids: &mut Vec, + currently_assigned: &BTreeSet, + ) { + let blocks_per_session = Period::get(); + para_ids.retain(|para_id| { + with_transaction(|| { + let max_tip = + pallet_services_payment::MaxTip::::get(para_id).unwrap_or_default(); + TransactionOutcome::Rollback(Self::charge_para_ids_internal( + blocks_per_session, + *para_id, + currently_assigned, + &Some(max_tip), + )) + }) + .is_ok() + }); + } - // This should take into account whether we tank goes below ED - // The true refers to keepAlive - Balances::can_withdraw(&pallet_services_payment::Pallet::::parachain_tank(*para_id), remaining_to_pay).into_result(true).is_ok() + fn post_assignment_remove_para_ids_with_no_credits( + current_assigned: &BTreeSet, + new_assigned: &mut BTreeMap>, + maybe_tip: &Option>, + ) -> Weight { + let blocks_per_session = Period::get(); + let mut total_weight = Weight::zero(); + new_assigned.retain(|¶_id, collators| { + // Short-circuit in case collators are empty + if collators.is_empty() { + return true; + } + with_storage_layer(|| { + Self::charge_para_ids_internal( + blocks_per_session, + para_id, + current_assigned, + maybe_tip, + ) + }) + .inspect(|weight| { + total_weight += *weight; + }) + .is_ok() }); + total_weight } /// Make those para ids valid by giving them enough credits, for benchmarking. @@ -743,7 +821,6 @@ impl pallet_collator_assignment::Config for Runtime { type GetRandomnessForNextBlock = (); type RemoveInvulnerables = RemoveInvulnerablesImpl; type RemoveParaIdsWithNoCredits = RemoveParaIdsWithNoCreditsImpl; - type CollatorAssignmentHook = ServicesPayment; type CollatorAssignmentTip = ServicesPayment; type Currency = Balances; type ForceEmptyOrchestrator = ConstBool; diff --git a/solo-chains/runtime/starlight/src/lib.rs b/solo-chains/runtime/starlight/src/lib.rs index c0bd7adeb..2f28701e7 100644 --- a/solo-chains/runtime/starlight/src/lib.rs +++ b/solo-chains/runtime/starlight/src/lib.rs @@ -20,6 +20,7 @@ // `construct_runtime!` does a lot of recursion and requires us to increase the limit. #![recursion_limit = "512"] +use frame_support::storage::{with_storage_layer, with_transaction}; // Fix compile error in impl_runtime_weights! macro use runtime_common as polkadot_runtime_common; use { @@ -516,11 +517,13 @@ parameter_types! { #[cfg(feature = "runtime-benchmarks")] pub struct TreasuryBenchmarkHelper(PhantomData); -#[cfg(feature = "runtime-benchmarks")] use frame_support::traits::Currency; +use frame_support::traits::{ExistenceRequirement, OnUnbalanced, WithdrawReasons}; +use pallet_services_payment::BalanceOf; #[cfg(feature = "runtime-benchmarks")] use pallet_treasury::ArgumentsFactory; use runtime_parachains::configuration::HostConfiguration; +use sp_runtime::{DispatchError, TransactionOutcome}; #[cfg(feature = "runtime-benchmarks")] impl ArgumentsFactory<(), T::AccountId> for TreasuryBenchmarkHelper @@ -2906,54 +2909,124 @@ impl frame_support::traits::Randomness for BabeCurrentBlockRa pub struct RemoveParaIdsWithNoCreditsImpl; -impl RemoveParaIdsWithNoCredits for RemoveParaIdsWithNoCreditsImpl { - fn remove_para_ids_with_no_credits( - para_ids: &mut Vec, +impl RemoveParaIdsWithNoCreditsImpl { + fn charge_para_ids_internal( + blocks_per_session: BlockNumber, + para_id: ParaId, currently_assigned: &BTreeSet, - ) { - let blocks_per_session = EpochDurationInBlocks::get(); - - para_ids.retain(|para_id| { - // If the para has been assigned collators for this session it must have enough block credits - // for the current and the next session. - let block_credits_needed = if currently_assigned.contains(para_id) { - blocks_per_session * 2 + maybe_tip: &Option>, + ) -> Result { + // Check if the container chain has enough credits for a session assignments + let maybe_assignment_imbalance = + if pallet_services_payment::Pallet::::burn_collator_assignment_free_credit_for_para(¶_id).is_err() { + let (amount_to_charge, _weight) = + ::ProvideCollatorAssignmentCost::collator_assignment_cost(¶_id); + Some(::Currency::withdraw( + &pallet_services_payment::Pallet::::parachain_tank(para_id), + amount_to_charge, + WithdrawReasons::FEE, + ExistenceRequirement::KeepAlive, + )?) } else { - blocks_per_session + None }; - // Check if the container chain has enough credits for producing blocks - let free_block_credits = pallet_services_payment::BlockProductionCredits::::get(para_id) - .unwrap_or_default(); - - // Check if the container chain has enough credits for a session assignments - let free_session_credits = pallet_services_payment::CollatorAssignmentCredits::::get(para_id) - .unwrap_or_default(); - - // If para's max tip is set it should have enough to pay for one assignment with tip - let max_tip = pallet_services_payment::MaxTip::::get(para_id).unwrap_or_default() ; - - // Return if we can survive with free credits - if free_block_credits >= block_credits_needed && free_session_credits >= 1 { - // Max tip should always be checked, as it can be withdrawn even if free credits were used - return Balances::can_withdraw(&pallet_services_payment::Pallet::::parachain_tank(*para_id), max_tip).into_result(true).is_ok() + if let Some(tip) = maybe_tip { + if let Err(e) = pallet_services_payment::Pallet::::charge_tip(¶_id, tip) { + // Return assignment imbalance to tank on error + if let Some(assignment_imbalance) = maybe_assignment_imbalance { + ::Currency::resolve_creating( + &pallet_services_payment::Pallet::::parachain_tank(para_id), + assignment_imbalance, + ); + } + return Err(e); } + } - let remaining_block_credits = block_credits_needed.saturating_sub(free_block_credits); - let remaining_session_credits = 1u32.saturating_sub(free_session_credits); + if let Some(assignment_imbalance) = maybe_assignment_imbalance { + ::OnChargeForCollatorAssignment::on_unbalanced(assignment_imbalance); + } - let (block_production_costs, _) = ::ProvideBlockProductionCost::block_cost(para_id); - let (collator_assignment_costs, _) = ::ProvideCollatorAssignmentCost::collator_assignment_cost(para_id); - // let's check if we can withdraw - let remaining_block_credits_to_pay = u128::from(remaining_block_credits).saturating_mul(block_production_costs); - let remaining_session_credits_to_pay = u128::from(remaining_session_credits).saturating_mul(collator_assignment_costs); + // If the para has been assigned collators for this session it must have enough block credits + // for the current and the next session. + let block_credits_needed = if currently_assigned.contains(¶_id) { + blocks_per_session * 2 + } else { + blocks_per_session + }; + // Check if the container chain has enough credits for producing blocks + let free_block_credits = + pallet_services_payment::BlockProductionCredits::::get(para_id) + .unwrap_or_default(); + let remaining_block_credits = block_credits_needed.saturating_sub(free_block_credits); + let (block_production_costs, _) = + ::ProvideBlockProductionCost::block_cost( + ¶_id, + ); + // Check if we can withdraw + let remaining_block_credits_to_pay = + u128::from(remaining_block_credits).saturating_mul(block_production_costs); + let remaining_to_pay = remaining_block_credits_to_pay; + // This should take into account whether we tank goes below ED + // The true refers to keepAlive + Balances::can_withdraw( + &pallet_services_payment::Pallet::::parachain_tank(para_id), + remaining_to_pay, + ) + .into_result(true)?; + // TODO: Have proper weight + Ok(Weight::zero()) + } +} - let remaining_to_pay = remaining_block_credits_to_pay.saturating_add(remaining_session_credits_to_pay).saturating_add(max_tip); +impl RemoveParaIdsWithNoCredits, AC> for RemoveParaIdsWithNoCreditsImpl { + fn pre_assignment_remove_para_ids_with_no_credits( + para_ids: &mut Vec, + currently_assigned: &BTreeSet, + ) { + let blocks_per_session = EpochDurationInBlocks::get(); + para_ids.retain(|para_id| { + with_transaction(|| { + let max_tip = + pallet_services_payment::MaxTip::::get(para_id).unwrap_or_default(); + TransactionOutcome::Rollback(Self::charge_para_ids_internal( + blocks_per_session, + *para_id, + currently_assigned, + &Some(max_tip), + )) + }) + .is_ok() + }); + } - // This should take into account whether we tank goes below ED - // The true refers to keepAlive - Balances::can_withdraw(&pallet_services_payment::Pallet::::parachain_tank(*para_id), remaining_to_pay).into_result(true).is_ok() + fn post_assignment_remove_para_ids_with_no_credits( + current_assigned: &BTreeSet, + new_assigned: &mut BTreeMap>, + maybe_tip: &Option>, + ) -> Weight { + let blocks_per_session = EpochDurationInBlocks::get(); + let mut total_weight = Weight::zero(); + new_assigned.retain(|¶_id, collators| { + // Short-circuit in case collators are empty + if collators.is_empty() { + return true; + } + with_storage_layer(|| { + Self::charge_para_ids_internal( + blocks_per_session, + para_id, + current_assigned, + maybe_tip, + ) + }) + .inspect(|weight| { + total_weight += *weight; + }) + .is_ok() }); + total_weight } /// Make those para ids valid by giving them enough credits, for benchmarking. @@ -3043,7 +3116,6 @@ impl pallet_collator_assignment::Config for Runtime { type GetRandomnessForNextBlock = BabeGetRandomnessForNextBlock; type RemoveInvulnerables = (); type RemoveParaIdsWithNoCredits = RemoveParaIdsWithNoCreditsImpl; - type CollatorAssignmentHook = ServicesPayment; type CollatorAssignmentTip = ServicesPayment; type Currency = Balances; type ForceEmptyOrchestrator = ConstBool;