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

fix: undecodable ForeignInvestmentInfo on Demo #1819

Merged
merged 5 commits into from
Apr 19, 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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pallets/foreign-investments/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ pub mod pallet {
/// NOTE: The storage is killed once the investment is fully collected, or
/// decreased.
#[pallet::storage]
pub(super) type ForeignInvestmentInfo<T: Config> = StorageDoubleMap<
pub type ForeignInvestmentInfo<T: Config> = StorageDoubleMap<
_,
Blake2_128Concat,
T::AccountId,
Expand Down
55 changes: 55 additions & 0 deletions runtime/common/src/migrations/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,58 @@
pub mod increase_storage_version;
pub mod nuke;
pub mod precompile_account_codes;

pub mod utils {
use frame_support::storage::unhashed;
use sp_arithmetic::traits::Saturating;

/// Iterates keys of storage and removes undecodable keys
///
/// WARNING: USE WITH EXTREME CAUTION! Ensure the cleanup can be performed
/// beforehand!
pub fn remove_undecodable_storage_keys<T: parity_scale_codec::Decode + Sized>(

Check warning on line 27 in runtime/common/src/migrations/mod.rs

View check run for this annotation

Codecov / codecov/patch

runtime/common/src/migrations/mod.rs#L27

Added line #L27 was not covered by tests
prefix: [u8; 32],
) -> (u64, u64) {
let mut previous_key = prefix.clone().to_vec();
let mut reads: u64 = 1;
let mut writes: u64 = 0;
while let Some(next) =
sp_io::storage::next_key(&previous_key).filter(|n| n.starts_with(&prefix))

Check warning on line 34 in runtime/common/src/migrations/mod.rs

View check run for this annotation

Codecov / codecov/patch

runtime/common/src/migrations/mod.rs#L30-L34

Added lines #L30 - L34 were not covered by tests
{
reads.saturating_accrue(1);
previous_key = next;
let maybe_value = unhashed::get::<T>(&previous_key);
match maybe_value {
Some(_) => continue,
None => {
log::debug!(

Check warning on line 42 in runtime/common/src/migrations/mod.rs

View check run for this annotation

Codecov / codecov/patch

runtime/common/src/migrations/mod.rs#L36-L42

Added lines #L36 - L42 were not covered by tests
"Removing key which could not be decoded at {:?}",
previous_key

Check warning on line 44 in runtime/common/src/migrations/mod.rs

View check run for this annotation

Codecov / codecov/patch

runtime/common/src/migrations/mod.rs#L44

Added line #L44 was not covered by tests
);
unhashed::kill(&previous_key);
writes.saturating_accrue(1);
continue;

Check warning on line 48 in runtime/common/src/migrations/mod.rs

View check run for this annotation

Codecov / codecov/patch

runtime/common/src/migrations/mod.rs#L46-L48

Added lines #L46 - L48 were not covered by tests
}
}
}

(reads, writes)

Check warning on line 53 in runtime/common/src/migrations/mod.rs

View check run for this annotation

Codecov / codecov/patch

runtime/common/src/migrations/mod.rs#L53

Added line #L53 was not covered by tests
}

/// Iterates all keys of a storage and returns the count.
///
/// NOTE: Necessary because `Storage::<T>::iter().count()` does not account
/// for keys which cannot be decoded.
pub fn count_storage_keys(prefix: [u8; 32]) -> u32 {
let mut previous_key = prefix.clone().to_vec();
let mut n: u32 = 0;
while let Some(next) =
sp_io::storage::next_key(&previous_key).filter(|n| n.starts_with(&prefix))

Check warning on line 64 in runtime/common/src/migrations/mod.rs

View check run for this annotation

Codecov / codecov/patch

runtime/common/src/migrations/mod.rs#L60-L64

Added lines #L60 - L64 were not covered by tests
{
n.saturating_accrue(1);
previous_key = next;

Check warning on line 67 in runtime/common/src/migrations/mod.rs

View check run for this annotation

Codecov / codecov/patch

runtime/common/src/migrations/mod.rs#L66-L67

Added lines #L66 - L67 were not covered by tests
}

n

Check warning on line 70 in runtime/common/src/migrations/mod.rs

View check run for this annotation

Codecov / codecov/patch

runtime/common/src/migrations/mod.rs#L70

Added line #L70 was not covered by tests
}
}
11 changes: 2 additions & 9 deletions runtime/development/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ documentation.workspace = true
getrandom = { workspace = true }
hex = { workspace = true }
hex-literal = { workspace = true, optional = true }
log = { workspace = true }
parity-scale-codec = { workspace = true }
scale-info = { workspace = true }
serde = { workspace = true, optional = true }
Expand Down Expand Up @@ -148,9 +149,9 @@ std = [
"parity-scale-codec/std",
"getrandom/std",
"hex/std",
"log/std",
"scale-info/std",
"serde/std",

# Substrate related
"sp-api/std",
"sp-runtime/std",
Expand Down Expand Up @@ -187,14 +188,12 @@ std = [
"pallet-transaction-payment-rpc-runtime-api/std",
"polkadot-runtime-common/std",
"polkadot-parachain-primitives/std",

# Locals
"cfg-primitives/std",
"cfg-traits/std",
"cfg-types/std",
"runtime-common/std",
"liquidity-pools-gateway-routers/std",

# Pallet list
"axelar-gateway-precompile/std",
"chainbridge/std",
Expand Down Expand Up @@ -273,7 +272,6 @@ runtime-benchmarks = [
"frame-system-benchmarking/runtime-benchmarks",
"frame-benchmarking/runtime-benchmarks",
"cumulus-pallet-session-benchmarking/runtime-benchmarks",

# Substrate related
"sp-runtime/runtime-benchmarks",
"sp-staking/runtime-benchmarks",
Expand All @@ -284,14 +282,12 @@ runtime-benchmarks = [
"xcm-primitives/runtime-benchmarks",
"polkadot-runtime-common/runtime-benchmarks",
"polkadot-parachain-primitives/runtime-benchmarks",

# Locals
"cfg-primitives/runtime-benchmarks",
"cfg-traits/runtime-benchmarks",
"cfg-types/runtime-benchmarks",
"runtime-common/runtime-benchmarks",
"liquidity-pools-gateway-routers/runtime-benchmarks",

# Pallet list
"axelar-gateway-precompile/runtime-benchmarks",
"chainbridge/runtime-benchmarks",
Expand Down Expand Up @@ -356,22 +352,19 @@ runtime-benchmarks = [
try-runtime = [
# Enabling optional
"frame-try-runtime/try-runtime",

# Substrate related
"sp-runtime/try-runtime",
"frame-support/try-runtime",
"frame-system/try-runtime",
"frame-executive/try-runtime",
"fp-self-contained/try-runtime",
"polkadot-runtime-common/try-runtime",

# Locals
"cfg-primitives/try-runtime",
"cfg-traits/try-runtime",
"cfg-types/try-runtime",
"runtime-common/try-runtime",
"liquidity-pools-gateway-routers/try-runtime",

# Pallet list
"axelar-gateway-precompile/try-runtime",
"chainbridge/try-runtime",
Expand Down
4 changes: 2 additions & 2 deletions runtime/development/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
spec_name: create_runtime_str!("centrifuge-devel"),
impl_name: create_runtime_str!("centrifuge-devel"),
authoring_version: 1,
spec_version: 1046,
spec_version: 1047,
impl_version: 1,
apis: RUNTIME_API_VERSIONS,
transaction_version: 2,
Expand Down Expand Up @@ -2026,7 +2026,7 @@ pub type Executive = frame_executive::Executive<
frame_system::ChainContext<Runtime>,
Runtime,
AllPalletsWithSystem,
crate::migrations::UpgradeDevelopment1046,
crate::migrations::UpgradeDevelopment1047,
>;

// Frame Order in this block dictates the index of each one in the metadata
Expand Down
81 changes: 79 additions & 2 deletions runtime/development/src/migrations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,82 @@

/// The migration set for Development & Demo.
/// It includes all the migrations that have to be applied on that chain.
pub type UpgradeDevelopment1046 =
pallet_collator_selection::migration::v1::MigrateToV1<crate::Runtime>;
pub type UpgradeDevelopment1047 = (
pallet_collator_selection::migration::v1::MigrateToV1<crate::Runtime>,
cleanup_foreign_investments::Migration<crate::Runtime>,
);

mod cleanup_foreign_investments {
use cfg_types::tokens::CurrencyId;
use frame_support::{
storage::StoragePrefixedMap,
traits::{Get, OnRuntimeUpgrade},
weights::Weight,
};
#[cfg(feature = "try-runtime")]
use runtime_common::migrations::utils::count_storage_keys;
use runtime_common::migrations::utils::remove_undecodable_storage_keys;
#[cfg(feature = "try-runtime")]
use sp_runtime::DispatchError;
#[cfg(feature = "try-runtime")]
use sp_runtime::SaturatedConversion;

pub struct Migration<T>(sp_std::marker::PhantomData<T>);

const LOG_PREFIX: &str = "CleanupForeignInvestments";
impl<T> OnRuntimeUpgrade for Migration<T>
where
T: pallet_foreign_investments::Config + frame_system::Config,
{
fn on_runtime_upgrade() -> Weight {
log::info!("{LOG_PREFIX} Initiating removal of undecodable keys");
let (reads, writes) = remove_undecodable_storage_keys::<CurrencyId>(
pallet_foreign_investments::ForeignInvestmentInfo::<T>::final_prefix(),
);

log::info!("{LOG_PREFIX} Removed {writes} undecodable keys");

T::DbWeight::get().reads_writes(reads, writes)
}

#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<sp_std::vec::Vec<u8>, DispatchError> {
let n: u32 = count_storage_keys(
pallet_foreign_investments::ForeignInvestmentInfo::<T>::final_prefix(),
);
let n_count: u32 = pallet_foreign_investments::ForeignInvestmentInfo::<T>::iter_keys()
.count()
.saturated_into();

if n == n_count {
log::info!(
"{LOG_PREFIX} Storage cleanup can be skipped because all keys can be decoded"
);
} else {
log::info!(
"{LOG_PREFIX} Failed to decode {} keys, cleanup necessary",
n.saturating_sub(n_count)
);
}

log::info!("{LOG_PREFIX} pre_upgrade done!",);

Ok(sp_std::vec![])
}

#[cfg(feature = "try-runtime")]
fn post_upgrade(_pre_state: sp_std::vec::Vec<u8>) -> Result<(), DispatchError> {
let n: u32 = count_storage_keys(
pallet_foreign_investments::ForeignInvestmentInfo::<T>::final_prefix(),
);
let n_count: u32 = pallet_foreign_investments::ForeignInvestmentInfo::<T>::iter_keys()
.count()
.saturated_into();
assert_eq!(n, n_count);

log::info!("{LOG_PREFIX} post_upgrade done with {n} remaining storage keys!",);

Ok(())
}
}
}
Loading