Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Polkadot v1.7.2 migrations #1849

Merged
merged 7 commits into from
May 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions runtime/altair/src/migrations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@

use crate::{ForeignInvestments, OraclePriceCollection, OraclePriceFeed, OrderBook};

// Number of identities on Altair Chain on 30.05.2024 was 34
const IDENTITY_MIGRATION_KEY_LIMIT: u64 = 1000;

/// The migration set for Altair @ Kusama.
/// It includes all the migrations that have to be applied on that chain.
pub type UpgradeAltair1035 = (
Expand All @@ -20,4 +23,15 @@ pub type UpgradeAltair1035 = (
runtime_common::migrations::increase_storage_version::Migration<OrderBook, 0, 1>,
runtime_common::migrations::increase_storage_version::Migration<ForeignInvestments, 0, 1>,
pallet_collator_selection::migration::v1::MigrateToV1<crate::Runtime>,
runtime_common::migrations::loans::AddWithLinearPricing<crate::Runtime>,
// As of May 2024, the `pallet_balances::Hold` storage was empty. But better be safe.
runtime_common::migrations::hold_reason::MigrateTransferAllowListHolds<
crate::Runtime,
crate::RuntimeHoldReason,
>,
// Migrations below this comment originate from Polkadot SDK
pallet_xcm::migration::MigrateToLatestXcmVersion<crate::Runtime>,
cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4<crate::Runtime>,
pallet_identity::migration::versioned::V0ToV1<crate::Runtime, IDENTITY_MIGRATION_KEY_LIMIT>,
pallet_uniques::migration::MigrateV0ToV1<crate::Runtime, ()>,
);
12 changes: 12 additions & 0 deletions runtime/centrifuge/src/migrations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,23 @@

use crate::{OraclePriceCollection, OraclePriceFeed};

// Number of identities on Centrifuge Chain on 29.05.2024 was 34
const IDENTITY_MIGRATION_KEY_LIMIT: u64 = 1000;

/// The migration set for Centrifuge @ Polkadot.
/// It includes all the migrations that have to be applied on that chain.
pub type UpgradeCentrifuge1029 = (
runtime_common::migrations::increase_storage_version::Migration<OraclePriceFeed, 0, 1>,
runtime_common::migrations::increase_storage_version::Migration<OraclePriceCollection, 0, 1>,
pallet_collator_selection::migration::v1::MigrateToV1<crate::Runtime>,
runtime_common::migrations::loans::AddWithLinearPricing<crate::Runtime>,
runtime_common::migrations::hold_reason::MigrateTransferAllowListHolds<
crate::Runtime,
crate::RuntimeHoldReason,
>,
// Migrations below this comment originate from Polkadot SDK
pallet_xcm::migration::MigrateToLatestXcmVersion<crate::Runtime>,
cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4<crate::Runtime>,
pallet_identity::migration::versioned::V0ToV1<crate::Runtime, IDENTITY_MIGRATION_KEY_LIMIT>,
pallet_uniques::migration::MigrateV0ToV1<crate::Runtime, ()>,
);
151 changes: 151 additions & 0 deletions runtime/common/src/migrations/hold_reason.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
// Copyright 2023 Centrifuge Foundation (centrifuge.io).
//
// This file is part of the Centrifuge chain project.
// Centrifuge is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version (see http://www.gnu.org/licenses).
// Centrifuge is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.

use cfg_primitives::{AccountId, Balance};
use frame_support::{
pallet_prelude::ValueQuery,
storage_alias,
traits::{ConstU32, Get, Len, OnRuntimeUpgrade, VariantCountOf},
Blake2_128Concat, BoundedVec, Parameter,
};
use pallet_balances::IdAmount;
use pallet_order_book::weights::Weight;
use pallet_transfer_allowlist::HoldReason;
#[cfg(feature = "try-runtime")]
use parity_scale_codec::{Decode, Encode};
use parity_scale_codec::{FullCodec, FullEncode};
#[cfg(feature = "try-runtime")]
use sp_runtime::Saturating;
#[cfg(feature = "try-runtime")]
use sp_runtime::TryRuntimeError;
use sp_runtime::{traits::Member, SaturatedConversion};
use sp_std::{vec, vec::Vec};

const LOG_PREFIX: &str = "MigrateTransferAllowList HoldReason:";

pub struct MigrateTransferAllowListHolds<T, RuntimeHoldReason>(
sp_std::marker::PhantomData<(T, RuntimeHoldReason)>,
);

type OldHolds = BoundedVec<IdAmount<(), Balance>, ConstU32<10>>;
type NewHolds<RuntimeHoldReason> =
BoundedVec<IdAmount<RuntimeHoldReason, Balance>, VariantCountOf<RuntimeHoldReason>>;

#[storage_alias]
pub type Holds<T: pallet_balances::Config> =
StorageMap<pallet_balances::Pallet<T>, Blake2_128Concat, AccountId, OldHolds, ValueQuery>;

impl<T, RuntimeHoldReason> OnRuntimeUpgrade for MigrateTransferAllowListHolds<T, RuntimeHoldReason>
where
T: pallet_balances::Config<Balance = Balance, RuntimeHoldReason = RuntimeHoldReason>
+ pallet_transfer_allowlist::Config
+ frame_system::Config<AccountId = AccountId>,
<T as pallet_balances::Config>::RuntimeHoldReason: From<pallet_transfer_allowlist::HoldReason>,
RuntimeHoldReason: frame_support::traits::VariantCount
+ FullCodec
+ FullEncode
+ Parameter
+ Member
+ sp_std::fmt::Debug,
{
fn on_runtime_upgrade() -> Weight {
let transfer_allowlist_accounts: Vec<AccountId> =
pallet_transfer_allowlist::AccountCurrencyTransferAllowance::<T>::iter_keys()
.map(|(a, _, _)| a)
.collect();
let mut weight =
T::DbWeight::get().reads(transfer_allowlist_accounts.len().saturated_into());

pallet_balances::Holds::<T>::translate::<OldHolds, _>(|who, holds| {
if Self::account_can_be_migrated(&who, &transfer_allowlist_accounts) {
log::info!("{LOG_PREFIX} Migrating hold for account {who:?}");
let id_amount = IdAmount::<RuntimeHoldReason, Balance> {
id: HoldReason::TransferAllowance.into(),
// Non-emptiness ensured above
amount: holds[0].amount,
};
weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1));

Some(NewHolds::truncate_from(vec![id_amount]))
} else {
Copy link
Contributor

@lemunozm lemunozm May 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if Self::account_can_be_migrated(&who, &transfer_allowlist_accounts) is false, then if the old Hold storage still has an entry, that means that we need to migrate it with pallet_restricted_tokens::HoldReason::NativeIndex.into() because it means that someone used that pallet to hold something thought that pallet.

Or am I wrong in my assumption?

The flow will be:

  • if hold in balances and account in allowlist
    • then: pallet_transfer_allowlist::HoldReason::TransferAllowance.into()
  • else if hold in balances
    • then: pallet_restricted_tokens::HoldReason::NativeIndex.into()
  • else
    • then: None

Still scared about how can we know that an account that has been used by transfer allowance has not also be used by restricted_tokens 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if Self::account_can_be_migrated(&who, &transfer_allowlist_accounts) is false, then if the old Hold storage still has an entry, that means that we need to migrate it with pallet_restricted_tokens::HoldReason::NativeIndex.into() because it means that someone used that pallet to hold something thought that pallet.

Or am I wrong in my assumption?

We have three code paths which use Currency::hold in our codebase:

  1. pallet-rewards when depositing stake
  2. pallet-transfer-allowlist when adding a transfer filter
  3. pallet-orderbook when placing or updating an order.

All three codepaths use the RestrictedTokens as their Currency trait which provides two implementations for hold: One for native currency (i.e. fungible which falls back to the impl of pallet-balances) and one for non-native assets (i.e. fungibles which falls back to the impl of orml-tokens). The crucial part is that the orml-tokens implementation of hold has not made use of the RuntimeHoldReason (it does not even have storage for that) such that we only need to migrate the Holds storage of pallet-balances.

The flow will be:

  • if hold in balances and account in allowlist
    • then: pallet_transfer_allowlist::HoldReason::TransferAllowance.into()
  • else if hold in balances
    • then: pallet_restricted_tokens::HoldReason::NativeIndex.into()
  • else
    • then: None

Now coming back to the three codepaths which make use of hold API: Both pallet-rewards and pallet-orderbook only hold fungibles because staking currency is not native and we can assume no CFG <-> * orders to exist until we execute the RU. So there can only be a single reason why there exists an entry in pallet_balances::Holds storage which points back to the transfer allowlist. There cannot be any other stored holds.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your reasoning!

Just one thing. Root can call the extrinsic restricted_tokens::set_balance() (not sure if this already happened), modifying the hold amount in the account for CFG

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we never use orderbook to create an order with CFG

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your reasoning!

Just one thing. Root can call the extrinsic restricted_tokens::set_balance() (not sure if this already happened), modifying the hold amount in the account for CFG

Luckily, set_balance is rather a dev tool than something which is used in production. Ironically, we have used it once on Centrifuge Chain to recover a lost wallet from an early Gnosis backer (-> Gov forum) but did not set any holds there. Again, I have checked all storage on all our chains to ensure this migration works. Before we propose the RU, I will redo that step.

Assuming the worst case of some unexpected hold to spawn in between RU proposal and execution, which is not tied to transfer allowlist, then we can fix this afterwards. The user will just not be able to remove the hold until then.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I think we have all these cases tied!

None
}
});

log::info!(
"{LOG_PREFIX} migrated {:?} accounts",
transfer_allowlist_accounts.len()
);

weight
}

