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

added migrations for key fix #2226

Merged
merged 9 commits into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from 8 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
3 changes: 2 additions & 1 deletion .github/workflows/verify-pr-commit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,10 @@ jobs:
steps:
- name: Check Out Repo
uses: actions/checkout@v4
# using older version of cargo deny since the new one requires rustc version >= 1.81
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to upgrade our base image container to a newer version?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We should do it but outside the scope of this PR.

- name: Set Up Cargo Deny
run: |
cargo install --force --locked cargo-deny
cargo install --force --locked cargo-deny@0.16.1
cargo generate-lockfile
- name: Run Cargo Deny
run: cargo deny check --hide-inclusion-graph -c deny.toml
Expand Down
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: 2 additions & 0 deletions pallets/capacity/src/migration/provider_boost_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ use frame_support::{
pallet_prelude::Weight,
traits::{Get, OnRuntimeUpgrade},
};
#[cfg(feature = "try-runtime")]
use sp_runtime::TryRuntimeError;

#[cfg(feature = "try-runtime")]
use sp_std::vec::Vec;
Expand Down
3 changes: 2 additions & 1 deletion pallets/stateful-storage/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ common-runtime = { path = "../../runtime/common", default-features = false }
env_logger = { workspace = true }
pretty_assertions = { workspace = true }
sp-keystore = { workspace = true }
hex = { workspace = true, default-features = false, features = ["alloc"] }

[features]
default = ['std']
Expand All @@ -57,4 +58,4 @@ std = [
"common-runtime/std",
]
try-runtime = ['frame-support/try-runtime']
test = []
test = []
31 changes: 30 additions & 1 deletion pallets/stateful-storage/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@
#[cfg(feature = "runtime-benchmarks")]
use common_primitives::benchmarks::{MsaBenchmarkHelper, SchemaBenchmarkHelper};
use sp_std::prelude::*;

/// storage migrations
pub mod migration;
mod stateful_child_tree;
pub mod types;
pub mod weights;
Expand All @@ -60,6 +61,8 @@
use sp_runtime::{traits::Convert, DispatchError, MultiSignature};
pub use weights::*;

const LOG_TARGET: &str = "runtime::stateful-storage";

#[frame_support::pallet]
pub mod pallet {
use super::*;
Expand Down Expand Up @@ -124,8 +127,13 @@
// Simple declaration of the `Pallet` type. It is placeholder we use to implement traits and
// method.
#[pallet::pallet]
#[pallet::storage_version(STATEFUL_STORAGE_VERSION)]
pub struct Pallet<T>(_);

/// A temporary storage for migration
#[pallet::storage]
pub(super) type MigrationPageIndex<T: Config> = StorageValue<_, u32, ValueQuery>;

#[pallet::error]
pub enum Error<T> {
/// Page would exceed the highest allowable PageId
Expand Down Expand Up @@ -220,6 +228,27 @@
},
}

#[pallet::hooks]
impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
fn on_initialize(_current: BlockNumberFor<T>) -> Weight {
// this should get removed after rolling out to testnet
#[cfg(any(feature = "frequency-testnet", test))]
{
let page_index = <MigrationPageIndex<T>>::get();
let (weight, continue_migration) = migration::v1::paginated_migration_testnet::<T>(
MIGRATION_PAGE_SIZE,
page_index,
);
if continue_migration {
<MigrationPageIndex<T>>::set(page_index.saturating_add(1));
}
T::DbWeight::get().reads_writes(1, 1).saturating_add(weight)
}
#[cfg(not(any(feature = "frequency-testnet", test)))]
Weight::zero()

Check warning on line 248 in pallets/stateful-storage/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

pallets/stateful-storage/src/lib.rs#L248

Added line #L248 was not covered by tests
}
}

