diff --git a/zrml/asset-router/src/lib.rs b/zrml/asset-router/src/lib.rs index 32a171c65..cfa1e7e6d 100644 --- a/zrml/asset-router/src/lib.rs +++ b/zrml/asset-router/src/lib.rs @@ -70,6 +70,9 @@ pub mod pallet { pub(crate) use zeitgeist_primitives::traits::CheckedDivPerComponent; pub(crate) const LOG_TARGET: &str = "runtime::asset-router"; + const MAX_ASSET_DESTRUCTIONS_PER_BLOCK: u8 = 128; + pub(crate) const MAX_ASSETS_IN_DESTRUCTION: u32 = 2048; + const MAX_INDESTRUCTIBLE_ASSETS: u32 = 256; pub trait AssetTraits: Create @@ -191,7 +194,7 @@ pub mod pallet { /// Keeps track of assets that can't be destroyed. #[pallet::storage] pub(crate) type IndestructibleAssets = - StorageValue<_, BoundedVec>, ValueQuery>; + StorageValue<_, BoundedVec>, ValueQuery>; #[pallet::error] pub enum Error { @@ -215,7 +218,26 @@ pub mod pallet { #[pallet::hooks] impl Hooks for Pallet { fn on_idle(_: T::BlockNumber, mut remaining_weight: Weight) -> Weight { - if !remaining_weight.all_gte(T::DbWeight::get().reads(1)) { + let max_proof_size_destructibles: u64 = + AssetInDestruction::::max_encoded_len() + .saturating_mul(MAX_ASSETS_IN_DESTRUCTION.saturated_into()) + .saturated_into(); + let max_proof_size_indestructibles: u64 = T::AssetType::max_encoded_len() + .saturating_mul(MAX_INDESTRUCTIBLE_ASSETS.saturated_into()) + .saturated_into(); + let max_proof_size_total = + max_proof_size_destructibles.saturating_add(max_proof_size_indestructibles); + + let max_extra_weight = Weight::from_parts( + // 1ms extra execution time + 1_000_000_000, + // Maximum proof size assuming writes on full storage + max_proof_size_total, + ); + + if !remaining_weight + .all_gte(max_extra_weight.saturating_add(T::DbWeight::get().reads(1))) + { return remaining_weight; }; @@ -224,18 +246,19 @@ pub mod pallet { return remaining_weight.saturating_sub(T::DbWeight::get().reads(1)); } - remaining_weight = - remaining_weight.saturating_sub(T::DbWeight::get().reads_writes(1, 1)); - let saftey_counter_outer_max = 128u8; + remaining_weight = remaining_weight + .saturating_sub(T::DbWeight::get().reads_writes(1, 1)) + .saturating_sub(max_extra_weight); + let saftey_counter_outer_max = MAX_ASSET_DESTRUCTIONS_PER_BLOCK; let mut saftey_counter_outer; 'outer: while let Some(mut asset) = destroy_assets.pop() { - let safety_counter_inner_max = 6u8; + let safety_counter_inner_max = DESTRUCTION_STATES; let mut safety_counter_inner = 0u8; while (*asset.state() != DestructionState::Destroyed && *asset.state() != DestructionState::Indestructible) - && safety_counter_inner < safety_counter_inner_max + && safety_counter_inner < safety_counter_inner_max { match asset.state() { DestructionState::Accounts => { diff --git a/zrml/asset-router/src/mock.rs b/zrml/asset-router/src/mock.rs index ccb4213c3..1291b4d6b 100644 --- a/zrml/asset-router/src/mock.rs +++ b/zrml/asset-router/src/mock.rs @@ -66,7 +66,7 @@ pub(super) const CUSTOM_ASSET_INITIAL_AMOUNT: Balance = 20; pub(super) const MARKET_ASSET_INITIAL_AMOUNT: Balance = 30; pub(super) const CURRENCY_INITIAL_AMOUNT: Balance = 40; -pub(super) const DESTROY_WEIGHT: Weight = Weight::from_all(1000); +pub(super) const DESTROY_WEIGHT: Weight = Weight::from_parts(1000, 0); pub(super) type AccountId = ::AccountId; pub(super) type CustomAssetsInstance = pallet_assets::Instance1; diff --git a/zrml/asset-router/src/tests/managed_destroy.rs b/zrml/asset-router/src/tests/managed_destroy.rs index 36f894ac5..58a315a9e 100644 --- a/zrml/asset-router/src/tests/managed_destroy.rs +++ b/zrml/asset-router/src/tests/managed_destroy.rs @@ -18,8 +18,12 @@ #![cfg(test)] use super::*; -use crate::AssetInDestruction; -use frame_support::{traits::tokens::fungibles::Inspect, BoundedVec}; +use crate::{AssetInDestruction, Weight}; +use frame_support::{ + traits::{tokens::fungibles::Inspect, Get}, + weights::RuntimeDbWeight, + BoundedVec, +}; use pallet_assets::ManagedDestroy; #[test] @@ -121,7 +125,7 @@ fn destroys_assets_fully_works_properly() { assert_ok!(managed_destroy_multi_transactional(assets.clone())); assert_eq!(crate::DestroyAssets::::get().len(), 3); - let available_weight = 1_000_000_000.into(); + let available_weight = 10_000_000_000.into(); let remaining_weight = AssetRouter::on_idle(0, available_weight); assert!(!AssetRouter::asset_exists(CAMPAIGN_ASSET)); assert!(!AssetRouter::asset_exists(CUSTOM_ASSET)); @@ -129,7 +133,9 @@ fn destroys_assets_fully_works_properly() { assert_eq!(crate::IndestructibleAssets::::get(), vec![]); assert_eq!(crate::DestroyAssets::::get(), vec![]); - let consumed_weight = available_weight - 3u64 * 3u64 * DESTROY_WEIGHT; + let mut consumed_weight = available_weight - 3u64 * 3u64 * DESTROY_WEIGHT; + // Consider safety buffer for extra execution time and storage proof size + consumed_weight = consumed_weight.saturating_sub(Weight::from_parts(1_000_000_000, 45_824)); assert_eq!(remaining_weight, consumed_weight); }) } @@ -147,15 +153,17 @@ fn destroys_assets_partially_properly() { assert_ok!(managed_destroy_multi_transactional(assets.clone())); assert_eq!(crate::DestroyAssets::::get().len(), 3); - let available_weight = DESTROY_WEIGHT * 3; + let mut available_weight: Weight = Weight::from_all(1_000_000_000) + 2u64 * DESTROY_WEIGHT; // Make on_idle only partially delete the first asset - let _ = AssetRouter::on_idle(0, available_weight - 2u32 * DESTROY_WEIGHT); + let _ = AssetRouter::on_idle(0, available_weight); assert_eq!(crate::DestroyAssets::::get().len(), 3); // Now delete each asset one by one by supplying exactly the required weight + available_weight = Weight::from_all(1_000_000_000) + DESTROY_WEIGHT; let _ = AssetRouter::on_idle(0, available_weight); assert_eq!(crate::DestroyAssets::::get().len(), 2); + available_weight = Weight::from_all(1_000_000_000) + 3u64 * DESTROY_WEIGHT; let _ = AssetRouter::on_idle(0, available_weight); assert_eq!(crate::DestroyAssets::::get().len(), 1); @@ -173,7 +181,7 @@ fn properly_handles_indestructible_assets() { ExtBuilder::default().build().execute_with(|| { let assets_raw = vec![CAMPAIGN_ASSET, CUSTOM_ASSET, MARKET_ASSET]; let mut destroy_assets = crate::DestroyAssets::::get(); - let available_weight = 1_000_000_000.into(); + let available_weight = 10_000_000_000.into(); for asset in assets_raw { destroy_assets.force_push(AssetInDestruction::new(asset)); @@ -226,7 +234,9 @@ fn properly_handles_indestructible_assets() { crate::DestroyAssets::::put(destroy_assets); assert_eq!(crate::DestroyAssets::::get().len(), 3); let remaining_weight = AssetRouter::on_idle(0, available_weight); - let consumed_weight = available_weight - 2u32 * 3u32 * DESTROY_WEIGHT - DESTROY_WEIGHT; + let mut consumed_weight = available_weight - 2u32 * 3u32 * DESTROY_WEIGHT - DESTROY_WEIGHT; + // Consider safety buffer for extra execution time and storage proof size + consumed_weight = consumed_weight.saturating_sub(Weight::from_parts(1_000_000_000, 45_824)); assert_eq!(remaining_weight, consumed_weight); assert_eq!(crate::DestroyAssets::::get().len(), 0); assert_eq!(crate::IndestructibleAssets::::get().len(), 1); @@ -236,3 +246,31 @@ fn properly_handles_indestructible_assets() { assert!(!AssetRouter::asset_exists(MARKET_ASSET)); }) } + +#[test] +fn does_not_execute_on_insufficient_weight() { + ExtBuilder::default().build().execute_with(|| { + assert_ok!(AssetRouter::create(CAMPAIGN_ASSET, ALICE, true, CAMPAIGN_ASSET_MIN_BALANCE)); + assert_ok!(AssetRouter::managed_destroy(CAMPAIGN_ASSET, None)); + assert_eq!(crate::DestroyAssets::::get().len(), 1); + + let db_weight: RuntimeDbWeight = ::DbWeight::get(); + let mut available_weight = + Weight::from_parts(1_000_000_000, 45_824) + db_weight.reads(1) - 1u64.into(); + let mut remaining_weight = AssetRouter::on_idle(0, available_weight); + assert_eq!(available_weight, remaining_weight); + assert_eq!(crate::DestroyAssets::::get().len(), 1); + + available_weight += 1u64.into(); + let mut remaining_weight_expected: Weight = 0u64.into(); + remaining_weight = AssetRouter::on_idle(0, available_weight); + assert_eq!(remaining_weight_expected, remaining_weight); + assert_eq!(crate::DestroyAssets::::get().len(), 1); + + remaining_weight_expected = 1u64.into(); + available_weight += 3u64 * DESTROY_WEIGHT + remaining_weight_expected; + remaining_weight = AssetRouter::on_idle(0, available_weight); + assert_eq!(remaining_weight_expected, remaining_weight); + assert_eq!(crate::DestroyAssets::::get().len(), 0); + }) +} diff --git a/zrml/asset-router/src/types.rs b/zrml/asset-router/src/types.rs index f44d8104e..91ec53a22 100644 --- a/zrml/asset-router/src/types.rs +++ b/zrml/asset-router/src/types.rs @@ -15,13 +15,15 @@ // You should have received a copy of the GNU General Public License // along with Zeitgeist. If not, see . -use crate::{BoundedVec, Config, ConstU32, Weight}; +use crate::{BoundedVec, Config, ConstU32, Weight, MAX_ASSETS_IN_DESTRUCTION}; use core::cmp::Ordering; use parity_scale_codec::{Decode, Encode, MaxEncodedLen}; use scale_info::TypeInfo; -pub(crate) type DestroyAssetsT = - BoundedVec::AssetType>, ConstU32<8192>>; +pub(crate) type DestroyAssetsT = BoundedVec< + AssetInDestruction<::AssetType>, + ConstU32<{ MAX_ASSETS_IN_DESTRUCTION }>, +>; pub(crate) enum DestructionOk { Complete(Weight), @@ -45,6 +47,7 @@ pub(crate) enum DestructionState { Destroyed, Indestructible, } +pub(crate) const DESTRUCTION_STATES: u8 = 5; #[derive(Clone, Copy, Encode, Eq, Debug, Decode, MaxEncodedLen, PartialEq, TypeInfo)] pub(crate) struct AssetInDestruction {