#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, TryRuntimeError> {
let transfer_allowlist_accounts: Vec<AccountId> =
pallet_transfer_allowlist::AccountCurrencyTransferAllowance::<T>::iter_keys()
.map(|(a, _, _)| a)
.collect();

let mut n = 0u64;
for (acc, _) in Holds::<T>::iter() {
assert!(Self::account_can_be_migrated(
&acc,
&transfer_allowlist_accounts
));
n.saturating_accrue(1);
}

log::info!("{LOG_PREFIX} pre checks done");
Ok(n.encode())
}

#[cfg(feature = "try-runtime")]
fn post_upgrade(pre_state: Vec<u8>) -> Result<(), TryRuntimeError> {
let count_pre: u64 = Decode::decode(&mut pre_state.as_slice())
.expect("pre_upgrade provides a valid state; qed");

let holds_post = pallet_balances::Holds::<T>::iter();
let count_post: u64 = holds_post.count().saturated_into();
assert_eq!(count_pre, count_post);

for (_, hold) in pallet_balances::Holds::<T>::iter() {
assert_eq!(hold.len(), 1);
assert_eq!(hold[0].id, HoldReason::TransferAllowance.into());
}

log::info!("{LOG_PREFIX} post checks done");
Ok(())
}
}

impl<T, RuntimeHoldReason> MigrateTransferAllowListHolds<T, RuntimeHoldReason>
where
T: pallet_balances::Config
+ pallet_transfer_allowlist::Config
+ frame_system::Config<AccountId = AccountId>,
{
fn account_can_be_migrated(who: &AccountId, whitelist: &[AccountId]) -> bool {
if !whitelist.iter().any(|a| a == who) {
log::warn!("{LOG_PREFIX} Account {who:?} is skipped due to missing AccountCurrencyTransferAllowance storage entry");
return false;
}

match Holds::<T>::get(who) {
holds if holds.len() == 1 => true,
_ => {
log::warn!("{LOG_PREFIX} Account {who:?} does not meet Hold storage assumptions");
false
}
}
}
}
1 change: 1 addition & 0 deletions runtime/common/src/migrations/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