#[pallet::call]
impl<T: Config> Pallet<T> {
/// Applies the Add or Delete Actions on the requested Itemized page.
Expand Down
2 changes: 2 additions & 0 deletions pallets/stateful-storage/src/migration/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
/// migrations to v1
pub mod v1;
258 changes: 258 additions & 0 deletions pallets/stateful-storage/src/migration/v1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,258 @@
#[cfg(feature = "try-runtime")]
use crate::types::STATEFUL_STORAGE_VERSION;
use crate::{
stateful_child_tree::StatefulChildTree,
types::{
ItemAction, ItemizedKey, ItemizedOperations, ItemizedPage, Page, ITEMIZED_STORAGE_PREFIX,
PALLET_STORAGE_PREFIX,
},
Config, Pallet, LOG_TARGET,
};
use common_primitives::{
msa::MessageSourceId,
utils::{get_chain_type_by_genesis_hash, DetectedChainType},
};
use frame_support::{pallet_prelude::*, traits::OnRuntimeUpgrade, weights::Weight};
use frame_system::pallet_prelude::BlockNumberFor;
use log;
#[cfg(feature = "try-runtime")]
use sp_core::hexdisplay::HexDisplay;
use sp_runtime::Saturating;
#[cfg(feature = "try-runtime")]
use sp_runtime::TryRuntimeError;
use sp_std::{vec, vec::Vec};

/// testnet specific msa ids for migration
#[cfg(any(feature = "frequency-testnet", test))]
pub fn get_testnet_msa_ids() -> Vec<MessageSourceId> {
vec![
8004, 8009, 8816, 8817, 8818, 8819, 8820, 8822, 8823, 8824, 8825, 8826, 9384, 9753, 9919,
9992, 9994, 9996, 9997, 10009, 10010, 10012, 10013, 10014, 10015, 10019, 10020, 10021,
10022, 10023, 10024, 10025, 10026, 10027, 10028, 10029, 10030, 10031, 10032, 10033, 10034,
10035, 10036, 10037, 10038, 10039, 10040, 10041, 10042, 10043, 10044, 10045, 10046, 10047,
10048, 10049, 10050, 10051, 10052, 10053, 10054, 10055, 10056, 10057, 10058, 10059, 10061,
10062, 10064, 10067, 10068, 10069, 10070, 10071, 10072, 10075, 10076, 10077, 10078, 10079,
10138, 10139, 10140, 10206, 10207, 10209, 10212, 10218, 10219, 10220, 10221, 10222, 10223,
10224, 10231, 10232, 10233, 10234, 10235, 10236, 10237, 10238, 10239, 10240, 10241, 10242,
10243, 10247, 10248, 10251, 10253, 10254, 10255, 10256, 10257, 10258, 10259, 10260, 10261,
10262, 10263, 10264, 10265, 10266, 10267, 10268, 10269, 10270, 10271, 10272, 10273, 10274,
10275, 10287, 10288, 10289, 10290, 10291, 10292, 10293, 10294, 10295, 10296, 10297, 10298,
10299, 10300, 10301, 10302, 10303, 10304, 10305, 10306, 10307, 10308, 10309, 10311, 10312,
10313, 10314, 10315, 10316, 10317, 10318, 10319, 10320, 10321, 10322, 10323, 10324, 10325,
10326, 10327, 10328, 10329,
]
}

/// returns the chain type from genesis hash
pub fn get_chain_type<T: Config>() -> DetectedChainType {
let genesis_block: BlockNumberFor<T> = 0u32.into();
let genesis = <frame_system::Pallet<T>>::block_hash(genesis_block);
get_chain_type_by_genesis_hash(&genesis.encode()[..])
}

/// get the msa ids with key migrations
pub fn get_msa_ids<T: Config>() -> Vec<MessageSourceId> {
let chain_type = get_chain_type::<T>();
if let DetectedChainType::FrequencyMainNet = chain_type {
vec![

Check warning on line 57 in pallets/stateful-storage/src/migration/v1.rs

View check run for this annotation

Codecov / codecov/patch

pallets/stateful-storage/src/migration/v1.rs#L57

Added line #L57 was not covered by tests
227, 542, 1249820, 1287729, 1288925, 1309067, 1309241, 1309258, 1309367, 1309397,
1329112, 1329535, 1330067,
]
} else {
if cfg!(test) {
// this allows to test the mainnet path
vec![1]
} else {
// we are going to use hooks for this multi-block migration so this is empty to only flag that
// it as done for consistency
vec![]

Check warning on line 68 in pallets/stateful-storage/src/migration/v1.rs

View check run for this annotation

Codecov / codecov/patch

pallets/stateful-storage/src/migration/v1.rs#L68

Added line #L68 was not covered by tests
}
}
}

/// migration to v1 implementation
pub struct MigrateToV1<T>(PhantomData<T>);

impl<T: Config> OnRuntimeUpgrade for MigrateToV1<T> {
fn on_runtime_upgrade() -> Weight {
migrate_to_v1::<T>()
}

Check warning on line 79 in pallets/stateful-storage/src/migration/v1.rs

View check run for this annotation

Codecov / codecov/patch

pallets/stateful-storage/src/migration/v1.rs#L77-L79

Added lines #L77 - L79 were not covered by tests

#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, TryRuntimeError> {
log::info!(target: LOG_TARGET, "Running pre_upgrade...");
let on_chain_version = Pallet::<T>::on_chain_storage_version();
let genesis_block: BlockNumberFor<T> = 0u32.into();
let genesis = <frame_system::Pallet<T>>::block_hash(genesis_block);
if on_chain_version >= 1 {
return Ok(Vec::new())
}
log::info!(target: LOG_TARGET, "Found genesis... {:?}", genesis);
let detected_chain = get_chain_type_by_genesis_hash(&genesis.encode()[..]);
log::info!(target: LOG_TARGET,"Detected Chain is {:?}", detected_chain);
Ok(Vec::new())
}

#[cfg(feature = "try-runtime")]
fn post_upgrade(_: Vec<u8>) -> Result<(), TryRuntimeError> {
log::info!(target: LOG_TARGET, "Running post_upgrade...");
let on_chain_version = Pallet::<T>::on_chain_storage_version();
if on_chain_version > 1 {
return Ok(())
}
let onchain_version = Pallet::<T>::on_chain_storage_version();
assert_eq!(onchain_version, STATEFUL_STORAGE_VERSION);
// check to ensure updates took place
let schema_id = get_schema_id();
let msa_ids = get_msa_ids::<T>();
for msa_id in msa_ids.into_iter() {
let itemized: ItemizedPage<T> = Pallet::<T>::get_itemized_page_for(msa_id, schema_id)
.map_err(|_| TryRuntimeError::Other("can not get storage"))?
.ok_or(TryRuntimeError::Other("no storage"))?;
let (_, val) = <Page<T::MaxItemizedPageSizeBytes> as ItemizedOperations<T>>::try_parse(
&itemized, false,
)
.map_err(|_| TryRuntimeError::Other("can not parse storage"))?
.items
.clone()
.into_iter()
.next()
.ok_or(TryRuntimeError::Other("no item"))?;

assert_eq!(val.len(), 33);
log::info!(target: LOG_TARGET, "{:?}", HexDisplay::from(&val));
}
log::info!(target: LOG_TARGET, "Finished post_upgrade");
Ok(())
}
}

