From a233102ef73a50213ad09d9c4a94db10d412c2ec Mon Sep 17 00:00:00 2001 From: Krayt78 Date: Wed, 20 Nov 2024 20:15:34 +0100 Subject: [PATCH 1/4] Did some migration to v2, stuck on error --- polkadot/runtime/common/src/slots/mod.rs | 56 ++++++++++++++++-------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/polkadot/runtime/common/src/slots/mod.rs b/polkadot/runtime/common/src/slots/mod.rs index 333f14c6608a..958d4fa1fd76 100644 --- a/polkadot/runtime/common/src/slots/mod.rs +++ b/polkadot/runtime/common/src/slots/mod.rs @@ -973,7 +973,7 @@ mod benchmarking { use polkadot_runtime_parachains::paras; use sp_runtime::traits::{Bounded, One}; - use frame_benchmarking::{account, benchmarks, whitelisted_caller, BenchmarkError}; + use frame_benchmarking::v2::*; use crate::slots::Pallet as Slots; @@ -1009,10 +1009,15 @@ mod benchmarking { (para, leaser) } - benchmarks! { + #[benchmarks( where_clause { where T: paras::Config } + )] - force_lease { + mod benchmarks { + use super::*; + + #[benchmark] + fn force_lease() -> Result<(), BenchmarkError> { // If there is an offset, we need to be on that block to be able to do lease things. frame_system::Pallet::::set_block_number(T::LeaseOffset::get() + One::one()); let para = ParaId::from(1337); @@ -1023,8 +1028,10 @@ mod benchmarking { let period_count = 3u32.into(); let origin = T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - }: _(origin, para, leaser.clone(), amount, period_begin, period_count) - verify { + + #[extrinsic_call] + _(origin, para, leaser.clone(), amount, period_begin, period_count); + assert_last_event::(Event::::Leased { para_id: para, leaser, period_begin, @@ -1032,14 +1039,14 @@ mod benchmarking { extra_reserved: amount, total_amount: amount, }.into()); + + Ok(()) } // Worst case scenario, T on-demand parachains onboard, and C lease holding parachains offboard. - manage_lease_period_start { - // Assume reasonable maximum of 100 paras at any time - let c in 0 .. 100; - let t in 0 .. 100; - + // Assume reasonable maximum of 100 paras at any time + #[benchmark] + fn manage_lease_period_start(c: Linear<0,100>, n : Linear<0,100>) -> Result<(), BenchmarkError> { let period_begin = 1u32.into(); let period_count = 4u32.into(); @@ -1078,9 +1085,9 @@ mod benchmarking { for i in 200 .. 200 + c { assert!(T::Registrar::is_parachain(ParaId::from(i))); } - }: { - Slots::::manage_lease_period_start(period_begin); - } verify { + #[extrinsic_call] + Slots::::manage_lease_period_start(period_begin); + // All paras should have switched. T::Registrar::execute_pending_transitions(); for i in 0 .. t { @@ -1089,11 +1096,14 @@ mod benchmarking { for i in 200 .. 200 + c { assert!(T::Registrar::is_parathread(ParaId::from(i))); } + + Ok(()) } // Assume that at most 8 people have deposits for leases on a parachain. // This would cover at least 4 years of leases in the worst case scenario. - clear_all_leases { + #[benchmark] + fn clear_all_leases() -> Result<(), BenchmarkError> { let max_people = 8; let (para, _) = register_a_parathread::(1); @@ -1120,24 +1130,32 @@ mod benchmarking { let origin = T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - }: _(origin, para) - verify { + + #[extrinsic_call] + _(origin, para); + for i in 0 .. max_people { let leaser = account("lease_deposit", i, 0); assert_eq!(T::Currency::reserved_balance(&leaser), 0u32.into()); } + + Ok(()) } - trigger_onboard { + #[benchmark] + fn trigger_onboard() -> Result<(), BenchmarkError> { // get a parachain into a bad state where they did not onboard let (para, _) = register_a_parathread::(1); Leases::::insert(para, vec![Some((account::("lease_insert", 0, 0), BalanceOf::::default()))]); assert!(T::Registrar::is_parathread(para)); let caller = whitelisted_caller(); - }: _(RawOrigin::Signed(caller), para) - verify { + + #[extrinsic_call] + _(RawOrigin::Signed(caller), para); + T::Registrar::execute_pending_transitions(); assert!(T::Registrar::is_parachain(para)); + Ok(()) } impl_benchmark_test_suite!( From 1bf298dece291a9069c4582367c49aba32b1cd0f Mon Sep 17 00:00:00 2001 From: Krayt78 Date: Thu, 21 Nov 2024 21:55:53 +0100 Subject: [PATCH 2/4] fix where clause issue --- polkadot/runtime/common/src/slots/mod.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/polkadot/runtime/common/src/slots/mod.rs b/polkadot/runtime/common/src/slots/mod.rs index 958d4fa1fd76..43cc765a1f03 100644 --- a/polkadot/runtime/common/src/slots/mod.rs +++ b/polkadot/runtime/common/src/slots/mod.rs @@ -1010,7 +1010,8 @@ mod benchmarking { } #[benchmarks( - where_clause { where T: paras::Config } + where + T: paras::Config, )] mod benchmarks { @@ -1030,7 +1031,7 @@ mod benchmarking { T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; #[extrinsic_call] - _(origin, para, leaser.clone(), amount, period_begin, period_count); + _(origin as T::RuntimeOrigin, para, leaser.clone(), amount, period_begin, period_count); assert_last_event::(Event::::Leased { para_id: para, @@ -1046,7 +1047,7 @@ mod benchmarking { // Worst case scenario, T on-demand parachains onboard, and C lease holding parachains offboard. // Assume reasonable maximum of 100 paras at any time #[benchmark] - fn manage_lease_period_start(c: Linear<0,100>, n : Linear<0,100>) -> Result<(), BenchmarkError> { + fn manage_lease_period_start(c: Linear<0,100>, t : Linear<0,100>) -> Result<(), BenchmarkError> { let period_begin = 1u32.into(); let period_count = 4u32.into(); @@ -1132,7 +1133,7 @@ mod benchmarking { T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; #[extrinsic_call] - _(origin, para); + _(origin as T::RuntimeOrigin, para); for i in 0 .. max_people { let leaser = account("lease_deposit", i, 0); From ac7c452b6ce532505dd0401ed3dc1865160f626c Mon Sep 17 00:00:00 2001 From: Krayt78 Date: Fri, 22 Nov 2024 17:50:46 +0100 Subject: [PATCH 3/4] fix issue and fmt --- polkadot/runtime/common/src/slots/mod.rs | 94 ++++++++++++++---------- 1 file changed, 54 insertions(+), 40 deletions(-) diff --git a/polkadot/runtime/common/src/slots/mod.rs b/polkadot/runtime/common/src/slots/mod.rs index 43cc765a1f03..4fa0774bc3a4 100644 --- a/polkadot/runtime/common/src/slots/mod.rs +++ b/polkadot/runtime/common/src/slots/mod.rs @@ -149,7 +149,7 @@ pub mod pallet { if let Some((lease_period, first_block)) = Self::lease_period_index(n) { // If we're beginning a new lease period then handle that. if first_block { - return Self::manage_lease_period_start(lease_period) + return Self::manage_lease_period_start(lease_period); } } @@ -237,7 +237,7 @@ impl Pallet { let mut parachains = Vec::new(); for (para, mut lease_periods) in Leases::::iter() { if lease_periods.is_empty() { - continue + continue; } // ^^ should never be empty since we would have deleted the entry otherwise. @@ -312,10 +312,11 @@ impl Pallet { let mut tracker = alloc::collections::btree_map::BTreeMap::new(); Leases::::get(para).into_iter().for_each(|lease| match lease { Some((who, amount)) => match tracker.get(&who) { - Some(prev_amount) => + Some(prev_amount) => { if amount > *prev_amount { tracker.insert(who, amount); - }, + } + }, None => { tracker.insert(who, amount); }, @@ -381,7 +382,7 @@ impl Leaser> for Pallet { // attempt. // // We bail, not giving any lease and leave it for governance to sort out. - return Err(LeaseError::AlreadyLeased) + return Err(LeaseError::AlreadyLeased); } } else if d.len() == i { // Doesn't exist. This is usual. @@ -430,12 +431,13 @@ impl Leaser> for Pallet { Leases::::get(para) .into_iter() .map(|lease| match lease { - Some((who, amount)) => + Some((who, amount)) => { if &who == leaser { amount } else { Zero::zero() - }, + } + }, None => Zero::zero(), }) .max() @@ -488,7 +490,7 @@ impl Leaser> for Pallet { for slot in offset..=offset + period_count { if let Some(Some(_)) = leases.get(slot) { // If there exists any lease period, we exit early and return true. - return true + return true; } } @@ -1010,8 +1012,7 @@ mod benchmarking { } #[benchmarks( - where - T: paras::Config, + where T: paras::Config, )] mod benchmarks { @@ -1029,17 +1030,21 @@ mod benchmarking { let period_count = 3u32.into(); let origin = T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - - #[extrinsic_call] - _(origin as T::RuntimeOrigin, para, leaser.clone(), amount, period_begin, period_count); - assert_last_event::(Event::::Leased { - para_id: para, - leaser, period_begin, - period_count, - extra_reserved: amount, - total_amount: amount, - }.into()); + #[extrinsic_call] + _(origin as T::RuntimeOrigin, para, leaser.clone(), amount, period_begin, period_count); + + assert_last_event::( + Event::::Leased { + para_id: para, + leaser, + period_begin, + period_count, + extra_reserved: amount, + total_amount: amount, + } + .into(), + ); Ok(()) } @@ -1047,7 +1052,10 @@ mod benchmarking { // Worst case scenario, T on-demand parachains onboard, and C lease holding parachains offboard. // Assume reasonable maximum of 100 paras at any time #[benchmark] - fn manage_lease_period_start(c: Linear<0,100>, t : Linear<0,100>) -> Result<(), BenchmarkError> { + fn manage_lease_period_start( + c: Linear<0, 100>, + t: Linear<0, 100>, + ) -> Result<(), BenchmarkError> { let period_begin = 1u32.into(); let period_count = 4u32.into(); @@ -1055,9 +1063,7 @@ mod benchmarking { frame_system::Pallet::::set_block_number(T::LeaseOffset::get() + One::one()); // Make T parathreads (on-demand parachains) - let paras_info = (0..t).map(|i| { - register_a_parathread::(i) - }).collect::>(); + let paras_info = (0..t).map(|i| register_a_parathread::(i)).collect::>(); T::Registrar::execute_pending_transitions(); @@ -1072,29 +1078,31 @@ mod benchmarking { T::Registrar::execute_pending_transitions(); // C lease holding parachains are downgrading to on-demand parachains - for i in 200 .. 200 + c { - let (para, leaser) = register_a_parathread::(i); + for i in 200..200 + c { + let (para, _) = register_a_parathread::(i); T::Registrar::make_parachain(para)?; } T::Registrar::execute_pending_transitions(); - for i in 0 .. t { + for i in 0..t { assert!(T::Registrar::is_parathread(ParaId::from(i))); } - for i in 200 .. 200 + c { + for i in 200..200 + c { assert!(T::Registrar::is_parachain(ParaId::from(i))); } - #[extrinsic_call] - Slots::::manage_lease_period_start(period_begin); + #[block] + { + let _ = Slots::::manage_lease_period_start(period_begin); + } // All paras should have switched. T::Registrar::execute_pending_transitions(); - for i in 0 .. t { + for i in 0..t { assert!(T::Registrar::is_parachain(ParaId::from(i))); } - for i in 200 .. 200 + c { + for i in 200..200 + c { assert!(T::Registrar::is_parathread(ParaId::from(i))); } @@ -1111,7 +1119,7 @@ mod benchmarking { // If there is an offset, we need to be on that block to be able to do lease things. frame_system::Pallet::::set_block_number(T::LeaseOffset::get() + One::one()); - for i in 0 .. max_people { + for i in 0..max_people { let leaser = account("lease_deposit", i, 0); let amount = T::Currency::minimum_balance(); T::Currency::make_free_balance_be(&leaser, BalanceOf::::max_value()); @@ -1124,18 +1132,18 @@ mod benchmarking { Slots::::force_lease(origin, para, leaser, amount, period_begin, period_count)?; } - for i in 0 .. max_people { + for i in 0..max_people { let leaser = account("lease_deposit", i, 0); assert_eq!(T::Currency::reserved_balance(&leaser), T::Currency::minimum_balance()); } let origin = T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - - #[extrinsic_call] - _(origin as T::RuntimeOrigin, para); - for i in 0 .. max_people { + #[extrinsic_call] + _(origin as T::RuntimeOrigin, para); + + for i in 0..max_people { let leaser = account("lease_deposit", i, 0); assert_eq!(T::Currency::reserved_balance(&leaser), 0u32.into()); } @@ -1147,10 +1155,16 @@ mod benchmarking { fn trigger_onboard() -> Result<(), BenchmarkError> { // get a parachain into a bad state where they did not onboard let (para, _) = register_a_parathread::(1); - Leases::::insert(para, vec![Some((account::("lease_insert", 0, 0), BalanceOf::::default()))]); + Leases::::insert( + para, + vec![Some(( + account::("lease_insert", 0, 0), + BalanceOf::::default(), + ))], + ); assert!(T::Registrar::is_parathread(para)); let caller = whitelisted_caller(); - + #[extrinsic_call] _(RawOrigin::Signed(caller), para); From 86841fb33a5fe0cdccb04e2897ca37dd5afe7868 Mon Sep 17 00:00:00 2001 From: Krayt78 Date: Wed, 27 Nov 2024 16:06:39 +0100 Subject: [PATCH 4/4] nightly fmt --- polkadot/runtime/common/src/slots/mod.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/polkadot/runtime/common/src/slots/mod.rs b/polkadot/runtime/common/src/slots/mod.rs index 4fa0774bc3a4..b25959953de5 100644 --- a/polkadot/runtime/common/src/slots/mod.rs +++ b/polkadot/runtime/common/src/slots/mod.rs @@ -312,11 +312,10 @@ impl Pallet { let mut tracker = alloc::collections::btree_map::BTreeMap::new(); Leases::::get(para).into_iter().for_each(|lease| match lease { Some((who, amount)) => match tracker.get(&who) { - Some(prev_amount) => { + Some(prev_amount) => if amount > *prev_amount { tracker.insert(who, amount); - } - }, + }, None => { tracker.insert(who, amount); }, @@ -431,13 +430,12 @@ impl Leaser> for Pallet { Leases::::get(para) .into_iter() .map(|lease| match lease { - Some((who, amount)) => { + Some((who, amount)) => if &who == leaser { amount } else { Zero::zero() - } - }, + }, None => Zero::zero(), }) .max() @@ -1049,8 +1047,8 @@ mod benchmarking { Ok(()) } - // Worst case scenario, T on-demand parachains onboard, and C lease holding parachains offboard. - // Assume reasonable maximum of 100 paras at any time + // Worst case scenario, T on-demand parachains onboard, and C lease holding parachains + // offboard. Assume reasonable maximum of 100 paras at any time #[benchmark] fn manage_lease_period_start( c: Linear<0, 100>,