From a35c509fa70327bcfb16282e33cb51aa4e3c9a27 Mon Sep 17 00:00:00 2001 From: muharem Date: Fri, 20 Jan 2023 21:16:45 +0100 Subject: [PATCH 1/4] Update benchmark's successful origin api --- runtime/common/src/auctions.rs | 14 +++++++++----- runtime/common/src/slots/mod.rs | 14 +++++++++----- xcm/pallet-xcm/src/benchmarking.rs | 12 ++++++++---- 3 files changed, 26 insertions(+), 14 deletions(-) diff --git a/runtime/common/src/auctions.rs b/runtime/common/src/auctions.rs index aafc273e7138..d7f20dc8ce01 100644 --- a/runtime/common/src/auctions.rs +++ b/runtime/common/src/auctions.rs @@ -1730,7 +1730,7 @@ mod benchmarking { use frame_system::RawOrigin; use sp_runtime::{traits::Bounded, SaturatedConversion}; - use frame_benchmarking::{account, benchmarks, whitelisted_caller}; + use frame_benchmarking::{account, benchmarks, whitelisted_caller, BenchmarkError}; fn assert_last_event(generic_event: ::RuntimeEvent) { let events = frame_system::Pallet::::events(); @@ -1786,7 +1786,8 @@ mod benchmarking { new_auction { let duration = T::BlockNumber::max_value(); let lease_period_index = LeasePeriodOf::::max_value(); - let origin = T::InitiateOrigin::successful_origin(); + let origin = + T::InitiateOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; }: _(origin, duration, lease_period_index) verify { assert_last_event::(Event::::AuctionStarted { @@ -1805,7 +1806,8 @@ mod benchmarking { // Create a new auction let duration = T::BlockNumber::max_value(); let lease_period_index = LeasePeriodOf::::zero(); - let origin = T::InitiateOrigin::successful_origin(); + let origin = + T::InitiateOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; Auctions::::new_auction(origin, duration, lease_period_index)?; let para = ParaId::from(0); @@ -1857,7 +1859,8 @@ mod benchmarking { let duration: T::BlockNumber = lease_length / 2u32.into(); let lease_period_index = LeasePeriodOf::::zero(); let now = frame_system::Pallet::::block_number(); - let origin = T::InitiateOrigin::successful_origin(); + let origin = + T::InitiateOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; Auctions::::new_auction(origin, duration, lease_period_index)?; fill_winners::(lease_period_index); @@ -1901,7 +1904,8 @@ mod benchmarking { let duration: T::BlockNumber = lease_length / 2u32.into(); let lease_period_index = LeasePeriodOf::::zero(); let now = frame_system::Pallet::::block_number(); - let origin = T::InitiateOrigin::successful_origin(); + let origin = + T::InitiateOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; Auctions::::new_auction(origin, duration, lease_period_index)?; fill_winners::(lease_period_index); diff --git a/runtime/common/src/slots/mod.rs b/runtime/common/src/slots/mod.rs index e5e94a898d3e..d79b6c42dc09 100644 --- a/runtime/common/src/slots/mod.rs +++ b/runtime/common/src/slots/mod.rs @@ -987,7 +987,7 @@ mod benchmarking { use frame_system::RawOrigin; use sp_runtime::traits::{Bounded, One}; - use frame_benchmarking::{account, benchmarks, whitelisted_caller}; + use frame_benchmarking::{account, benchmarks, whitelisted_caller, BenchmarkError}; use crate::slots::Pallet as Slots; @@ -1027,7 +1027,8 @@ mod benchmarking { let amount = T::Currency::minimum_balance(); let period_begin = 69u32.into(); let period_count = 3u32.into(); - let origin = T::ForceOrigin::successful_origin(); + let origin = + T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; }: _(origin, para, leaser.clone(), amount, period_begin, period_count) verify { assert_last_event::(Event::::Leased { @@ -1061,7 +1062,8 @@ mod benchmarking { // T parathread are upgrading to parachains for (para, leaser) in paras_info { let amount = T::Currency::minimum_balance(); - let origin = T::ForceOrigin::successful_origin(); + let origin = + T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; Slots::::force_lease(origin, para, leaser, amount, period_begin, period_count)?; } @@ -1112,7 +1114,8 @@ mod benchmarking { // Average slot has 4 lease periods. let period_count: LeasePeriodOf = 4u32.into(); let period_begin = period_count * i.into(); - let origin = T::ForceOrigin::successful_origin(); + let origin = + T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; Slots::::force_lease(origin, para, leaser, amount, period_begin, period_count)?; } @@ -1121,7 +1124,8 @@ mod benchmarking { assert_eq!(T::Currency::reserved_balance(&leaser), T::Currency::minimum_balance()); } - let origin = T::ForceOrigin::successful_origin(); + let origin = + T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; }: _(origin, para) verify { for i in 0 .. max_people { diff --git a/xcm/pallet-xcm/src/benchmarking.rs b/xcm/pallet-xcm/src/benchmarking.rs index fff4b8b2ead4..7f0c5ddde1e1 100644 --- a/xcm/pallet-xcm/src/benchmarking.rs +++ b/xcm/pallet-xcm/src/benchmarking.rs @@ -26,7 +26,8 @@ type RuntimeOrigin = ::RuntimeOrigin; benchmarks! { send { - let send_origin = T::SendXcmOrigin::successful_origin(); + let send_origin = + T::SendXcmOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; if T::SendXcmOrigin::try_origin(send_origin.clone()).is_err() { return Err(BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX))) } @@ -40,7 +41,8 @@ benchmarks! { teleport_assets { let asset: MultiAsset = (Here, 10).into(); - let send_origin = T::ExecuteXcmOrigin::successful_origin(); + let send_origin = + T::ExecuteXcmOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let origin_location = T::ExecuteXcmOrigin::try_origin(send_origin.clone()) .map_err(|_| BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))?; if !T::XcmTeleportFilter::contains(&(origin_location, vec![asset.clone()])) { @@ -59,7 +61,8 @@ benchmarks! { reserve_transfer_assets { let asset: MultiAsset = (Here, 10).into(); - let send_origin = T::ExecuteXcmOrigin::successful_origin(); + let send_origin = + T::ExecuteXcmOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let origin_location = T::ExecuteXcmOrigin::try_origin(send_origin.clone()) .map_err(|_| BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))?; if !T::XcmReserveTransferFilter::contains(&(origin_location, vec![asset.clone()])) { @@ -77,7 +80,8 @@ benchmarks! { }: _>(send_origin, Box::new(versioned_dest), Box::new(versioned_beneficiary), Box::new(versioned_assets), 0) execute { - let execute_origin = T::ExecuteXcmOrigin::successful_origin(); + let execute_origin = + T::ExecuteXcmOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let origin_location = T::ExecuteXcmOrigin::try_origin(execute_origin.clone()) .map_err(|_| BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))?; let msg = Xcm(vec![ClearOrigin]); From 002b0167d880d047c105c4b0b0e41c4f4a56886d Mon Sep 17 00:00:00 2001 From: muharem Date: Sun, 22 Jan 2023 19:23:20 +0100 Subject: [PATCH 2/4] rustfmt --- runtime/common/src/auctions.rs | 8 ++++---- runtime/common/src/slots/mod.rs | 8 ++++---- xcm/pallet-xcm/src/benchmarking.rs | 8 ++++---- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/runtime/common/src/auctions.rs b/runtime/common/src/auctions.rs index d7f20dc8ce01..b2bfb7a49411 100644 --- a/runtime/common/src/auctions.rs +++ b/runtime/common/src/auctions.rs @@ -1786,7 +1786,7 @@ mod benchmarking { new_auction { let duration = T::BlockNumber::max_value(); let lease_period_index = LeasePeriodOf::::max_value(); - let origin = + let origin = T::InitiateOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; }: _(origin, duration, lease_period_index) verify { @@ -1806,7 +1806,7 @@ mod benchmarking { // Create a new auction let duration = T::BlockNumber::max_value(); let lease_period_index = LeasePeriodOf::::zero(); - let origin = + let origin = T::InitiateOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; Auctions::::new_auction(origin, duration, lease_period_index)?; @@ -1859,7 +1859,7 @@ mod benchmarking { let duration: T::BlockNumber = lease_length / 2u32.into(); let lease_period_index = LeasePeriodOf::::zero(); let now = frame_system::Pallet::::block_number(); - let origin = + let origin = T::InitiateOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; Auctions::::new_auction(origin, duration, lease_period_index)?; @@ -1904,7 +1904,7 @@ mod benchmarking { let duration: T::BlockNumber = lease_length / 2u32.into(); let lease_period_index = LeasePeriodOf::::zero(); let now = frame_system::Pallet::::block_number(); - let origin = + let origin = T::InitiateOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; Auctions::::new_auction(origin, duration, lease_period_index)?; diff --git a/runtime/common/src/slots/mod.rs b/runtime/common/src/slots/mod.rs index d79b6c42dc09..045cc5d50df4 100644 --- a/runtime/common/src/slots/mod.rs +++ b/runtime/common/src/slots/mod.rs @@ -1027,7 +1027,7 @@ mod benchmarking { let amount = T::Currency::minimum_balance(); let period_begin = 69u32.into(); let period_count = 3u32.into(); - let origin = + let origin = T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; }: _(origin, para, leaser.clone(), amount, period_begin, period_count) verify { @@ -1062,7 +1062,7 @@ mod benchmarking { // T parathread are upgrading to parachains for (para, leaser) in paras_info { let amount = T::Currency::minimum_balance(); - let origin = + let origin = T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; Slots::::force_lease(origin, para, leaser, amount, period_begin, period_count)?; } @@ -1114,7 +1114,7 @@ mod benchmarking { // Average slot has 4 lease periods. let period_count: LeasePeriodOf = 4u32.into(); let period_begin = period_count * i.into(); - let origin = + let origin = T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; Slots::::force_lease(origin, para, leaser, amount, period_begin, period_count)?; } @@ -1124,7 +1124,7 @@ mod benchmarking { assert_eq!(T::Currency::reserved_balance(&leaser), T::Currency::minimum_balance()); } - let origin = + let origin = T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; }: _(origin, para) verify { diff --git a/xcm/pallet-xcm/src/benchmarking.rs b/xcm/pallet-xcm/src/benchmarking.rs index 7f0c5ddde1e1..3809965d34eb 100644 --- a/xcm/pallet-xcm/src/benchmarking.rs +++ b/xcm/pallet-xcm/src/benchmarking.rs @@ -26,7 +26,7 @@ type RuntimeOrigin = ::RuntimeOrigin; benchmarks! { send { - let send_origin = + let send_origin = T::SendXcmOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; if T::SendXcmOrigin::try_origin(send_origin.clone()).is_err() { return Err(BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX))) @@ -41,7 +41,7 @@ benchmarks! { teleport_assets { let asset: MultiAsset = (Here, 10).into(); - let send_origin = + let send_origin = T::ExecuteXcmOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let origin_location = T::ExecuteXcmOrigin::try_origin(send_origin.clone()) .map_err(|_| BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))?; @@ -61,7 +61,7 @@ benchmarks! { reserve_transfer_assets { let asset: MultiAsset = (Here, 10).into(); - let send_origin = + let send_origin = T::ExecuteXcmOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let origin_location = T::ExecuteXcmOrigin::try_origin(send_origin.clone()) .map_err(|_| BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))?; @@ -80,7 +80,7 @@ benchmarks! { }: _>(send_origin, Box::new(versioned_dest), Box::new(versioned_beneficiary), Box::new(versioned_assets), 0) execute { - let execute_origin = + let execute_origin = T::ExecuteXcmOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; let origin_location = T::ExecuteXcmOrigin::try_origin(execute_origin.clone()) .map_err(|_| BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))?; From ef72c96f19004fa089bae8049b8c91b6b86d56dc Mon Sep 17 00:00:00 2001 From: muharem Date: Mon, 23 Jan 2023 13:26:30 +0100 Subject: [PATCH 3/4] unwrap for indirect origin dep --- runtime/common/src/auctions.rs | 9 +++------ runtime/common/src/slots/mod.rs | 6 ++---- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/runtime/common/src/auctions.rs b/runtime/common/src/auctions.rs index b2bfb7a49411..772ea041fb15 100644 --- a/runtime/common/src/auctions.rs +++ b/runtime/common/src/auctions.rs @@ -1806,8 +1806,7 @@ mod benchmarking { // Create a new auction let duration = T::BlockNumber::max_value(); let lease_period_index = LeasePeriodOf::::zero(); - let origin = - T::InitiateOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; + let origin = T::InitiateOrigin::try_successful_origin().unwrap(); Auctions::::new_auction(origin, duration, lease_period_index)?; let para = ParaId::from(0); @@ -1859,8 +1858,7 @@ mod benchmarking { let duration: T::BlockNumber = lease_length / 2u32.into(); let lease_period_index = LeasePeriodOf::::zero(); let now = frame_system::Pallet::::block_number(); - let origin = - T::InitiateOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; + let origin = T::InitiateOrigin::try_successful_origin().unwrap(); Auctions::::new_auction(origin, duration, lease_period_index)?; fill_winners::(lease_period_index); @@ -1904,8 +1902,7 @@ mod benchmarking { let duration: T::BlockNumber = lease_length / 2u32.into(); let lease_period_index = LeasePeriodOf::::zero(); let now = frame_system::Pallet::::block_number(); - let origin = - T::InitiateOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; + let origin = T::InitiateOrigin::try_successful_origin().unwrap(); Auctions::::new_auction(origin, duration, lease_period_index)?; fill_winners::(lease_period_index); diff --git a/runtime/common/src/slots/mod.rs b/runtime/common/src/slots/mod.rs index 045cc5d50df4..a93c28c3fb4c 100644 --- a/runtime/common/src/slots/mod.rs +++ b/runtime/common/src/slots/mod.rs @@ -1062,8 +1062,7 @@ mod benchmarking { // T parathread are upgrading to parachains for (para, leaser) in paras_info { let amount = T::Currency::minimum_balance(); - let origin = - T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; + let origin = T::ForceOrigin::try_successful_origin().unwrap(); Slots::::force_lease(origin, para, leaser, amount, period_begin, period_count)?; } @@ -1114,8 +1113,7 @@ mod benchmarking { // Average slot has 4 lease periods. let period_count: LeasePeriodOf = 4u32.into(); let period_begin = period_count * i.into(); - let origin = - T::ForceOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; + let origin = T::ForceOrigin::try_successful_origin().unwrap(); Slots::::force_lease(origin, para, leaser, amount, period_begin, period_count)?; } From b642aada0717b7206e2d308c64b9d23d4b96d0e7 Mon Sep 17 00:00:00 2001 From: muharem Date: Tue, 24 Jan 2023 12:29:05 +0100 Subject: [PATCH 4/4] replace unwrap by expect with a message --- runtime/common/src/auctions.rs | 9 ++++++--- runtime/common/src/slots/mod.rs | 6 ++++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/runtime/common/src/auctions.rs b/runtime/common/src/auctions.rs index 772ea041fb15..3a6951b6fc79 100644 --- a/runtime/common/src/auctions.rs +++ b/runtime/common/src/auctions.rs @@ -1806,7 +1806,8 @@ mod benchmarking { // Create a new auction let duration = T::BlockNumber::max_value(); let lease_period_index = LeasePeriodOf::::zero(); - let origin = T::InitiateOrigin::try_successful_origin().unwrap(); + let origin = T::InitiateOrigin::try_successful_origin() + .expect("InitiateOrigin has no successful origin required for the benchmark"); Auctions::::new_auction(origin, duration, lease_period_index)?; let para = ParaId::from(0); @@ -1858,7 +1859,8 @@ mod benchmarking { let duration: T::BlockNumber = lease_length / 2u32.into(); let lease_period_index = LeasePeriodOf::::zero(); let now = frame_system::Pallet::::block_number(); - let origin = T::InitiateOrigin::try_successful_origin().unwrap(); + let origin = T::InitiateOrigin::try_successful_origin() + .expect("InitiateOrigin has no successful origin required for the benchmark"); Auctions::::new_auction(origin, duration, lease_period_index)?; fill_winners::(lease_period_index); @@ -1902,7 +1904,8 @@ mod benchmarking { let duration: T::BlockNumber = lease_length / 2u32.into(); let lease_period_index = LeasePeriodOf::::zero(); let now = frame_system::Pallet::::block_number(); - let origin = T::InitiateOrigin::try_successful_origin().unwrap(); + let origin = T::InitiateOrigin::try_successful_origin() + .expect("InitiateOrigin has no successful origin required for the benchmark"); Auctions::::new_auction(origin, duration, lease_period_index)?; fill_winners::(lease_period_index); diff --git a/runtime/common/src/slots/mod.rs b/runtime/common/src/slots/mod.rs index a93c28c3fb4c..23781cd89338 100644 --- a/runtime/common/src/slots/mod.rs +++ b/runtime/common/src/slots/mod.rs @@ -1062,7 +1062,8 @@ mod benchmarking { // T parathread are upgrading to parachains for (para, leaser) in paras_info { let amount = T::Currency::minimum_balance(); - let origin = T::ForceOrigin::try_successful_origin().unwrap(); + let origin = T::ForceOrigin::try_successful_origin() + .expect("ForceOrigin has no successful origin required for the benchmark"); Slots::::force_lease(origin, para, leaser, amount, period_begin, period_count)?; } @@ -1113,7 +1114,8 @@ mod benchmarking { // Average slot has 4 lease periods. let period_count: LeasePeriodOf = 4u32.into(); let period_begin = period_count * i.into(); - let origin = T::ForceOrigin::try_successful_origin().unwrap(); + let origin = T::ForceOrigin::try_successful_origin() + .expect("ForceOrigin has no successful origin required for the benchmark"); Slots::::force_lease(origin, para, leaser, amount, period_begin, period_count)?; }