From 929a273ae1ba647628c4ba6e2f8737e58b596d6a Mon Sep 17 00:00:00 2001
From: Muharem <ismailov.m.h@gmail.com>
Date: Wed, 26 Jun 2024 18:36:33 +0200
Subject: [PATCH] pallet assets: optional auto-increment for the asset ID
(#4757)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Introduce an optional auto-increment setup for the IDs of new assets.
---------
Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
Co-authored-by: Bastian Köcher <git@kchr.de>
---
Cargo.lock | 1 +
.../xcm/xcm-builder/src/tests/pay/mock.rs | 1 +
polkadot/xcm/xcm-runtime-apis/tests/mock.rs | 1 +
prdoc/pr_4757.prdoc | 18 +++++
.../cli/tests/res/default_genesis_config.json | 6 +-
substrate/frame/assets-freezer/src/mock.rs | 1 +
substrate/frame/assets/Cargo.toml | 1 +
substrate/frame/assets/src/functions.rs | 3 +
substrate/frame/assets/src/lib.rs | 74 +++++++++++++++++--
substrate/frame/assets/src/migration.rs | 24 ++++++
substrate/frame/assets/src/mock.rs | 3 +-
substrate/frame/assets/src/tests.rs | 68 +++++++++++++++++
.../frame/contracts/mock-network/src/lib.rs | 1 +
13 files changed, 194 insertions(+), 8 deletions(-)
create mode 100644 prdoc/pr_4757.prdoc
diff --git a/Cargo.lock b/Cargo.lock
index 5a1e5d4a744..15b5ea3431c 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -9806,6 +9806,7 @@ dependencies = [
"frame-benchmarking",
"frame-support",
"frame-system",
+ "impl-trait-for-tuples",
"log",
"pallet-balances",
"parity-scale-codec",
diff --git a/polkadot/xcm/xcm-builder/src/tests/pay/mock.rs b/polkadot/xcm/xcm-builder/src/tests/pay/mock.rs
index 10e9f4c6c08..18bde3aab48 100644
--- a/polkadot/xcm/xcm-builder/src/tests/pay/mock.rs
+++ b/polkadot/xcm/xcm-builder/src/tests/pay/mock.rs
@@ -299,6 +299,7 @@ pub fn new_test_ext() -> sp_io::TestExternalities {
(1, TreasuryAccountId::get(), INITIAL_BALANCE),
(100, TreasuryAccountId::get(), INITIAL_BALANCE),
],
+ next_asset_id: None,
}
.assimilate_storage(&mut t)
.unwrap();
diff --git a/polkadot/xcm/xcm-runtime-apis/tests/mock.rs b/polkadot/xcm/xcm-runtime-apis/tests/mock.rs
index 3b73070da45..e723e254635 100644
--- a/polkadot/xcm/xcm-runtime-apis/tests/mock.rs
+++ b/polkadot/xcm/xcm-runtime-apis/tests/mock.rs
@@ -389,6 +389,7 @@ pub fn new_test_ext_with_balances_and_assets(
(1, "Relay Token".into(), "RLY".into(), 12),
],
accounts: assets,
+ next_asset_id: None,
}
.assimilate_storage(&mut t)
.unwrap();
diff --git a/prdoc/pr_4757.prdoc b/prdoc/pr_4757.prdoc
new file mode 100644
index 00000000000..d94a20d7bb1
--- /dev/null
+++ b/prdoc/pr_4757.prdoc
@@ -0,0 +1,18 @@
+title: "pallet assets: optional auto-increment for the asset ID"
+
+doc:
+ - audience: Runtime Dev
+ description: |
+ Introduce an optional auto-increment setup for the IDs of new assets.
+
+crates:
+ - name: pallet-assets
+ bump: major
+ - name: staging-xcm-builder
+ bump: patch
+ - name: staging-xcm
+ bump: patch
+ - name: pallet-assets-freezer
+ bump: patch
+ - name: pallet-contracts
+ bump: patch
diff --git a/substrate/bin/node/cli/tests/res/default_genesis_config.json b/substrate/bin/node/cli/tests/res/default_genesis_config.json
index bba2076f400..b63e5ff549e 100644
--- a/substrate/bin/node/cli/tests/res/default_genesis_config.json
+++ b/substrate/bin/node/cli/tests/res/default_genesis_config.json
@@ -81,12 +81,14 @@
"assets": {
"assets": [],
"metadata": [],
- "accounts": []
+ "accounts": [],
+ "nextAssetId": null
},
"poolAssets": {
"assets": [],
"metadata": [],
- "accounts": []
+ "accounts": [],
+ "nextAssetId": null
},
"transactionStorage": {
"byteFee": 10,
diff --git a/substrate/frame/assets-freezer/src/mock.rs b/substrate/frame/assets-freezer/src/mock.rs
index b4e8c857fba..5e04dfe8e2b 100644
--- a/substrate/frame/assets-freezer/src/mock.rs
+++ b/substrate/frame/assets-freezer/src/mock.rs
@@ -137,6 +137,7 @@ pub fn new_test_ext(execute: impl FnOnce()) -> sp_io::TestExternalities {
assets: vec![(1, 0, true, 1)],
metadata: vec![],
accounts: vec![(1, 1, 100)],
+ next_asset_id: None,
},
system: Default::default(),
balances: Default::default(),
diff --git a/substrate/frame/assets/Cargo.toml b/substrate/frame/assets/Cargo.toml
index 412524f0b35..d0b5dc77789 100644
--- a/substrate/frame/assets/Cargo.toml
+++ b/substrate/frame/assets/Cargo.toml
@@ -17,6 +17,7 @@ targets = ["x86_64-unknown-linux-gnu"]
[dependencies]
codec = { workspace = true }
+impl-trait-for-tuples = "0.2.2"
log = { workspace = true }
scale-info = { features = ["derive"], workspace = true }
sp-std = { workspace = true }
diff --git a/substrate/frame/assets/src/functions.rs b/substrate/frame/assets/src/functions.rs
index 9309d010117..2fb8aee1a97 100644
--- a/substrate/frame/assets/src/functions.rs
+++ b/substrate/frame/assets/src/functions.rs
@@ -709,6 +709,9 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
) -> DispatchResult {
ensure!(!Asset::<T, I>::contains_key(&id), Error::<T, I>::InUse);
ensure!(!min_balance.is_zero(), Error::<T, I>::MinBalanceZero);
+ if let Some(next_id) = NextAssetId::<T, I>::get() {
+ ensure!(id == next_id, Error::<T, I>::BadAssetId);
+ }
Asset::<T, I>::insert(
&id,
diff --git a/substrate/frame/assets/src/lib.rs b/substrate/frame/assets/src/lib.rs
index d5214922555..6dbce717a8e 100644
--- a/substrate/frame/assets/src/lib.rs
+++ b/substrate/frame/assets/src/lib.rs
@@ -174,6 +174,7 @@ use sp_runtime::{
};
use sp_std::prelude::*;
+use core::marker::PhantomData;
use frame_support::{
dispatch::DispatchResult,
ensure,
@@ -182,7 +183,7 @@ use frame_support::{
traits::{
tokens::{fungibles, DepositConsequence, WithdrawConsequence},
BalanceStatus::Reserved,
- Currency, EnsureOriginWithArg, ReservableCurrency, StoredMap,
+ Currency, EnsureOriginWithArg, Incrementable, ReservableCurrency, StoredMap,
},
};
use frame_system::Config as SystemConfig;
@@ -206,8 +207,37 @@ pub trait AssetsCallback<AssetId, AccountId> {
}
}
-/// Empty implementation in case no callbacks are required.
-impl<AssetId, AccountId> AssetsCallback<AssetId, AccountId> for () {}
+#[impl_trait_for_tuples::impl_for_tuples(10)]
+impl<AssetId, AccountId> AssetsCallback<AssetId, AccountId> for Tuple {
+ fn created(id: &AssetId, owner: &AccountId) -> Result<(), ()> {
+ for_tuples!( #( Tuple::created(id, owner)?; )* );
+ Ok(())
+ }
+
+ fn destroyed(id: &AssetId) -> Result<(), ()> {
+ for_tuples!( #( Tuple::destroyed(id)?; )* );
+ Ok(())
+ }
+}
+
+/// Auto-increment the [`NextAssetId`] when an asset is created.
+///
+/// This has not effect if the [`NextAssetId`] value is not present.
+pub struct AutoIncAssetId<T, I = ()>(PhantomData<(T, I)>);
+impl<T: Config<I>, I> AssetsCallback<T::AssetId, T::AccountId> for AutoIncAssetId<T, I>
+where
+ T::AssetId: Incrementable,
+{
+ fn created(_: &T::AssetId, _: &T::AccountId) -> Result<(), ()> {
+ let Some(next_id) = NextAssetId::<T, I>::get() else {
+ // Auto increment for the asset id is not enabled.
+ return Ok(());
+ };
+ let next_id = next_id.increment().ok_or(())?;
+ NextAssetId::<T, I>::put(next_id);
+ Ok(())
+ }
+}
#[frame_support::pallet]
pub mod pallet {
@@ -361,6 +391,11 @@ pub mod pallet {
type Extra: Member + Parameter + Default + MaxEncodedLen;
/// Callback methods for asset state change (e.g. asset created or destroyed)
+ ///
+ /// Types implementing the [`AssetsCallback`] can be chained when listed together as a
+ /// tuple.
+ /// The [`AutoIncAssetId`] callback, in conjunction with the [`NextAssetId`], can be
+ /// used to set up auto-incrementing asset IDs for this collection.
type CallbackHandle: AssetsCallback<Self::AssetId, Self::AccountId>;
/// Weight information for extrinsics in this pallet.
@@ -415,6 +450,18 @@ pub mod pallet {
ValueQuery,
>;
+ /// The asset ID enforced for the next asset creation, if any present. Otherwise, this storage
+ /// item has no effect.
+ ///
+ /// This can be useful for setting up constraints for IDs of the new assets. For example, by
+ /// providing an initial [`NextAssetId`] and using the [`crate::AutoIncAssetId`] callback, an
+ /// auto-increment model can be applied to all new asset IDs.
+ ///
+ /// The initial next asset ID can be set using the [`GenesisConfig`] or the
+ /// [SetNextAssetId](`migration::next_asset_id::SetNextAssetId`) migration.
+ #[pallet::storage]
+ pub type NextAssetId<T: Config<I>, I: 'static = ()> = StorageValue<_, T::AssetId, OptionQuery>;
+
#[pallet::genesis_config]
#[derive(frame_support::DefaultNoBound)]
pub struct GenesisConfig<T: Config<I>, I: 'static = ()> {
@@ -424,6 +471,13 @@ pub mod pallet {
pub metadata: Vec<(T::AssetId, Vec<u8>, Vec<u8>, u8)>,
/// Genesis accounts: id, account_id, balance
pub accounts: Vec<(T::AssetId, T::AccountId, T::Balance)>,
+ /// Genesis [`NextAssetId`].
+ ///
+ /// Refer to the [`NextAssetId`] item for more information.
+ ///
+ /// This does not enforce the asset ID for the [assets](`GenesisConfig::assets`) within the
+ /// genesis config. It sets the [`NextAssetId`] after they have been created.
+ pub next_asset_id: Option<T::AssetId>,
}
#[pallet::genesis_build]
@@ -485,6 +539,10 @@ pub mod pallet {
);
assert!(result.is_ok());
}
+
+ if let Some(next_asset_id) = &self.next_asset_id {
+ NextAssetId::<T, I>::put(next_asset_id);
+ }
}
}
@@ -622,6 +680,8 @@ pub mod pallet {
NotFrozen,
/// Callback action resulted in error
CallbackFailed,
+ /// The asset ID must be equal to the [`NextAssetId`].
+ BadAssetId,
}
#[pallet::call(weight(<T as Config<I>>::WeightInfo))]
@@ -636,7 +696,7 @@ pub mod pallet {
///
/// Parameters:
/// - `id`: The identifier of the new asset. This must not be currently in use to identify
- /// an existing asset.
+ /// an existing asset. If [`NextAssetId`] is set, then this must be equal to it.
/// - `admin`: The admin of this class of assets. The admin is the initial address of each
/// member of the asset class's admin team.
/// - `min_balance`: The minimum balance of this new asset that any single account must
@@ -659,6 +719,10 @@ pub mod pallet {
ensure!(!Asset::<T, I>::contains_key(&id), Error::<T, I>::InUse);
ensure!(!min_balance.is_zero(), Error::<T, I>::MinBalanceZero);
+ if let Some(next_id) = NextAssetId::<T, I>::get() {
+ ensure!(id == next_id, Error::<T, I>::BadAssetId);
+ }
+
let deposit = T::AssetDeposit::get();
T::Currency::reserve(&owner, deposit)?;
@@ -698,7 +762,7 @@ pub mod pallet {
/// Unlike `create`, no funds are reserved.
///
/// - `id`: The identifier of the new asset. This must not be currently in use to identify
- /// an existing asset.
+ /// an existing asset. If [`NextAssetId`] is set, then this must be equal to it.
/// - `owner`: The owner of this class of assets. The owner has full superuser permissions
/// over this asset, but may later change and configure the permissions using
/// `transfer_ownership` and `set_team`.
diff --git a/substrate/frame/assets/src/migration.rs b/substrate/frame/assets/src/migration.rs
index dd7c12293e8..9096f25fb79 100644
--- a/substrate/frame/assets/src/migration.rs
+++ b/substrate/frame/assets/src/migration.rs
@@ -22,6 +22,30 @@ use log;
#[cfg(feature = "try-runtime")]
use sp_runtime::TryRuntimeError;
+pub mod next_asset_id {
+ use super::*;
+ use sp_core::Get;
+
+ /// Set [`NextAssetId`] to the value of `ID` if [`NextAssetId`] does not exist yet.
+ pub struct SetNextAssetId<ID, T: Config<I>, I: 'static = ()>(
+ core::marker::PhantomData<(ID, T, I)>,
+ );
+ impl<ID, T: Config<I>, I: 'static> OnRuntimeUpgrade for SetNextAssetId<ID, T, I>
+ where
+ T::AssetId: Incrementable,
+ ID: Get<T::AssetId>,
+ {
+ fn on_runtime_upgrade() -> frame_support::weights::Weight {
+ if !NextAssetId::<T, I>::exists() {
+ NextAssetId::<T, I>::put(ID::get());
+ T::DbWeight::get().reads_writes(1, 1)
+ } else {
+ T::DbWeight::get().reads(1)
+ }
+ }
+ }
+}
+
pub mod v1 {
use frame_support::{pallet_prelude::*, weights::Weight};
diff --git a/substrate/frame/assets/src/mock.rs b/substrate/frame/assets/src/mock.rs
index 694ef234dff..2c160840e14 100644
--- a/substrate/frame/assets/src/mock.rs
+++ b/substrate/frame/assets/src/mock.rs
@@ -103,7 +103,7 @@ impl Config for Test {
type CreateOrigin = AsEnsureOriginWithArg<frame_system::EnsureSigned<u64>>;
type ForceOrigin = frame_system::EnsureRoot<u64>;
type Freezer = TestFreezer;
- type CallbackHandle = AssetsCallbackHandle;
+ type CallbackHandle = (AssetsCallbackHandle, AutoIncAssetId<Test>);
}
use std::collections::HashMap;
@@ -167,6 +167,7 @@ pub(crate) fn new_test_ext() -> sp_io::TestExternalities {
// id, account_id, balance
(999, 1, 100),
],
+ next_asset_id: None,
};
config.assimilate_storage(&mut storage).unwrap();
diff --git a/substrate/frame/assets/src/tests.rs b/substrate/frame/assets/src/tests.rs
index c7021bcad53..62106d47a15 100644
--- a/substrate/frame/assets/src/tests.rs
+++ b/substrate/frame/assets/src/tests.rs
@@ -1777,3 +1777,71 @@ fn asset_destroy_refund_existence_deposit() {
assert_eq!(Balances::reserved_balance(&admin), 0);
});
}
+
+#[test]
+fn asset_id_cannot_be_reused() {
+ new_test_ext().execute_with(|| {
+ Balances::make_free_balance_be(&1, 100);
+ // Asset id can be reused till auto increment is not enabled.
+ assert_ok!(Assets::create(RuntimeOrigin::signed(1), 0, 1, 1));
+
+ assert_ok!(Assets::start_destroy(RuntimeOrigin::signed(1), 0));
+ assert_ok!(Assets::finish_destroy(RuntimeOrigin::signed(1), 0));
+
+ assert!(!Asset::<Test>::contains_key(0));
+
+ // Asset id `0` is reused.
+ assert_ok!(Assets::create(RuntimeOrigin::signed(1), 0, 1, 1));
+ assert!(Asset::<Test>::contains_key(0));
+
+ assert_ok!(Assets::start_destroy(RuntimeOrigin::signed(1), 0));
+ assert_ok!(Assets::finish_destroy(RuntimeOrigin::signed(1), 0));
+
+ assert!(!Asset::<Test>::contains_key(0));
+
+ // Enable auto increment. Next asset id must be 5.
+ pallet::NextAssetId::<Test>::put(5);
+
+ assert_noop!(Assets::create(RuntimeOrigin::signed(1), 0, 1, 1), Error::<Test>::BadAssetId);
+ assert_noop!(Assets::create(RuntimeOrigin::signed(1), 1, 1, 1), Error::<Test>::BadAssetId);
+ assert_noop!(
+ Assets::force_create(RuntimeOrigin::root(), 0, 1, false, 1),
+ Error::<Test>::BadAssetId
+ );
+ assert_noop!(
+ Assets::force_create(RuntimeOrigin::root(), 1, 1, true, 1),
+ Error::<Test>::BadAssetId
+ );
+
+ // Asset with id `5` is created.
+ assert_ok!(Assets::create(RuntimeOrigin::signed(1), 5, 1, 1));
+ assert!(Asset::<Test>::contains_key(5));
+
+ // Destroy asset with id `6`.
+ assert_ok!(Assets::start_destroy(RuntimeOrigin::signed(1), 5));
+ assert_ok!(Assets::finish_destroy(RuntimeOrigin::signed(1), 5));
+
+ assert!(!Asset::<Test>::contains_key(0));
+
+ // Asset id `5` cannot be reused.
+ assert_noop!(Assets::create(RuntimeOrigin::signed(1), 5, 1, 1), Error::<Test>::BadAssetId);
+
+ assert_ok!(Assets::create(RuntimeOrigin::signed(1), 6, 1, 1));
+ assert!(Asset::<Test>::contains_key(6));
+
+ // Destroy asset with id `6`.
+ assert_ok!(Assets::start_destroy(RuntimeOrigin::signed(1), 6));
+ assert_ok!(Assets::finish_destroy(RuntimeOrigin::signed(1), 6));
+
+ assert!(!Asset::<Test>::contains_key(6));
+
+ // Asset id `6` cannot be reused with force.
+ assert_noop!(
+ Assets::force_create(RuntimeOrigin::root(), 6, 1, false, 1),
+ Error::<Test>::BadAssetId
+ );
+
+ assert_ok!(Assets::force_create(RuntimeOrigin::root(), 7, 1, false, 1));
+ assert!(Asset::<Test>::contains_key(7));
+ });
+}
diff --git a/substrate/frame/contracts/mock-network/src/lib.rs b/substrate/frame/contracts/mock-network/src/lib.rs
index 20ded0f4a0b..34cc95f2eae 100644
--- a/substrate/frame/contracts/mock-network/src/lib.rs
+++ b/substrate/frame/contracts/mock-network/src/lib.rs
@@ -112,6 +112,7 @@ pub fn para_ext(para_id: u32) -> sp_io::TestExternalities {
(0u128, ALICE, INITIAL_BALANCE),
(0u128, relay_sovereign_account_id(), INITIAL_BALANCE),
],
+ next_asset_id: None,
}
.assimilate_storage(&mut t)
.unwrap();
--
GitLab