/// migrating to v1
pub fn migrate_to_v1<T: Config>() -> Weight {
log::info!(target: LOG_TARGET, "Running storage migration...");
let onchain_version = Pallet::<T>::on_chain_storage_version();
let current_version = Pallet::<T>::in_code_storage_version();
log::info!(target: LOG_TARGET, "onchain_version= {:?}, current_version={:?}", onchain_version, current_version);
if onchain_version < 1 {
let msa_ids = get_msa_ids::<T>();
let weights = migrate_msa_ids::<T>(&msa_ids[..]);
// Set storage version to `1`.
StorageVersion::new(1).put::<Pallet<T>>();
let total_weight = T::DbWeight::get().writes(1).saturating_add(weights);
log::info!(target: LOG_TARGET, "Migration Calculated weights={:?}",total_weight);
total_weight
} else {
log::info!(

Check warning on line 145 in pallets/stateful-storage/src/migration/v1.rs

View check run for this annotation

Codecov / codecov/patch

pallets/stateful-storage/src/migration/v1.rs#L145

Added line #L145 was not covered by tests
target: LOG_TARGET,
"Migration did not execute. This probably should be removed onchain:{:?}, current:{:?}",
onchain_version,
current_version
);
T::DbWeight::get().reads(1)

Check warning on line 151 in pallets/stateful-storage/src/migration/v1.rs

View check run for this annotation

Codecov / codecov/patch

pallets/stateful-storage/src/migration/v1.rs#L151

Added line #L151 was not covered by tests
}
}

