diff --git a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs index 9fd0093840d3..37a6ea1d1c5f 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs @@ -832,7 +832,7 @@ impl_runtime_apis! { let begin = 0; let end = 42; - let region_id = pallet_broker::Pallet::::issue(core, begin, end, None, None); + let region_id = pallet_broker::Pallet::::issue(core, begin, pallet_broker::CoreMask::complete(), end, None, None); Some(( Asset { fun: NonFungible(Index(region_id.into())), diff --git a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs index 7907f252cf8e..b6847dddb6c0 100644 --- a/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/coretime/coretime-westend/src/lib.rs @@ -823,7 +823,7 @@ impl_runtime_apis! { let begin = 0; let end = 42; - let region_id = pallet_broker::Pallet::::issue(core, begin, end, None, None); + let region_id = pallet_broker::Pallet::::issue(core, begin, pallet_broker::CoreMask::complete(), end, None, None); Some(( Asset { fun: NonFungible(Index(region_id.into())), diff --git a/prdoc/pr_5067.prdoc b/prdoc/pr_5067.prdoc new file mode 100644 index 000000000000..9a11f96b5104 --- /dev/null +++ b/prdoc/pr_5067.prdoc @@ -0,0 +1,19 @@ +# 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: Fix region nonfungible implementation + +doc: + - audience: Runtime User + description: | + PR fixes the issue with the current implementation where minting causes + the region coremask to be set to `Coremask::complete` regardless of the + actual coremask of the region. + +crates: +- name: pallet-broker + bump: major +- name: coretime-rococo-runtime + bump: patch +- name: coretime-westend-runtime + bump: patch \ No newline at end of file diff --git a/substrate/frame/broker/src/dispatchable_impls.rs b/substrate/frame/broker/src/dispatchable_impls.rs index 9e7a56e52812..a8ded084a801 100644 --- a/substrate/frame/broker/src/dispatchable_impls.rs +++ b/substrate/frame/broker/src/dispatchable_impls.rs @@ -128,8 +128,14 @@ impl Pallet { let core = Self::purchase_core(&who, price, &mut sale)?; 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, + CoreMask::complete(), + 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 e272ecbe0081..acbcba1dae49 100644 --- a/substrate/frame/broker/src/nonfungible_impl.rs +++ b/substrate/frame/broker/src/nonfungible_impl.rs @@ -74,7 +74,14 @@ impl Mutate for Pallet { // '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); + Self::issue( + region_id.core, + region_id.begin, + region_id.mask, + 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 2a8ea24b447a..3ea68c5a74d6 100644 --- a/substrate/frame/broker/src/tests.rs +++ b/substrate/frame/broker/src/tests.rs @@ -240,6 +240,49 @@ fn mutate_operations_work() { }); } +#[test] +fn mutate_operations_work_with_partitioned_region() { + TestExt::new().endow(1, 1000).execute_with(|| { + assert_ok!(Broker::do_start_sales(100, 1)); + advance_to(2); + let region = Broker::do_purchase(1, u64::max_value()).unwrap(); + let (region1, _region2) = Broker::do_partition(region, None, 2).unwrap(); + let record_1 = Regions::::get(region1).unwrap(); + + // 'withdraw' the region from user 1: + assert_ok!(>::burn(®ion1.into(), Some(&1))); + assert_eq!(Regions::::get(region1).unwrap().owner, None); + + // `mint_into` works after burning: + assert_ok!(>::mint_into(®ion1.into(), &1)); + + // Ensure the region minted is the same as the one we burned previously: + assert_eq!(Regions::::get(region1).unwrap(), record_1); + }); +} + +#[test] +fn mutate_operations_work_with_interlaced_region() { + TestExt::new().endow(1, 1000).execute_with(|| { + assert_ok!(Broker::do_start_sales(100, 1)); + advance_to(2); + let region = Broker::do_purchase(1, u64::max_value()).unwrap(); + let (region1, _region2) = + Broker::do_interlace(region, None, CoreMask::from_chunk(0, 40)).unwrap(); + let record_1 = Regions::::get(region1).unwrap(); + + // 'withdraw' the region from user 1: + assert_ok!(>::burn(®ion1.into(), Some(&1))); + assert_eq!(Regions::::get(region1).unwrap().owner, None); + + // `mint_into` works after burning: + assert_ok!(>::mint_into(®ion1.into(), &1)); + + // Ensure the region minted is the same as the one we burned previously: + assert_eq!(Regions::::get(region1).unwrap(), record_1); + }); +} + #[test] fn permanent_is_not_reassignable() { TestExt::new().endow(1, 1000).execute_with(|| { diff --git a/substrate/frame/broker/src/utility_impls.rs b/substrate/frame/broker/src/utility_impls.rs index 9cceb7f970a9..5c66ebb9674b 100644 --- a/substrate/frame/broker/src/utility_impls.rs +++ b/substrate/frame/broker/src/utility_impls.rs @@ -94,11 +94,12 @@ impl Pallet { pub fn issue( core: CoreIndex, begin: Timeslice, + mask: CoreMask, end: Timeslice, owner: Option, paid: Option>, ) -> RegionId { - let id = RegionId { begin, core, mask: CoreMask::complete() }; + let id = RegionId { begin, core, mask }; let record = RegionRecord { end, owner, paid }; Regions::::insert(&id, &record); id