From aa9e2caf5c6e3640c1f67e8b52c075a20fcb18ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=B3nal=20Murray?= Date: Wed, 20 Sep 2023 12:40:29 +0100 Subject: [PATCH 1/7] Update HRMP pallet benchmarking to use benchmarks v2 --- .../parachains/src/hrmp/benchmarking.rs | 124 +++++++++++------- 1 file changed, 80 insertions(+), 44 deletions(-) diff --git a/polkadot/runtime/parachains/src/hrmp/benchmarking.rs b/polkadot/runtime/parachains/src/hrmp/benchmarking.rs index 9ffa264a7dc8..46f798f0cbab 100644 --- a/polkadot/runtime/parachains/src/hrmp/benchmarking.rs +++ b/polkadot/runtime/parachains/src/hrmp/benchmarking.rs @@ -14,12 +14,15 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . +#![cfg(feature = "runtime-benchmarks")] + use crate::{ configuration::Pallet as Configuration, hrmp::{Pallet as Hrmp, *}, paras::{Pallet as Paras, ParaKind, ParachainsCache}, shared::Pallet as Shared, }; +use frame_benchmarking::v2::*; use frame_support::{assert_ok, traits::Currency}; type BalanceOf = @@ -138,10 +141,12 @@ static_assertions::const_assert!(MAX_UNIQUE_CHANNELS < PREFIX_0); static_assertions::const_assert!(HRMP_MAX_INBOUND_CHANNELS_BOUND < PREFIX_0); static_assertions::const_assert!(HRMP_MAX_OUTBOUND_CHANNELS_BOUND < PREFIX_0); -frame_benchmarking::benchmarks! { - where_clause { where ::RuntimeOrigin: From } +#[benchmarks(where ::RuntimeOrigin: From)] +mod benchmarks { + use super::*; - hrmp_init_open_channel { + #[benchmark] + fn hrmp_init_open_channel () { let sender_id: ParaId = 1u32.into(); let sender_origin: crate::Origin = 1u32.into(); @@ -155,38 +160,47 @@ frame_benchmarking::benchmarks! { let capacity = Configuration::::config().hrmp_channel_max_capacity; let message_size = Configuration::::config().hrmp_channel_max_message_size; - }: _(sender_origin, recipient_id, capacity, message_size) - verify { + + #[extrinsic_call] + _(sender_origin, recipient_id, capacity, message_size); + assert_last_event::( Event::::OpenChannelRequested(sender_id, recipient_id, capacity, message_size).into() ); } - hrmp_accept_open_channel { + #[benchmark] + fn hrmp_accept_open_channel () { let [(sender, _), (recipient, recipient_origin)] = establish_para_connection::(1, 2, ParachainSetupStep::Requested); - }: _(recipient_origin, sender) - verify { + + #[extrinsic_call] + _(recipient_origin, sender); + assert_last_event::(Event::::OpenChannelAccepted(sender, recipient).into()); } - hrmp_close_channel { + #[benchmark] + fn hrmp_close_channel () { let [(sender, sender_origin), (recipient, _)] = establish_para_connection::(1, 2, ParachainSetupStep::Established); let channel_id = HrmpChannelId { sender, recipient }; - }: _(sender_origin, channel_id.clone()) - verify { + + #[extrinsic_call] + _(sender_origin, channel_id.clone()); + assert_last_event::(Event::::ChannelClosed(sender, channel_id).into()); } // NOTE: a single parachain should have the maximum number of allowed ingress and egress // channels. - force_clean_hrmp { + #[benchmark] + fn force_clean_hrmp ( // ingress channels to a single leaving parachain that need to be closed. - let i in 0 .. (HRMP_MAX_INBOUND_CHANNELS_BOUND - 1); + i: Linear<0, { HRMP_MAX_INBOUND_CHANNELS_BOUND - 1 }>, // egress channels to a single leaving parachain that need to be closed. - let e in 0 .. (HRMP_MAX_OUTBOUND_CHANNELS_BOUND - 1); - + e: Linear<0, { HRMP_MAX_OUTBOUND_CHANNELS_BOUND - 1 }> + ) { // first, update the configs to support this many open channels... assert_ok!( Configuration::::set_hrmp_max_parachain_outbound_channels(frame_system::RawOrigin::Root.into(), e + 1) @@ -225,7 +239,10 @@ frame_benchmarking::benchmarks! { // all in all, we have created this many channels. assert_eq!(HrmpChannels::::iter().count() as u32, i + e); - }: _(frame_system::Origin::::Root, para, i, e) verify { + + #[extrinsic_call] + _(frame_system::Origin::::Root, para, i, e); + // all in all, all of them must be gone by now. assert_eq!(HrmpChannels::::iter().count() as u32, 0); // borrow this function from the tests to make sure state is clear, given that we do a lot @@ -233,40 +250,47 @@ frame_benchmarking::benchmarks! { Hrmp::::assert_storage_consistency_exhaustive(); } - force_process_hrmp_open { + #[benchmark] + fn force_process_hrmp_open ( // number of channels that need to be processed. Worse case is an N-M relation: unique // sender and recipients for all channels. - let c in 0 .. MAX_UNIQUE_CHANNELS; - + c: Linear<0, MAX_UNIQUE_CHANNELS> + ) { for id in 0 .. c { let _ = establish_para_connection::(PREFIX_0 + id, PREFIX_1 + id, ParachainSetupStep::Accepted); } assert_eq!(HrmpOpenChannelRequestsList::::decode_len().unwrap_or_default() as u32, c); - }: _(frame_system::Origin::::Root, c) - verify { + + #[extrinsic_call] + _(frame_system::Origin::::Root, c); + assert_eq!(HrmpOpenChannelRequestsList::::decode_len().unwrap_or_default() as u32, 0); } - force_process_hrmp_close { + #[benchmark] + fn force_process_hrmp_close ( // number of channels that need to be processed. Worse case is an N-M relation: unique // sender and recipients for all channels. - let c in 0 .. MAX_UNIQUE_CHANNELS; - + c: Linear<0, MAX_UNIQUE_CHANNELS> + ) { for id in 0 .. c { let _ = establish_para_connection::(PREFIX_0 + id, PREFIX_1 + id, ParachainSetupStep::CloseRequested); } assert_eq!(HrmpCloseChannelRequestsList::::decode_len().unwrap_or_default() as u32, c); - }: _(frame_system::Origin::::Root, c) - verify { + + #[extrinsic_call] + _(frame_system::Origin::::Root, c); + assert_eq!(HrmpCloseChannelRequestsList::::decode_len().unwrap_or_default() as u32, 0); } - hrmp_cancel_open_request { + #[benchmark] + fn hrmp_cancel_open_request ( // number of items already existing in the `HrmpOpenChannelRequestsList`, other than the // one that we remove. - let c in 0 .. MAX_UNIQUE_CHANNELS; - + c: Linear<0, MAX_UNIQUE_CHANNELS> + ) { for id in 0 .. c { let _ = establish_para_connection::(PREFIX_0 + id, PREFIX_1 + id, ParachainSetupStep::Requested); } @@ -275,16 +299,18 @@ frame_benchmarking::benchmarks! { establish_para_connection::(1, 2, ParachainSetupStep::Requested); assert_eq!(HrmpOpenChannelRequestsList::::decode_len().unwrap_or_default() as u32, c + 1); let channel_id = HrmpChannelId { sender, recipient }; - }: _(sender_origin, channel_id, c + 1) - verify { + + #[extrinsic_call] + _(sender_origin, channel_id, c + 1); + assert_eq!(HrmpOpenChannelRequestsList::::decode_len().unwrap_or_default() as u32, c); } // worse case will be `n` parachain channel requests, where in all of them either the sender or // the recipient need to be cleaned. This enforces the deposit of at least one to be processed. // No code path for triggering two deposit process exists. - clean_open_channel_requests { - let c in 0 .. MAX_UNIQUE_CHANNELS; + #[benchmark] + fn clean_open_channel_requests (c: Linear<0, MAX_UNIQUE_CHANNELS>) { for id in 0 .. c { let _ = establish_para_connection::(PREFIX_0 + id, PREFIX_1 + id, ParachainSetupStep::Requested); @@ -293,13 +319,15 @@ frame_benchmarking::benchmarks! { assert_eq!(HrmpOpenChannelRequestsList::::decode_len().unwrap_or_default() as u32, c); let outgoing = (0..c).map(|id| (id + PREFIX_1).into()).collect::>(); let config = Configuration::::config(); - }: { + + #[extrinsic_call] Hrmp::::clean_open_channel_requests(&config, &outgoing); - } verify { + assert_eq!(HrmpOpenChannelRequestsList::::decode_len().unwrap_or_default() as u32, 0); } - force_open_hrmp_channel { + #[benchmark] + fn force_open_hrmp_channel () { let sender_id: ParaId = 1u32.into(); let sender_origin: crate::Origin = 1u32.into(); let recipient_id: ParaId = 2u32.into(); @@ -344,14 +372,17 @@ frame_benchmarking::benchmarks! { // but the _channel_ should not exist assert!(HrmpChannels::::get(&channel_id).is_none()); - }: _(frame_system::Origin::::Root, sender_id, recipient_id, capacity, message_size) - verify { + + #[extrinsic_call] + _(frame_system::Origin::::Root, sender_id, recipient_id, capacity, message_size); + assert_last_event::( Event::::HrmpChannelForceOpened(sender_id, recipient_id, capacity, message_size).into() ); } - establish_system_channel { + #[benchmark] + fn establish_system_channel () { let sender_id: ParaId = 1u32.into(); let recipient_id: ParaId = 2u32.into(); @@ -364,14 +395,17 @@ frame_benchmarking::benchmarks! { let capacity = config.hrmp_channel_max_capacity; let message_size = config.hrmp_channel_max_message_size; - }: _(frame_system::RawOrigin::Signed(caller), sender_id, recipient_id) - verify { + + #[extrinsic_call] + _(frame_system::RawOrigin::Signed(caller), sender_id, recipient_id); + assert_last_event::( Event::::HrmpSystemChannelOpened(sender_id, recipient_id, capacity, message_size).into() ); } - poke_channel_deposits { + #[benchmark] + fn poke_channel_deposits () { let sender_id: ParaId = 1u32.into(); let recipient_id: ParaId = 2u32.into(); let channel_id = HrmpChannelId {sender: sender_id, recipient: recipient_id }; @@ -403,8 +437,10 @@ frame_benchmarking::benchmarks! { // Actually reserve the deposits. let _ = T::Currency::reserve(&sender_id.into_account_truncating(), sender_deposit); let _ = T::Currency::reserve(&recipient_id.into_account_truncating(), recipient_deposit); - }: _(frame_system::RawOrigin::Signed(caller), sender_id, recipient_id) - verify { + + #[extrinsic_call] + _(frame_system::RawOrigin::Signed(caller), sender_id, recipient_id); + assert_last_event::( Event::::OpenChannelDepositsUpdated(sender_id, recipient_id).into() ); From ec1153a3710d73f29a4e889372ed9f967bfd62cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=B3nal=20Murray?= Date: Thu, 21 Sep 2023 09:15:58 +0100 Subject: [PATCH 2/7] Replace extrinsic_call with block for clean_open_channel_requests --- polkadot/runtime/parachains/src/hrmp/benchmarking.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/polkadot/runtime/parachains/src/hrmp/benchmarking.rs b/polkadot/runtime/parachains/src/hrmp/benchmarking.rs index 46f798f0cbab..86c63b3fc906 100644 --- a/polkadot/runtime/parachains/src/hrmp/benchmarking.rs +++ b/polkadot/runtime/parachains/src/hrmp/benchmarking.rs @@ -320,8 +320,10 @@ mod benchmarks { let outgoing = (0..c).map(|id| (id + PREFIX_1).into()).collect::>(); let config = Configuration::::config(); - #[extrinsic_call] - Hrmp::::clean_open_channel_requests(&config, &outgoing); + #[block] + { + Hrmp::::clean_open_channel_requests(&config, &outgoing); + } assert_eq!(HrmpOpenChannelRequestsList::::decode_len().unwrap_or_default() as u32, 0); } From 23d64918be3ca68413050383c5d8a73f63d451dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=B3nal=20Murray?= Date: Fri, 22 Sep 2023 11:46:00 +0100 Subject: [PATCH 3/7] Fix downward message size issue in HRMP benchmark Increase max_downward_message_size to fix force_clean_hrmp benchmark issue. Increased to 1024. Previously it was inheriting the tests config of 8, which was too small for the message struct. 1KiB randomly chosen as a reasonable number which won't present issues for small changes to the message format. --- polkadot/runtime/parachains/src/hrmp/benchmarking.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/polkadot/runtime/parachains/src/hrmp/benchmarking.rs b/polkadot/runtime/parachains/src/hrmp/benchmarking.rs index 86c63b3fc906..26a7de662535 100644 --- a/polkadot/runtime/parachains/src/hrmp/benchmarking.rs +++ b/polkadot/runtime/parachains/src/hrmp/benchmarking.rs @@ -208,6 +208,9 @@ mod benchmarks { assert_ok!( Configuration::::set_hrmp_max_parachain_inbound_channels(frame_system::RawOrigin::Root.into(), i + 1) ); + assert_ok!( + Configuration::::set_max_downward_message_size(frame_system::RawOrigin::Root.into(), 1024) + ); // .. and enact it. Configuration::::initializer_on_new_session(&Shared::::scheduled_session()); From 070443e83009bb4faf4fb052679ad04edf1223ad Mon Sep 17 00:00:00 2001 From: command-bot <> Date: Fri, 22 Sep 2023 11:25:24 +0000 Subject: [PATCH 4/7] ".git/.scripts/commands/fmt/fmt.sh" --- .../parachains/src/hrmp/benchmarking.rs | 127 ++++++++++++------ 1 file changed, 83 insertions(+), 44 deletions(-) diff --git a/polkadot/runtime/parachains/src/hrmp/benchmarking.rs b/polkadot/runtime/parachains/src/hrmp/benchmarking.rs index 26a7de662535..c23c5ee85df9 100644 --- a/polkadot/runtime/parachains/src/hrmp/benchmarking.rs +++ b/polkadot/runtime/parachains/src/hrmp/benchmarking.rs @@ -146,7 +146,7 @@ mod benchmarks { use super::*; #[benchmark] - fn hrmp_init_open_channel () { + fn hrmp_init_open_channel() { let sender_id: ParaId = 1u32.into(); let sender_origin: crate::Origin = 1u32.into(); @@ -154,7 +154,8 @@ mod benchmarks { // make sure para is registered, and has enough balance. let ed = T::Currency::minimum_balance(); - let deposit: BalanceOf = Configuration::::config().hrmp_sender_deposit.unique_saturated_into(); + let deposit: BalanceOf = + Configuration::::config().hrmp_sender_deposit.unique_saturated_into(); register_parachain_with_balance::(sender_id, deposit + ed); register_parachain_with_balance::(recipient_id, deposit + ed); @@ -165,12 +166,13 @@ mod benchmarks { _(sender_origin, recipient_id, capacity, message_size); assert_last_event::( - Event::::OpenChannelRequested(sender_id, recipient_id, capacity, message_size).into() + Event::::OpenChannelRequested(sender_id, recipient_id, capacity, message_size) + .into(), ); } #[benchmark] - fn hrmp_accept_open_channel () { + fn hrmp_accept_open_channel() { let [(sender, _), (recipient, recipient_origin)] = establish_para_connection::(1, 2, ParachainSetupStep::Requested); @@ -181,7 +183,7 @@ mod benchmarks { } #[benchmark] - fn hrmp_close_channel () { + fn hrmp_close_channel() { let [(sender, sender_origin), (recipient, _)] = establish_para_connection::(1, 2, ParachainSetupStep::Established); let channel_id = HrmpChannelId { sender, recipient }; @@ -195,22 +197,25 @@ mod benchmarks { // NOTE: a single parachain should have the maximum number of allowed ingress and egress // channels. #[benchmark] - fn force_clean_hrmp ( + fn force_clean_hrmp( // ingress channels to a single leaving parachain that need to be closed. i: Linear<0, { HRMP_MAX_INBOUND_CHANNELS_BOUND - 1 }>, // egress channels to a single leaving parachain that need to be closed. - e: Linear<0, { HRMP_MAX_OUTBOUND_CHANNELS_BOUND - 1 }> + e: Linear<0, { HRMP_MAX_OUTBOUND_CHANNELS_BOUND - 1 }>, ) { // first, update the configs to support this many open channels... - assert_ok!( - Configuration::::set_hrmp_max_parachain_outbound_channels(frame_system::RawOrigin::Root.into(), e + 1) - ); - assert_ok!( - Configuration::::set_hrmp_max_parachain_inbound_channels(frame_system::RawOrigin::Root.into(), i + 1) - ); - assert_ok!( - Configuration::::set_max_downward_message_size(frame_system::RawOrigin::Root.into(), 1024) - ); + assert_ok!(Configuration::::set_hrmp_max_parachain_outbound_channels( + frame_system::RawOrigin::Root.into(), + e + 1 + )); + assert_ok!(Configuration::::set_hrmp_max_parachain_inbound_channels( + frame_system::RawOrigin::Root.into(), + i + 1 + )); + assert_ok!(Configuration::::set_max_downward_message_size( + frame_system::RawOrigin::Root.into(), + 1024 + )); // .. and enact it. Configuration::::initializer_on_new_session(&Shared::::scheduled_session()); @@ -225,7 +230,11 @@ mod benchmarks { for ingress_para_id in 0..i { // establish ingress channels to `para`. let ingress_para_id = ingress_para_id + PREFIX_0; - let _ = establish_para_connection::(ingress_para_id, para.into(), ParachainSetupStep::Established); + let _ = establish_para_connection::( + ingress_para_id, + para.into(), + ParachainSetupStep::Established, + ); } // nothing should be left unprocessed. @@ -234,7 +243,11 @@ mod benchmarks { for egress_para_id in 0..e { // establish egress channels to `para`. let egress_para_id = egress_para_id + PREFIX_1; - let _ = establish_para_connection::(para.into(), egress_para_id, ParachainSetupStep::Established); + let _ = establish_para_connection::( + para.into(), + egress_para_id, + ParachainSetupStep::Established, + ); } // nothing should be left unprocessed. @@ -254,13 +267,17 @@ mod benchmarks { } #[benchmark] - fn force_process_hrmp_open ( + fn force_process_hrmp_open( // number of channels that need to be processed. Worse case is an N-M relation: unique // sender and recipients for all channels. - c: Linear<0, MAX_UNIQUE_CHANNELS> + c: Linear<0, MAX_UNIQUE_CHANNELS>, ) { - for id in 0 .. c { - let _ = establish_para_connection::(PREFIX_0 + id, PREFIX_1 + id, ParachainSetupStep::Accepted); + for id in 0..c { + let _ = establish_para_connection::( + PREFIX_0 + id, + PREFIX_1 + id, + ParachainSetupStep::Accepted, + ); } assert_eq!(HrmpOpenChannelRequestsList::::decode_len().unwrap_or_default() as u32, c); @@ -271,13 +288,17 @@ mod benchmarks { } #[benchmark] - fn force_process_hrmp_close ( + fn force_process_hrmp_close( // number of channels that need to be processed. Worse case is an N-M relation: unique // sender and recipients for all channels. - c: Linear<0, MAX_UNIQUE_CHANNELS> + c: Linear<0, MAX_UNIQUE_CHANNELS>, ) { - for id in 0 .. c { - let _ = establish_para_connection::(PREFIX_0 + id, PREFIX_1 + id, ParachainSetupStep::CloseRequested); + for id in 0..c { + let _ = establish_para_connection::( + PREFIX_0 + id, + PREFIX_1 + id, + ParachainSetupStep::CloseRequested, + ); } assert_eq!(HrmpCloseChannelRequestsList::::decode_len().unwrap_or_default() as u32, c); @@ -289,18 +310,25 @@ mod benchmarks { } #[benchmark] - fn hrmp_cancel_open_request ( + fn hrmp_cancel_open_request( // number of items already existing in the `HrmpOpenChannelRequestsList`, other than the // one that we remove. - c: Linear<0, MAX_UNIQUE_CHANNELS> + c: Linear<0, MAX_UNIQUE_CHANNELS>, ) { - for id in 0 .. c { - let _ = establish_para_connection::(PREFIX_0 + id, PREFIX_1 + id, ParachainSetupStep::Requested); + for id in 0..c { + let _ = establish_para_connection::( + PREFIX_0 + id, + PREFIX_1 + id, + ParachainSetupStep::Requested, + ); } let [(sender, sender_origin), (recipient, _)] = establish_para_connection::(1, 2, ParachainSetupStep::Requested); - assert_eq!(HrmpOpenChannelRequestsList::::decode_len().unwrap_or_default() as u32, c + 1); + assert_eq!( + HrmpOpenChannelRequestsList::::decode_len().unwrap_or_default() as u32, + c + 1 + ); let channel_id = HrmpChannelId { sender, recipient }; #[extrinsic_call] @@ -313,10 +341,13 @@ mod benchmarks { // the recipient need to be cleaned. This enforces the deposit of at least one to be processed. // No code path for triggering two deposit process exists. #[benchmark] - fn clean_open_channel_requests (c: Linear<0, MAX_UNIQUE_CHANNELS>) { - - for id in 0 .. c { - let _ = establish_para_connection::(PREFIX_0 + id, PREFIX_1 + id, ParachainSetupStep::Requested); + fn clean_open_channel_requests(c: Linear<0, MAX_UNIQUE_CHANNELS>) { + for id in 0..c { + let _ = establish_para_connection::( + PREFIX_0 + id, + PREFIX_1 + id, + ParachainSetupStep::Requested, + ); } assert_eq!(HrmpOpenChannelRequestsList::::decode_len().unwrap_or_default() as u32, c); @@ -332,7 +363,7 @@ mod benchmarks { } #[benchmark] - fn force_open_hrmp_channel () { + fn force_open_hrmp_channel() { let sender_id: ParaId = 1u32.into(); let sender_origin: crate::Origin = 1u32.into(); let recipient_id: ParaId = 2u32.into(); @@ -382,12 +413,13 @@ mod benchmarks { _(frame_system::Origin::::Root, sender_id, recipient_id, capacity, message_size); assert_last_event::( - Event::::HrmpChannelForceOpened(sender_id, recipient_id, capacity, message_size).into() + Event::::HrmpChannelForceOpened(sender_id, recipient_id, capacity, message_size) + .into(), ); } #[benchmark] - fn establish_system_channel () { + fn establish_system_channel() { let sender_id: ParaId = 1u32.into(); let recipient_id: ParaId = 2u32.into(); @@ -405,15 +437,16 @@ mod benchmarks { _(frame_system::RawOrigin::Signed(caller), sender_id, recipient_id); assert_last_event::( - Event::::HrmpSystemChannelOpened(sender_id, recipient_id, capacity, message_size).into() + Event::::HrmpSystemChannelOpened(sender_id, recipient_id, capacity, message_size) + .into(), ); } #[benchmark] - fn poke_channel_deposits () { + fn poke_channel_deposits() { let sender_id: ParaId = 1u32.into(); let recipient_id: ParaId = 2u32.into(); - let channel_id = HrmpChannelId {sender: sender_id, recipient: recipient_id }; + let channel_id = HrmpChannelId { sender: sender_id, recipient: recipient_id }; let caller: T::AccountId = frame_benchmarking::whitelisted_caller(); let config = Configuration::::config(); @@ -447,15 +480,21 @@ mod benchmarks { _(frame_system::RawOrigin::Signed(caller), sender_id, recipient_id); assert_last_event::( - Event::::OpenChannelDepositsUpdated(sender_id, recipient_id).into() + Event::::OpenChannelDepositsUpdated(sender_id, recipient_id).into(), ); let channel = HrmpChannels::::get(&channel_id).unwrap(); // Check that the deposit was updated in the channel state. assert_eq!(channel.sender_deposit, 0); assert_eq!(channel.recipient_deposit, 0); // And that the funds were unreserved. - assert_eq!(T::Currency::reserved_balance(&sender_id.into_account_truncating()), 0u128.unique_saturated_into()); - assert_eq!(T::Currency::reserved_balance(&recipient_id.into_account_truncating()), 0u128.unique_saturated_into()); + assert_eq!( + T::Currency::reserved_balance(&sender_id.into_account_truncating()), + 0u128.unique_saturated_into() + ); + assert_eq!( + T::Currency::reserved_balance(&recipient_id.into_account_truncating()), + 0u128.unique_saturated_into() + ); } } From 8e22f9f9373e06cb7abf87ea004ab8b3df92dc0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=B3nal=20Murray?= Date: Fri, 22 Sep 2023 13:03:53 +0100 Subject: [PATCH 5/7] Move missed weight param into function signature --- .../parachains/src/hrmp/benchmarking.rs | 43 +++++++++---------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/polkadot/runtime/parachains/src/hrmp/benchmarking.rs b/polkadot/runtime/parachains/src/hrmp/benchmarking.rs index c23c5ee85df9..085c4a553b8e 100644 --- a/polkadot/runtime/parachains/src/hrmp/benchmarking.rs +++ b/polkadot/runtime/parachains/src/hrmp/benchmarking.rs @@ -363,7 +363,11 @@ mod benchmarks { } #[benchmark] - fn force_open_hrmp_channel() { + fn force_open_hrmp_channel( + // Weight parameter only accepts `u32`, `0` and `1` used to represent `false` and `true`, + // respectively. + c: Linear<0, 1>, + ) { let sender_id: ParaId = 1u32.into(); let sender_origin: crate::Origin = 1u32.into(); let recipient_id: ParaId = 2u32.into(); @@ -379,31 +383,26 @@ mod benchmarks { let capacity = Configuration::::config().hrmp_channel_max_capacity; let message_size = Configuration::::config().hrmp_channel_max_message_size; - // Weight parameter only accepts `u32`, `0` and `1` used to represent `false` and `true`, - // respectively. - let c = [0, 1]; let channel_id = HrmpChannelId { sender: sender_id, recipient: recipient_id }; - for channels_to_close in c { - if channels_to_close == 1 { - // this will consume more weight if a channel _request_ already exists, because it - // will need to clear the request. - assert_ok!(Hrmp::::hrmp_init_open_channel( + if c == 1 { + // this will consume more weight if a channel _request_ already exists, because it + // will need to clear the request. + assert_ok!(Hrmp::::hrmp_init_open_channel( + sender_origin.clone().into(), + recipient_id, + capacity, + message_size + )); + assert!(HrmpOpenChannelRequests::::get(&channel_id).is_some()); + } else { + if HrmpOpenChannelRequests::::get(&channel_id).is_some() { + assert_ok!(Hrmp::::hrmp_cancel_open_request( sender_origin.clone().into(), - recipient_id, - capacity, - message_size + channel_id.clone(), + MAX_UNIQUE_CHANNELS, )); - assert!(HrmpOpenChannelRequests::::get(&channel_id).is_some()); - } else { - if HrmpOpenChannelRequests::::get(&channel_id).is_some() { - assert_ok!(Hrmp::::hrmp_cancel_open_request( - sender_origin.clone().into(), - channel_id.clone(), - MAX_UNIQUE_CHANNELS, - )); - } - assert!(HrmpOpenChannelRequests::::get(&channel_id).is_none()); } + assert!(HrmpOpenChannelRequests::::get(&channel_id).is_none()); } // but the _channel_ should not exist From 5e40a7404317c9399f94c697ada0b46f0f180c02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=B3nal=20Murray?= Date: Fri, 22 Sep 2023 14:06:24 +0100 Subject: [PATCH 6/7] Remove unused variable in benchmark --- polkadot/runtime/parachains/src/hrmp/benchmarking.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/polkadot/runtime/parachains/src/hrmp/benchmarking.rs b/polkadot/runtime/parachains/src/hrmp/benchmarking.rs index 085c4a553b8e..f4bba6705403 100644 --- a/polkadot/runtime/parachains/src/hrmp/benchmarking.rs +++ b/polkadot/runtime/parachains/src/hrmp/benchmarking.rs @@ -223,7 +223,6 @@ mod benchmarks { let deposit: BalanceOf = config.hrmp_sender_deposit.unique_saturated_into(); let para: ParaId = 1u32.into(); - let para_origin: crate::Origin = 1u32.into(); register_parachain_with_balance::(para, deposit); T::Currency::make_free_balance_be(¶.into_account_truncating(), deposit * 256u32.into()); From ba28b46853c6800fa0203e0773f2195bba1ba648 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=B3nal=20Murray?= Date: Fri, 22 Sep 2023 14:30:18 +0100 Subject: [PATCH 7/7] Fix impl benchmark test suite position --- .../parachains/src/hrmp/benchmarking.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/polkadot/runtime/parachains/src/hrmp/benchmarking.rs b/polkadot/runtime/parachains/src/hrmp/benchmarking.rs index f4bba6705403..7d82bcc99a75 100644 --- a/polkadot/runtime/parachains/src/hrmp/benchmarking.rs +++ b/polkadot/runtime/parachains/src/hrmp/benchmarking.rs @@ -22,7 +22,7 @@ use crate::{ paras::{Pallet as Paras, ParaKind, ParachainsCache}, shared::Pallet as Shared, }; -use frame_benchmarking::v2::*; +use frame_benchmarking::{impl_benchmark_test_suite, v2::*, whitelisted_caller}; use frame_support::{assert_ok, traits::Currency}; type BalanceOf = @@ -421,7 +421,7 @@ mod benchmarks { let sender_id: ParaId = 1u32.into(); let recipient_id: ParaId = 2u32.into(); - let caller: T::AccountId = frame_benchmarking::whitelisted_caller(); + let caller: T::AccountId = whitelisted_caller(); let config = Configuration::::config(); // make sure para is registered, and has zero balance. @@ -446,7 +446,7 @@ mod benchmarks { let recipient_id: ParaId = 2u32.into(); let channel_id = HrmpChannelId { sender: sender_id, recipient: recipient_id }; - let caller: T::AccountId = frame_benchmarking::whitelisted_caller(); + let caller: T::AccountId = whitelisted_caller(); let config = Configuration::::config(); // make sure para is registered, and has balance to reserve. @@ -494,10 +494,10 @@ mod benchmarks { 0u128.unique_saturated_into() ); } -} -frame_benchmarking::impl_benchmark_test_suite!( - Hrmp, - crate::mock::new_test_ext(crate::hrmp::tests::GenesisConfigBuilder::default().build()), - crate::mock::Test -); + impl_benchmark_test_suite!( + Hrmp, + crate::mock::new_test_ext(crate::hrmp::tests::GenesisConfigBuilder::default().build()), + crate::mock::Test + ); +}