diff --git a/pallet/src/lib.rs b/pallet/src/lib.rs index 6f7777b6..c22f275f 100644 --- a/pallet/src/lib.rs +++ b/pallet/src/lib.rs @@ -22,6 +22,7 @@ extern crate alloc; pub use pallet::*; pub mod impls; +pub mod migrations; pub mod runtime_api; pub mod weights; pub use weights::WeightInfo; @@ -71,7 +72,12 @@ pub mod pallet { BlockNumberFor, >; + /// In-code storage version. v1 backfills the `commitment_nonce` field added + /// to `BucketSnapshot` by #125; see [`crate::migrations::v1`]. + const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); + #[pallet::pallet] + #[pallet::storage_version(STORAGE_VERSION)] pub struct Pallet(_); #[pallet::hooks] diff --git a/pallet/src/migrations.rs b/pallet/src/migrations.rs new file mode 100644 index 00000000..115d89a7 --- /dev/null +++ b/pallet/src/migrations.rs @@ -0,0 +1,120 @@ +// SPDX-License-Identifier: Apache-2.0 + +//! Storage migrations for `pallet-storage-provider`. + +extern crate alloc; + +/// v0 -> v1: backfill the `commitment_nonce` field added to `BucketSnapshot` by +/// the challenge flow overhaul (#125). +/// +/// `BucketSnapshot` (nested in `Bucket::snapshot`) gained a trailing +/// `commitment_nonce: u64` field, required by `extend_checkpoint` to verify a +/// late-arriving signature against the payload the original signers signed. +/// `Buckets` entries checkpointed before that change still encode the old +/// (nonce-less) layout, so the current `Bucket`/`BucketSnapshot` decode treats +/// them as undecodable — `try-runtime` reports the keys as undecodable. This +/// re-stores every entry in the new layout, defaulting `commitment_nonce` to +/// `0` (harmless: the nonce only matters to late-signature verification for a +/// checkpoint still accepting signers, which no pre-existing snapshot has). +pub mod v1 { + use crate::{Bucket, Buckets, Config, Member, Pallet}; + use frame_support::{pallet_prelude::*, traits::UncheckedOnRuntimeUpgrade, weights::Weight}; + use frame_system::pallet_prelude::BlockNumberFor; + use sp_core::H256; + use storage_primitives::{BucketSnapshot, Commitment}; + + /// The pre-migration `Bucket`/`BucketSnapshot`, used only to decode the old + /// `Buckets` values. Mirrors the pre-#125 layout, i.e. the current layout + /// minus the trailing `commitment_nonce` field. + mod old { + use super::*; + use alloc::vec::Vec; + + #[derive(Decode)] + pub struct BucketSnapshot { + pub commitment: Commitment, + pub checkpoint_block: BlockNumber, + pub primary_signers: Vec, + } + + #[derive(Decode)] + pub struct Bucket { + pub members: BoundedVec, T::MaxMembers>, + pub frozen_start_seq: Option, + pub min_providers: u32, + pub primary_providers: BoundedVec, + pub snapshot: Option>>, + pub historical_roots: [(u32, H256); 6], + pub total_snapshots: u32, + } + } + + pub struct InnerMigrateV0ToV1(core::marker::PhantomData); + + impl UncheckedOnRuntimeUpgrade for InnerMigrateV0ToV1 { + fn on_runtime_upgrade() -> Weight { + let total = Buckets::::iter_keys().count() as u64; + let mut translated = 0u64; + Buckets::::translate::, _>(|_bucket_id, old| { + translated = translated.saturating_add(1); + Some(Bucket { + members: old.members, + frozen_start_seq: old.frozen_start_seq, + min_providers: old.min_providers, + primary_providers: old.primary_providers, + snapshot: old.snapshot.map(|s| BucketSnapshot { + commitment: s.commitment, + checkpoint_block: s.checkpoint_block, + primary_signers: s.primary_signers, + commitment_nonce: 0, + }), + historical_roots: old.historical_roots, + total_snapshots: old.total_snapshots, + }) + }); + // `translate` reads every key in the map (whether or not it + // decodes under the old layout) but only rewrites the ones that + // do, so `total` reads and `translated` writes is the true upper + // bound rather than under-counting reads on decode failures. + T::DbWeight::get().reads_writes(total, translated) + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, sp_runtime::TryRuntimeError> { + // Count existing keys before migration so post_upgrade can confirm + // none were dropped. + let count = Buckets::::iter_keys().count() as u64; + Ok(count.encode()) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(state: alloc::vec::Vec) -> Result<(), sp_runtime::TryRuntimeError> { + let before = u64::decode(&mut &state[..]).map_err(|_| "invalid pre_upgrade state")?; + // `iter_keys()` only enumerates keys; `iter()` fully decodes every + // value under the NEW layout. If any entry still failed to decode, + // `translate` would have left it as a raw, undecodable blob that + // `iter()` silently skips, so the two counts would diverge. + let after_keys = Buckets::::iter_keys().count() as u64; + let after_values = Buckets::::iter().count() as u64; + ensure!( + before == after_keys, + "Buckets entry count changed during migration" + ); + ensure!( + after_keys == after_values, + "some Buckets entry failed to decode under the new layout" + ); + Ok(()) + } + } + + /// Runs [`InnerMigrateV0ToV1`] only when the on-chain storage version is 0, + /// then bumps it to 1. + pub type MigrateV0ToV1 = frame_support::migrations::VersionedMigration< + 0, + 1, + InnerMigrateV0ToV1, + Pallet, + ::DbWeight, + >; +} diff --git a/pallet/src/tests/migrations.rs b/pallet/src/tests/migrations.rs new file mode 100644 index 00000000..5d932845 --- /dev/null +++ b/pallet/src/tests/migrations.rs @@ -0,0 +1,106 @@ +// SPDX-License-Identifier: Apache-2.0 + +//! Tests for the v0 -> v1 `commitment_nonce` backfill migration. + +use super::*; +use crate::migrations::v1::InnerMigrateV0ToV1; +use codec::Encode; +use frame_support::{storage::unhashed, traits::UncheckedOnRuntimeUpgrade, BoundedVec}; +use sp_core::H256; +use storage_primitives::{BucketId, Commitment}; + +/// Mirrors the pre-#125 `BucketSnapshot` layout (no `commitment_nonce`), used +/// only to encode a raw old-format value for this test. +#[derive(Encode)] +struct OldBucketSnapshot { + commitment: Commitment, + checkpoint_block: u64, + primary_signers: Vec, +} + +/// Mirrors the pre-#125 `Bucket` layout, used only to encode a raw old-format +/// value for this test. +#[derive(Encode)] +struct OldBucket { + members: BoundedVec, ::MaxMembers>, + frozen_start_seq: Option, + min_providers: u32, + primary_providers: BoundedVec::MaxPrimaryProviders>, + snapshot: Option, + historical_roots: [(u32, H256); 6], + total_snapshots: u32, +} + +fn put_old_bucket(bucket_id: BucketId, old: OldBucket) { + unhashed::put_raw(&Buckets::::hashed_key_for(bucket_id), &old.encode()); +} + +#[test] +fn migration_backfills_commitment_nonce_on_existing_snapshot() { + new_test_ext().execute_with(|| { + let bucket_id: BucketId = 1; + let member = Member { + account: 42u64, + role: Role::Admin, + }; + put_old_bucket( + bucket_id, + OldBucket { + members: vec![member.clone()].try_into().unwrap(), + frozen_start_seq: None, + min_providers: 1, + primary_providers: vec![42u64].try_into().unwrap(), + snapshot: Some(OldBucketSnapshot { + commitment: Commitment { + mmr_root: H256::repeat_byte(0xAB), + start_seq: 0, + leaf_count: 2, + }, + checkpoint_block: 100, + primary_signers: vec![0b1], + }), + historical_roots: [(0, H256::default()); 6], + total_snapshots: 1, + }, + ); + + InnerMigrateV0ToV1::::on_runtime_upgrade(); + + let migrated = Buckets::::get(bucket_id).expect("bucket must still decode"); + assert_eq!(migrated.members.into_inner(), vec![member]); + assert_eq!(migrated.frozen_start_seq, None); + assert_eq!(migrated.min_providers, 1); + assert_eq!(migrated.primary_providers.into_inner(), vec![42u64]); + assert_eq!(migrated.total_snapshots, 1); + let snapshot = migrated.snapshot.expect("snapshot must survive migration"); + assert_eq!(snapshot.checkpoint_block, 100); + assert_eq!(snapshot.primary_signers, vec![0b1]); + assert_eq!(snapshot.commitment.leaf_count, 2); + assert_eq!(snapshot.commitment_nonce, 0); + }); +} + +#[test] +fn migration_preserves_bucket_with_no_snapshot() { + new_test_ext().execute_with(|| { + let bucket_id: BucketId = 2; + put_old_bucket( + bucket_id, + OldBucket { + members: BoundedVec::default(), + frozen_start_seq: Some(5), + min_providers: 0, + primary_providers: BoundedVec::default(), + snapshot: None, + historical_roots: [(0, H256::default()); 6], + total_snapshots: 0, + }, + ); + + InnerMigrateV0ToV1::::on_runtime_upgrade(); + + let migrated = Buckets::::get(bucket_id).expect("bucket must still decode"); + assert_eq!(migrated.frozen_start_seq, Some(5)); + assert!(migrated.snapshot.is_none()); + }); +} diff --git a/pallet/src/tests/mod.rs b/pallet/src/tests/mod.rs index cebbc9b3..d18aa1bd 100644 --- a/pallet/src/tests/mod.rs +++ b/pallet/src/tests/mod.rs @@ -21,6 +21,7 @@ mod error_paths; mod extend_topup; mod genesis; mod member_buckets; +mod migrations; mod misc; mod provider; mod replica; diff --git a/runtimes/web3-storage-paseo/src/migrations.rs b/runtimes/web3-storage-paseo/src/migrations.rs index f91df07c..866982cb 100644 --- a/runtimes/web3-storage-paseo/src/migrations.rs +++ b/runtimes/web3-storage-paseo/src/migrations.rs @@ -29,4 +29,8 @@ pub type Migrations = ( // Drop the `payment` field from `DriveInfo` (#105). A real data transform, // so it stays a `VersionedMigration` gated on the pallet's storage version. pallet_drive_registry::migrations::v1::MigrateV0ToV1, + // Backfill the `commitment_nonce` field added to `BucketSnapshot` by the + // challenge flow overhaul (#125). A real data transform, so it stays a + // `VersionedMigration` gated on the pallet's storage version. + pallet_storage_provider::migrations::v1::MigrateV0ToV1, );