Storage migration for pallet-xcm SafeXcmVersion constant

.gitignore

@@ -25,3 +25,5 @@ frequency.log
 scripts/js/onboard/.yarnrc.yml
 scripts/js/onboard/.yarn/cache
 scripts/js/onboard/.yarn/*.gz
+
+.markdownlint.json

AGENTS.md

@@ -0,0 +1,65 @@
+# AGENT GUIDELINES
+- This document provides guidelines for contributing to the project.
+- Please ensure that your contributions adhere to the project's coding standards and style guidelines.
+
+## How to Contribute
+- Fork the repository and create a new branch for your feature or bug fix.
+- Make your changes and commit them with clear, descriptive commit messages.
+- Submit a pull request with a detailed description of your changes.
+
+## Code Review Process
+- All contributions will be reviewed by the project maintainers.
+- Feedback will be provided, and changes may be requested before merging.
+
+## Testing Your Changes
+- Ensure that your changes are covered by tests.
+- Run the test suite to verify that all tests pass before submitting your pull request.
+
+## Documentation
+- Update the documentation to reflect any changes made to the codebase.
+- Ensure that the documentation is clear and easy to understand.
+
+## Issues and Bug Reports
+- If you find a bug, please open an issue in the repository.
+- Provide as much detail as possible, including steps to reproduce the issue and any relevant logs or screenshots.
+
+## Coding Standards
+- Follow the project's coding standards and style guidelines.
+- Use meaningful variable and function names.
+- Write clear and concise comments where necessary.
+
+## License
+- By contributing to this project, you agree that your contributions will be licensed under the project's license.
+
+## Dev Environment Tips
+- Set up your development environment according to the project's guidelines.
+- Use the provided scripts or tools to manage dependencies and run the project locally.
+- Regularly pull the latest changes from the main branch to keep your branch up to date.
+
+## Testing Instructions
+- Find the CI plan in the .github directory. `verify-pr-commit.yml` is the main CI plan that runs on every PR.
+- First run `make check` to ensure that cargo is installed and the project is set up correctly.
+- Run the build to ensure that the project compiles without errors.
+    ```bash
+    make build-no-relay
+    ```
+- Rust builds can take up to 20 minutes, so be patient.
+- Run the unit test suite using the provided commands. Network access is not required for these tests.
+    ```bash
+    make test
+    ```
+- Run the e2e test suite using the provided commands. Network access is not required for these tests, only that you have a local node running.
+    ```bash
+    make e2e-tests
+    ```
+- Run the linter to check for code style issues.
+    ```bash
+    make lint
+    ```
+- Run the formatter to ensure code style consistency.
+    ```bash
+    make lint-fix
+    ```
+
+## PR Instructions
+Title Format: `[AGENT] <short description>`

Makefile

@@ -373,29 +373,55 @@ check-try-runtime-installed:
 try-runtime-create-snapshot-paseo-testnet: check-try-runtime-installed
 	try-runtime create-snapshot --uri wss://0.rpc.testnet.amplica.io:443 testnet-paseo-all-pallets.state
 
+try-runtime-create-snapshot-westend-testnet: check-try-runtime-installed
+	@if [ -z "$(ONFINALITY_APIKEY)" ]; then \
+		echo "Error: ONFINALITY_APIKEY environment variable is not set. Please set it before running this target."; \
+		echo "Example: ONFINALITY_APIKEY=your-api-key-here make try-runtime-check-migrations-westend-testnet"; \
+		exit 1; \
+	fi
+	try-runtime create-snapshot --uri wss://node-7330371704012918784.nv.onfinality.io/ws?apikey=$(ONFINALITY_APIKEY) testnet-paseo-all-pallets.state
+
 # mainnet snapshot takes as many as 24 hours to complete
 try-runtime-create-snapshot-mainnet: check-try-runtime-installed
 	try-runtime create-snapshot --uri wss://1.rpc.frequency.xyz:443 mainnet-all-pallets.state
 
 try-runtime-upgrade-paseo-testnet: check-try-runtime-installed
 	cargo build --release --features frequency-testnet,try-runtime && \
-	try-runtime --runtime ./target/release/wbuild/frequency-runtime/frequency_runtime.wasm on-runtime-upgrade live --uri wss://0.rpc.testnet.amplica.io:443
+	try-runtime --runtime ./target/release/wbuild/frequency-runtime/frequency_runtime.wasm on-runtime-upgrade --blocktime=6000 live --uri wss://0.rpc.testnet.amplica.io:443
 
 try-runtime-upgrade-mainnet: check-try-runtime-installed
 	cargo build --release --features frequency,try-runtime && \
-	try-runtime --runtime ./target/release/wbuild/frequency-runtime/frequency_runtime.wasm on-runtime-upgrade live --uri wss://1.rpc.frequency.xyz:443
+	try-runtime --runtime ./target/release/wbuild/frequency-runtime/frequency_runtime.wasm on-runtime-upgrade --blocktime=6000 live --uri wss://1.rpc.frequency.xyz:443
 
 try-runtime-use-snapshot-paseo-testnet: check-try-runtime-installed
 	cargo build --release --features frequency-testnet,try-runtime && \
-	try-runtime --runtime ./target/release/wbuild/frequency-runtime/frequency_runtime.wasm on-runtime-upgrade snap --path testnet-paseo-all-pallets.state
+	try-runtime --runtime ./target/release/wbuild/frequency-runtime/frequency_runtime.wasm on-runtime-upgrade --blocktime=6000 snap --path testnet-paseo-all-pallets.state
 
 try-runtime-use-snapshot-mainnet: check-try-runtime-installed
 	cargo build --release --features frequency,try-runtime && \
-	try-runtime --runtime ./target/release/wbuild/frequency-runtime/frequency_runtime.wasm on-runtime-upgrade snap --path mainnet-all-pallets.state
+	try-runtime --runtime ./target/release/wbuild/frequency-runtime/frequency_runtime.wasm on-runtime-upgrade --blocktime=6000 snap --path mainnet-all-pallets.state
 
 try-runtime-check-migrations-paseo-testnet: check-try-runtime-installed
 	cargo build --release --features frequency-testnet,try-runtime -q --locked && \
-	try-runtime --runtime ./target/release/wbuild/frequency-runtime/frequency_runtime.wasm on-runtime-upgrade --checks="pre-and-post" --disable-spec-version-check --no-weight-warnings live --uri wss://0.rpc.testnet.amplica.io:443
+	try-runtime --runtime ./target/release/wbuild/frequency-runtime/frequency_runtime.wasm on-runtime-upgrade --blocktime=6000 --checks="pre-and-post" --disable-spec-version-check --no-weight-warnings live --uri wss://0.rpc.testnet.amplica.io:443
+
+try-runtime-check-migrations-westend-testnet: check-try-runtime-installed
+	@if [ -z "$(ONFINALITY_APIKEY)" ]; then \
+		echo "Error: ONFINALITY_APIKEY environment variable is not set. Please set it before running this target."; \
+		echo "Example: ONFINALITY_APIKEY=your-api-key-here make try-runtime-check-migrations-westend-testnet"; \
+		exit 1; \
+	fi
+	cargo build --release --features frequency-westend,frequency-bridging,try-runtime -q --locked && \
+	try-runtime --runtime ./target/release/wbuild/frequency-runtime/frequency_runtime.wasm on-runtime-upgrade --blocktime=6000 --checks="pre-and-post" --disable-spec-version-check --no-weight-warnings live --uri wss://node-7330371704012918784.nv.onfinality.io/ws?apikey=$(ONFINALITY_APIKEY)
+
+try-runtime-check-migrations-local: check-try-runtime-installed
+	cargo build --features frequency-local,frequency-bridging,try-runtime -q --locked && \
+	try-runtime --runtime ./target/debug/wbuild/frequency-runtime/frequency_runtime.wasm on-runtime-upgrade --blocktime=6000 --checks="pre-and-post" --disable-spec-version-check --disable-mbm-checks --no-weight-warnings live --uri ws://localhost:9944
+
+try-runtime-check-migrations-none-local: check-try-runtime-installed
+	cargo build --features frequency-local,frequency-bridging,try-runtime -q --locked && \
+	try-runtime --runtime ./target/debug/wbuild/frequency-runtime/frequency_runtime.wasm on-runtime-upgrade --blocktime=6000 --checks="none" --disable-spec-version-check --disable-mbm-checks --no-weight-warnings live --uri ws://localhost:9944
+
 # Pull the Polkadot version from the polkadot-cli package in the Cargo.lock file.
 # This will break if the lock file format changes
 POLKADOT_VERSION=$(shell grep "^polkadot-cli" Cargo.toml | grep -o 'branch[[:space:]]*=[[:space:]]*"\(.*\)"' | sed 's/branch *= *"\(.*\)"/\1/' | head -n 1)

runtime/common/src/constants.rs

@@ -400,3 +400,12 @@ pub type CapacityRewardEraLength =
 	ConstU32<{ prod_or_testnet_or_local!(14 * DAYS, 1 * HOURS, 50) }>;
 
 // -end- Capacity Pallet ---
+
+// --- XCM Version ---
+#[cfg(feature = "frequency-bridging")]
+pub mod xcm_version {
+	/// The default XCM version considered safe for the network.
+	/// This is not the latest version, but the one that is considered stable and safe to use.
+	/// It is used to ensure that the network can handle XCM messages without issues.
+	pub const SAFE_XCM_VERSION: u32 = 4;
+}

runtime/frequency/src/lib.rs

@@ -34,6 +34,9 @@ use xcm_commons::{RelayOrigin, ReservedDmpWeight, ReservedXcmpWeight};
 #[cfg(feature = "frequency-bridging")]
 mod xcm; // Tests are contained the xcm directory
 
+#[cfg(test)]
+mod migration_tests;
+
 use alloc::borrow::Cow;
 use common_runtime::constants::currency::UNITS;
 
@@ -426,6 +429,17 @@ pub type UncheckedExtrinsic =
 	generic::UncheckedExtrinsic<Address, RuntimeCall, Signature, TxExtension>;
 
 /// Executive: handles dispatch to the various modules.
+#[cfg(feature = "frequency-bridging")]
+pub type Executive = frame_executive::Executive<
+	Runtime,
+	Block,
+	frame_system::ChainContext<Runtime>,
+	Runtime,
+	AllPalletsWithSystem,
+	(MigratePalletsCurrentStorage<Runtime>, SetSafeXcmVersion<Runtime>),
+>;
+
+#[cfg(not(feature = "frequency-bridging"))]
 pub type Executive = frame_executive::Executive<
 	Runtime,
 	Block,
@@ -454,6 +468,106 @@ impl<T: pallet_collator_selection::Config> OnRuntimeUpgrade for MigratePalletsCu
 	}
 }
 
+/// Migration to set the initial safe XCM version for the XCM pallet.
+#[cfg(feature = "frequency-bridging")]
+pub struct SetSafeXcmVersion<T>(core::marker::PhantomData<T>);
+
+#[cfg(feature = "frequency-bridging")]
+use common_runtime::constants::xcm_version::SAFE_XCM_VERSION;
+
+#[cfg(feature = "frequency-bridging")]
+impl<T: pallet_xcm::Config> OnRuntimeUpgrade for SetSafeXcmVersion<T> {
+	fn on_runtime_upgrade() -> Weight {
+		use sp_core::Get;
+
+		// Access storage directly using storage key because `pallet_xcm` does not provide a direct API to get the safe XCM version.
+		let storage_key = frame_support::storage::storage_prefix(b"PolkadotXcm", b"SafeXcmVersion");
+		log::info!("Checking SafeXcmVersion in storage with key: {:?}", storage_key);
+
+		let current_version = frame_support::storage::unhashed::get::<u32>(&storage_key);
+		match current_version {
+			Some(version) if version == SAFE_XCM_VERSION => {
+				log::info!("SafeXcmVersion already set to {}, skipping migration.", version);
+				T::DbWeight::get().reads(1)
+			},
+			Some(version) => {
+				log::info!(
+					"SafeXcmVersion currently set to {}, updating to {}",
+					version,
+					SAFE_XCM_VERSION
+				);
+				// Set the safe XCM version directly in storage
+				frame_support::storage::unhashed::put(&storage_key, &(SAFE_XCM_VERSION));
+				T::DbWeight::get().reads(1).saturating_add(T::DbWeight::get().writes(1))
+			},
+			None => {
+				log::info!("SafeXcmVersion not set, setting to {}", SAFE_XCM_VERSION);
+				// Set the safe XCM version directly in storage
+				frame_support::storage::unhashed::put(&storage_key, &(SAFE_XCM_VERSION));
+				T::DbWeight::get().reads(1).saturating_add(T::DbWeight::get().writes(1))
+			},
+		}
+	}
+}
+
+#[cfg(all(feature = "frequency-bridging", feature = "try-runtime"))]
+impl<T: pallet_xcm::Config> SetSafeXcmVersion<T> {
+	pub fn pre_upgrade() -> Result<Vec<u8>, sp_runtime::TryRuntimeError> {
+		use parity_scale_codec::Encode;
+
+		// For testing purposes, simulate that SafeXcmVersion is not set (None)
+		let test_pre_upgrade_state: Option<u32> = None;
+
+		log::info!("pre_upgrade: Test state SafeXcmVersion = {:?}", test_pre_upgrade_state);
+
+		// Return the test state encoded for post_upgrade verification
+		Ok(test_pre_upgrade_state.encode())
+	}
+
+	pub fn post_upgrade(state: Vec<u8>) -> Result<(), sp_runtime::TryRuntimeError> {
+		use parity_scale_codec::Decode;
+
+		// Decode the pre-upgrade state
+		let pre_upgrade_version = Option::<u32>::decode(&mut &state[..])
+			.map_err(|_| "Failed to decode pre-upgrade state")?;
+
+		let storage_key = frame_support::storage::storage_prefix(b"PolkadotXcm", b"SafeXcmVersion");
+		let current_version = frame_support::storage::unhashed::get::<u32>(&storage_key);
+
+		log::info!(
+			"post_upgrade: Pre-upgrade version = {:?}, Current version = {:?}",
+			pre_upgrade_version,
+			current_version
+		);
+
+		// Verify the migration worked correctly
+		match current_version {
+			Some(version) if version == SAFE_XCM_VERSION => {
+				log::info!(
+					"post_upgrade: Migration successful - SafeXcmVersion correctly set to {}",
+					version
+				);
+			},
+			Some(version) => {
+				log::error!("post_upgrade: Migration failed - SafeXcmVersion was set to {}, but expected {}", version, SAFE_XCM_VERSION);
+				return Err(sp_runtime::TryRuntimeError::Other(
+					"SafeXcmVersion was set to incorrect version after migration",
+				));
+			},
+			None => {
+				return Err(sp_runtime::TryRuntimeError::Other(
+					"SafeXcmVersion should be set after migration but found None",
+				));
+			},
+		}
+
+		Ok(())
+	}
+}
+
+#[cfg(not(feature = "frequency-bridging"))]
+pub struct SetSafeXcmVersion<T>(core::marker::PhantomData<T>);
+
 /// Opaque types. These are used by the CLI to instantiate machinery that don't need to know
 /// the specifics of the runtime. They can then be made to be agnostic over specific formats
 /// of data like extrinsics, allowing for them to continue syncing the network through upgrades
@@ -489,7 +603,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	spec_name: Cow::Borrowed("frequency"),
 	impl_name: Cow::Borrowed("frequency"),
 	authoring_version: 1,
-	spec_version: 159,
+	spec_version: 163,
 	impl_version: 0,
 	apis: RUNTIME_API_VERSIONS,
 	transaction_version: 1,
@@ -503,7 +617,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	spec_name: Cow::Borrowed("frequency-testnet"),
 	impl_name: Cow::Borrowed("frequency"),
 	authoring_version: 1,
-	spec_version: 159,
+	spec_version: 163,
 	impl_version: 0,
 	apis: RUNTIME_API_VERSIONS,
 	transaction_version: 1,

runtime/frequency/src/migration_tests.rs

@@ -0,0 +1,75 @@
+#![cfg(all(feature = "frequency-bridging", feature = "try-runtime"))]
+use super::*;
+use crate::xcm::tests::mock::{new_test_ext_with_balances, TestRuntime};
+use common_runtime::constants::xcm_version::SAFE_XCM_VERSION;
+use parity_scale_codec::Decode;
+
+#[test]
+fn pre_upgrade_returns_current_value() {
+	new_test_ext_with_balances(vec![]).execute_with(|| {
+		let encoded_result = SetSafeXcmVersion::<TestRuntime>::pre_upgrade().unwrap();
+		let decoded_result = Option::<u32>::decode(&mut &encoded_result[..]).unwrap();
+		assert_eq!(decoded_result, None);
+	});
+}
+
+#[test]
+fn post_upgrade_sets_safe_version() {
+	new_test_ext_with_balances(vec![]).execute_with(|| {
+		let storage_key = frame_support::storage::storage_prefix(b"PolkadotXcm", b"SafeXcmVersion");
+
+		// Clear the storage
+		frame_support::storage::unhashed::kill(&storage_key);
+		let pre = SetSafeXcmVersion::<TestRuntime>::pre_upgrade().unwrap();
+		SetSafeXcmVersion::<TestRuntime>::on_runtime_upgrade();
+		SetSafeXcmVersion::<TestRuntime>::post_upgrade(pre).unwrap();
+		assert_eq!(
+			frame_support::storage::unhashed::get::<u32>(&storage_key).unwrap(),
+			SAFE_XCM_VERSION
+		);
+	});
+}
+
+#[test]
+fn migration_is_idempotent() {
+	new_test_ext_with_balances(vec![]).execute_with(|| {
+		let storage_key = frame_support::storage::storage_prefix(b"PolkadotXcm", b"SafeXcmVersion");
+
+		// Clear the storage initially
+		frame_support::storage::unhashed::kill(&storage_key);
+
+		// Run migration first time
+		let weight1 = SetSafeXcmVersion::<TestRuntime>::on_runtime_upgrade();
+		assert_eq!(
+			frame_support::storage::unhashed::get::<u32>(&storage_key).unwrap(),
+			SAFE_XCM_VERSION
+		);
+
+		// Run migration second time - should be a no-op
+		let weight2 = SetSafeXcmVersion::<TestRuntime>::on_runtime_upgrade();
+		assert_eq!(
+			frame_support::storage::unhashed::get::<u32>(&storage_key).unwrap(),
+			SAFE_XCM_VERSION
+		);
+
+		// Second run should only do a read (less weight)
+		assert!(weight2.ref_time() < weight1.ref_time());
+	});
+}
+
+#[test]
+fn test_weight_calculation() {
+	new_test_ext_with_balances(vec![]).execute_with(|| {
+		let storage_key = frame_support::storage::storage_prefix(b"PolkadotXcm", b"SafeXcmVersion");
+
+		// Test weight when migration is needed
+		frame_support::storage::unhashed::kill(&storage_key);
+		let weight_with_write = SetSafeXcmVersion::<TestRuntime>::on_runtime_upgrade();
+
+		// Test weight when migration is not needed (value already exists)
+		let weight_read_only = SetSafeXcmVersion::<TestRuntime>::on_runtime_upgrade();
+
+		// Weight with write should be greater than read-only weight
+		assert!(weight_with_write.ref_time() > weight_read_only.ref_time());
+	});
+}

runtime/frequency/src/xcm/tests/mock.rs

@@ -17,6 +17,7 @@
 //! Mock runtime for tests.
 //! Implements both runtime APIs for fee estimation and getting the messages for transfers.
 
+use common_runtime::weights::rocksdb_weights::constants::RocksDbWeight;
 use core::{cell::RefCell, marker::PhantomData};
 use frame_support::{
 	construct_runtime, derive_impl, parameter_types, sp_runtime,
@@ -81,6 +82,7 @@ impl frame_system::Config for TestRuntime {
 	type AccountId = AccountId;
 	type AccountData = pallet_balances::AccountData<Balance>;
 	type Lookup = IdentityLookup<AccountId>;
+	type DbWeight = RocksDbWeight;
 }
 
 #[derive_impl(pallet_balances::config_preludes::TestDefaultConfig)]

runtime/frequency/src/xcm/tests/mod.rs

@@ -1,4 +1,4 @@
-mod mock;
+pub mod mock;
 
 mod check_whitelist;
 mod fee_estimation;