From 58a50da24b3de76b1929d8348e849aea65d332b6 Mon Sep 17 00:00:00 2001 From: muraca Date: Thu, 28 Sep 2023 15:15:48 +0200 Subject: [PATCH 01/16] implemented `deactivate` and `reactivate`, started working on migration Signed-off-by: muraca --- substrate/frame/assets/src/functions.rs | 1 + substrate/frame/assets/src/impl_fungibles.rs | 16 ++- substrate/frame/assets/src/lib.rs | 2 + substrate/frame/assets/src/migration.rs | 138 ------------------- substrate/frame/assets/src/migration/mod.rs | 21 +++ substrate/frame/assets/src/migration/v1.rs | 134 ++++++++++++++++++ substrate/frame/assets/src/migration/v2.rs | 112 +++++++++++++++ substrate/frame/assets/src/types.rs | 2 + 8 files changed, 287 insertions(+), 139 deletions(-) delete mode 100644 substrate/frame/assets/src/migration.rs create mode 100644 substrate/frame/assets/src/migration/mod.rs create mode 100644 substrate/frame/assets/src/migration/v1.rs create mode 100644 substrate/frame/assets/src/migration/v2.rs diff --git a/substrate/frame/assets/src/functions.rs b/substrate/frame/assets/src/functions.rs index c2c1b6839060..134665c126e7 100644 --- a/substrate/frame/assets/src/functions.rs +++ b/substrate/frame/assets/src/functions.rs @@ -713,6 +713,7 @@ impl, I: 'static> Pallet { admin: owner.clone(), freezer: owner.clone(), supply: Zero::zero(), + inactive: Zero::zero(), deposit: Zero::zero(), min_balance, is_sufficient, diff --git a/substrate/frame/assets/src/impl_fungibles.rs b/substrate/frame/assets/src/impl_fungibles.rs index 123abeba8283..47c437403b3f 100644 --- a/substrate/frame/assets/src/impl_fungibles.rs +++ b/substrate/frame/assets/src/impl_fungibles.rs @@ -164,7 +164,21 @@ impl, I: 'static> fungibles::Unbalanced for Pallet::mutate(asset, |maybe_asset| { + if let Some(ref mut asset) = maybe_asset { + asset.inactive.saturating_accrue(amount); + } + }); + } + + fn reactivate(asset: Self::AssetId, amount: Self::Balance) { + Asset::::mutate(asset, |maybe_asset| { + if let Some(ref mut asset) = maybe_asset { + asset.inactive.saturating_reduce(amount); + } + }); + } } impl, I: 'static> fungibles::Create for Pallet { diff --git a/substrate/frame/assets/src/lib.rs b/substrate/frame/assets/src/lib.rs index 79e4fe300187..2e3da78bd1bd 100644 --- a/substrate/frame/assets/src/lib.rs +++ b/substrate/frame/assets/src/lib.rs @@ -393,6 +393,7 @@ pub mod pallet { admin: owner.clone(), freezer: owner.clone(), supply: Zero::zero(), + inactive: Zero::zero(), deposit: Zero::zero(), min_balance: *min_balance, is_sufficient: *is_sufficient, @@ -619,6 +620,7 @@ pub mod pallet { admin: admin.clone(), freezer: admin.clone(), supply: Zero::zero(), + inactive: Zero::zero(), deposit, min_balance, is_sufficient: false, diff --git a/substrate/frame/assets/src/migration.rs b/substrate/frame/assets/src/migration.rs deleted file mode 100644 index efe77714c524..000000000000 --- a/substrate/frame/assets/src/migration.rs +++ /dev/null @@ -1,138 +0,0 @@ -// 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 frame_support::traits::OnRuntimeUpgrade; -use log; - -#[cfg(feature = "try-runtime")] -use sp_runtime::TryRuntimeError; - -pub mod v1 { - use frame_support::{pallet_prelude::*, weights::Weight}; - - use super::*; - - #[derive(Decode)] - pub struct OldAssetDetails { - pub owner: AccountId, - pub issuer: AccountId, - pub admin: AccountId, - pub freezer: AccountId, - pub supply: Balance, - pub deposit: DepositBalance, - pub min_balance: Balance, - pub is_sufficient: bool, - pub accounts: u32, - pub sufficients: u32, - pub approvals: u32, - pub is_frozen: bool, - } - - impl OldAssetDetails { - fn migrate_to_v1(self) -> AssetDetails { - let status = if self.is_frozen { AssetStatus::Frozen } else { AssetStatus::Live }; - - AssetDetails { - owner: self.owner, - issuer: self.issuer, - admin: self.admin, - freezer: self.freezer, - supply: self.supply, - deposit: self.deposit, - min_balance: self.min_balance, - is_sufficient: self.is_sufficient, - accounts: self.accounts, - sufficients: self.sufficients, - approvals: self.approvals, - status, - } - } - } - - pub struct MigrateToV1(sp_std::marker::PhantomData); - impl OnRuntimeUpgrade for MigrateToV1 { - fn on_runtime_upgrade() -> Weight { - let current_version = Pallet::::current_storage_version(); - let onchain_version = Pallet::::on_chain_storage_version(); - if onchain_version == 0 && current_version == 1 { - let mut translated = 0u64; - Asset::::translate::< - OldAssetDetails>, - _, - >(|_key, old_value| { - translated.saturating_inc(); - Some(old_value.migrate_to_v1()) - }); - current_version.put::>(); - log::info!( - target: LOG_TARGET, - "Upgraded {} pools, storage to version {:?}", - translated, - current_version - ); - T::DbWeight::get().reads_writes(translated + 1, translated + 1) - } else { - log::info!( - target: LOG_TARGET, - "Migration did not execute. This probably should be removed" - ); - T::DbWeight::get().reads(1) - } - } - - #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, TryRuntimeError> { - frame_support::ensure!( - Pallet::::on_chain_storage_version() == 0, - "must upgrade linearly" - ); - let prev_count = Asset::::iter().count(); - Ok((prev_count as u32).encode()) - } - - #[cfg(feature = "try-runtime")] - fn post_upgrade(prev_count: Vec) -> Result<(), TryRuntimeError> { - let prev_count: u32 = Decode::decode(&mut prev_count.as_slice()).expect( - "the state parameter should be something that was generated by pre_upgrade", - ); - let post_count = Asset::::iter().count() as u32; - ensure!( - prev_count == post_count, - "the asset count before and after the migration should be the same" - ); - - let current_version = Pallet::::current_storage_version(); - let onchain_version = Pallet::::on_chain_storage_version(); - - frame_support::ensure!(current_version == 1, "must_upgrade"); - ensure!( - current_version == onchain_version, - "after migration, the current_version and onchain_version should be the same" - ); - - Asset::::iter().try_for_each(|(_id, asset)| -> Result<(), TryRuntimeError> { - ensure!( - asset.status == AssetStatus::Live || asset.status == AssetStatus::Frozen, - "assets should only be live or frozen. None should be in destroying status, or undefined state" - ); - Ok(()) - })?; - Ok(()) - } - } -} diff --git a/substrate/frame/assets/src/migration/mod.rs b/substrate/frame/assets/src/migration/mod.rs new file mode 100644 index 000000000000..4507940f0e85 --- /dev/null +++ b/substrate/frame/assets/src/migration/mod.rs @@ -0,0 +1,21 @@ +// 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. + +/// Version 1. +pub mod v1; +/// Version 2. +pub mod v2; diff --git a/substrate/frame/assets/src/migration/v1.rs b/substrate/frame/assets/src/migration/v1.rs new file mode 100644 index 000000000000..88140729e9e4 --- /dev/null +++ b/substrate/frame/assets/src/migration/v1.rs @@ -0,0 +1,134 @@ +// 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 crate::{migration::v2::V1AssetDetails, *}; +use frame_support::traits::OnRuntimeUpgrade; +use log; + +#[cfg(feature = "try-runtime")] +use sp_runtime::TryRuntimeError; + +use frame_support::{pallet_prelude::*, weights::Weight}; + +#[derive(Decode)] +pub struct V0AssetDetails { + pub owner: AccountId, + pub issuer: AccountId, + pub admin: AccountId, + pub freezer: AccountId, + pub supply: Balance, + pub deposit: DepositBalance, + pub min_balance: Balance, + pub is_sufficient: bool, + pub accounts: u32, + pub sufficients: u32, + pub approvals: u32, + pub is_frozen: bool, +} + +impl V0AssetDetails { + fn migrate_to_v1(self) -> V1AssetDetails { + let status = if self.is_frozen { AssetStatus::Frozen } else { AssetStatus::Live }; + + V1AssetDetails { + owner: self.owner, + issuer: self.issuer, + admin: self.admin, + freezer: self.freezer, + supply: self.supply, + deposit: self.deposit, + min_balance: self.min_balance, + is_sufficient: self.is_sufficient, + accounts: self.accounts, + sufficients: self.sufficients, + approvals: self.approvals, + status, + } + } +} + +pub struct MigrateToV1(sp_std::marker::PhantomData); +impl OnRuntimeUpgrade for MigrateToV1 { + fn on_runtime_upgrade() -> Weight { + let current_version = Pallet::::current_storage_version(); + let onchain_version = Pallet::::on_chain_storage_version(); + if onchain_version == 0 && current_version == 1 { + let mut translated = 0u64; + /* TODO: fix this + Asset::::translate::>, _>( + |_key, old_value| { + translated.saturating_inc(); + Some(old_value.migrate_to_v1()) + }, + ); + */ + current_version.put::>(); + log::info!( + target: LOG_TARGET, + "Upgraded {} pools, storage to version {:?}", + translated, + current_version + ); + T::DbWeight::get().reads_writes(translated + 1, translated + 1) + } else { + log::info!( + target: LOG_TARGET, + "Migration did not execute. This probably should be removed" + ); + T::DbWeight::get().reads(1) + } + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, TryRuntimeError> { + frame_support::ensure!( + Pallet::::on_chain_storage_version() == 0, + "must upgrade linearly" + ); + let prev_count = Asset::::iter().count(); + Ok((prev_count as u32).encode()) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(prev_count: Vec) -> Result<(), TryRuntimeError> { + let prev_count: u32 = Decode::decode(&mut prev_count.as_slice()) + .expect("the state parameter should be something that was generated by pre_upgrade"); + let post_count = Asset::::iter().count() as u32; + ensure!( + prev_count == post_count, + "the asset count before and after the migration should be the same" + ); + + let current_version = Pallet::::current_storage_version(); + let onchain_version = Pallet::::on_chain_storage_version(); + + frame_support::ensure!(current_version == 1, "must_upgrade"); + ensure!( + current_version == onchain_version, + "after migration, the current_version and onchain_version should be the same" + ); + + Asset::::iter().try_for_each(|(_id, asset)| -> Result<(), TryRuntimeError> { + ensure!( + asset.status == AssetStatus::Live || asset.status == AssetStatus::Frozen, + "assets should only be live or frozen. None should be in destroying status, or undefined state" + ); + Ok(()) + })?; + Ok(()) + } +} diff --git a/substrate/frame/assets/src/migration/v2.rs b/substrate/frame/assets/src/migration/v2.rs new file mode 100644 index 000000000000..4fdebee24190 --- /dev/null +++ b/substrate/frame/assets/src/migration/v2.rs @@ -0,0 +1,112 @@ +// 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 crate::*; +use frame_support::traits::OnRuntimeUpgrade; + +#[cfg(feature = "try-runtime")] +use sp_runtime::TryRuntimeError; + +use frame_support::{pallet_prelude::*, weights::Weight}; + +#[derive(Decode)] +pub struct V1AssetDetails { + pub owner: AccountId, + pub issuer: AccountId, + pub admin: AccountId, + pub freezer: AccountId, + pub supply: Balance, + pub deposit: DepositBalance, + pub min_balance: Balance, + pub is_sufficient: bool, + pub accounts: u32, + pub sufficients: u32, + pub approvals: u32, + pub status: AssetStatus, +} + +impl V1AssetDetails { + fn migrate_to_v2(self, inactive: Balance) -> AssetDetails { + AssetDetails { + owner: self.owner, + issuer: self.issuer, + admin: self.admin, + freezer: self.freezer, + supply: self.supply, + inactive, + deposit: self.deposit, + min_balance: self.min_balance, + is_sufficient: self.is_sufficient, + accounts: self.accounts, + sufficients: self.sufficients, + approvals: self.approvals, + status: self.status, + } + } +} + +/// This migration moves all the state to v2 of Assets +pub struct VersionUncheckedMigrateToV2, I: 'static, A>( + sp_std::marker::PhantomData<(T, I, A)>, +); + +impl, I: 'static, A> OnRuntimeUpgrade for VersionUncheckedMigrateToV2 { + fn on_runtime_upgrade() -> Weight { + let mut translated = 0u64; + Asset::::translate::< + V1AssetDetails>, + _, + >(|_asset_id, old_value| { + translated.saturating_inc(); + let inactive = T::Balance::zero(); + // TODO compute inactives based on input of A + Some(old_value.migrate_to_v2(inactive)) + }); + + T::DbWeight::get().reads_writes(translated, translated) + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, TryRuntimeError> { + log::info!("pre-migration assets v2"); + let prev_count = Asset::::iter().count(); + Ok((prev_count as u32).encode()) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(prev_count: Vec) -> Result<(), TryRuntimeError> { + log::info!("post-migration assets v2"); + let prev_count: u32 = Decode::decode(&mut prev_count.as_slice()) + .expect("the state parameter should be something that was generated by pre_upgrade"); + let post_count = Asset::::iter().count() as u32; + ensure!( + prev_count == post_count, + "the asset count before and after the migration should be the same" + ); + Ok(()) + } +} + +/// [`VersionUncheckedMigrateToV2`] wrapped in a [`frame_support::migrations::VersionedMigration`], +/// ensuring the migration is only performed when on-chain version is 1. +pub type VersionCheckedMigrateToV2 = frame_support::migrations::VersionedMigration< + 1, + 2, + VersionUncheckedMigrateToV2, + crate::pallet::Pallet, + ::DbWeight, +>; diff --git a/substrate/frame/assets/src/types.rs b/substrate/frame/assets/src/types.rs index 67f9bf07f5e7..1924dff5b034 100644 --- a/substrate/frame/assets/src/types.rs +++ b/substrate/frame/assets/src/types.rs @@ -60,6 +60,8 @@ pub struct AssetDetails { pub(super) freezer: AccountId, /// The total supply across all accounts. pub(super) supply: Balance, + /// The total units of deactivated supply across all accounts. + pub(super) inactive: Balance, /// The balance deposited for this asset. This pays for the data stored here. pub(super) deposit: DepositBalance, /// The ED for virtual accounts. From 1167d685bdea69b3565dcf8fdcc27b2260df2d33 Mon Sep 17 00:00:00 2001 From: muraca Date: Thu, 28 Sep 2023 16:25:28 +0200 Subject: [PATCH 02/16] fixed migrations Signed-off-by: muraca --- substrate/frame/assets/src/migration/v1.rs | 98 ++++++++++++---------- substrate/frame/assets/src/migration/v2.rs | 96 +++++++++++++-------- substrate/frame/assets/src/tests.rs | 63 ++++++++++++++ 3 files changed, 175 insertions(+), 82 deletions(-) diff --git a/substrate/frame/assets/src/migration/v1.rs b/substrate/frame/assets/src/migration/v1.rs index 88140729e9e4..90817aae7d71 100644 --- a/substrate/frame/assets/src/migration/v1.rs +++ b/substrate/frame/assets/src/migration/v1.rs @@ -15,48 +15,57 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::{migration::v2::V1AssetDetails, *}; -use frame_support::traits::OnRuntimeUpgrade; +use crate::{ + migration::v2::old::{Asset, AssetDetails}, + AssetStatus, Config, DepositBalanceOf, Pallet, LOG_TARGET, +}; +use frame_support::{ + pallet_prelude::*, sp_runtime::traits::Saturating, traits::OnRuntimeUpgrade, weights::Weight, +}; use log; #[cfg(feature = "try-runtime")] use sp_runtime::TryRuntimeError; -use frame_support::{pallet_prelude::*, weights::Weight}; +mod old { + use super::*; -#[derive(Decode)] -pub struct V0AssetDetails { - pub owner: AccountId, - pub issuer: AccountId, - pub admin: AccountId, - pub freezer: AccountId, - pub supply: Balance, - pub deposit: DepositBalance, - pub min_balance: Balance, - pub is_sufficient: bool, - pub accounts: u32, - pub sufficients: u32, - pub approvals: u32, - pub is_frozen: bool, -} + #[derive(Decode)] + pub struct AssetDetails { + pub owner: AccountId, + pub issuer: AccountId, + pub admin: AccountId, + pub freezer: AccountId, + pub supply: Balance, + pub deposit: DepositBalance, + pub min_balance: Balance, + pub is_sufficient: bool, + pub accounts: u32, + pub sufficients: u32, + pub approvals: u32, + pub is_frozen: bool, + } -impl V0AssetDetails { - fn migrate_to_v1(self) -> V1AssetDetails { - let status = if self.is_frozen { AssetStatus::Frozen } else { AssetStatus::Live }; + impl AssetDetails { + pub(super) fn migrate_to_v1( + self, + ) -> super::AssetDetails { + let status = if self.is_frozen { AssetStatus::Frozen } else { AssetStatus::Live }; - V1AssetDetails { - owner: self.owner, - issuer: self.issuer, - admin: self.admin, - freezer: self.freezer, - supply: self.supply, - deposit: self.deposit, - min_balance: self.min_balance, - is_sufficient: self.is_sufficient, - accounts: self.accounts, - sufficients: self.sufficients, - approvals: self.approvals, - status, + super::AssetDetails { + owner: self.owner, + issuer: self.issuer, + admin: self.admin, + freezer: self.freezer, + supply: self.supply, + deposit: self.deposit, + min_balance: self.min_balance, + is_sufficient: self.is_sufficient, + accounts: self.accounts, + sufficients: self.sufficients, + approvals: self.approvals, + status, + } } } } @@ -68,14 +77,13 @@ impl OnRuntimeUpgrade for MigrateToV1 { let onchain_version = Pallet::::on_chain_storage_version(); if onchain_version == 0 && current_version == 1 { let mut translated = 0u64; - /* TODO: fix this - Asset::::translate::>, _>( - |_key, old_value| { - translated.saturating_inc(); - Some(old_value.migrate_to_v1()) - }, - ); - */ + Asset::::translate::< + old::AssetDetails>, + _, + >(|_key, old_value| { + translated.saturating_inc(); + Some(old_value.migrate_to_v1()) + }); current_version.put::>(); log::info!( target: LOG_TARGET, @@ -99,7 +107,7 @@ impl OnRuntimeUpgrade for MigrateToV1 { Pallet::::on_chain_storage_version() == 0, "must upgrade linearly" ); - let prev_count = Asset::::iter().count(); + let prev_count = Asset::::iter().count(); Ok((prev_count as u32).encode()) } @@ -107,7 +115,7 @@ impl OnRuntimeUpgrade for MigrateToV1 { fn post_upgrade(prev_count: Vec) -> Result<(), TryRuntimeError> { let prev_count: u32 = Decode::decode(&mut prev_count.as_slice()) .expect("the state parameter should be something that was generated by pre_upgrade"); - let post_count = Asset::::iter().count() as u32; + let post_count = Asset::::iter().count() as u32; ensure!( prev_count == post_count, "the asset count before and after the migration should be the same" @@ -122,7 +130,7 @@ impl OnRuntimeUpgrade for MigrateToV1 { "after migration, the current_version and onchain_version should be the same" ); - Asset::::iter().try_for_each(|(_id, asset)| -> Result<(), TryRuntimeError> { + Asset::::iter().try_for_each(|(_id, asset)| -> Result<(), TryRuntimeError> { ensure!( asset.status == AssetStatus::Live || asset.status == AssetStatus::Frozen, "assets should only be live or frozen. None should be in destroying status, or undefined state" diff --git a/substrate/frame/assets/src/migration/v2.rs b/substrate/frame/assets/src/migration/v2.rs index 4fdebee24190..ef5b3cc0d28f 100644 --- a/substrate/frame/assets/src/migration/v2.rs +++ b/substrate/frame/assets/src/migration/v2.rs @@ -15,50 +15,72 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::*; -use frame_support::traits::OnRuntimeUpgrade; +use crate::{Asset, AssetDetails, AssetStatus, Config, DepositBalanceOf, Pallet}; +use frame_support::{ + pallet_prelude::*, + sp_runtime::traits::{Saturating, Zero}, + traits::OnRuntimeUpgrade, + weights::Weight, +}; #[cfg(feature = "try-runtime")] use sp_runtime::TryRuntimeError; -use frame_support::{pallet_prelude::*, weights::Weight}; +pub mod old { + use super::*; -#[derive(Decode)] -pub struct V1AssetDetails { - pub owner: AccountId, - pub issuer: AccountId, - pub admin: AccountId, - pub freezer: AccountId, - pub supply: Balance, - pub deposit: DepositBalance, - pub min_balance: Balance, - pub is_sufficient: bool, - pub accounts: u32, - pub sufficients: u32, - pub approvals: u32, - pub status: AssetStatus, -} + #[frame_support::storage_alias] + /// Details of an asset. + pub type Asset, I: 'static> = StorageMap< + Pallet, + Blake2_128Concat, + >::AssetId, + AssetDetails< + >::Balance, + ::AccountId, + DepositBalanceOf, + >, + >; + + #[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, TypeInfo)] + pub(crate) struct AssetDetails { + pub owner: AccountId, + pub issuer: AccountId, + pub admin: AccountId, + pub freezer: AccountId, + pub supply: Balance, + pub deposit: DepositBalance, + pub min_balance: Balance, + pub is_sufficient: bool, + pub accounts: u32, + pub sufficients: u32, + pub approvals: u32, + pub status: AssetStatus, + } -impl V1AssetDetails { - fn migrate_to_v2(self, inactive: Balance) -> AssetDetails { - AssetDetails { - owner: self.owner, - issuer: self.issuer, - admin: self.admin, - freezer: self.freezer, - supply: self.supply, - inactive, - deposit: self.deposit, - min_balance: self.min_balance, - is_sufficient: self.is_sufficient, - accounts: self.accounts, - sufficients: self.sufficients, - approvals: self.approvals, - status: self.status, + impl AssetDetails { + pub(super) fn migrate_to_v2( + self, + inactive: Balance, + ) -> super::AssetDetails { + super::AssetDetails { + owner: self.owner, + issuer: self.issuer, + admin: self.admin, + freezer: self.freezer, + supply: self.supply, + inactive, + deposit: self.deposit, + min_balance: self.min_balance, + is_sufficient: self.is_sufficient, + accounts: self.accounts, + sufficients: self.sufficients, + approvals: self.approvals, + status: self.status, + } } } } - /// This migration moves all the state to v2 of Assets pub struct VersionUncheckedMigrateToV2, I: 'static, A>( sp_std::marker::PhantomData<(T, I, A)>, @@ -68,7 +90,7 @@ impl, I: 'static, A> OnRuntimeUpgrade for VersionUncheckedMigrateTo fn on_runtime_upgrade() -> Weight { let mut translated = 0u64; Asset::::translate::< - V1AssetDetails>, + old::AssetDetails>, _, >(|_asset_id, old_value| { translated.saturating_inc(); @@ -83,7 +105,7 @@ impl, I: 'static, A> OnRuntimeUpgrade for VersionUncheckedMigrateTo #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, TryRuntimeError> { log::info!("pre-migration assets v2"); - let prev_count = Asset::::iter().count(); + let prev_count = old::Asset::::iter().count(); Ok((prev_count as u32).encode()) } diff --git a/substrate/frame/assets/src/tests.rs b/substrate/frame/assets/src/tests.rs index f1b116a0f4a0..b0457933c7a9 100644 --- a/substrate/frame/assets/src/tests.rs +++ b/substrate/frame/assets/src/tests.rs @@ -22,6 +22,7 @@ use crate::{mock::*, Error}; use frame_support::{ assert_noop, assert_ok, dispatch::GetDispatchInfo, + pallet_prelude::StorageVersion, traits::{fungibles::InspectEnumerable, tokens::Preservation::Protect, Currency}, }; use pallet_balances::Error as BalancesError; @@ -1775,3 +1776,65 @@ fn asset_destroy_refund_existence_deposit() { assert_eq!(Balances::reserved_balance(&admin), 0); }); } + +#[test] +fn migrate_to_v2_works() { + new_test_ext().execute_with(|| { + use crate::migration::v2::old as v1; + use frame_support::traits::OnRuntimeUpgrade; + StorageVersion::new(1).put::>(); + + v1::Asset::::insert( + 1, + v1::AssetDetails::< + ::Balance, + ::AccountId, + DepositBalanceOf, + > { + owner: 1, + issuer: 1, + admin: 1, + freezer: 1, + supply: 102, + deposit: 0, + min_balance: 1, + is_sufficient: false, + accounts: 0, + sufficients: 0, + approvals: 0, + status: AssetStatus::Live, + }, + ); + + Account::::insert( + 1, + 1, + AssetAccountOf:: { + balance: 100, + status: AccountStatus::Liquid, + reason: ExistenceReason::<_, _>::Sufficient, + extra: Default::default(), + }, + ); + Account::::insert( + 1, + 2, + AssetAccountOf:: { + balance: 2, + status: AccountStatus::Liquid, + reason: ExistenceReason::<_, _>::Sufficient, + extra: Default::default(), + }, + ); + + // Run migration. + assert_ok!( + crate::migration::v2::VersionCheckedMigrateToV2::::try_on_runtime_upgrade( + true + ) + ); + + // TODO check with 2 after setting account 2's balance as inactive + assert_eq!(Asset::::get(1).unwrap().inactive, 0); + }); +} From ebdb21e61069d645d92cad223b9779f5ac4e3fe3 Mon Sep 17 00:00:00 2001 From: muraca Date: Thu, 28 Sep 2023 22:08:41 +0200 Subject: [PATCH 03/16] inactive computation in migration Signed-off-by: muraca --- substrate/frame/assets/src/migration/v2.rs | 64 +++++++++++++++------- substrate/frame/assets/src/tests.rs | 33 ++++++++--- 2 files changed, 69 insertions(+), 28 deletions(-) diff --git a/substrate/frame/assets/src/migration/v2.rs b/substrate/frame/assets/src/migration/v2.rs index ef5b3cc0d28f..ec73d30f3969 100644 --- a/substrate/frame/assets/src/migration/v2.rs +++ b/substrate/frame/assets/src/migration/v2.rs @@ -15,7 +15,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::{Asset, AssetDetails, AssetStatus, Config, DepositBalanceOf, Pallet}; +use crate::{Account, Asset, AssetDetails, AssetStatus, Config, DepositBalanceOf, Pallet}; use frame_support::{ pallet_prelude::*, sp_runtime::traits::{Saturating, Zero}, @@ -58,10 +58,12 @@ pub mod old { pub status: AssetStatus, } - impl AssetDetails { + impl AssetDetails + where + Balance: Zero, + { pub(super) fn migrate_to_v2( self, - inactive: Balance, ) -> super::AssetDetails { super::AssetDetails { owner: self.owner, @@ -69,7 +71,7 @@ pub mod old { admin: self.admin, freezer: self.freezer, supply: self.supply, - inactive, + inactive: Zero::zero(), deposit: self.deposit, min_balance: self.min_balance, is_sufficient: self.is_sufficient, @@ -82,24 +84,47 @@ pub mod old { } } /// This migration moves all the state to v2 of Assets -pub struct VersionUncheckedMigrateToV2, I: 'static, A>( - sp_std::marker::PhantomData<(T, I, A)>, -); +pub struct VersionUncheckedMigrateToV2< + T: Config, + I: 'static, + A: Get>, +>(sp_std::marker::PhantomData<(T, I, A)>); -impl, I: 'static, A> OnRuntimeUpgrade for VersionUncheckedMigrateToV2 { +impl, I: 'static, A: Get>> OnRuntimeUpgrade + for VersionUncheckedMigrateToV2 +{ fn on_runtime_upgrade() -> Weight { let mut translated = 0u64; + Asset::::translate::< old::AssetDetails>, _, >(|_asset_id, old_value| { translated.saturating_inc(); - let inactive = T::Balance::zero(); - // TODO compute inactives based on input of A - Some(old_value.migrate_to_v2(inactive)) + Some(old_value.migrate_to_v2()) }); - T::DbWeight::get().reads_writes(translated, translated) + let mut read = 0u64; + for (asset_id, account) in A::get() { + if let Some(acc) = Account::::get(&asset_id, &account) { + Asset::::mutate(&asset_id, |asset| { + if let Some(asset) = asset { + asset.inactive.saturating_accrue(acc.balance); + } else { + log::info!("inactive migration: asset {:?} not found", asset_id); + } + }); + } else { + log::info!( + "inactive migration: account {:?} not found for asset {:?}", + account, + asset_id + ); + } + read.saturating_inc(); + } + + T::DbWeight::get().reads_writes(translated + read, translated) } #[cfg(feature = "try-runtime")] @@ -125,10 +150,11 @@ impl, I: 'static, A> OnRuntimeUpgrade for VersionUncheckedMigrateTo /// [`VersionUncheckedMigrateToV2`] wrapped in a [`frame_support::migrations::VersionedMigration`], /// ensuring the migration is only performed when on-chain version is 1. -pub type VersionCheckedMigrateToV2 = frame_support::migrations::VersionedMigration< - 1, - 2, - VersionUncheckedMigrateToV2, - crate::pallet::Pallet, - ::DbWeight, ->; +pub type VersionCheckedMigrateToV2 = + frame_support::migrations::VersionedMigration< + 1, + 2, + VersionUncheckedMigrateToV2, + crate::pallet::Pallet, + ::DbWeight, + >; diff --git a/substrate/frame/assets/src/tests.rs b/substrate/frame/assets/src/tests.rs index b0457933c7a9..938a62642fa6 100644 --- a/substrate/frame/assets/src/tests.rs +++ b/substrate/frame/assets/src/tests.rs @@ -27,7 +27,10 @@ use frame_support::{ }; use pallet_balances::Error as BalancesError; use sp_io::storage; -use sp_runtime::{traits::ConvertInto, TokenError}; +use sp_runtime::{ + traits::{ConvertInto, Get}, + TokenError, +}; fn asset_ids() -> Vec { let mut s: Vec<_> = Assets::asset_ids().collect(); @@ -1784,6 +1787,7 @@ fn migrate_to_v2_works() { use frame_support::traits::OnRuntimeUpgrade; StorageVersion::new(1).put::>(); + // Create an asset with id 1 v1::Asset::::insert( 1, v1::AssetDetails::< @@ -1806,6 +1810,7 @@ fn migrate_to_v2_works() { }, ); + // Populate balances of asset 1 Account::::insert( 1, 1, @@ -1827,14 +1832,24 @@ fn migrate_to_v2_works() { }, ); - // Run migration. - assert_ok!( - crate::migration::v2::VersionCheckedMigrateToV2::::try_on_runtime_upgrade( - true - ) - ); + // Define a constant Vec for migration. + // This is a workaround for the fact that we can't implement Get for Vec. + struct ConstVecForMigrationToV2; + impl Get> for ConstVecForMigrationToV2 { + fn get() -> Vec<(u32, u64)> { + // AssetId 1, AccountId 2 will be inactive. + vec![(1, 2)] + } + } - // TODO check with 2 after setting account 2's balance as inactive - assert_eq!(Asset::::get(1).unwrap().inactive, 0); + // Run migration. + assert_ok!(crate::migration::v2::VersionCheckedMigrateToV2::< + Test, + (), + ConstVecForMigrationToV2, + >::try_on_runtime_upgrade(true)); + + // Total inactive balance of asset 1 should be 2. + assert_eq!(Asset::::get(1).unwrap().inactive, 2); }); } From 8af1f3920e91bebef2a1be48517da75c5b4cbc38 Mon Sep 17 00:00:00 2001 From: muraca Date: Thu, 28 Sep 2023 22:33:02 +0200 Subject: [PATCH 04/16] feature try-runtime for migration test Signed-off-by: muraca --- substrate/frame/assets/src/tests.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/substrate/frame/assets/src/tests.rs b/substrate/frame/assets/src/tests.rs index 938a62642fa6..c9b462bc5625 100644 --- a/substrate/frame/assets/src/tests.rs +++ b/substrate/frame/assets/src/tests.rs @@ -22,15 +22,11 @@ use crate::{mock::*, Error}; use frame_support::{ assert_noop, assert_ok, dispatch::GetDispatchInfo, - pallet_prelude::StorageVersion, traits::{fungibles::InspectEnumerable, tokens::Preservation::Protect, Currency}, }; use pallet_balances::Error as BalancesError; use sp_io::storage; -use sp_runtime::{ - traits::{ConvertInto, Get}, - TokenError, -}; +use sp_runtime::{traits::ConvertInto, TokenError}; fn asset_ids() -> Vec { let mut s: Vec<_> = Assets::asset_ids().collect(); @@ -1780,11 +1776,13 @@ fn asset_destroy_refund_existence_deposit() { }); } +#[cfg(all(feature = "try-runtime", test))] #[test] fn migrate_to_v2_works() { new_test_ext().execute_with(|| { use crate::migration::v2::old as v1; - use frame_support::traits::OnRuntimeUpgrade; + use frame_support::{pallet_prelude::StorageVersion, traits::OnRuntimeUpgrade}; + use sp_runtime::traits::Get; StorageVersion::new(1).put::>(); // Create an asset with id 1 From 632d1a88a73f0838a7f6bbddd3bb47759b26a823 Mon Sep 17 00:00:00 2001 From: muraca Date: Fri, 29 Sep 2023 09:31:44 +0200 Subject: [PATCH 05/16] fixed leak crate-private type Signed-off-by: muraca --- substrate/frame/assets/src/migration/v2.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/frame/assets/src/migration/v2.rs b/substrate/frame/assets/src/migration/v2.rs index ec73d30f3969..03e183fc2cad 100644 --- a/substrate/frame/assets/src/migration/v2.rs +++ b/substrate/frame/assets/src/migration/v2.rs @@ -15,7 +15,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::{Account, Asset, AssetDetails, AssetStatus, Config, DepositBalanceOf, Pallet}; +use crate::{Account, Asset, AssetDetails, AssetStatus, Config, DepositBalanceOf, Pallet, Vec}; use frame_support::{ pallet_prelude::*, sp_runtime::traits::{Saturating, Zero}, @@ -31,7 +31,7 @@ pub mod old { #[frame_support::storage_alias] /// Details of an asset. - pub type Asset, I: 'static> = StorageMap< + pub(crate) type Asset, I: 'static> = StorageMap< Pallet, Blake2_128Concat, >::AssetId, From 57ec30a8838f1bd4ba225e8a98437d546b2e5d40 Mon Sep 17 00:00:00 2001 From: muraca Date: Fri, 29 Sep 2023 09:49:43 +0200 Subject: [PATCH 06/16] import Vec for migration v1 Signed-off-by: muraca --- substrate/frame/assets/src/migration/v1.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/substrate/frame/assets/src/migration/v1.rs b/substrate/frame/assets/src/migration/v1.rs index 90817aae7d71..5edfd6b1260c 100644 --- a/substrate/frame/assets/src/migration/v1.rs +++ b/substrate/frame/assets/src/migration/v1.rs @@ -24,6 +24,8 @@ use frame_support::{ }; use log; +#[cfg(feature = "try-runtime")] +use crate::Vec; #[cfg(feature = "try-runtime")] use sp_runtime::TryRuntimeError; From 180d91b19db2fb875281a6b66528c325bd0d6533 Mon Sep 17 00:00:00 2001 From: muraca Date: Mon, 2 Oct 2023 12:01:05 +0200 Subject: [PATCH 07/16] addressed review comments Signed-off-by: muraca --- substrate/frame/assets/src/impl_fungibles.rs | 14 ++-- substrate/frame/assets/src/migration/mod.rs | 4 +- substrate/frame/assets/src/migration/v2.rs | 69 ++++++++++++++------ 3 files changed, 56 insertions(+), 31 deletions(-) diff --git a/substrate/frame/assets/src/impl_fungibles.rs b/substrate/frame/assets/src/impl_fungibles.rs index 47c437403b3f..196e356c302e 100644 --- a/substrate/frame/assets/src/impl_fungibles.rs +++ b/substrate/frame/assets/src/impl_fungibles.rs @@ -165,18 +165,16 @@ impl, I: 'static> fungibles::Unbalanced for Pallet::mutate(asset, |maybe_asset| { - if let Some(ref mut asset) = maybe_asset { - asset.inactive.saturating_accrue(amount); - } + Asset::::mutate(&asset, |maybe_asset| match maybe_asset { + Some(ref mut asset) => asset.inactive.saturating_accrue(amount), + None => log::error!("Called deactivate for nonexistent asset {:?}", asset), }); } fn reactivate(asset: Self::AssetId, amount: Self::Balance) { - Asset::::mutate(asset, |maybe_asset| { - if let Some(ref mut asset) = maybe_asset { - asset.inactive.saturating_reduce(amount); - } + Asset::::mutate(&asset, |maybe_asset| match maybe_asset { + Some(ref mut asset) => asset.inactive.saturating_reduce(amount), + None => log::error!("Called reactivate for nonexistent asset {:?}", asset), }); } } diff --git a/substrate/frame/assets/src/migration/mod.rs b/substrate/frame/assets/src/migration/mod.rs index 4507940f0e85..aa13a113abf4 100644 --- a/substrate/frame/assets/src/migration/mod.rs +++ b/substrate/frame/assets/src/migration/mod.rs @@ -15,7 +15,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -/// Version 1. +/// Version 0 to 1. pub mod v1; -/// Version 2. +/// Version 1 to 2. pub mod v2; diff --git a/substrate/frame/assets/src/migration/v2.rs b/substrate/frame/assets/src/migration/v2.rs index 03e183fc2cad..c4fb927a3107 100644 --- a/substrate/frame/assets/src/migration/v2.rs +++ b/substrate/frame/assets/src/migration/v2.rs @@ -104,46 +104,73 @@ impl, I: 'static, A: Get>> OnRuntim Some(old_value.migrate_to_v2()) }); - let mut read = 0u64; + let mut reads = 0u64; for (asset_id, account) in A::get() { - if let Some(acc) = Account::::get(&asset_id, &account) { - Asset::::mutate(&asset_id, |asset| { - if let Some(asset) = asset { - asset.inactive.saturating_accrue(acc.balance); - } else { - log::info!("inactive migration: asset {:?} not found", asset_id); - } - }); - } else { + reads.saturating_inc(); + let Some(acc) = Account::::get(&asset_id, &account) else { log::info!( "inactive migration: account {:?} not found for asset {:?}", account, asset_id ); - } - read.saturating_inc(); + continue + }; + Asset::::mutate(&asset_id, |asset| match asset { + Some(asset) => asset.inactive.saturating_accrue(acc.balance), + None => log::info!("inactive migration: asset {:?} not found", asset_id), + }); } - T::DbWeight::get().reads_writes(translated + read, translated) + T::DbWeight::get().reads_writes(translated.saturating_add(reads), translated) } #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, TryRuntimeError> { log::info!("pre-migration assets v2"); - let prev_count = old::Asset::::iter().count(); - Ok((prev_count as u32).encode()) + Ok((old::Asset::::iter().collect::>()).encode()) } #[cfg(feature = "try-runtime")] - fn post_upgrade(prev_count: Vec) -> Result<(), TryRuntimeError> { + fn post_upgrade(old_status: Vec) -> Result<(), TryRuntimeError> { log::info!("post-migration assets v2"); - let prev_count: u32 = Decode::decode(&mut prev_count.as_slice()) + + let mut old_assets: Vec<( + T::AssetId, + old::AssetDetails>, + )> = Decode::decode(&mut old_status.as_slice()) .expect("the state parameter should be something that was generated by pre_upgrade"); - let post_count = Asset::::iter().count() as u32; - ensure!( - prev_count == post_count, - "the asset count before and after the migration should be the same" + + assert_eq!( + old_assets.len(), + Asset::::iter().count(), + "the number of assets should be the same" + ); + + let new_assets = + old_assets.drain(..).map(|(k, v)| (k, v.migrate_to_v2())).collect::>(); + + let inactives = A::get().iter().fold( + Vec::<(T::AssetId, T::Balance)>::new(), + |mut acc, (asset_id, account)| { + let Some(details) = Account::::get(&asset_id, &account) else { return acc }; + match acc.iter().position(|(id, _)| id == asset_id) { + Some(idx) => acc[idx].1.saturating_accrue(details.balance), + None => acc.push((asset_id.clone(), details.balance)), + } + acc + }, ); + + for (asset_id, mut asset) in new_assets { + let inactive = inactives + .iter() + .find(|(id, _)| *id == asset_id) + .map(|(_, b)| *b) + .unwrap_or_else(|| Zero::zero()); + asset.inactive = inactive; + assert_eq!(Asset::::get(&asset_id), Some(asset)); + } + Ok(()) } } From ea52ad0c577420afc72c7c30e253614274fb95e0 Mon Sep 17 00:00:00 2001 From: muraca Date: Mon, 2 Oct 2023 19:25:13 +0200 Subject: [PATCH 08/16] assert to ensure in migration post-upgrade Signed-off-by: muraca --- substrate/frame/assets/src/migration/v2.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/substrate/frame/assets/src/migration/v2.rs b/substrate/frame/assets/src/migration/v2.rs index c4fb927a3107..cb828737f8c3 100644 --- a/substrate/frame/assets/src/migration/v2.rs +++ b/substrate/frame/assets/src/migration/v2.rs @@ -137,12 +137,12 @@ impl, I: 'static, A: Get>> OnRuntim let mut old_assets: Vec<( T::AssetId, old::AssetDetails>, - )> = Decode::decode(&mut old_status.as_slice()) - .expect("the state parameter should be something that was generated by pre_upgrade"); + )> = Decode::decode(&mut old_status.as_slice()).map_err(|_| { + "the state parameter should be something that was generated by pre_upgrade" + })?; - assert_eq!( - old_assets.len(), - Asset::::iter().count(), + ensure!( + old_assets.len() == Asset::::iter().count(), "the number of assets should be the same" ); @@ -168,7 +168,10 @@ impl, I: 'static, A: Get>> OnRuntim .map(|(_, b)| *b) .unwrap_or_else(|| Zero::zero()); asset.inactive = inactive; - assert_eq!(Asset::::get(&asset_id), Some(asset)); + ensure!( + Asset::::get(&asset_id) == Some(asset), + "migrated asset does not match expected inactive value" + ); } Ok(()) From 1b61d666c5687d2120ce0f0d4cb1a347e43cf911 Mon Sep 17 00:00:00 2001 From: muraca Date: Mon, 16 Oct 2023 12:23:09 +0200 Subject: [PATCH 09/16] migration v1 doesn't depend on v2 anymore Signed-off-by: muraca --- substrate/frame/assets/src/migration/v1.rs | 40 ++++++++++++++++++---- substrate/frame/assets/src/migration/v2.rs | 1 + 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/substrate/frame/assets/src/migration/v1.rs b/substrate/frame/assets/src/migration/v1.rs index 5edfd6b1260c..e123244cb315 100644 --- a/substrate/frame/assets/src/migration/v1.rs +++ b/substrate/frame/assets/src/migration/v1.rs @@ -15,10 +15,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::{ - migration::v2::old::{Asset, AssetDetails}, - AssetStatus, Config, DepositBalanceOf, Pallet, LOG_TARGET, -}; +use crate::{AssetStatus, Config, DepositBalanceOf, Pallet, LOG_TARGET}; use frame_support::{ pallet_prelude::*, sp_runtime::traits::Saturating, traits::OnRuntimeUpgrade, weights::Weight, }; @@ -72,6 +69,35 @@ mod old { } } +#[frame_support::storage_alias] +/// Details of an asset. +pub(crate) type Asset, I: 'static> = StorageMap< + Pallet, + Blake2_128Concat, + >::AssetId, + AssetDetails< + >::Balance, + ::AccountId, + DepositBalanceOf, + >, +>; + +#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, TypeInfo)] +pub(crate) struct AssetDetails { + pub owner: AccountId, + pub issuer: AccountId, + pub admin: AccountId, + pub freezer: AccountId, + pub supply: Balance, + pub deposit: DepositBalance, + pub min_balance: Balance, + pub is_sufficient: bool, + pub accounts: u32, + pub sufficients: u32, + pub approvals: u32, + pub status: AssetStatus, +} + pub struct MigrateToV1(sp_std::marker::PhantomData); impl OnRuntimeUpgrade for MigrateToV1 { fn on_runtime_upgrade() -> Weight { @@ -134,9 +160,9 @@ impl OnRuntimeUpgrade for MigrateToV1 { Asset::::iter().try_for_each(|(_id, asset)| -> Result<(), TryRuntimeError> { ensure!( - asset.status == AssetStatus::Live || asset.status == AssetStatus::Frozen, - "assets should only be live or frozen. None should be in destroying status, or undefined state" - ); + asset.status == AssetStatus::Live || asset.status == AssetStatus::Frozen, + "assets should only be live or frozen. None should be in destroying status, or undefined state" + ); Ok(()) })?; Ok(()) diff --git a/substrate/frame/assets/src/migration/v2.rs b/substrate/frame/assets/src/migration/v2.rs index cb828737f8c3..54e6942c987b 100644 --- a/substrate/frame/assets/src/migration/v2.rs +++ b/substrate/frame/assets/src/migration/v2.rs @@ -83,6 +83,7 @@ pub mod old { } } } + /// This migration moves all the state to v2 of Assets pub struct VersionUncheckedMigrateToV2< T: Config, From 45506565188052ab7b0d992e1b70041cbed11fb5 Mon Sep 17 00:00:00 2001 From: muraca Date: Fri, 27 Oct 2023 17:35:57 +0200 Subject: [PATCH 10/16] migration v1 now supports pallet instance Signed-off-by: muraca --- substrate/frame/assets/src/migration/v1.rs | 26 +++++++++++----------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/substrate/frame/assets/src/migration/v1.rs b/substrate/frame/assets/src/migration/v1.rs index e123244cb315..e32e3787b536 100644 --- a/substrate/frame/assets/src/migration/v1.rs +++ b/substrate/frame/assets/src/migration/v1.rs @@ -98,21 +98,21 @@ pub(crate) struct AssetDetails { pub status: AssetStatus, } -pub struct MigrateToV1(sp_std::marker::PhantomData); -impl OnRuntimeUpgrade for MigrateToV1 { +pub struct MigrateToV1(sp_std::marker::PhantomData<(T, I)>); +impl, I: 'static> OnRuntimeUpgrade for MigrateToV1 { fn on_runtime_upgrade() -> Weight { - let current_version = Pallet::::current_storage_version(); - let onchain_version = Pallet::::on_chain_storage_version(); + let current_version = Pallet::::current_storage_version(); + let onchain_version = Pallet::::on_chain_storage_version(); if onchain_version == 0 && current_version == 1 { let mut translated = 0u64; - Asset::::translate::< - old::AssetDetails>, + Asset::::translate::< + old::AssetDetails>, _, >(|_key, old_value| { translated.saturating_inc(); Some(old_value.migrate_to_v1()) }); - current_version.put::>(); + current_version.put::>(); log::info!( target: LOG_TARGET, "Upgraded {} pools, storage to version {:?}", @@ -132,10 +132,10 @@ impl OnRuntimeUpgrade for MigrateToV1 { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, TryRuntimeError> { frame_support::ensure!( - Pallet::::on_chain_storage_version() == 0, + Pallet::::on_chain_storage_version() == 0, "must upgrade linearly" ); - let prev_count = Asset::::iter().count(); + let prev_count = Asset::::iter().count(); Ok((prev_count as u32).encode()) } @@ -143,14 +143,14 @@ impl OnRuntimeUpgrade for MigrateToV1 { fn post_upgrade(prev_count: Vec) -> Result<(), TryRuntimeError> { let prev_count: u32 = Decode::decode(&mut prev_count.as_slice()) .expect("the state parameter should be something that was generated by pre_upgrade"); - let post_count = Asset::::iter().count() as u32; + let post_count = Asset::::iter().count() as u32; ensure!( prev_count == post_count, "the asset count before and after the migration should be the same" ); - let current_version = Pallet::::current_storage_version(); - let onchain_version = Pallet::::on_chain_storage_version(); + let current_version = Pallet::::current_storage_version(); + let onchain_version = Pallet::::on_chain_storage_version(); frame_support::ensure!(current_version == 1, "must_upgrade"); ensure!( @@ -158,7 +158,7 @@ impl OnRuntimeUpgrade for MigrateToV1 { "after migration, the current_version and onchain_version should be the same" ); - Asset::::iter().try_for_each(|(_id, asset)| -> Result<(), TryRuntimeError> { + Asset::::iter().try_for_each(|(_id, asset)| -> Result<(), TryRuntimeError> { ensure!( asset.status == AssetStatus::Live || asset.status == AssetStatus::Frozen, "assets should only be live or frozen. None should be in destroying status, or undefined state" From f69f85ac5b259feaa60facd704b267fa31355894 Mon Sep 17 00:00:00 2001 From: muraca Date: Sat, 28 Oct 2023 10:40:54 +0200 Subject: [PATCH 11/16] bump pallet version to v2 Signed-off-by: muraca --- substrate/frame/assets/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/assets/src/lib.rs b/substrate/frame/assets/src/lib.rs index 2e3da78bd1bd..8c900c815ecd 100644 --- a/substrate/frame/assets/src/lib.rs +++ b/substrate/frame/assets/src/lib.rs @@ -209,7 +209,7 @@ pub mod pallet { use frame_system::pallet_prelude::*; /// The current storage version. - const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); + const STORAGE_VERSION: StorageVersion = StorageVersion::new(2); #[pallet::pallet] #[pallet::storage_version(STORAGE_VERSION)] From ecea308da36c807378fe7c68f8c22ef2e2b94c44 Mon Sep 17 00:00:00 2001 From: muraca Date: Mon, 30 Oct 2023 09:36:01 +0100 Subject: [PATCH 12/16] migration to v1 only checks on chain storage versions Signed-off-by: muraca --- substrate/frame/assets/src/migration/v1.rs | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/substrate/frame/assets/src/migration/v1.rs b/substrate/frame/assets/src/migration/v1.rs index e32e3787b536..7b8cc83b54cf 100644 --- a/substrate/frame/assets/src/migration/v1.rs +++ b/substrate/frame/assets/src/migration/v1.rs @@ -101,9 +101,8 @@ pub(crate) struct AssetDetails { pub struct MigrateToV1(sp_std::marker::PhantomData<(T, I)>); impl, I: 'static> OnRuntimeUpgrade for MigrateToV1 { fn on_runtime_upgrade() -> Weight { - let current_version = Pallet::::current_storage_version(); let onchain_version = Pallet::::on_chain_storage_version(); - if onchain_version == 0 && current_version == 1 { + if onchain_version == 0 { let mut translated = 0u64; Asset::::translate::< old::AssetDetails>, @@ -112,14 +111,15 @@ impl, I: 'static> OnRuntimeUpgrade for MigrateToV1 { translated.saturating_inc(); Some(old_value.migrate_to_v1()) }); - current_version.put::>(); + StorageVersion::new(1).put::>(); log::info!( target: LOG_TARGET, - "Upgraded {} pools, storage to version {:?}", + "Upgraded {} pools, storage to version 1", translated, - current_version ); - T::DbWeight::get().reads_writes(translated + 1, translated + 1) + translated.saturating_inc(); + + T::DbWeight::get().reads_writes(translated, translated) } else { log::info!( target: LOG_TARGET, @@ -149,13 +149,11 @@ impl, I: 'static> OnRuntimeUpgrade for MigrateToV1 { "the asset count before and after the migration should be the same" ); - let current_version = Pallet::::current_storage_version(); let onchain_version = Pallet::::on_chain_storage_version(); - frame_support::ensure!(current_version == 1, "must_upgrade"); - ensure!( - current_version == onchain_version, - "after migration, the current_version and onchain_version should be the same" + frame_support::ensure!( + onchain_version == 1, + "on chain storage version for assets should be 1 after migration" ); Asset::::iter().try_for_each(|(_id, asset)| -> Result<(), TryRuntimeError> { From 7af51dd92fd1ac4b92a162b20c7d0beff4463338 Mon Sep 17 00:00:00 2001 From: muraca Date: Fri, 10 Nov 2023 15:31:52 +0100 Subject: [PATCH 13/16] renamed `VersionCheckedMigrateToV2` to `MigrateToV2` https://github.com/paritytech/polkadot-sdk/pull/2264 Signed-off-by: muraca --- Cargo.lock | 74 ++++++++-------------- substrate/frame/assets/src/migration/v2.rs | 15 ++--- substrate/frame/assets/src/tests.rs | 2 +- 3 files changed, 36 insertions(+), 55 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2a091ce6817d..3e87678a088d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -655,7 +655,7 @@ dependencies = [ "num-traits", "rusticata-macros", "thiserror", - "time 0.3.27", + "time", ] [[package]] @@ -671,7 +671,7 @@ dependencies = [ "num-traits", "rusticata-macros", "thiserror", - "time 0.3.27", + "time", ] [[package]] @@ -2741,15 +2741,14 @@ dependencies = [ [[package]] name = "chrono" -version = "0.4.27" +version = "0.4.30" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f56b4c72906975ca04becb8a30e102dfecddd0c06181e3e95ddc444be28881f8" +checksum = "defd4e7873dbddba6c7c91e199c7fcb946abc4a6a4ac3195400bcfb01b5de877" dependencies = [ "android-tzdata", "iana-time-zone", "js-sys", "num-traits", - "time 0.1.45", "wasm-bindgen", "windows-targets 0.48.5", ] @@ -7351,9 +7350,9 @@ checksum = "884e2677b40cc8c339eaefcb701c32ef1fd2493d71118dc0ca4b6a736c93bd67" [[package]] name = "libc" -version = "0.2.149" +version = "0.2.150" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a08173bc88b7955d1b3145aa561539096c421ac8debde8cbc3612ec635fee29b" +checksum = "89d92a4743f9a61002fae18374ed11e7973f530cb3a3255fb354818118b2203c" [[package]] name = "libflate" @@ -7937,9 +7936,9 @@ checksum = "ef53942eb7bf7ff43a617b3e2c1c4a5ecf5944a7c1bc12d7ee39bbb15e5c1519" [[package]] name = "linux-raw-sys" -version = "0.4.10" +version = "0.4.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "da2479e8c062e40bf0066ffa0bc823de0a9368974af99c9f6df941d2c231e03f" +checksum = "969488b55f8ac402214f3f5fd243ebb7206cf82de60d3172994707a4bcc2b829" [[package]] name = "lioness" @@ -13703,9 +13702,9 @@ dependencies = [ [[package]] name = "proc-macro2" -version = "1.0.69" +version = "1.0.68" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "134c189feb4956b20f6f547d2cf727d4c0fe06722b20a0eec87ed445a97f92da" +checksum = "5b1106fec09662ec6dd98ccac0f81cef56984d0b49f75c92d8cbad76e20c005c" dependencies = [ "unicode-ident", ] @@ -13928,9 +13927,9 @@ dependencies = [ [[package]] name = "quinn-proto" -version = "0.9.5" +version = "0.9.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c956be1b23f4261676aed05a0046e204e8a6836e50203902683a718af0797989" +checksum = "f31999cfc7927c4e212e60fd50934ab40e8e8bfd2d493d6095d2d306bc0764d9" dependencies = [ "bytes", "rand 0.8.5", @@ -14085,7 +14084,7 @@ checksum = "6413f3de1edee53342e6138e75b56d32e7bc6e332b3bd62d497b1929d4cfbcdd" dependencies = [ "pem", "ring 0.16.20", - "time 0.3.27", + "time", "x509-parser 0.13.2", "yasna", ] @@ -14098,7 +14097,7 @@ checksum = "ffbe84efe2f38dea12e9bfc1f65377fdf03e53a18cb3b995faedf7934c7e785b" dependencies = [ "pem", "ring 0.16.20", - "time 0.3.27", + "time", "yasna", ] @@ -14715,7 +14714,7 @@ dependencies = [ "bitflags 2.4.0", "errno", "libc", - "linux-raw-sys 0.4.10", + "linux-raw-sys 0.4.11", "windows-sys 0.48.0", ] @@ -17459,7 +17458,7 @@ dependencies = [ [[package]] name = "sp-crypto-ec-utils" version = "0.4.1" -source = "git+https://github.com/paritytech/polkadot-sdk#fe9435db2fda7c9e2f4e29521564c72cac38f59b" +source = "git+https://github.com/paritytech/polkadot-sdk#84ddbaf68445fed23a88a086ee37bdcdbcb7356a" dependencies = [ "ark-bls12-377", "ark-bls12-377-ext", @@ -17497,7 +17496,7 @@ dependencies = [ [[package]] name = "sp-debug-derive" version = "8.0.0" -source = "git+https://github.com/paritytech/polkadot-sdk#fe9435db2fda7c9e2f4e29521564c72cac38f59b" +source = "git+https://github.com/paritytech/polkadot-sdk#84ddbaf68445fed23a88a086ee37bdcdbcb7356a" dependencies = [ "proc-macro2", "quote", @@ -17517,7 +17516,7 @@ dependencies = [ [[package]] name = "sp-externalities" version = "0.19.0" -source = "git+https://github.com/paritytech/polkadot-sdk#fe9435db2fda7c9e2f4e29521564c72cac38f59b" +source = "git+https://github.com/paritytech/polkadot-sdk#84ddbaf68445fed23a88a086ee37bdcdbcb7356a" dependencies = [ "environmental", "parity-scale-codec", @@ -17749,7 +17748,7 @@ dependencies = [ [[package]] name = "sp-runtime-interface" version = "17.0.0" -source = "git+https://github.com/paritytech/polkadot-sdk#fe9435db2fda7c9e2f4e29521564c72cac38f59b" +source = "git+https://github.com/paritytech/polkadot-sdk#84ddbaf68445fed23a88a086ee37bdcdbcb7356a" dependencies = [ "bytes", "impl-trait-for-tuples", @@ -17778,7 +17777,7 @@ dependencies = [ [[package]] name = "sp-runtime-interface-proc-macro" version = "11.0.0" -source = "git+https://github.com/paritytech/polkadot-sdk#fe9435db2fda7c9e2f4e29521564c72cac38f59b" +source = "git+https://github.com/paritytech/polkadot-sdk#84ddbaf68445fed23a88a086ee37bdcdbcb7356a" dependencies = [ "Inflector", "proc-macro-crate", @@ -17906,7 +17905,7 @@ version = "8.0.0" [[package]] name = "sp-std" version = "8.0.0" -source = "git+https://github.com/paritytech/polkadot-sdk#fe9435db2fda7c9e2f4e29521564c72cac38f59b" +source = "git+https://github.com/paritytech/polkadot-sdk#84ddbaf68445fed23a88a086ee37bdcdbcb7356a" [[package]] name = "sp-storage" @@ -17923,7 +17922,7 @@ dependencies = [ [[package]] name = "sp-storage" version = "13.0.0" -source = "git+https://github.com/paritytech/polkadot-sdk#fe9435db2fda7c9e2f4e29521564c72cac38f59b" +source = "git+https://github.com/paritytech/polkadot-sdk#84ddbaf68445fed23a88a086ee37bdcdbcb7356a" dependencies = [ "impl-serde", "parity-scale-codec", @@ -17972,7 +17971,7 @@ dependencies = [ [[package]] name = "sp-tracing" version = "10.0.0" -source = "git+https://github.com/paritytech/polkadot-sdk#fe9435db2fda7c9e2f4e29521564c72cac38f59b" +source = "git+https://github.com/paritytech/polkadot-sdk#84ddbaf68445fed23a88a086ee37bdcdbcb7356a" dependencies = [ "parity-scale-codec", "sp-std 8.0.0 (git+https://github.com/paritytech/polkadot-sdk)", @@ -18073,7 +18072,7 @@ dependencies = [ [[package]] name = "sp-wasm-interface" version = "14.0.0" -source = "git+https://github.com/paritytech/polkadot-sdk#fe9435db2fda7c9e2f4e29521564c72cac38f59b" +source = "git+https://github.com/paritytech/polkadot-sdk#84ddbaf68445fed23a88a086ee37bdcdbcb7356a" dependencies = [ "anyhow", "impl-trait-for-tuples", @@ -19266,17 +19265,6 @@ dependencies = [ "tikv-jemalloc-sys", ] -[[package]] -name = "time" -version = "0.1.45" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1b797afad3f312d1c66a56d11d0316f916356d11bd158fbc6ca6389ff6bf805a" -dependencies = [ - "libc", - "wasi 0.10.0+wasi-snapshot-preview1", - "winapi", -] - [[package]] name = "time" version = "0.3.27" @@ -20109,12 +20097,6 @@ version = "0.9.0+wasi-snapshot-preview1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cccddf32554fecc6acb585f82a32a72e28b48f8c4c1883ddfeeeaa96f7d8e519" -[[package]] -name = "wasi" -version = "0.10.0+wasi-snapshot-preview1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1a143597ca7c7793eff794def352d41792a93c481eb1042423ff7ff72ba2c31f" - [[package]] name = "wasi" version = "0.11.0+wasi-snapshot-preview1" @@ -20617,7 +20599,7 @@ dependencies = [ "sha2 0.10.7", "stun", "thiserror", - "time 0.3.27", + "time", "tokio", "turn", "url", @@ -21295,7 +21277,7 @@ dependencies = [ "ring 0.16.20", "rusticata-macros", "thiserror", - "time 0.3.27", + "time", ] [[package]] @@ -21313,7 +21295,7 @@ dependencies = [ "oid-registry 0.6.1", "rusticata-macros", "thiserror", - "time 0.3.27", + "time", ] [[package]] @@ -21485,7 +21467,7 @@ version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e17bb3549cc1321ae1296b9cdc2698e2b6cb1992adfa19a8c72e5b7a738f44cd" dependencies = [ - "time 0.3.27", + "time", ] [[package]] diff --git a/substrate/frame/assets/src/migration/v2.rs b/substrate/frame/assets/src/migration/v2.rs index 54e6942c987b..b8e865b9c39f 100644 --- a/substrate/frame/assets/src/migration/v2.rs +++ b/substrate/frame/assets/src/migration/v2.rs @@ -181,11 +181,10 @@ impl, I: 'static, A: Get>> OnRuntim /// [`VersionUncheckedMigrateToV2`] wrapped in a [`frame_support::migrations::VersionedMigration`], /// ensuring the migration is only performed when on-chain version is 1. -pub type VersionCheckedMigrateToV2 = - frame_support::migrations::VersionedMigration< - 1, - 2, - VersionUncheckedMigrateToV2, - crate::pallet::Pallet, - ::DbWeight, - >; +pub type MigrateToV2 = frame_support::migrations::VersionedMigration< + 1, + 2, + VersionUncheckedMigrateToV2, + crate::pallet::Pallet, + ::DbWeight, +>; diff --git a/substrate/frame/assets/src/tests.rs b/substrate/frame/assets/src/tests.rs index c9b462bc5625..935acfc2fc95 100644 --- a/substrate/frame/assets/src/tests.rs +++ b/substrate/frame/assets/src/tests.rs @@ -1841,7 +1841,7 @@ fn migrate_to_v2_works() { } // Run migration. - assert_ok!(crate::migration::v2::VersionCheckedMigrateToV2::< + assert_ok!(crate::migration::v2::MigrateToV2::< Test, (), ConstVecForMigrationToV2, From 91ac946b0faad348f7574e4bfc504d3267dad135 Mon Sep 17 00:00:00 2001 From: Matteo Muraca Date: Thu, 7 Mar 2024 10:31:03 +0100 Subject: [PATCH 14/16] inactive amount can't exceed supply Signed-off-by: Matteo Muraca --- substrate/frame/assets/src/impl_fungibles.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/substrate/frame/assets/src/impl_fungibles.rs b/substrate/frame/assets/src/impl_fungibles.rs index 196e356c302e..a1f56fd5b2dd 100644 --- a/substrate/frame/assets/src/impl_fungibles.rs +++ b/substrate/frame/assets/src/impl_fungibles.rs @@ -166,7 +166,10 @@ impl, I: 'static> fungibles::Unbalanced for Pallet::mutate(&asset, |maybe_asset| match maybe_asset { - Some(ref mut asset) => asset.inactive.saturating_accrue(amount), + Some(ref mut asset) =>{ + // Inactive amount can't exceed supply + asset.inactive = asset.inactive.saturating_add(amount).max(asset.supply); + }, None => log::error!("Called deactivate for nonexistent asset {:?}", asset), }); } From c2b3f2b2f7b30c499e66e00dd503919b3452ac34 Mon Sep 17 00:00:00 2001 From: Matteo Muraca Date: Thu, 7 Mar 2024 11:43:11 +0100 Subject: [PATCH 15/16] prdoc for #1747 Signed-off-by: Matteo Muraca --- prdoc/pr_1747.prdoc | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 prdoc/pr_1747.prdoc diff --git a/prdoc/pr_1747.prdoc b/prdoc/pr_1747.prdoc new file mode 100644 index 000000000000..d8031b96c95d --- /dev/null +++ b/prdoc/pr_1747.prdoc @@ -0,0 +1,13 @@ +# 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: Implement Inactive balance tracking in Assets pallet + +doc: + - audience: Runtime Dev + description: | + The trait `fungibles::Unbalanced` provides methods to increase and decrease active issuance. + This PR addresses the lack of this feature in Assets, by introducing a new field `inactive` in the `Asset` struct and implementing the methods accordingly. + +crates: + - name: pallet-assets From 6c7a19708f6453baeee42176791dba0bd6175b60 Mon Sep 17 00:00:00 2001 From: Matteo Muraca Date: Thu, 7 Mar 2024 12:59:43 +0100 Subject: [PATCH 16/16] fixed fmt & clippy Signed-off-by: Matteo Muraca --- substrate/frame/assets/src/impl_fungibles.rs | 2 +- substrate/frame/assets/src/migration/v1.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/frame/assets/src/impl_fungibles.rs b/substrate/frame/assets/src/impl_fungibles.rs index a1f56fd5b2dd..d5706270122a 100644 --- a/substrate/frame/assets/src/impl_fungibles.rs +++ b/substrate/frame/assets/src/impl_fungibles.rs @@ -166,7 +166,7 @@ impl, I: 'static> fungibles::Unbalanced for Pallet::mutate(&asset, |maybe_asset| match maybe_asset { - Some(ref mut asset) =>{ + Some(ref mut asset) => { // Inactive amount can't exceed supply asset.inactive = asset.inactive.saturating_add(amount).max(asset.supply); }, diff --git a/substrate/frame/assets/src/migration/v1.rs b/substrate/frame/assets/src/migration/v1.rs index aff1c199f941..9a0c402bed9d 100644 --- a/substrate/frame/assets/src/migration/v1.rs +++ b/substrate/frame/assets/src/migration/v1.rs @@ -151,7 +151,7 @@ impl, I: 'static> OnRuntimeUpgrade for MigrateToV1 { "the asset count before and after the migration should be the same" ); - let in_code_version = Pallet::::in_code_storage_version(); + let in_code_version = Pallet::::in_code_storage_version(); let on_chain_version = Pallet::::on_chain_storage_version(); frame_support::ensure!(in_code_version == 1, "must_upgrade");