/// migrating all msa_ids
pub fn migrate_msa_ids<T: Config>(msa_ids: &[MessageSourceId]) -> Weight {
let schema_id = get_schema_id();
let key: ItemizedKey = (schema_id,);
let each_layer_access: u64 = 33 * 16;
let mut reads = 1u64;
let mut writes = 0u64;
let mut bytes = 0u64;

for msa_id in msa_ids.iter() {
reads.saturating_inc();
// get the itemized storages
let itemized_result: Result<Option<ItemizedPage<T>>, _> =
Pallet::<T>::get_itemized_page_for(*msa_id, schema_id);
match itemized_result {
Ok(Some(existing_page)) => {
bytes = bytes.saturating_add(existing_page.encode().len() as u64);
bytes = bytes.saturating_add(each_layer_access * 3); // three layers in merkle tree

match <Page<T::MaxItemizedPageSizeBytes> as ItemizedOperations<T>>::try_parse(
&existing_page,
false,
) {
Ok(parsed_page) => match parsed_page.items.clone().into_iter().next() {
Some((_, existing_value)) => match existing_value.len() {
32usize => {
// 64 is decimal value for 0x40
let mut prefixed = vec![64u8];
prefixed.extend_from_slice(existing_value);
let bounded: BoundedVec<u8, T::MaxItemizedBlobSizeBytes> =
prefixed.try_into().unwrap_or_default();

let empty_page = ItemizedPage::<T>::default();
match <Page<<T as Config>::MaxItemizedPageSizeBytes> as ItemizedOperations<T>>::apply_item_actions(&empty_page,&vec![ItemAction::Add {
data: bounded,
}]) {
Ok(mut updated_page) => {
updated_page.nonce = existing_page.nonce;
StatefulChildTree::<T::KeyHasher>::write(
msa_id,
PALLET_STORAGE_PREFIX,
ITEMIZED_STORAGE_PREFIX,
&key,
&updated_page,
);
bytes = bytes.saturating_add(updated_page.encode().len() as u64);
writes.saturating_inc();
},
Err(e) =>
log::error!(target: LOG_TARGET, "Error appending prefixed value {:?} and schema_id {:?} with {:?}", msa_id, schema_id, e),

Check warning on line 204 in pallets/stateful-storage/src/migration/v1.rs

View check run for this annotation

Codecov / codecov/patch

pallets/stateful-storage/src/migration/v1.rs#L203-L204

Added lines #L203 - L204 were not covered by tests
}
},
33usize =>
log::warn!(target: LOG_TARGET, "Itemized page item for msa_id {:?} and schema_id {:?} has correct size", msa_id, schema_id),

Check warning on line 208 in pallets/stateful-storage/src/migration/v1.rs

View check run for this annotation

Codecov / codecov/patch

pallets/stateful-storage/src/migration/v1.rs#L208

Added line #L208 was not covered by tests
_ =>
log::warn!(target: LOG_TARGET, "Itemized page item for msa_id {:?} and schema_id {:?} has invalid size {:?}", msa_id, schema_id, existing_value.len()),

Check warning on line 210 in pallets/stateful-storage/src/migration/v1.rs

View check run for this annotation

Codecov / codecov/patch

pallets/stateful-storage/src/migration/v1.rs#L210

Added line #L210 was not covered by tests
},
None =>
log::warn!(target: LOG_TARGET, "Itemized page was empty for msa_id {:?} and schema_id {:?}", msa_id, schema_id),

Check warning on line 213 in pallets/stateful-storage/src/migration/v1.rs

View check run for this annotation

Codecov / codecov/patch

pallets/stateful-storage/src/migration/v1.rs#L213

Added line #L213 was not covered by tests
},
Err(e) =>
log::error!(target: LOG_TARGET, "Error parsing page for msa_id {:?} and schema_id {:?} with {:?}", msa_id, schema_id, e),

Check warning on line 216 in pallets/stateful-storage/src/migration/v1.rs

View check run for this annotation

Codecov / codecov/patch

pallets/stateful-storage/src/migration/v1.rs#L215-L216

Added lines #L215 - L216 were not covered by tests
}
},
Ok(None) =>
log::warn!(target: LOG_TARGET, "No page found for msa_id {:?} and schema_id {:?}", msa_id, schema_id),
Err(e) =>
log::error!(target: LOG_TARGET, "Error getting the page for msa_id {:?} and schema_id {:?} with {:?}", msa_id, schema_id, e),

Check warning on line 222 in pallets/stateful-storage/src/migration/v1.rs

View check run for this annotation

Codecov / codecov/patch

pallets/stateful-storage/src/migration/v1.rs#L221-L222

Added lines #L221 - L222 were not covered by tests
}
}
log::info!(target: LOG_TARGET, "Storage migrated to version 1 read={:?}, write={:?}, bytes={:?}", reads, writes, bytes);
let weights = T::DbWeight::get().reads_writes(reads, writes).add_proof_size(bytes);
log::info!(target: LOG_TARGET, "migrate_msa_ids weights={:?}",weights);
weights
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

logic looks good
nit: reduce nesting but since its a one time migration

Copy link
Collaborator Author

@aramikm aramikm Nov 21, 2024

Choose a reason for hiding this comment

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

Yeah I was on the fence about that but since I wanted to log all the failure cases the other alternative was to use a lot of continue in the loop for failure cases which is also not that great.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's cleared up once you get down to the alternate cases, but a couple of comments would be nice. I wonder if you have found any empty itemized pages stored.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I couldn't find any empty itemized pages but still had to handle it just in case.


/// paginated migration for testnet
#[cfg(any(feature = "frequency-testnet", test))]
pub fn paginated_migration_testnet<T: Config>(page_size: u32, page_index: u32) -> (Weight, bool) {
let msa_ids: Vec<MessageSourceId> = get_testnet_msa_ids();
let mut chunks = msa_ids.chunks(page_size as usize);
let chunk_len = chunks.len() as u32;
let mut current = 0u32;
while current < page_index && current < chunk_len {
let _ = chunks.next();
current += 1;
}
match chunks.next() {
Some(page) => {
let weight = migrate_msa_ids::<T>(page);
(weight, true)
},
None => (Weight::zero(), false),
}
}

fn get_schema_id() -> u16 {
if cfg!(test) {
// Supported ITEMIZED_APPEND_ONLY_SCHEMA for tests
103
} else {
7

Check warning on line 256 in pallets/stateful-storage/src/migration/v1.rs

View check run for this annotation

Codecov / codecov/patch

pallets/stateful-storage/src/migration/v1.rs#L256

Added line #L256 was not covered by tests
}
}
Loading
Loading