From 5581f4268021b749c1711ede78d70a711a34cd97 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Fri, 23 Feb 2024 10:50:36 +0100 Subject: [PATCH 01/40] Fix XCM benchmarks --- .../contracts-rococo/src/xcm_config.rs | 2 +- .../coretime/coretime-rococo/src/lib.rs | 15 ++++++++-- .../coretime-rococo/src/xcm_config.rs | 2 +- .../coretime/coretime-westend/src/lib.rs | 15 ++++++++-- .../coretime-westend/src/xcm_config.rs | 2 +- polkadot/xcm/pallet-xcm/src/benchmarking.rs | 28 +++++++++++++------ 6 files changed, 48 insertions(+), 16 deletions(-) diff --git a/cumulus/parachains/runtimes/contracts/contracts-rococo/src/xcm_config.rs b/cumulus/parachains/runtimes/contracts/contracts-rococo/src/xcm_config.rs index e8f3209eb67f..a310b84d43ba 100644 --- a/cumulus/parachains/runtimes/contracts/contracts-rococo/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/contracts/contracts-rococo/src/xcm_config.rs @@ -227,7 +227,7 @@ impl pallet_xcm::Config for Runtime { type XcmExecuteFilter = Nothing; type XcmExecutor = XcmExecutor; type XcmTeleportFilter = Everything; - type XcmReserveTransferFilter = Everything; + type XcmReserveTransferFilter = Nothing; type Weigher = FixedWeightBounds; type UniversalLocation = UniversalLocation; type RuntimeOrigin = RuntimeOrigin; diff --git a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs index f07bac8b2ef0..5c66b7d3f42f 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs @@ -731,8 +731,19 @@ impl_runtime_apis! { } fn reserve_transferable_asset_and_dest() -> Option<(Asset, Location)> { - // Reserve transfers are disabled - None + // Coretime chain can reserve transfer regions to some random parachain. + let random_para_id = 43211234; + ParachainSystem::open_outbound_hrmp_channel_for_benchmarks_or_tests( + random_para_id.into() + ); + let raw_region_id = 42; + Some(( + Asset { + fun: NonFungible(Index(raw_region_id)), + id: AssetId(Location::new(0, [PalletInstance(50)])) + }, + ParentThen(Parachain(random_para_id).into()).into(), + )) } } diff --git a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/xcm_config.rs b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/xcm_config.rs index 37bb8809daba..79e56fe6eaf8 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/xcm_config.rs @@ -286,7 +286,7 @@ impl pallet_xcm::Config for Runtime { type XcmExecuteFilter = Nothing; type XcmExecutor = XcmExecutor; type XcmTeleportFilter = Everything; - type XcmReserveTransferFilter = Nothing; // This parachain is not meant as a reserve location. + type XcmReserveTransferFilter = Everything; type Weigher = WeightInfoBounds< crate::weights::xcm::CoretimeRococoXcmWeight, RuntimeCall, diff --git a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs index 85d70d2f17b6..7d819c654660 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs @@ -722,8 +722,19 @@ impl_runtime_apis! { } fn reserve_transferable_asset_and_dest() -> Option<(Asset, Location)> { - // Reserve transfers are disabled - None + // Coretime chain can reserve transfer regions to some random parachain. + let random_para_id = 43211234; + ParachainSystem::open_outbound_hrmp_channel_for_benchmarks_or_tests( + random_para_id.into() + ); + let raw_region_id = 42; + Some(( + Asset { + fun: NonFungible(Index(raw_region_id)), + id: AssetId(Location::new(0, [PalletInstance(50)])) + }, + ParentThen(Parachain(random_para_id).into()).into(), + )) } } diff --git a/cumulus/parachains/runtimes/coretime/coretime-westend/src/xcm_config.rs b/cumulus/parachains/runtimes/coretime/coretime-westend/src/xcm_config.rs index 44049adf0271..3588adec6de2 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-westend/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-westend/src/xcm_config.rs @@ -298,7 +298,7 @@ impl pallet_xcm::Config for Runtime { type XcmExecuteFilter = Nothing; type XcmExecutor = XcmExecutor; type XcmTeleportFilter = Everything; - type XcmReserveTransferFilter = Nothing; // This parachain is not meant as a reserve location. + type XcmReserveTransferFilter = Everything; type Weigher = WeightInfoBounds< crate::weights::xcm::CoretimeWestendXcmWeight, RuntimeCall, diff --git a/polkadot/xcm/pallet-xcm/src/benchmarking.rs b/polkadot/xcm/pallet-xcm/src/benchmarking.rs index c7d8fb24e9df..fdc7b20e0da6 100644 --- a/polkadot/xcm/pallet-xcm/src/benchmarking.rs +++ b/polkadot/xcm/pallet-xcm/src/benchmarking.rs @@ -140,22 +140,27 @@ benchmarks! { BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)), )?; - let transferred_amount = match &asset.fun { - Fungible(amount) => *amount, - _ => return Err(BenchmarkError::Stop("Benchmark asset not fungible")), - }.into(); - let assets: Assets = asset.into(); - let existential_deposit = T::ExistentialDeposit::get(); let caller = whitelisted_caller(); // Give some multiple of the existential deposit let balance = existential_deposit.saturating_mul(ED_MULTIPLIER.into()); - assert!(balance >= transferred_amount); let _ = as Currency<_>>::make_free_balance_be(&caller, balance); // verify initial balance assert_eq!(pallet_balances::Pallet::::free_balance(&caller), balance); + match &asset.fun { + Fungible(transferred_amount) => { + assert!(balance >= (*transferred_amount).into()); + }, + NonFungible(instance) => { + // TODO: Mint instance + todo!() + } + } + + let assets: Assets = asset.clone().into(); + let send_origin = RawOrigin::Signed(caller.clone()); let origin_location = T::ExecuteXcmOrigin::try_origin(send_origin.clone().into()) .map_err(|_| BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))?; @@ -170,8 +175,13 @@ benchmarks! { let versioned_assets: VersionedAssets = assets.into(); }: _>(send_origin.into(), Box::new(versioned_dest), Box::new(versioned_beneficiary), Box::new(versioned_assets), 0) verify { - // verify balance after transfer, decreased by transferred amount (+ maybe XCM delivery fees) - assert!(pallet_balances::Pallet::::free_balance(&caller) <= balance - transferred_amount); + if let Fungible(transferred_amount) = asset.fun { + // verify balance after transfer, decreased by transferred amount (+ maybe XCM delivery fees) + assert!(pallet_balances::Pallet::::free_balance(&caller) <= balance - transferred_amount.into()); + }else { + // TODO: Ensure the caller is no longer owner of the instance. + todo!() + } } transfer_assets { From 89b8829a82a6940cfe09347a6a1c98798cde363b Mon Sep 17 00:00:00 2001 From: Szegoo Date: Fri, 23 Feb 2024 11:30:52 +0100 Subject: [PATCH 02/40] nonfungible implementation --- polkadot/xcm/pallet-xcm/src/benchmarking.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/polkadot/xcm/pallet-xcm/src/benchmarking.rs b/polkadot/xcm/pallet-xcm/src/benchmarking.rs index fdc7b20e0da6..e922ac7b975a 100644 --- a/polkadot/xcm/pallet-xcm/src/benchmarking.rs +++ b/polkadot/xcm/pallet-xcm/src/benchmarking.rs @@ -149,21 +149,22 @@ benchmarks! { // verify initial balance assert_eq!(pallet_balances::Pallet::::free_balance(&caller), balance); + let send_origin = RawOrigin::Signed(caller.clone()); + let origin_location = T::ExecuteXcmOrigin::try_origin(send_origin.clone().into()) + .map_err(|_| BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))?; + match &asset.fun { Fungible(transferred_amount) => { assert!(balance >= (*transferred_amount).into()); }, NonFungible(instance) => { - // TODO: Mint instance - todo!() + ::AssetTransactor::deposit_asset(&asset, &origin_location, None) + .map_err(|_| BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))?; } } let assets: Assets = asset.clone().into(); - let send_origin = RawOrigin::Signed(caller.clone()); - let origin_location = T::ExecuteXcmOrigin::try_origin(send_origin.clone().into()) - .map_err(|_| BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))?; if !T::XcmReserveTransferFilter::contains(&(origin_location, assets.clone().into_inner())) { return Err(BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX))) } @@ -178,9 +179,6 @@ benchmarks! { if let Fungible(transferred_amount) = asset.fun { // verify balance after transfer, decreased by transferred amount (+ maybe XCM delivery fees) assert!(pallet_balances::Pallet::::free_balance(&caller) <= balance - transferred_amount.into()); - }else { - // TODO: Ensure the caller is no longer owner of the instance. - todo!() } } From 55844cfc63572c2394213c4719d5ddd8bc548088 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Fri, 23 Feb 2024 16:25:50 +0100 Subject: [PATCH 03/40] progress --- polkadot/xcm/pallet-xcm/src/benchmarking.rs | 59 +++++++++++---------- polkadot/xcm/pallet-xcm/src/lib.rs | 2 + 2 files changed, 34 insertions(+), 27 deletions(-) diff --git a/polkadot/xcm/pallet-xcm/src/benchmarking.rs b/polkadot/xcm/pallet-xcm/src/benchmarking.rs index e922ac7b975a..fc364c31babf 100644 --- a/polkadot/xcm/pallet-xcm/src/benchmarking.rs +++ b/polkadot/xcm/pallet-xcm/src/benchmarking.rs @@ -101,25 +101,30 @@ benchmarks! { BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)), )?; - let transferred_amount = match &asset.fun { - Fungible(amount) => *amount, - _ => return Err(BenchmarkError::Stop("Benchmark asset not fungible")), - }.into(); - let assets: Assets = asset.into(); + let caller: T::AccountId = whitelisted_caller(); - let existential_deposit = T::ExistentialDeposit::get(); - let caller = whitelisted_caller(); + let send_origin = RawOrigin::Signed(caller.clone()); + let origin_location = T::ExecuteXcmOrigin::try_origin(send_origin.clone().into()) + .map_err(|_| BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))?; + let existential_deposit = T::ExistentialDeposit::get(); // Give some multiple of the existential deposit let balance = existential_deposit.saturating_mul(ED_MULTIPLIER.into()); - assert!(balance >= transferred_amount); - let _ = as Currency<_>>::make_free_balance_be(&caller, balance); + make_free_balance_be::(origin_location.clone(), asset.id.0.clone(), balance.into()); // verify initial balance assert_eq!(pallet_balances::Pallet::::free_balance(&caller), balance); - let send_origin = RawOrigin::Signed(caller.clone()); - let origin_location = T::ExecuteXcmOrigin::try_origin(send_origin.clone().into()) - .map_err(|_| BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))?; + match &asset.fun { + Fungible(transferred_amount) => { + assert!(balance >= (*transferred_amount).into()); + }, + NonFungible(instance) => { + ::AssetTransactor::deposit_asset(&asset, &origin_location, None) + .map_err(|_| BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))?; + } + }; + let assets: Assets = asset.into(); + if !T::XcmTeleportFilter::contains(&(origin_location, assets.clone().into_inner())) { return Err(BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX))) } @@ -130,10 +135,6 @@ benchmarks! { AccountId32 { network: None, id: recipient.into() }.into(); let versioned_assets: VersionedAssets = assets.into(); }: _>(send_origin.into(), Box::new(versioned_dest), Box::new(versioned_beneficiary), Box::new(versioned_assets), 0) - verify { - // verify balance after transfer, decreased by transferred amount (+ maybe XCM delivery fees) - assert!(pallet_balances::Pallet::::free_balance(&caller) <= balance - transferred_amount); - } reserve_transfer_assets { let (asset, destination) = T::reserve_transferable_asset_and_dest().ok_or( @@ -141,17 +142,14 @@ benchmarks! { )?; let existential_deposit = T::ExistentialDeposit::get(); - let caller = whitelisted_caller(); + let caller: T::AccountId = whitelisted_caller(); // Give some multiple of the existential deposit let balance = existential_deposit.saturating_mul(ED_MULTIPLIER.into()); - let _ = as Currency<_>>::make_free_balance_be(&caller, balance); - // verify initial balance - assert_eq!(pallet_balances::Pallet::::free_balance(&caller), balance); - let send_origin = RawOrigin::Signed(caller.clone()); let origin_location = T::ExecuteXcmOrigin::try_origin(send_origin.clone().into()) .map_err(|_| BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))?; + make_free_balance_be::(origin_location.clone(), asset.id.0.clone(), balance.into()); match &asset.fun { Fungible(transferred_amount) => { @@ -175,12 +173,6 @@ benchmarks! { AccountId32 { network: None, id: recipient.into() }.into(); let versioned_assets: VersionedAssets = assets.into(); }: _>(send_origin.into(), Box::new(versioned_dest), Box::new(versioned_beneficiary), Box::new(versioned_assets), 0) - verify { - if let Fungible(transferred_amount) = asset.fun { - // verify balance after transfer, decreased by transferred amount (+ maybe XCM delivery fees) - assert!(pallet_balances::Pallet::::free_balance(&caller) <= balance - transferred_amount.into()); - } - } transfer_assets { let (assets, fee_index, destination, verify) = T::set_up_complex_asset_transfer().ok_or( @@ -344,8 +336,21 @@ benchmarks! { ); } +pub fn make_free_balance_be(who: Location, asset_location: Location, balance: u128) +where + T: Config + pallet_balances::Config, +{ + ::AssetTransactor::deposit_asset( + &Asset { fun: Fungible(balance), id: AssetId(asset_location) }, + &who, + None, + ) + .unwrap(); +} + pub mod helpers { use super::*; + pub fn native_teleport_as_asset_transfer( native_asset_location: Location, destination: Location, diff --git a/polkadot/xcm/pallet-xcm/src/lib.rs b/polkadot/xcm/pallet-xcm/src/lib.rs index 5e1a3e55f9b6..69da175e8de9 100644 --- a/polkadot/xcm/pallet-xcm/src/lib.rs +++ b/polkadot/xcm/pallet-xcm/src/lib.rs @@ -316,6 +316,7 @@ pub mod pallet { let weight_used = outcome.weight_used(); outcome.ensure_complete().map_err(|error| { log::error!(target: "xcm::pallet_xcm::execute", "XCM execution failed with error {:?}", error); + println!("HEYY2"); Error::::LocalExecutionIncomplete.with_weight( weight_used.saturating_add( ::execute(), @@ -1680,6 +1681,7 @@ impl Pallet { weight, ); Self::deposit_event(Event::Attempted { outcome: outcome.clone() }); + println!("HEYY"); outcome.ensure_complete().map_err(|error| { log::error!( target: "xcm::pallet_xcm::build_and_execute_xcm_transfer_type", From c39893052d3ed39901fe7f97c00db82c8a1db8d2 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Fri, 23 Feb 2024 17:17:27 +0100 Subject: [PATCH 04/40] works --- polkadot/xcm/pallet-xcm/src/benchmarking.rs | 86 ++++++++------------- polkadot/xcm/pallet-xcm/src/lib.rs | 2 - 2 files changed, 34 insertions(+), 54 deletions(-) diff --git a/polkadot/xcm/pallet-xcm/src/benchmarking.rs b/polkadot/xcm/pallet-xcm/src/benchmarking.rs index fc364c31babf..f5588556d097 100644 --- a/polkadot/xcm/pallet-xcm/src/benchmarking.rs +++ b/polkadot/xcm/pallet-xcm/src/benchmarking.rs @@ -17,15 +17,14 @@ use super::*; use bounded_collections::{ConstU32, WeakBoundedVec}; use frame_benchmarking::{benchmarks, whitelisted_caller, BenchmarkError, BenchmarkResult}; -use frame_support::{traits::Currency, weights::Weight}; +use frame_support::weights::Weight; use frame_system::RawOrigin; use sp_std::prelude::*; use xcm::{latest::prelude::*, v2}; type RuntimeOrigin = ::RuntimeOrigin; -// existential deposit multiplier -const ED_MULTIPLIER: u32 = 100; +const BALANCE: u128 = 1_000_000_000_000; /// Pallet we're benchmarking here. pub struct Pallet(crate::Pallet); @@ -77,11 +76,6 @@ pub trait Config: crate::Config { } benchmarks! { - where_clause { - where - T: pallet_balances::Config, - ::Balance: From + Into, - } send { let send_origin = T::SendXcmOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; @@ -107,16 +101,11 @@ benchmarks! { let origin_location = T::ExecuteXcmOrigin::try_origin(send_origin.clone().into()) .map_err(|_| BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))?; - let existential_deposit = T::ExistentialDeposit::get(); - // Give some multiple of the existential deposit - let balance = existential_deposit.saturating_mul(ED_MULTIPLIER.into()); - make_free_balance_be::(origin_location.clone(), asset.id.0.clone(), balance.into()); - // verify initial balance - assert_eq!(pallet_balances::Pallet::::free_balance(&caller), balance); + helpers::make_free_balance_be::(origin_location.clone(), asset.id.0.clone(), BALANCE.into()); match &asset.fun { Fungible(transferred_amount) => { - assert!(balance >= (*transferred_amount).into()); + assert!(BALANCE >= (*transferred_amount).into()); }, NonFungible(instance) => { ::AssetTransactor::deposit_asset(&asset, &origin_location, None) @@ -141,19 +130,16 @@ benchmarks! { BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)), )?; - let existential_deposit = T::ExistentialDeposit::get(); let caller: T::AccountId = whitelisted_caller(); - // Give some multiple of the existential deposit - let balance = existential_deposit.saturating_mul(ED_MULTIPLIER.into()); let send_origin = RawOrigin::Signed(caller.clone()); let origin_location = T::ExecuteXcmOrigin::try_origin(send_origin.clone().into()) .map_err(|_| BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))?; - make_free_balance_be::(origin_location.clone(), asset.id.0.clone(), balance.into()); + helpers::make_free_balance_be::(origin_location.clone(), asset.id.0.clone(), BALANCE.into()); match &asset.fun { Fungible(transferred_amount) => { - assert!(balance >= (*transferred_amount).into()); + assert!(BALANCE >= (*transferred_amount).into()); }, NonFungible(instance) => { ::AssetTransactor::deposit_asset(&asset, &origin_location, None) @@ -336,48 +322,44 @@ benchmarks! { ); } -pub fn make_free_balance_be(who: Location, asset_location: Location, balance: u128) -where - T: Config + pallet_balances::Config, -{ - ::AssetTransactor::deposit_asset( - &Asset { fun: Fungible(balance), id: AssetId(asset_location) }, - &who, - None, - ) - .unwrap(); -} - pub mod helpers { use super::*; - pub fn native_teleport_as_asset_transfer( + pub fn make_free_balance_be(who: Location, asset_location: Location, balance: u128) { + ::AssetTransactor::deposit_asset( + &Asset { fun: Fungible(balance), id: AssetId(asset_location) }, + &who, + None, + ) + .unwrap(); + } + + pub fn native_teleport_as_asset_transfer( native_asset_location: Location, destination: Location, - ) -> Option<(Assets, u32, Location, Box)> - where - T: Config + pallet_balances::Config, - u128: From<::Balance>, - { + ) -> Option<(Assets, u32, Location, Box)> { // Relay/native token can be teleported to/from AH. - let amount = T::ExistentialDeposit::get() * 100u32.into(); - let assets: Assets = - Asset { fun: Fungible(amount.into()), id: AssetId(native_asset_location) }.into(); + let amount = BALANCE; + let asset = Asset { fun: Fungible(amount.into()), id: AssetId(native_asset_location) }; let fee_index = 0u32; // Give some multiple of transferred amount - let balance = amount * 10u32.into(); - let who = whitelisted_caller(); - let _ = - as frame_support::traits::Currency<_>>::make_free_balance_be(&who, balance); - // verify initial balance - assert_eq!(pallet_balances::Pallet::::free_balance(&who), balance); + let balance = amount * 10; + let who: T::AccountId = whitelisted_caller(); + + let send_origin = RawOrigin::Signed(who.clone()); + let Ok(origin_location) = T::ExecuteXcmOrigin::try_origin(send_origin.clone().into()) + else { + return None; + }; + helpers::make_free_balance_be::( + origin_location.clone(), + asset.id.0.clone(), + balance.into(), + ); // verify transferred successfully - let verify = Box::new(move || { - // verify balance after transfer, decreased by transferred amount (and delivery fees) - assert!(pallet_balances::Pallet::::free_balance(&who) <= balance - amount); - }); - Some((assets, fee_index, destination, verify)) + let verify = Box::new(move || {}); + Some((asset.into(), fee_index, destination, verify)) } } diff --git a/polkadot/xcm/pallet-xcm/src/lib.rs b/polkadot/xcm/pallet-xcm/src/lib.rs index 69da175e8de9..5e1a3e55f9b6 100644 --- a/polkadot/xcm/pallet-xcm/src/lib.rs +++ b/polkadot/xcm/pallet-xcm/src/lib.rs @@ -316,7 +316,6 @@ pub mod pallet { let weight_used = outcome.weight_used(); outcome.ensure_complete().map_err(|error| { log::error!(target: "xcm::pallet_xcm::execute", "XCM execution failed with error {:?}", error); - println!("HEYY2"); Error::::LocalExecutionIncomplete.with_weight( weight_used.saturating_add( ::execute(), @@ -1681,7 +1680,6 @@ impl Pallet { weight, ); Self::deposit_event(Event::Attempted { outcome: outcome.clone() }); - println!("HEYY"); outcome.ensure_complete().map_err(|error| { log::error!( target: "xcm::pallet_xcm::build_and_execute_xcm_transfer_type", From 9200399180f66abe6b8312a38985e0230a8ff807 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Fri, 23 Feb 2024 17:23:33 +0100 Subject: [PATCH 05/40] prdoc --- polkadot/xcm/pallet-xcm/Cargo.toml | 2 +- prdoc/pr_3455.prdoc | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 prdoc/pr_3455.prdoc diff --git a/polkadot/xcm/pallet-xcm/Cargo.toml b/polkadot/xcm/pallet-xcm/Cargo.toml index 4840b6127f55..e3a66faa331b 100644 --- a/polkadot/xcm/pallet-xcm/Cargo.toml +++ b/polkadot/xcm/pallet-xcm/Cargo.toml @@ -29,10 +29,10 @@ xcm-builder = { package = "staging-xcm-builder", path = "../xcm-builder", defaul # marked optional, used in benchmarking frame-benchmarking = { path = "../../../substrate/frame/benchmarking", default-features = false, optional = true } -pallet-balances = { path = "../../../substrate/frame/balances", default-features = false, optional = true } [dev-dependencies] pallet-assets = { path = "../../../substrate/frame/assets" } +pallet-balances = { path = "../../../substrate/frame/balances" } polkadot-runtime-parachains = { path = "../../runtime/parachains" } polkadot-parachain-primitives = { path = "../../parachain" } diff --git a/prdoc/pr_3455.prdoc b/prdoc/pr_3455.prdoc new file mode 100644 index 000000000000..86358c9ebef3 --- /dev/null +++ b/prdoc/pr_3455.prdoc @@ -0,0 +1,18 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: pallet-xcm benchmarking fixes + +doc: + - audience: Runtime Dev + description: | + With this PR, the benchmarks for pallet-xcm no longer assume the use of + `pallet-balances` for fungible implementations. Furthermore, with this PR + non-fungible teleport and reserve transfers are properly benchmarked. + +crates: + - name: pallet-xcm + - name: contracts-rococo-runtime + - name: coretime-rococo-runtime + - name: coretime-westend-runtime + - name: coretime-westend-runtime From 507eb47e16d49a7ebe1c53b348ab5d0e185aa43a Mon Sep 17 00:00:00 2001 From: Szegoo Date: Tue, 27 Feb 2024 10:31:37 +0100 Subject: [PATCH 06/40] conflict readjustements --- polkadot/xcm/pallet-xcm/src/benchmarking.rs | 61 +++++++++------------ 1 file changed, 26 insertions(+), 35 deletions(-) diff --git a/polkadot/xcm/pallet-xcm/src/benchmarking.rs b/polkadot/xcm/pallet-xcm/src/benchmarking.rs index 934fe9ef793f..f2971c075772 100644 --- a/polkadot/xcm/pallet-xcm/src/benchmarking.rs +++ b/polkadot/xcm/pallet-xcm/src/benchmarking.rs @@ -18,7 +18,6 @@ use super::*; use bounded_collections::{ConstU32, WeakBoundedVec}; use frame_benchmarking::{benchmarks, whitelisted_caller, BenchmarkError, BenchmarkResult}; use frame_support::{ - traits::fungible::{Inspect, Mutate}, weights::Weight, }; use frame_system::RawOrigin; @@ -101,11 +100,7 @@ benchmarks! { BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)), )?; - let transferred_amount = match &asset.fun { - Fungible(amount) => *amount, - _ => return Err(BenchmarkError::Stop("Benchmark asset not fungible")), - }.into(); - let assets: Assets = asset.into(); + let assets: Assets = asset.clone().into(); let caller: T::AccountId = whitelisted_caller(); let send_origin = RawOrigin::Signed(caller.clone()); @@ -122,13 +117,16 @@ benchmarks! { FeeReason::ChargeFees, ); - // Actual balance (e.g. `ensure_successful_delivery` could drip delivery fees, ...) - let balance = as Inspect<_>>::balance(&caller); - // Add transferred_amount to origin - as Mutate<_>>::mint_into(&caller, transferred_amount)?; - // verify initial balance - let balance = balance + transferred_amount; - assert_eq!( as Inspect<_>>::balance(&caller), balance); + match &asset.fun { + Fungible(amount) => { + // Add transferred_amount to origin + helpers::mint_into::(origin_location, asset.id.0, *amount); + }, + NonFungible(instance) => { + ::AssetTransactor::deposit_asset(&asset, &origin_location, None) + .map_err(|_| BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))?; + } + }; let recipient = [0u8; 32]; let versioned_dest: VersionedLocation = destination.into(); @@ -136,21 +134,13 @@ benchmarks! { AccountId32 { network: None, id: recipient.into() }.into(); let versioned_assets: VersionedAssets = assets.into(); }: _>(send_origin.into(), Box::new(versioned_dest), Box::new(versioned_beneficiary), Box::new(versioned_assets), 0) - verify { - // verify balance after transfer, decreased by transferred amount (+ maybe XCM delivery fees) - assert!( as Inspect<_>>::balance(&caller) <= balance - transferred_amount); - } reserve_transfer_assets { let (asset, destination) = T::reserve_transferable_asset_and_dest().ok_or( BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)), )?; - let transferred_amount = match &asset.fun { - Fungible(amount) => *amount, - _ => return Err(BenchmarkError::Stop("Benchmark asset not fungible")), - }.into(); - let assets: Assets = asset.into(); + let assets: Assets = asset.clone().into(); let caller: T::AccountId = whitelisted_caller(); let send_origin = RawOrigin::Signed(caller.clone()); @@ -167,13 +157,16 @@ benchmarks! { FeeReason::ChargeFees, ); - // Actual balance (e.g. `ensure_successful_delivery` could drip delivery fees, ...) - let balance = as Inspect<_>>::balance(&caller); - // Add transferred_amount to origin - as Mutate<_>>::mint_into(&caller, transferred_amount)?; - // verify initial balance - let balance = balance + transferred_amount; - assert_eq!( as Inspect<_>>::balance(&caller), balance); + match &asset.fun { + Fungible(amount) => { + // Add transferred_amount to origin + helpers::mint_into::(origin_location, asset.id.0, *amount); + }, + NonFungible(instance) => { + ::AssetTransactor::deposit_asset(&asset, &origin_location, None) + .map_err(|_| BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))?; + } + }; let recipient = [0u8; 32]; let versioned_dest: VersionedLocation = destination.into(); @@ -181,10 +174,6 @@ benchmarks! { AccountId32 { network: None, id: recipient.into() }.into(); let versioned_assets: VersionedAssets = assets.into(); }: _>(send_origin.into(), Box::new(versioned_dest), Box::new(versioned_beneficiary), Box::new(versioned_assets), 0) - verify { - // verify balance after transfer, decreased by transferred amount (+ maybe XCM delivery fees) - assert!( as Inspect<_>>::balance(&caller) <= balance - transferred_amount); - } transfer_assets { let (assets, fee_index, destination, verify) = T::set_up_complex_asset_transfer().ok_or( @@ -351,7 +340,7 @@ benchmarks! { pub mod helpers { use super::*; - pub fn make_free_balance_be(who: Location, asset_location: Location, balance: u128) { + pub fn mint_into(who: Location, asset_location: Location, balance: u128) { ::AssetTransactor::deposit_asset( &Asset { fun: Fungible(balance), id: AssetId(asset_location) }, &who, @@ -360,6 +349,7 @@ pub mod helpers { .unwrap(); } + /* TODO: FIX pub fn native_teleport_as_asset_transfer( native_asset_location: Location, destination: Location, @@ -378,7 +368,7 @@ pub mod helpers { else { return None; }; - helpers::make_free_balance_be::( + helpers::mint_into::( origin_location.clone(), asset.id.0.clone(), balance.into(), @@ -388,4 +378,5 @@ pub mod helpers { let verify = Box::new(move || {}); Some((asset.into(), fee_index, destination, verify)) } + */ } From 4b0ee5d8c75ee8dc4b689f27a076237ac36de316 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Tue, 27 Feb 2024 13:46:47 +0100 Subject: [PATCH 07/40] fix --- polkadot/xcm/pallet-xcm/Cargo.toml | 2 +- polkadot/xcm/pallet-xcm/src/benchmarking.rs | 66 ++++++++++----------- 2 files changed, 32 insertions(+), 36 deletions(-) diff --git a/polkadot/xcm/pallet-xcm/Cargo.toml b/polkadot/xcm/pallet-xcm/Cargo.toml index e3a66faa331b..4840b6127f55 100644 --- a/polkadot/xcm/pallet-xcm/Cargo.toml +++ b/polkadot/xcm/pallet-xcm/Cargo.toml @@ -29,10 +29,10 @@ xcm-builder = { package = "staging-xcm-builder", path = "../xcm-builder", defaul # marked optional, used in benchmarking frame-benchmarking = { path = "../../../substrate/frame/benchmarking", default-features = false, optional = true } +pallet-balances = { path = "../../../substrate/frame/balances", default-features = false, optional = true } [dev-dependencies] pallet-assets = { path = "../../../substrate/frame/assets" } -pallet-balances = { path = "../../../substrate/frame/balances" } polkadot-runtime-parachains = { path = "../../runtime/parachains" } polkadot-parachain-primitives = { path = "../../parachain" } diff --git a/polkadot/xcm/pallet-xcm/src/benchmarking.rs b/polkadot/xcm/pallet-xcm/src/benchmarking.rs index f2971c075772..2823d160bc18 100644 --- a/polkadot/xcm/pallet-xcm/src/benchmarking.rs +++ b/polkadot/xcm/pallet-xcm/src/benchmarking.rs @@ -17,9 +17,7 @@ use super::*; use bounded_collections::{ConstU32, WeakBoundedVec}; use frame_benchmarking::{benchmarks, whitelisted_caller, BenchmarkError, BenchmarkResult}; -use frame_support::{ - weights::Weight, -}; +use frame_support::weights::Weight; use frame_system::RawOrigin; use sp_std::prelude::*; use xcm::{latest::prelude::*, v2}; @@ -120,7 +118,11 @@ benchmarks! { match &asset.fun { Fungible(amount) => { // Add transferred_amount to origin - helpers::mint_into::(origin_location, asset.id.0, *amount); + ::AssetTransactor::deposit_asset( + &Asset { fun: Fungible(*amount), id: asset.id }, + &origin_location, + None, + ).map_err(|_| BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))?; }, NonFungible(instance) => { ::AssetTransactor::deposit_asset(&asset, &origin_location, None) @@ -160,7 +162,11 @@ benchmarks! { match &asset.fun { Fungible(amount) => { // Add transferred_amount to origin - helpers::mint_into::(origin_location, asset.id.0, *amount); + ::AssetTransactor::deposit_asset( + &Asset { fun: Fungible(*amount), id: asset.id }, + &origin_location, + None, + ).map_err(|_| BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))?; }, NonFungible(instance) => { ::AssetTransactor::deposit_asset(&asset, &origin_location, None) @@ -340,43 +346,33 @@ benchmarks! { pub mod helpers { use super::*; - pub fn mint_into(who: Location, asset_location: Location, balance: u128) { - ::AssetTransactor::deposit_asset( - &Asset { fun: Fungible(balance), id: AssetId(asset_location) }, - &who, - None, - ) - .unwrap(); - } - - /* TODO: FIX - pub fn native_teleport_as_asset_transfer( + pub fn native_teleport_as_asset_transfer( native_asset_location: Location, destination: Location, - ) -> Option<(Assets, u32, Location, Box)> { + ) -> Option<(Assets, u32, Location, Box)> + where + T: Config + pallet_balances::Config, + u128: From<::Balance>, + { // Relay/native token can be teleported to/from AH. - let amount = BALANCE; - let asset = Asset { fun: Fungible(amount.into()), id: AssetId(native_asset_location) }; + let amount = T::ExistentialDeposit::get() * 100u32.into(); + let assets: Assets = + Asset { fun: Fungible(amount.into()), id: AssetId(native_asset_location) }.into(); let fee_index = 0u32; // Give some multiple of transferred amount - let balance = amount * 10; - let who: T::AccountId = whitelisted_caller(); - - let send_origin = RawOrigin::Signed(who.clone()); - let Ok(origin_location) = T::ExecuteXcmOrigin::try_origin(send_origin.clone().into()) - else { - return None; - }; - helpers::mint_into::( - origin_location.clone(), - asset.id.0.clone(), - balance.into(), - ); + let balance = amount * 10u32.into(); + let who = whitelisted_caller(); + let _ = + as frame_support::traits::Currency<_>>::make_free_balance_be(&who, balance); + // verify initial balance + assert_eq!(pallet_balances::Pallet::::free_balance(&who), balance); // verify transferred successfully - let verify = Box::new(move || {}); - Some((asset.into(), fee_index, destination, verify)) + let verify = Box::new(move || { + // verify balance after transfer, decreased by transferred amount (and delivery fees) + assert!(pallet_balances::Pallet::::free_balance(&who) <= balance - amount); + }); + Some((assets, fee_index, destination, verify)) } - */ } From eb7a61b069a536a4bbd392014421d7d2d061acdb Mon Sep 17 00:00:00 2001 From: Szegoo Date: Tue, 27 Feb 2024 13:49:26 +0100 Subject: [PATCH 08/40] .. --- polkadot/xcm/pallet-xcm/src/benchmarking.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/polkadot/xcm/pallet-xcm/src/benchmarking.rs b/polkadot/xcm/pallet-xcm/src/benchmarking.rs index 2823d160bc18..29ba4062e5ed 100644 --- a/polkadot/xcm/pallet-xcm/src/benchmarking.rs +++ b/polkadot/xcm/pallet-xcm/src/benchmarking.rs @@ -345,7 +345,6 @@ benchmarks! { pub mod helpers { use super::*; - pub fn native_teleport_as_asset_transfer( native_asset_location: Location, destination: Location, From e1f99dd5167553c4e4d933f14cffd22cc44c4707 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Sat, 2 Mar 2024 17:16:23 +0100 Subject: [PATCH 09/40] Region reserve transfers --- .../frame/broker/src/dispatchable_impls.rs | 8 +-- substrate/frame/broker/src/lib.rs | 4 +- .../frame/broker/src/nonfungible_impl.rs | 53 +++++++++++++++---- substrate/frame/broker/src/types.rs | 2 +- substrate/frame/broker/src/utility_impls.rs | 4 +- 5 files changed, 53 insertions(+), 18 deletions(-) diff --git a/substrate/frame/broker/src/dispatchable_impls.rs b/substrate/frame/broker/src/dispatchable_impls.rs index f2451013251f..b44318993145 100644 --- a/substrate/frame/broker/src/dispatchable_impls.rs +++ b/substrate/frame/broker/src/dispatchable_impls.rs @@ -178,11 +178,11 @@ impl Pallet { let mut region = Regions::::get(®ion_id).ok_or(Error::::UnknownRegion)?; if let Some(check_owner) = maybe_check_owner { - ensure!(check_owner == region.owner, Error::::NotOwner); + ensure!(Some(check_owner) == region.owner, Error::::NotOwner); } let old_owner = region.owner; - region.owner = new_owner; + region.owner = Some(new_owner); Regions::::insert(®ion_id, ®ion); let duration = region.end.saturating_sub(region_id.begin); Self::deposit_event(Event::Transferred { @@ -203,7 +203,7 @@ impl Pallet { let mut region = Regions::::get(®ion_id).ok_or(Error::::UnknownRegion)?; if let Some(check_owner) = maybe_check_owner { - ensure!(check_owner == region.owner, Error::::NotOwner); + ensure!(Some(check_owner) == region.owner, Error::::NotOwner); } let pivot = region_id.begin.saturating_add(pivot_offset); ensure!(pivot < region.end, Error::::PivotTooLate); @@ -227,7 +227,7 @@ impl Pallet { let region = Regions::::get(®ion_id).ok_or(Error::::UnknownRegion)?; if let Some(check_owner) = maybe_check_owner { - ensure!(check_owner == region.owner, Error::::NotOwner); + ensure!(Some(check_owner) == region.owner, Error::::NotOwner); } ensure!((pivot & !region_id.mask).is_void(), Error::::ExteriorPivot); diff --git a/substrate/frame/broker/src/lib.rs b/substrate/frame/broker/src/lib.rs index a669463aa02d..445050ac2f67 100644 --- a/substrate/frame/broker/src/lib.rs +++ b/substrate/frame/broker/src/lib.rs @@ -214,9 +214,9 @@ pub mod pallet { /// The duration of the Region. duration: Timeslice, /// The old owner of the Region. - old_owner: T::AccountId, + old_owner: Option, /// The new owner of the Region. - owner: T::AccountId, + owner: Option, }, /// A Region has been split into two non-overlapping Regions. Partitioned { diff --git a/substrate/frame/broker/src/nonfungible_impl.rs b/substrate/frame/broker/src/nonfungible_impl.rs index b2e88bf09a0e..13d98be833bf 100644 --- a/substrate/frame/broker/src/nonfungible_impl.rs +++ b/substrate/frame/broker/src/nonfungible_impl.rs @@ -25,12 +25,13 @@ use sp_std::vec::Vec; impl Inspect for Pallet { type ItemId = u128; - fn owner(index: &Self::ItemId) -> Option { - Regions::::get(RegionId::from(*index)).map(|r| r.owner) + fn owner(item: &Self::ItemId) -> Option { + let record = Regions::::get(RegionId::from(*item))?; + record.owner } - fn attribute(index: &Self::ItemId, key: &[u8]) -> Option> { - let id = RegionId::from(*index); + fn attribute(item: &Self::ItemId, key: &[u8]) -> Option> { + let id = RegionId::from(*item); let item = Regions::::get(id)?; match key { b"begin" => Some(id.begin.encode()), @@ -46,11 +47,45 @@ impl Inspect for Pallet { } impl Transfer for Pallet { - fn transfer(index: &Self::ItemId, dest: &T::AccountId) -> DispatchResult { - Self::do_transfer((*index).into(), None, dest.clone()).map_err(Into::into) + fn transfer(item: &Self::ItemId, dest: &T::AccountId) -> DispatchResult { + Self::do_transfer((*item).into(), None, dest.clone()).map_err(Into::into) } } -// We don't allow any of the mutate operations, so the default implementation is used, which will -// return `TokenError::Unsupported` in case any of the operations is called. -impl Mutate for Pallet {} +/// We don't really support burning and minting. +/// +/// We only need this to allow the region to be reserve transferable. +/// +/// For reserve transfers that are not 'local', the asset must first be withdrawn to the holding +/// register and then deposited into the designated account. This process necessitates that the +/// asset is capable of being 'burned' and 'minted'. +/// +/// Since each region is associated with specific record data, we will not actually burn the asset. +/// If we did, we wouldn't know what record to assign to the newly minted region. Therefore, instead +/// of burning, we set the asset's owner to `None`. In essence, 'burning' a region involves setting +/// its owner to `None`, whereas 'minting' the region assigns its owner to an actual account. This +/// way we never lose track of the associated record data. +impl Mutate for Pallet { + fn mint_into(item: &Self::ItemId, who: &T::AccountId) -> DispatchResult { + let region_id: RegionId = (*item).into(); + let record = Regions::::get(®ion_id).ok_or(Error::::UnknownRegion)?; + + // 'Minting' can only occur if the asset has previously been burned(i.e. moved to the + // holding registrar) + ensure!(record.owner.is_none(), Error::::NotAllowed); + Self::issue(region_id.core, region_id.begin, record.end, who.clone(), record.paid); + + Ok(()) + } + + fn burn(item: &Self::ItemId, maybe_check_owner: Option<&T::AccountId>) -> DispatchResult { + let region_id: RegionId = (*item).into(); + let mut record = Regions::::get(®ion_id).ok_or(Error::::UnknownRegion)?; + if let Some(owner) = maybe_check_owner { + ensure!(Some(owner.clone()) == record.owner, Error::::NotOwner); + } + + record.owner = None; + Ok(()) + } +} diff --git a/substrate/frame/broker/src/types.rs b/substrate/frame/broker/src/types.rs index 7e9f351723a5..f0fecbedcdb3 100644 --- a/substrate/frame/broker/src/types.rs +++ b/substrate/frame/broker/src/types.rs @@ -84,7 +84,7 @@ pub struct RegionRecord { /// The end of the Region. pub end: Timeslice, /// The owner of the Region. - pub owner: AccountId, + pub owner: Option, /// The amount paid to Polkadot for this Region, or `None` if renewal is not allowed. pub paid: Option, } diff --git a/substrate/frame/broker/src/utility_impls.rs b/substrate/frame/broker/src/utility_impls.rs index 3dba5be5b398..3d35805b91a4 100644 --- a/substrate/frame/broker/src/utility_impls.rs +++ b/substrate/frame/broker/src/utility_impls.rs @@ -80,7 +80,7 @@ impl Pallet { paid: Option>, ) -> RegionId { let id = RegionId { begin, core, mask: CoreMask::complete() }; - let record = RegionRecord { end, owner, paid }; + let record = RegionRecord { end, owner: Some(owner), paid }; Regions::::insert(&id, &record); id } @@ -94,7 +94,7 @@ impl Pallet { let region = Regions::::get(®ion_id).ok_or(Error::::UnknownRegion)?; if let Some(check_owner) = maybe_check_owner { - ensure!(check_owner == region.owner, Error::::NotOwner); + ensure!(Some(check_owner) == region.owner, Error::::NotOwner); } Regions::::remove(®ion_id); From 01ffbe01fe61ca4852ee7fd6504c7ee68a0f8d66 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Mon, 4 Mar 2024 17:33:45 +0100 Subject: [PATCH 10/40] tests & migration --- Cargo.lock | 1 + substrate/frame/broker/Cargo.toml | 2 + substrate/frame/broker/src/lib.rs | 7 ++ substrate/frame/broker/src/migration.rs | 67 +++++++++++++++++++ .../frame/broker/src/nonfungible_impl.rs | 2 + substrate/frame/broker/src/tests.rs | 25 +++++-- 6 files changed, 98 insertions(+), 6 deletions(-) create mode 100644 substrate/frame/broker/src/migration.rs diff --git a/Cargo.lock b/Cargo.lock index 3838ed0e94a1..6f1f8a7ed243 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9572,6 +9572,7 @@ dependencies = [ "frame-benchmarking", "frame-support", "frame-system", + "log", "parity-scale-codec", "scale-info", "sp-arithmetic", diff --git a/substrate/frame/broker/Cargo.toml b/substrate/frame/broker/Cargo.toml index 31f9a6b63178..2b63bbdad888 100644 --- a/substrate/frame/broker/Cargo.toml +++ b/substrate/frame/broker/Cargo.toml @@ -15,6 +15,7 @@ workspace = true targets = ["x86_64-unknown-linux-gnu"] [dependencies] +log = { workspace = true } codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false, features = ["derive"] } scale-info = { version = "2.10.0", default-features = false, features = ["derive"] } bitvec = { version = "1.0.0", default-features = false } @@ -38,6 +39,7 @@ std = [ "frame-benchmarking?/std", "frame-support/std", "frame-system/std", + "log/std", "scale-info/std", "sp-arithmetic/std", "sp-core/std", diff --git a/substrate/frame/broker/src/lib.rs b/substrate/frame/broker/src/lib.rs index 445050ac2f67..edfe327ae2f2 100644 --- a/substrate/frame/broker/src/lib.rs +++ b/substrate/frame/broker/src/lib.rs @@ -36,6 +36,7 @@ mod tick_impls; mod types; mod utility_impls; +pub mod migration; pub mod weights; pub use weights::WeightInfo; @@ -44,6 +45,9 @@ pub use core_mask::*; pub use coretime_interface::*; pub use types::*; +/// The log target for this pallet. +const LOG_TARGET: &str = "runtime::broker"; + #[frame_support::pallet] pub mod pallet { use super::*; @@ -59,7 +63,10 @@ pub mod pallet { use sp_runtime::traits::{Convert, ConvertBack}; use sp_std::vec::Vec; + const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); + #[pallet::pallet] + #[pallet::storage_version(STORAGE_VERSION)] pub struct Pallet(_); #[pallet::config] diff --git a/substrate/frame/broker/src/migration.rs b/substrate/frame/broker/src/migration.rs new file mode 100644 index 000000000000..4f4fe886d64f --- /dev/null +++ b/substrate/frame/broker/src/migration.rs @@ -0,0 +1,67 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use super::*; +use crate::types::RegionRecord; +use codec::{Decode, Encode}; +use core::marker::PhantomData; +use frame_support::traits::{Get, OnRuntimeUpgrade}; +use sp_runtime::Saturating; + +#[derive(Encode, Decode)] +pub struct RegionRecordV0 { + /// The end of the Region. + pub end: Timeslice, + /// The owner of the Region. + pub owner: AccountId, + /// The amount paid to Polkadot for this Region, or `None` if renewal is not allowed. + pub paid: Option, +} + +mod v1 { + use super::*; + + pub struct MigrateToV1Impl(PhantomData); + + impl OnRuntimeUpgrade for MigrateToV1Impl { + fn on_runtime_upgrade() -> frame_support::weights::Weight { + let mut count: u64 = 0; + + >::translate::>, _>(|_, v0| { + count.saturating_inc(); + Some(RegionRecord { end: v0.end, owner: Some(v0.owner), paid: v0.paid }) + }); + + log::info!( + target: LOG_TARGET, + "Storage migration v1 for pallet-broker finished.", + ); + + // calculate and return migration weights + T::DbWeight::get().reads_writes(count as u64 + 1, count as u64 + 1) + } + } +} + +/// Migrate the pallet storage from `0` to `1`. +pub type MigrateV0ToV1 = frame_support::migrations::VersionedMigration< + 0, + 1, + v1::MigrateToV1Impl, + Pallet, + ::DbWeight, +>; diff --git a/substrate/frame/broker/src/nonfungible_impl.rs b/substrate/frame/broker/src/nonfungible_impl.rs index 13d98be833bf..f6ef7cccacb8 100644 --- a/substrate/frame/broker/src/nonfungible_impl.rs +++ b/substrate/frame/broker/src/nonfungible_impl.rs @@ -86,6 +86,8 @@ impl Mutate for Pallet { } record.owner = None; + Regions::::insert(region_id, record); + Ok(()) } } diff --git a/substrate/frame/broker/src/tests.rs b/substrate/frame/broker/src/tests.rs index 3e1e36f7d448..12672e6667e3 100644 --- a/substrate/frame/broker/src/tests.rs +++ b/substrate/frame/broker/src/tests.rs @@ -198,14 +198,27 @@ fn transfer_works() { } #[test] -fn mutate_operations_unsupported_for_regions() { - TestExt::new().execute_with(|| { +fn mutate_operations_work() { + TestExt::new().endow(1, 1000).execute_with(|| { let region_id = RegionId { begin: 0, core: 0, mask: CoreMask::complete() }; assert_noop!( >::mint_into(®ion_id.into(), &2), - TokenError::Unsupported + Error::::UnknownRegion ); - assert_noop!(>::burn(®ion_id.into(), None), TokenError::Unsupported); + + assert_ok!(Broker::do_start_sales(100, 1)); + advance_to(2); + let region_id = Broker::do_purchase(1, u64::max_value()).unwrap(); + assert_noop!( + >::mint_into(®ion_id.into(), &2), + Error::::NotAllowed + ); + assert_ok!(>::burn(®ion_id.into(), None)); + assert_eq!(Regions::::get(region_id).unwrap().owner, None); + assert_ok!(>::mint_into(®ion_id.into(), &2)); + assert_eq!(Regions::::get(region_id).unwrap().owner, Some(2)); + + // Unsupported operations: assert_noop!( >::set_attribute(®ion_id.into(), &[], &[]), TokenError::Unsupported @@ -283,7 +296,7 @@ fn nft_metadata_works() { assert_eq!(attribute::(region, b"begin"), 4); assert_eq!(attribute::(region, b"length"), 3); assert_eq!(attribute::(region, b"end"), 7); - assert_eq!(attribute::(region, b"owner"), 1); + assert_eq!(attribute::>(region, b"owner"), Some(1)); assert_eq!(attribute::(region, b"part"), 0xfffff_fffff_fffff_fffff.into()); assert_eq!(attribute::(region, b"core"), 0); assert_eq!(attribute::>(region, b"paid"), Some(100)); @@ -295,7 +308,7 @@ fn nft_metadata_works() { assert_eq!(attribute::(region, b"begin"), 6); assert_eq!(attribute::(region, b"length"), 1); assert_eq!(attribute::(region, b"end"), 7); - assert_eq!(attribute::(region, b"owner"), 42); + assert_eq!(attribute::>(region, b"owner"), Some(42)); assert_eq!(attribute::(region, b"part"), 0x00000_fffff_fffff_00000.into()); assert_eq!(attribute::(region, b"core"), 0); assert_eq!(attribute::>(region, b"paid"), None); From 6307dd34b5aff0f4c91010b9ca866e2b74ba4541 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Mon, 4 Mar 2024 18:04:16 +0100 Subject: [PATCH 11/40] fix benchmarks --- substrate/frame/broker/src/benchmarking.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/frame/broker/src/benchmarking.rs b/substrate/frame/broker/src/benchmarking.rs index 70f488e998cc..c45e5caf1dbf 100644 --- a/substrate/frame/broker/src/benchmarking.rs +++ b/substrate/frame/broker/src/benchmarking.rs @@ -313,8 +313,8 @@ mod benches { assert_last_event::( Event::Transferred { region_id: region, - old_owner: caller, - owner: recipient, + old_owner: Some(caller), + owner: Some(recipient), duration: 3u32.into(), } .into(), From c9c25041e5284cea3fe98accd498830dfb131c5d Mon Sep 17 00:00:00 2001 From: Szegoo Date: Mon, 4 Mar 2024 18:08:48 +0100 Subject: [PATCH 12/40] try runtmie --- substrate/frame/broker/src/migration.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/substrate/frame/broker/src/migration.rs b/substrate/frame/broker/src/migration.rs index 4f4fe886d64f..33be3fbcfe48 100644 --- a/substrate/frame/broker/src/migration.rs +++ b/substrate/frame/broker/src/migration.rs @@ -22,6 +22,9 @@ use core::marker::PhantomData; use frame_support::traits::{Get, OnRuntimeUpgrade}; use sp_runtime::Saturating; +#[cfg(feature = "try-runtime")] +use frame_support::ensure; + #[derive(Encode, Decode)] pub struct RegionRecordV0 { /// The end of the Region. @@ -54,6 +57,20 @@ mod v1 { // calculate and return migration weights T::DbWeight::get().reads_writes(count as u64 + 1, count as u64 + 1) } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, sp_runtime::TryRuntimeError> { + Ok((Regions::::iter_keys().count() as u32).encode()) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(state: Vec) -> Result<(), sp_runtime::TryRuntimeError> { + let old_count = u32::decode(&mut &state[..]).expect("Known good"); + let new_count = Regions::::iter_values().count() as u32; + + ensure!(old_count == new_count, "Regions count should not change"); + Ok(()) + } } } From 2ebe29d4401367ea21d47d1d303f553b2b4444e7 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Mon, 4 Mar 2024 18:23:13 +0100 Subject: [PATCH 13/40] fix --- substrate/frame/broker/src/migration.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/substrate/frame/broker/src/migration.rs b/substrate/frame/broker/src/migration.rs index 33be3fbcfe48..333958e85678 100644 --- a/substrate/frame/broker/src/migration.rs +++ b/substrate/frame/broker/src/migration.rs @@ -21,6 +21,7 @@ use codec::{Decode, Encode}; use core::marker::PhantomData; use frame_support::traits::{Get, OnRuntimeUpgrade}; use sp_runtime::Saturating; +use sp_std::prelude::vec::Vec; #[cfg(feature = "try-runtime")] use frame_support::ensure; From 5448324c11b7d88ba627a8606dcf0811bf6168ef Mon Sep 17 00:00:00 2001 From: Szegoo Date: Thu, 7 Mar 2024 13:32:27 +0100 Subject: [PATCH 14/40] fix --- substrate/frame/broker/src/migration.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/broker/src/migration.rs b/substrate/frame/broker/src/migration.rs index 333958e85678..2457e7862164 100644 --- a/substrate/frame/broker/src/migration.rs +++ b/substrate/frame/broker/src/migration.rs @@ -21,7 +21,7 @@ use codec::{Decode, Encode}; use core::marker::PhantomData; use frame_support::traits::{Get, OnRuntimeUpgrade}; use sp_runtime::Saturating; -use sp_std::prelude::vec::Vec; +use sp_std::vec::Vec; #[cfg(feature = "try-runtime")] use frame_support::ensure; From 911c8c2394b284060f093b99b22aaf442416f213 Mon Sep 17 00:00:00 2001 From: Sergej Sakac <73715684+Szegoo@users.noreply.github.com> Date: Thu, 7 Mar 2024 13:33:35 +0100 Subject: [PATCH 15/40] Update cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs Co-authored-by: Branislav Kontur --- cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs index 111d61940a68..0202c04fb3ff 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs @@ -751,7 +751,7 @@ impl_runtime_apis! { fun: NonFungible(Index(raw_region_id)), id: AssetId(Location::new(0, [PalletInstance(50)])) }, - ParentThen(Parachain(random_para_id).into()).into(), + ParentThen(Parachain(RandomParaId::get().into()).into()).into(), )) } From 30a9133a9a537015f1cf36358f5ca2735146f99f Mon Sep 17 00:00:00 2001 From: Sergej Sakac <73715684+Szegoo@users.noreply.github.com> Date: Thu, 7 Mar 2024 13:33:55 +0100 Subject: [PATCH 16/40] Update cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs Co-authored-by: Branislav Kontur --- .../parachains/runtimes/coretime/coretime-rococo/src/lib.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs index 0202c04fb3ff..2d3175c1e376 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs @@ -741,10 +741,6 @@ impl_runtime_apis! { fn reserve_transferable_asset_and_dest() -> Option<(Asset, Location)> { // Coretime chain can reserve transfer regions to some random parachain. - let random_para_id = 43211234; - ParachainSystem::open_outbound_hrmp_channel_for_benchmarks_or_tests( - random_para_id.into() - ); let raw_region_id = 42; Some(( Asset { From 8ec636b137f59b0852e5720d4cbbb496b3f361b5 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Thu, 7 Mar 2024 13:43:05 +0100 Subject: [PATCH 17/40] fix --- .../coretime/coretime-rococo/src/lib.rs | 20 ++++++++++---- .../coretime/coretime-westend/src/lib.rs | 26 ++++++++++++------- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs index 2d3175c1e376..291ad38581a4 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs @@ -764,15 +764,25 @@ impl_runtime_apis! { RocRelayLocation::get(), ExistentialDeposit::get() ).into()); + pub const RandomParaId: ParaId = ParaId::new(43211234); } impl pallet_xcm_benchmarks::Config for Runtime { type XcmConfig = xcm_config::XcmConfig; - type DeliveryHelper = cumulus_primitives_utility::ToParentDeliveryHelper< - xcm_config::XcmConfig, - ExistentialDepositAsset, - xcm_config::PriceForParentDelivery, - >; + type DeliveryHelper = ( + cumulus_primitives_utility::ToParentDeliveryHelper< + xcm_config::XcmConfig, + ExistentialDepositAsset, + xcm_config::PriceForParentDelivery, + >, + polkadot_runtime_common::xcm_sender::ToParachainDeliveryHelper< + xcm_config::XcmConfig, + ExistentialDepositAsset, + PriceForSiblingParachainDelivery, + RandomParaId, + ParachainSystem, + > + ); type AccountIdConverter = xcm_config::LocationToAccountId; fn valid_destination() -> Result { Ok(RocRelayLocation::get()) diff --git a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs index ae739ce4b915..0acb29c99078 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs @@ -222,6 +222,7 @@ impl pallet_authorship::Config for Runtime { parameter_types! { pub const ExistentialDeposit: Balance = EXISTENTIAL_DEPOSIT; + pub const RandomParaId: ParaId = ParaId::new(43211234); } impl pallet_balances::Config for Runtime { @@ -709,11 +710,20 @@ impl_runtime_apis! { use pallet_xcm::benchmarking::Pallet as PalletXcmExtrinsicsBenchmark; impl pallet_xcm::benchmarking::Config for Runtime { - type DeliveryHelper = cumulus_primitives_utility::ToParentDeliveryHelper< - xcm_config::XcmConfig, - ExistentialDepositAsset, - xcm_config::PriceForParentDelivery, - >; + type DeliveryHelper = ( + cumulus_primitives_utility::ToParentDeliveryHelper< + xcm_config::XcmConfig, + ExistentialDepositAsset, + xcm_config::PriceForParentDelivery, + >, + polkadot_runtime_common::xcm_sender::ToParachainDeliveryHelper< + xcm_config::XcmConfig, + ExistentialDepositAsset, + PriceForSiblingParachainDelivery, + RandomParaId, + ParachainSystem, + > + ); fn reachable_dest() -> Option { Some(Parent.into()) @@ -732,17 +742,13 @@ impl_runtime_apis! { fn reserve_transferable_asset_and_dest() -> Option<(Asset, Location)> { // Coretime chain can reserve transfer regions to some random parachain. - let random_para_id = 43211234; - ParachainSystem::open_outbound_hrmp_channel_for_benchmarks_or_tests( - random_para_id.into() - ); let raw_region_id = 42; Some(( Asset { fun: NonFungible(Index(raw_region_id)), id: AssetId(Location::new(0, [PalletInstance(50)])) }, - ParentThen(Parachain(random_para_id).into()).into(), + ParentThen(Parachain(RandomParaId::get().into()).into()).into(), )) } From 7809ebab9344ac10f3c4b5ce258acc6edb4f40c3 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Thu, 7 Mar 2024 13:59:26 +0100 Subject: [PATCH 18/40] check --- polkadot/xcm/pallet-xcm/src/benchmarking.rs | 24 ++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/polkadot/xcm/pallet-xcm/src/benchmarking.rs b/polkadot/xcm/pallet-xcm/src/benchmarking.rs index 4fdc1857caa4..078ed0abbeae 100644 --- a/polkadot/xcm/pallet-xcm/src/benchmarking.rs +++ b/polkadot/xcm/pallet-xcm/src/benchmarking.rs @@ -17,7 +17,7 @@ use super::*; use bounded_collections::{ConstU32, WeakBoundedVec}; use frame_benchmarking::{benchmarks, whitelisted_caller, BenchmarkError, BenchmarkResult}; -use frame_support::weights::Weight; +use frame_support::{assert_ok, weights::Weight}; use frame_system::RawOrigin; use sp_std::prelude::*; use xcm::{latest::prelude::*, v2}; @@ -170,7 +170,7 @@ benchmarks! { Fungible(amount) => { // Add transferred_amount to origin ::AssetTransactor::deposit_asset( - &Asset { fun: Fungible(*amount), id: asset.id }, + &Asset { fun: Fungible(*amount), id: asset.id.clone() }, &origin_location, None, ).map_err(|_| BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))?; @@ -182,11 +182,29 @@ benchmarks! { }; let recipient = [0u8; 32]; - let versioned_dest: VersionedLocation = destination.into(); + let versioned_dest: VersionedLocation = destination.clone().into(); let versioned_beneficiary: VersionedLocation = AccountId32 { network: None, id: recipient.into() }.into(); let versioned_assets: VersionedAssets = assets.into(); }: _>(send_origin.into(), Box::new(versioned_dest), Box::new(versioned_beneficiary), Box::new(versioned_assets), 0) + verify { + match &asset.fun { + Fungible(amount) => { + assert_ok!(::AssetTransactor::withdraw_asset( + &Asset { fun: Fungible(*amount), id: asset.id }, + &destination, + None, + )); + }, + NonFungible(instance) => { + assert_ok!(::AssetTransactor::withdraw_asset( + &asset, + &destination, + None, + )); + } + }; + } transfer_assets { let (assets, fee_index, destination, verify) = T::set_up_complex_asset_transfer().ok_or( From 663dc0bac625e25d87cf7aa6d879181b6aa07b59 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Fri, 8 Mar 2024 20:07:19 +0100 Subject: [PATCH 19/40] ... --- substrate/frame/broker/src/migration.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/substrate/frame/broker/src/migration.rs b/substrate/frame/broker/src/migration.rs index 2457e7862164..d6ae4d7a8fad 100644 --- a/substrate/frame/broker/src/migration.rs +++ b/substrate/frame/broker/src/migration.rs @@ -21,10 +21,11 @@ use codec::{Decode, Encode}; use core::marker::PhantomData; use frame_support::traits::{Get, OnRuntimeUpgrade}; use sp_runtime::Saturating; -use sp_std::vec::Vec; #[cfg(feature = "try-runtime")] use frame_support::ensure; +#[cfg(feature = "try-runtime")] +use sp_std::vec::Vec; #[derive(Encode, Decode)] pub struct RegionRecordV0 { From 799a1d90ed861a3a32c8183469367d873734bd8f Mon Sep 17 00:00:00 2001 From: Szegoo Date: Mon, 11 Mar 2024 11:08:50 +0100 Subject: [PATCH 20/40] remove duplicate --- prdoc/pr_3455.prdoc | 1 - 1 file changed, 1 deletion(-) diff --git a/prdoc/pr_3455.prdoc b/prdoc/pr_3455.prdoc index 86358c9ebef3..1d08a7099f32 100644 --- a/prdoc/pr_3455.prdoc +++ b/prdoc/pr_3455.prdoc @@ -15,4 +15,3 @@ crates: - name: contracts-rococo-runtime - name: coretime-rococo-runtime - name: coretime-westend-runtime - - name: coretime-westend-runtime From 52f7d4b997c6d25d49078c99186c6588cf118ef8 Mon Sep 17 00:00:00 2001 From: Sergej Sakac <73715684+Szegoo@users.noreply.github.com> Date: Mon, 11 Mar 2024 11:09:08 +0100 Subject: [PATCH 21/40] Update polkadot/xcm/pallet-xcm/src/benchmarking.rs Co-authored-by: Francisco Aguirre --- polkadot/xcm/pallet-xcm/src/benchmarking.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/polkadot/xcm/pallet-xcm/src/benchmarking.rs b/polkadot/xcm/pallet-xcm/src/benchmarking.rs index 078ed0abbeae..edc84a2f3e78 100644 --- a/polkadot/xcm/pallet-xcm/src/benchmarking.rs +++ b/polkadot/xcm/pallet-xcm/src/benchmarking.rs @@ -129,7 +129,10 @@ benchmarks! { &Asset { fun: Fungible(*amount), id: asset.id }, &origin_location, None, - ).map_err(|_| BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))?; + ).map_err(|error| { + log::error!("Asset couldn't be deposited, error: {:?}", error); + BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)) + })?; }, NonFungible(instance) => { ::AssetTransactor::deposit_asset(&asset, &origin_location, None) From 5b5f8de65d0f5fbdb8c8733e8bae8f853ac2abde Mon Sep 17 00:00:00 2001 From: Szegoo Date: Mon, 11 Mar 2024 11:13:45 +0100 Subject: [PATCH 22/40] log error --- polkadot/xcm/pallet-xcm/src/benchmarking.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/polkadot/xcm/pallet-xcm/src/benchmarking.rs b/polkadot/xcm/pallet-xcm/src/benchmarking.rs index edc84a2f3e78..081a4235b779 100644 --- a/polkadot/xcm/pallet-xcm/src/benchmarking.rs +++ b/polkadot/xcm/pallet-xcm/src/benchmarking.rs @@ -130,13 +130,16 @@ benchmarks! { &origin_location, None, ).map_err(|error| { - log::error!("Asset couldn't be deposited, error: {:?}", error); + log::error!("Fungible asset couldn't be deposited, error: {:?}", error); BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)) })?; }, NonFungible(instance) => { ::AssetTransactor::deposit_asset(&asset, &origin_location, None) - .map_err(|_| BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))?; + .map_err(|error| { + log::error!("Nonfungible asset couldn't be deposited, error: {:?}", error); + BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)) + })?; } }; @@ -176,11 +179,17 @@ benchmarks! { &Asset { fun: Fungible(*amount), id: asset.id.clone() }, &origin_location, None, - ).map_err(|_| BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))?; + ).map_err(|error| { + log::error!("Fungible asset couldn't be deposited, error: {:?}", error); + BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)) + })?; }, NonFungible(instance) => { ::AssetTransactor::deposit_asset(&asset, &origin_location, None) - .map_err(|_| BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))?; + .map_err(|error| { + log::error!("Nonfungible asset couldn't be deposited, error: {:?}", error); + BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)) + })?; } }; From b41485606c044d827c38aa6cb1a4d1275ed51e59 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Thu, 14 Mar 2024 18:06:02 +0100 Subject: [PATCH 23/40] improve docs & tests --- substrate/frame/broker/src/migration.rs | 1 + substrate/frame/broker/src/nonfungible_impl.rs | 2 ++ substrate/frame/broker/src/tests.rs | 7 ++++++- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/substrate/frame/broker/src/migration.rs b/substrate/frame/broker/src/migration.rs index d6ae4d7a8fad..8a4ecfc21c30 100644 --- a/substrate/frame/broker/src/migration.rs +++ b/substrate/frame/broker/src/migration.rs @@ -27,6 +27,7 @@ use frame_support::ensure; #[cfg(feature = "try-runtime")] use sp_std::vec::Vec; +/// V0 region record. #[derive(Encode, Decode)] pub struct RegionRecordV0 { /// The end of the Region. diff --git a/substrate/frame/broker/src/nonfungible_impl.rs b/substrate/frame/broker/src/nonfungible_impl.rs index f6ef7cccacb8..b4e4353f7d4c 100644 --- a/substrate/frame/broker/src/nonfungible_impl.rs +++ b/substrate/frame/broker/src/nonfungible_impl.rs @@ -66,6 +66,7 @@ impl Transfer for Pallet { /// its owner to `None`, whereas 'minting' the region assigns its owner to an actual account. This /// way we never lose track of the associated record data. impl Mutate for Pallet { + /// Deposit a region into an account. fn mint_into(item: &Self::ItemId, who: &T::AccountId) -> DispatchResult { let region_id: RegionId = (*item).into(); let record = Regions::::get(®ion_id).ok_or(Error::::UnknownRegion)?; @@ -78,6 +79,7 @@ impl Mutate for Pallet { Ok(()) } + /// Withdraw a region from account. fn burn(item: &Self::ItemId, maybe_check_owner: Option<&T::AccountId>) -> DispatchResult { let region_id: RegionId = (*item).into(); let mut record = Regions::::get(®ion_id).ok_or(Error::::UnknownRegion)?; diff --git a/substrate/frame/broker/src/tests.rs b/substrate/frame/broker/src/tests.rs index 12672e6667e3..f7a6c1338343 100644 --- a/substrate/frame/broker/src/tests.rs +++ b/substrate/frame/broker/src/tests.rs @@ -213,8 +213,13 @@ fn mutate_operations_work() { >::mint_into(®ion_id.into(), &2), Error::::NotAllowed ); - assert_ok!(>::burn(®ion_id.into(), None)); + + assert_noop!(>::burn(®ion_id.into(), Some(&2)), Error::::NotOwner); + // 'withdraw' the region from user 1: + assert_ok!(>::burn(®ion_id.into(), Some(&1))); assert_eq!(Regions::::get(region_id).unwrap().owner, None); + + // `mint_into` works after burning: assert_ok!(>::mint_into(®ion_id.into(), &2)); assert_eq!(Regions::::get(region_id).unwrap().owner, Some(2)); From fd70daaf3c6059e56511c24ff030696cb2cf08bc Mon Sep 17 00:00:00 2001 From: Szegoo Date: Thu, 14 Mar 2024 18:25:20 +0100 Subject: [PATCH 24/40] prdoc --- prdoc/pr_3553.prdoc | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 prdoc/pr_3553.prdoc diff --git a/prdoc/pr_3553.prdoc b/prdoc/pr_3553.prdoc new file mode 100644 index 000000000000..5f8c94667c3f --- /dev/null +++ b/prdoc/pr_3553.prdoc @@ -0,0 +1,28 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: Region reserve transfers fix + +doc: + - audience: Runtime User + description: | + This PR addresses the issue with cross-chain transferring regions back to the + Coretime chain. TL;DR: Remote reserve transfers are performed by withdrawing + and depositing the asset to and from the holding registry. This requires the + asset to support burning and minting functionality. + This PR adds burning and minting; however, they work a bit differently than usual + so that the associated region record is not lost when burning. Instead of removing + all the data, burning will set the owner of the region to None, and when minting + it back, it will set it to an actual value. So, when cross-chain transferring, + withdrawing into the registry will remove the region from its original owner, and + when depositing it from the registry, it will set its owner to another account + +migrations: + db: [] + runtime: + - reference: pallet-regions + description: | + The region owner is optional. + +crates: + - name: pallet-regions From d8eb54b49061082db5894b171661a1994b6c9c97 Mon Sep 17 00:00:00 2001 From: Sergej Sakac <73715684+Szegoo@users.noreply.github.com> Date: Wed, 27 Mar 2024 16:17:22 +0100 Subject: [PATCH 25/40] Update cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs Co-authored-by: Branislav Kontur --- cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs index 291ad38581a4..5b511eed62de 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs @@ -745,7 +745,7 @@ impl_runtime_apis! { Some(( Asset { fun: NonFungible(Index(raw_region_id)), - id: AssetId(Location::new(0, [PalletInstance(50)])) + id: AssetId(xcm_config::BrokerPalletLocation::get()) }, ParentThen(Parachain(RandomParaId::get().into()).into()).into(), )) From fdd21e4845b4f73183dd1b45f4a2f7604b801435 Mon Sep 17 00:00:00 2001 From: Sergej Sakac <73715684+Szegoo@users.noreply.github.com> Date: Wed, 27 Mar 2024 16:17:28 +0100 Subject: [PATCH 26/40] Update cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs Co-authored-by: Branislav Kontur --- .../parachains/runtimes/coretime/coretime-westend/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs index 0acb29c99078..d9c776e3be89 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs @@ -746,7 +746,7 @@ impl_runtime_apis! { Some(( Asset { fun: NonFungible(Index(raw_region_id)), - id: AssetId(Location::new(0, [PalletInstance(50)])) + id: AssetId(xcm_config::BrokerPalletLocation::get()) }, ParentThen(Parachain(RandomParaId::get().into()).into()).into(), )) From 9b2575ba265b4bb88b171fef9ecc7d14c9b63d9a Mon Sep 17 00:00:00 2001 From: Szegoo Date: Fri, 29 Mar 2024 14:44:18 +0100 Subject: [PATCH 27/40] region reserve transfer --- .../runtimes/coretime/coretime-rococo/src/lib.rs | 10 ++++++++-- substrate/frame/broker/src/dispatchable_impls.rs | 2 +- substrate/frame/broker/src/nonfungible_impl.rs | 2 +- substrate/frame/broker/src/tests.rs | 5 ++++- substrate/frame/broker/src/utility_impls.rs | 6 +++--- 5 files changed, 17 insertions(+), 8 deletions(-) diff --git a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs index 5b511eed62de..2f871203099f 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs @@ -741,10 +741,16 @@ impl_runtime_apis! { fn reserve_transferable_asset_and_dest() -> Option<(Asset, Location)> { // Coretime chain can reserve transfer regions to some random parachain. - let raw_region_id = 42; + + // Properties of a mock region: + let core = 0; + let begin = 0; + let end = 42; + + let region_id = pallet_broker::Pallet::::issue(core, begin, end, None, None); Some(( Asset { - fun: NonFungible(Index(raw_region_id)), + fun: NonFungible(Index(region_id.into())), id: AssetId(xcm_config::BrokerPalletLocation::get()) }, ParentThen(Parachain(RandomParaId::get().into()).into()).into(), diff --git a/substrate/frame/broker/src/dispatchable_impls.rs b/substrate/frame/broker/src/dispatchable_impls.rs index b44318993145..0cdfc3e1a88a 100644 --- a/substrate/frame/broker/src/dispatchable_impls.rs +++ b/substrate/frame/broker/src/dispatchable_impls.rs @@ -119,7 +119,7 @@ impl Pallet { sale.sellout_price = Some(price); } SaleInfo::::put(&sale); - let id = Self::issue(core, sale.region_begin, sale.region_end, who.clone(), Some(price)); + let id = Self::issue(core, sale.region_begin, sale.region_end, Some(who.clone()), Some(price)); let duration = sale.region_end.saturating_sub(sale.region_begin); Self::deposit_event(Event::Purchased { who, region_id: id, price, duration }); Ok(id) diff --git a/substrate/frame/broker/src/nonfungible_impl.rs b/substrate/frame/broker/src/nonfungible_impl.rs index b4e4353f7d4c..53f5995bc3bd 100644 --- a/substrate/frame/broker/src/nonfungible_impl.rs +++ b/substrate/frame/broker/src/nonfungible_impl.rs @@ -74,7 +74,7 @@ impl Mutate for Pallet { // 'Minting' can only occur if the asset has previously been burned(i.e. moved to the // holding registrar) ensure!(record.owner.is_none(), Error::::NotAllowed); - Self::issue(region_id.core, region_id.begin, record.end, who.clone(), record.paid); + Self::issue(region_id.core, region_id.begin, record.end, Some(who.clone()), record.paid); Ok(()) } diff --git a/substrate/frame/broker/src/tests.rs b/substrate/frame/broker/src/tests.rs index f7a6c1338343..2e1a7548b493 100644 --- a/substrate/frame/broker/src/tests.rs +++ b/substrate/frame/broker/src/tests.rs @@ -214,7 +214,10 @@ fn mutate_operations_work() { Error::::NotAllowed ); - assert_noop!(>::burn(®ion_id.into(), Some(&2)), Error::::NotOwner); + assert_noop!( + >::burn(®ion_id.into(), Some(&2)), + Error::::NotOwner + ); // 'withdraw' the region from user 1: assert_ok!(>::burn(®ion_id.into(), Some(&1))); assert_eq!(Regions::::get(region_id).unwrap().owner, None); diff --git a/substrate/frame/broker/src/utility_impls.rs b/substrate/frame/broker/src/utility_impls.rs index 3d35805b91a4..10b6335128af 100644 --- a/substrate/frame/broker/src/utility_impls.rs +++ b/substrate/frame/broker/src/utility_impls.rs @@ -72,15 +72,15 @@ impl Pallet { Ok(()) } - pub(crate) fn issue( + pub fn issue( core: CoreIndex, begin: Timeslice, end: Timeslice, - owner: T::AccountId, + owner: Option, paid: Option>, ) -> RegionId { let id = RegionId { begin, core, mask: CoreMask::complete() }; - let record = RegionRecord { end, owner: Some(owner), paid }; + let record = RegionRecord { end, owner: owner, paid }; Regions::::insert(&id, &record); id } From 09789f9b493c83e65f8af97f33d13565350d7d60 Mon Sep 17 00:00:00 2001 From: Sergej Date: Sat, 30 Mar 2024 08:24:45 +0100 Subject: [PATCH 28/40] fix delivery helper --- .../coretime/coretime-rococo/src/lib.rs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs index 2f871203099f..6dabb3df9adf 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs @@ -718,11 +718,20 @@ impl_runtime_apis! { use pallet_xcm::benchmarking::Pallet as PalletXcmExtrinsicsBenchmark; impl pallet_xcm::benchmarking::Config for Runtime { - type DeliveryHelper = cumulus_primitives_utility::ToParentDeliveryHelper< - xcm_config::XcmConfig, - ExistentialDepositAsset, - xcm_config::PriceForParentDelivery, - >; + type DeliveryHelper = ( + cumulus_primitives_utility::ToParentDeliveryHelper< + xcm_config::XcmConfig, + ExistentialDepositAsset, + xcm_config::PriceForParentDelivery, + >, + polkadot_runtime_common::xcm_sender::ToParachainDeliveryHelper< + xcm_config::XcmConfig, + ExistentialDepositAsset, + PriceForSiblingParachainDelivery, + RandomParaId, + ParachainSystem, + > + ); fn reachable_dest() -> Option { Some(Parent.into()) From afc31931888307d0c578cdf164e5e24af86d06d4 Mon Sep 17 00:00:00 2001 From: Sergej Date: Sat, 30 Mar 2024 08:34:30 +0100 Subject: [PATCH 29/40] apply same fixes in westend --- .../coretime/coretime-westend/src/lib.rs | 31 ++++++++++++++----- .../frame/broker/src/dispatchable_impls.rs | 3 +- substrate/frame/broker/src/utility_impls.rs | 2 +- 3 files changed, 27 insertions(+), 9 deletions(-) diff --git a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs index d9c776e3be89..34955ada6278 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs @@ -742,10 +742,16 @@ impl_runtime_apis! { fn reserve_transferable_asset_and_dest() -> Option<(Asset, Location)> { // Coretime chain can reserve transfer regions to some random parachain. - let raw_region_id = 42; + + // Properties of a mock region: + let core = 0; + let begin = 0; + let end = 42; + + let region_id = pallet_broker::Pallet::::issue(core, begin, end, None, None); Some(( Asset { - fun: NonFungible(Index(raw_region_id)), + fun: NonFungible(Index(region_id.into())), id: AssetId(xcm_config::BrokerPalletLocation::get()) }, ParentThen(Parachain(RandomParaId::get().into()).into()).into(), @@ -769,11 +775,22 @@ impl_runtime_apis! { impl pallet_xcm_benchmarks::Config for Runtime { type XcmConfig = xcm_config::XcmConfig; - type DeliveryHelper = cumulus_primitives_utility::ToParentDeliveryHelper< - xcm_config::XcmConfig, - ExistentialDepositAsset, - xcm_config::PriceForParentDelivery, - >; + + type DeliveryHelper = ( + cumulus_primitives_utility::ToParentDeliveryHelper< + xcm_config::XcmConfig, + ExistentialDepositAsset, + xcm_config::PriceForParentDelivery, + >, + polkadot_runtime_common::xcm_sender::ToParachainDeliveryHelper< + xcm_config::XcmConfig, + ExistentialDepositAsset, + PriceForSiblingParachainDelivery, + RandomParaId, + ParachainSystem, + > + ); + type AccountIdConverter = xcm_config::LocationToAccountId; fn valid_destination() -> Result { Ok(TokenRelayLocation::get()) diff --git a/substrate/frame/broker/src/dispatchable_impls.rs b/substrate/frame/broker/src/dispatchable_impls.rs index 0cdfc3e1a88a..dc32d3050d34 100644 --- a/substrate/frame/broker/src/dispatchable_impls.rs +++ b/substrate/frame/broker/src/dispatchable_impls.rs @@ -119,7 +119,8 @@ impl Pallet { sale.sellout_price = Some(price); } SaleInfo::::put(&sale); - let id = Self::issue(core, sale.region_begin, sale.region_end, Some(who.clone()), Some(price)); + let id = + Self::issue(core, sale.region_begin, sale.region_end, Some(who.clone()), Some(price)); let duration = sale.region_end.saturating_sub(sale.region_begin); Self::deposit_event(Event::Purchased { who, region_id: id, price, duration }); Ok(id) diff --git a/substrate/frame/broker/src/utility_impls.rs b/substrate/frame/broker/src/utility_impls.rs index 10b6335128af..4163817a8b58 100644 --- a/substrate/frame/broker/src/utility_impls.rs +++ b/substrate/frame/broker/src/utility_impls.rs @@ -80,7 +80,7 @@ impl Pallet { paid: Option>, ) -> RegionId { let id = RegionId { begin, core, mask: CoreMask::complete() }; - let record = RegionRecord { end, owner: owner, paid }; + let record = RegionRecord { end, owner, paid }; Regions::::insert(&id, &record); id } From c75b4864573d220c0da6b7e028059e8486c7d555 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Tue, 2 Apr 2024 10:13:58 +0200 Subject: [PATCH 30/40] update prdoc --- prdoc/pr_3455.prdoc | 22 +++++++++++++++++----- prdoc/pr_3553.prdoc | 28 ---------------------------- 2 files changed, 17 insertions(+), 33 deletions(-) delete mode 100644 prdoc/pr_3553.prdoc diff --git a/prdoc/pr_3455.prdoc b/prdoc/pr_3455.prdoc index 1d08a7099f32..587775b55353 100644 --- a/prdoc/pr_3455.prdoc +++ b/prdoc/pr_3455.prdoc @@ -1,16 +1,28 @@ # Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 # See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json -title: pallet-xcm benchmarking fixes +title: Region reserve transfers fix doc: - - audience: Runtime Dev + - audience: Runtime User description: | - With this PR, the benchmarks for pallet-xcm no longer assume the use of - `pallet-balances` for fungible implementations. Furthermore, with this PR - non-fungible teleport and reserve transfers are properly benchmarked. + This PR introduces changes enabling the transfer of coretime regions via XCM. + There are two primary issues that are resolved in this PR: + 1. The mint and burn functions were not implemented for coretime regions. These operations + are essential for moving assets to and from the XCM holding register. + 2. The transfer of non-fungible assets through XCM was previously disallowed. This was due + to incorrectly benchmarking non-fungible asset transfers via XCM, which led to assigning + it a weight of Weight::Max, effectively preventing its execution. + +migrations: + db: [] + runtime: + - reference: pallet-regions + description: | + The region owner is optional. crates: + - name: pallet-regions - name: pallet-xcm - name: contracts-rococo-runtime - name: coretime-rococo-runtime diff --git a/prdoc/pr_3553.prdoc b/prdoc/pr_3553.prdoc deleted file mode 100644 index 5f8c94667c3f..000000000000 --- a/prdoc/pr_3553.prdoc +++ /dev/null @@ -1,28 +0,0 @@ -# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 -# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json - -title: Region reserve transfers fix - -doc: - - audience: Runtime User - description: | - This PR addresses the issue with cross-chain transferring regions back to the - Coretime chain. TL;DR: Remote reserve transfers are performed by withdrawing - and depositing the asset to and from the holding registry. This requires the - asset to support burning and minting functionality. - This PR adds burning and minting; however, they work a bit differently than usual - so that the associated region record is not lost when burning. Instead of removing - all the data, burning will set the owner of the region to None, and when minting - it back, it will set it to an actual value. So, when cross-chain transferring, - withdrawing into the registry will remove the region from its original owner, and - when depositing it from the registry, it will set its owner to another account - -migrations: - db: [] - runtime: - - reference: pallet-regions - description: | - The region owner is optional. - -crates: - - name: pallet-regions From 3c714a5a239dbb6dc1182e27e335033b5d3d4080 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Tue, 2 Apr 2024 10:18:24 +0200 Subject: [PATCH 31/40] move RegionRecordV0 to v1 --- substrate/frame/broker/src/migration.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/substrate/frame/broker/src/migration.rs b/substrate/frame/broker/src/migration.rs index 8a4ecfc21c30..f6e3542ed70a 100644 --- a/substrate/frame/broker/src/migration.rs +++ b/substrate/frame/broker/src/migration.rs @@ -27,20 +27,20 @@ use frame_support::ensure; #[cfg(feature = "try-runtime")] use sp_std::vec::Vec; -/// V0 region record. -#[derive(Encode, Decode)] -pub struct RegionRecordV0 { - /// The end of the Region. - pub end: Timeslice, - /// The owner of the Region. - pub owner: AccountId, - /// The amount paid to Polkadot for this Region, or `None` if renewal is not allowed. - pub paid: Option, -} - mod v1 { use super::*; + /// V0 region record. + #[derive(Encode, Decode)] + struct RegionRecordV0 { + /// The end of the Region. + pub end: Timeslice, + /// The owner of the Region. + pub owner: AccountId, + /// The amount paid to Polkadot for this Region, or `None` if renewal is not allowed. + pub paid: Option, + } + pub struct MigrateToV1Impl(PhantomData); impl OnRuntimeUpgrade for MigrateToV1Impl { From 962b58974f90059bc3cb2115ad540079545f6390 Mon Sep 17 00:00:00 2001 From: Sergej Sakac <73715684+Szegoo@users.noreply.github.com> Date: Fri, 5 Apr 2024 17:29:51 +0200 Subject: [PATCH 32/40] Update prdoc/pr_3455.prdoc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Dónal Murray --- prdoc/pr_3455.prdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prdoc/pr_3455.prdoc b/prdoc/pr_3455.prdoc index 587775b55353..3eae0e0a9a08 100644 --- a/prdoc/pr_3455.prdoc +++ b/prdoc/pr_3455.prdoc @@ -17,7 +17,7 @@ doc: migrations: db: [] runtime: - - reference: pallet-regions + - reference: pallet-broker description: | The region owner is optional. From 94fc92f031052b96072d51d5e2b1510d8158d085 Mon Sep 17 00:00:00 2001 From: Sergej Sakac <73715684+Szegoo@users.noreply.github.com> Date: Fri, 5 Apr 2024 17:30:07 +0200 Subject: [PATCH 33/40] Update prdoc/pr_3455.prdoc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Dónal Murray --- prdoc/pr_3455.prdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prdoc/pr_3455.prdoc b/prdoc/pr_3455.prdoc index 3eae0e0a9a08..06bdff24422c 100644 --- a/prdoc/pr_3455.prdoc +++ b/prdoc/pr_3455.prdoc @@ -22,7 +22,7 @@ migrations: The region owner is optional. crates: - - name: pallet-regions + - name: pallet-broker - name: pallet-xcm - name: contracts-rococo-runtime - name: coretime-rococo-runtime From 883ded43cabe5bb28f6391d59a5d6d8e0f7711e5 Mon Sep 17 00:00:00 2001 From: Sergej Sakac <73715684+Szegoo@users.noreply.github.com> Date: Fri, 5 Apr 2024 17:30:13 +0200 Subject: [PATCH 34/40] Update substrate/frame/broker/src/nonfungible_impl.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Dónal Murray --- substrate/frame/broker/src/nonfungible_impl.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/frame/broker/src/nonfungible_impl.rs b/substrate/frame/broker/src/nonfungible_impl.rs index 53f5995bc3bd..80dcc175df53 100644 --- a/substrate/frame/broker/src/nonfungible_impl.rs +++ b/substrate/frame/broker/src/nonfungible_impl.rs @@ -71,8 +71,8 @@ impl Mutate for Pallet { let region_id: RegionId = (*item).into(); let record = Regions::::get(®ion_id).ok_or(Error::::UnknownRegion)?; - // 'Minting' can only occur if the asset has previously been burned(i.e. moved to the - // holding registrar) + // 'Minting' can only occur if the asset has previously been burned (i.e. moved to the + // holding register) ensure!(record.owner.is_none(), Error::::NotAllowed); Self::issue(region_id.core, region_id.begin, record.end, Some(who.clone()), record.paid); From 0f510c836cb5145a919f247bcb38b13dc62d9bcd Mon Sep 17 00:00:00 2001 From: Sergej Date: Fri, 5 Apr 2024 17:37:31 +0200 Subject: [PATCH 35/40] revert contracts-rococo xcm config --- .../runtimes/contracts/contracts-rococo/src/xcm_config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cumulus/parachains/runtimes/contracts/contracts-rococo/src/xcm_config.rs b/cumulus/parachains/runtimes/contracts/contracts-rococo/src/xcm_config.rs index a310b84d43ba..e8f3209eb67f 100644 --- a/cumulus/parachains/runtimes/contracts/contracts-rococo/src/xcm_config.rs +++ b/cumulus/parachains/runtimes/contracts/contracts-rococo/src/xcm_config.rs @@ -227,7 +227,7 @@ impl pallet_xcm::Config for Runtime { type XcmExecuteFilter = Nothing; type XcmExecutor = XcmExecutor; type XcmTeleportFilter = Everything; - type XcmReserveTransferFilter = Nothing; + type XcmReserveTransferFilter = Everything; type Weigher = FixedWeightBounds; type UniversalLocation = UniversalLocation; type RuntimeOrigin = RuntimeOrigin; From e6cc759aed6fde391d329dd8a21855f4b58c279a Mon Sep 17 00:00:00 2001 From: Sergej Date: Fri, 5 Apr 2024 18:00:08 +0200 Subject: [PATCH 36/40] update prdoc --- prdoc/pr_3455.prdoc | 1 - 1 file changed, 1 deletion(-) diff --git a/prdoc/pr_3455.prdoc b/prdoc/pr_3455.prdoc index 06bdff24422c..c16ac2244862 100644 --- a/prdoc/pr_3455.prdoc +++ b/prdoc/pr_3455.prdoc @@ -24,6 +24,5 @@ migrations: crates: - name: pallet-broker - name: pallet-xcm - - name: contracts-rococo-runtime - name: coretime-rococo-runtime - name: coretime-westend-runtime From 73e9c7f960c5c938f30572870da6eb0a8288e2bd Mon Sep 17 00:00:00 2001 From: Sergej Date: Fri, 5 Apr 2024 18:16:56 +0200 Subject: [PATCH 37/40] bump version --- substrate/frame/broker/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/broker/src/lib.rs b/substrate/frame/broker/src/lib.rs index edfe327ae2f2..a3d0f902888b 100644 --- a/substrate/frame/broker/src/lib.rs +++ b/substrate/frame/broker/src/lib.rs @@ -63,7 +63,7 @@ pub mod pallet { use sp_runtime::traits::{Convert, ConvertBack}; use sp_std::vec::Vec; - const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); + const STORAGE_VERSION: StorageVersion = StorageVersion::new(2); #[pallet::pallet] #[pallet::storage_version(STORAGE_VERSION)] From e312286eb4c8edc4529dc24c9ce97ae245936f65 Mon Sep 17 00:00:00 2001 From: Sergej Sakac <73715684+Szegoo@users.noreply.github.com> Date: Mon, 8 Apr 2024 17:50:36 +0200 Subject: [PATCH 38/40] Update substrate/frame/broker/src/lib.rs --- substrate/frame/broker/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/broker/src/lib.rs b/substrate/frame/broker/src/lib.rs index 0dd3b3b8b284..eb53bdbe6618 100644 --- a/substrate/frame/broker/src/lib.rs +++ b/substrate/frame/broker/src/lib.rs @@ -65,7 +65,7 @@ pub mod pallet { use sp_runtime::traits::{Convert, ConvertBack}; use sp_std::vec::Vec; - const STORAGE_VERSION: StorageVersion = StorageVersion::new(2); + const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); #[pallet::pallet] #[pallet::storage_version(STORAGE_VERSION)] From 1374042c610cd7dad41bcb724a2f3efba479eb8a Mon Sep 17 00:00:00 2001 From: Sergej Date: Tue, 16 Apr 2024 17:28:13 +0200 Subject: [PATCH 39/40] add missing migration in rococo & westend --- .../parachains/runtimes/coretime/coretime-rococo/src/lib.rs | 1 + .../parachains/runtimes/coretime/coretime-westend/src/lib.rs | 3 ++- substrate/frame/broker/src/migration.rs | 4 ++-- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs index c6bdb5eb31cd..ad065ee34774 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs @@ -109,6 +109,7 @@ pub type UncheckedExtrinsic = /// Migrations to apply on runtime upgrade. pub type Migrations = ( cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4, + pallet_broker::migration::MigrateV0ToV1, // permanent pallet_xcm::migration::MigrateToLatestXcmVersion, ); diff --git a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs index 6e7e025ce0ce..23e06016e423 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs @@ -108,7 +108,8 @@ pub type UncheckedExtrinsic = /// Migrations to apply on runtime upgrade. pub type Migrations = ( - cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4, + cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4, + pallet_broker::migration::MigrateV0ToV1, // permanent pallet_xcm::migration::MigrateToLatestXcmVersion, ); diff --git a/substrate/frame/broker/src/migration.rs b/substrate/frame/broker/src/migration.rs index f6e3542ed70a..95aa28250a62 100644 --- a/substrate/frame/broker/src/migration.rs +++ b/substrate/frame/broker/src/migration.rs @@ -19,7 +19,7 @@ use super::*; use crate::types::RegionRecord; use codec::{Decode, Encode}; use core::marker::PhantomData; -use frame_support::traits::{Get, OnRuntimeUpgrade}; +use frame_support::traits::{Get, UncheckedOnRuntimeUpgrade}; use sp_runtime::Saturating; #[cfg(feature = "try-runtime")] @@ -43,7 +43,7 @@ mod v1 { pub struct MigrateToV1Impl(PhantomData); - impl OnRuntimeUpgrade for MigrateToV1Impl { + impl UncheckedOnRuntimeUpgrade for MigrateToV1Impl { fn on_runtime_upgrade() -> frame_support::weights::Weight { let mut count: u64 = 0; From 8d1d338e28d367e69436ab3020869346e0112491 Mon Sep 17 00:00:00 2001 From: Sergej Date: Tue, 16 Apr 2024 17:29:12 +0200 Subject: [PATCH 40/40] fmt --- .../parachains/runtimes/coretime/coretime-westend/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs index 23e06016e423..0f0742268618 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs @@ -108,7 +108,7 @@ pub type UncheckedExtrinsic = /// Migrations to apply on runtime upgrade. pub type Migrations = ( - cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4, + cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4, pallet_broker::migration::MigrateV0ToV1, // permanent pallet_xcm::migration::MigrateToLatestXcmVersion,