From c39cc333b677da6e4f18ee836a38bf01130ca221 Mon Sep 17 00:00:00 2001
From: Sergej Sakac <73715684+Szegoo@users.noreply.github.com>
Date: Fri, 26 Jul 2024 19:49:38 +0200
Subject: [PATCH] Fix region nonfungible implementation (#5067)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The problem with the current implementation is that minting will cause
the region coremask to be set to `Coremask::complete` regardless of the
actual coremask.
This PR fixes that.
More details about the nonfungible implementation can be found here:
https://github.com/paritytech/polkadot-sdk/pull/3455
---------
Co-authored-by: Dónal Murray <donalm@seadanda.dev>
Co-authored-by: Branislav Kontur <bkontur@gmail.com>
Co-authored-by: Francisco Aguirre <franciscoaguirreperez@gmail.com>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
---
.../coretime/coretime-rococo/src/lib.rs | 2 +-
.../coretime/coretime-westend/src/lib.rs | 2 +-
prdoc/pr_5067.prdoc | 19 ++++++++
.../frame/broker/src/dispatchable_impls.rs | 10 ++++-
.../frame/broker/src/nonfungible_impl.rs | 9 +++-
substrate/frame/broker/src/tests.rs | 43 +++++++++++++++++++
substrate/frame/broker/src/utility_impls.rs | 3 +-
7 files changed, 82 insertions(+), 6 deletions(-)
create mode 100644 prdoc/pr_5067.prdoc
diff --git a/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs b/cumulus/parachains/runtimes/coretime/coretime-rococo/src/lib.rs
index 9fd0093840d..37a6ea1d1c5 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::<Runtime>::issue(core, begin, end, None, None);
+ let region_id = pallet_broker::Pallet::<Runtime>::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 7907f252cf8..b6847dddb6c 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::<Runtime>::issue(core, begin, end, None, None);
+ let region_id = pallet_broker::Pallet::<Runtime>::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 00000000000..9a11f96b510
--- /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 9e7a56e5281..a8ded084a80 100644
--- a/substrate/frame/broker/src/dispatchable_impls.rs
+++ b/substrate/frame/broker/src/dispatchable_impls.rs
@@ -128,8 +128,14 @@ impl<T: Config> Pallet<T> {
let core = Self::purchase_core(&who, price, &mut sale)?;
SaleInfo::<T>::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 e272ecbe008..acbcba1dae4 100644
--- a/substrate/frame/broker/src/nonfungible_impl.rs
+++ b/substrate/frame/broker/src/nonfungible_impl.rs
@@ -74,7 +74,14 @@ impl<T: Config> Mutate<T::AccountId> for Pallet<T> {
// 'Minting' can only occur if the asset has previously been burned (i.e. moved to the
// holding register)
ensure!(record.owner.is_none(), Error::<T>::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 2a8ea24b447..3ea68c5a74d 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::<Test>::get(region1).unwrap();
+
+ // 'withdraw' the region from user 1:
+ assert_ok!(<Broker as Mutate<_>>::burn(®ion1.into(), Some(&1)));
+ assert_eq!(Regions::<Test>::get(region1).unwrap().owner, None);
+
+ // `mint_into` works after burning:
+ assert_ok!(<Broker as Mutate<_>>::mint_into(®ion1.into(), &1));
+
+ // Ensure the region minted is the same as the one we burned previously:
+ assert_eq!(Regions::<Test>::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::<Test>::get(region1).unwrap();
+
+ // 'withdraw' the region from user 1:
+ assert_ok!(<Broker as Mutate<_>>::burn(®ion1.into(), Some(&1)));
+ assert_eq!(Regions::<Test>::get(region1).unwrap().owner, None);
+
+ // `mint_into` works after burning:
+ assert_ok!(<Broker as Mutate<_>>::mint_into(®ion1.into(), &1));
+
+ // Ensure the region minted is the same as the one we burned previously:
+ assert_eq!(Regions::<Test>::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 9cceb7f970a..5c66ebb9674 100644
--- a/substrate/frame/broker/src/utility_impls.rs
+++ b/substrate/frame/broker/src/utility_impls.rs
@@ -94,11 +94,12 @@ impl<T: Config> Pallet<T> {
pub fn issue(
core: CoreIndex,
begin: Timeslice,
+ mask: CoreMask,
end: Timeslice,
owner: Option<T::AccountId>,
paid: Option<BalanceOf<T>>,
) -> RegionId {
- let id = RegionId { begin, core, mask: CoreMask::complete() };
+ let id = RegionId { begin, core, mask };
let record = RegionRecord { end, owner, paid };
Regions::<T>::insert(&id, &record);
id
--
GitLab