//! Centrifuge Runtime-Common Migrations

pub mod hold_reason;
pub mod increase_storage_version;
pub mod loans;
pub mod nuke;
Expand Down
19 changes: 16 additions & 3 deletions runtime/development/src/migrations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ parameter_types! {
pub const AnnualTreasuryInflationPercent: u32 = 3;
}

// Number of identities on Dev and Demo Chain on 30.05.2024 was both 0
const IDENTITY_MIGRATION_KEY_LIMIT: u64 = 1000;

/// The migration set for Development & Demo.
/// It includes all the migrations that have to be applied on that chain.
pub type UpgradeDevelopment1047 = (
Expand All @@ -30,26 +33,36 @@ pub type UpgradeDevelopment1047 = (
// v0 -> v1
runtime_common::migrations::nuke::ResetPallet<crate::Democracy, crate::RocksDbWeight, 0>,
// v0 -> v1
pallet_xcm::migration::v1::VersionUncheckedMigrateToV1<crate::Runtime>,
runtime_common::migrations::nuke::ResetPallet<crate::PolkadotXcm, crate::RocksDbWeight, 0>,
runtime_common::migrations::increase_storage_version::Migration<crate::PoolSystem, 0, 2>,
runtime_common::migrations::increase_storage_version::Migration<crate::InterestAccrual, 0, 3>,
runtime_common::migrations::increase_storage_version::Migration<crate::Investments, 0, 1>,
runtime_common::migrations::increase_storage_version::Migration<crate::BlockRewards, 0, 2>,
runtime_common::migrations::increase_storage_version::Migration<crate::OraclePriceFeed, 0, 1>,
runtime_common::migrations::increase_storage_version::Migration<
crate::OraclePriceCollection,
0,
1,
>,
runtime_common::migrations::increase_storage_version::Migration<crate::OrmlAssetRegistry, 0, 2>,
// Reset Block rewards
// Reset Block rewards on Demo which is at v0
runtime_common::migrations::nuke::ResetPallet<crate::BlockRewards, crate::RocksDbWeight, 0>,
// Reset Block rewards on Dev which is at v2
runtime_common::migrations::nuke::ResetPallet<crate::BlockRewards, crate::RocksDbWeight, 2>,
pallet_block_rewards::migrations::init::InitBlockRewards<
crate::Runtime,
CollatorReward,
AnnualTreasuryInflationPercent,
>,
runtime_common::migrations::loans::AddWithLinearPricing<crate::Runtime>,
runtime_common::migrations::hold_reason::MigrateTransferAllowListHolds<
crate::Runtime,
crate::RuntimeHoldReason,
>,
// Migrations below this comment originate from Polkadot SDK
pallet_xcm::migration::MigrateToLatestXcmVersion<crate::Runtime>,
cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4<crate::Runtime>,
pallet_identity::migration::versioned::V0ToV1<crate::Runtime, IDENTITY_MIGRATION_KEY_LIMIT>,
pallet_uniques::migration::MigrateV0ToV1<crate::Runtime, ()>,
);

mod cleanup_foreign_investments {
Expand Down
Loading