From ea0117af5d7435f4720d18aebd32ee3c6f771382 Mon Sep 17 00:00:00 2001
From: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
Date: Thu, 23 Sep 2021 09:20:37 +0100
Subject: [PATCH] Add some docs about runtime migration best practices (#9837)
* Add some docs about runtime migration best practices as well.
* Update docs/CONTRIBUTING.adoc
* Update docs/CONTRIBUTING.adoc
Co-authored-by: Chevdor <chevdor@users.noreply.github.com>
Co-authored-by: Chevdor <chevdor@users.noreply.github.com>
---
substrate/docs/CONTRIBUTING.adoc | 5 +--
.../utils/frame/try-runtime/cli/src/lib.rs | 44 +++++++++++++++++--
2 files changed, 43 insertions(+), 6 deletions(-)
diff --git a/substrate/docs/CONTRIBUTING.adoc b/substrate/docs/CONTRIBUTING.adoc
index b0eaec04455..0a9a7ebacff 100644
--- a/substrate/docs/CONTRIBUTING.adoc
+++ b/substrate/docs/CONTRIBUTING.adoc
@@ -42,7 +42,7 @@ A Pull Request (PR) needs to be reviewed and approved by project maintainers unl
. PRs must be tagged with their release importance via the `C1-C9` labels.
. PRs must be tagged with their audit requirements via the `D1-D9` labels.
. PRs that must be backported to a stable branch must be tagged with https://github.com/paritytech/substrate/labels/E1-runtimemigration[`E0-patchthis`].
-. PRs that introduce runtime migrations must be tagged with https://github.com/paritytech/substrate/labels/E1-runtimemigration[`E1-runtimemigration`].
+. PRs that introduce runtime migrations must be tagged with https://github.com/paritytech/substrate/labels/E1-runtimemigration[`E1-runtimemigration`]. See the https://github.com/paritytech/substrate/blob/master/utils/frame/try-runtime/cli/src/lib.rs#L18[Migration Best Practices here] for more info about how to test runtime migrations.
. PRs that introduce irreversible database migrations must be tagged with https://github.com/paritytech/substrate/labels/E2-databasemigration[`E2-databasemigration`].
. PRs that add host functions must be tagged with with https://github.com/paritytech/substrate/labels/E4-newhostfunctions[`E4-newhostfunctions`].
. PRs that break the external API must be tagged with https://github.com/paritytech/substrate/labels/E5-breaksapi[`E5-breaksapi`].
@@ -88,8 +88,7 @@ To create a Polkadot companion PR:
- The bot will push a commit to the Polkadot PR updating its Substrate reference.
- If the polkadot PR origins from a fork then a project member may need to press `approve run` on the polkadot PR.
- The bot will merge the Polkadot PR once all its CI `{"build_allow_failure":false}` checks are green.
-
- Note: The merge-bot currently doesn't work with forks on org accounts, only individual accounts.
+ Note: The merge-bot currently doesn't work with forks on org accounts, only individual accounts.
If your PR is reviewed well, but a Polkadot PR is missing, signal it with https://github.com/paritytech/substrate/labels/A7-needspolkadotpr[`A7-needspolkadotpr`] to prevent it from getting automatically merged.
diff --git a/substrate/utils/frame/try-runtime/cli/src/lib.rs b/substrate/utils/frame/try-runtime/cli/src/lib.rs
index e7407f2f408..d5ccca95602 100644
--- a/substrate/utils/frame/try-runtime/cli/src/lib.rs
+++ b/substrate/utils/frame/try-runtime/cli/src/lib.rs
@@ -29,9 +29,9 @@
//!
//! Some resources about the above:
//!
-//! 1. https://substrate.dev/docs/en/knowledgebase/integrate/try-runtime
-//! 2. https://www.crowdcast.io/e/substrate-seminar/41
-//! 3. https://substrate.dev/docs/en/knowledgebase/advanced/executor
+//! 1. <https://substrate.dev/docs/en/knowledgebase/integrate/try-runtime>
+//! 2. <https://www.crowdcast.io/e/substrate-seminar/41>
+//! 3. <https://substrate.dev/docs/en/knowledgebase/advanced/executor>
//!
//! ---
//!
@@ -117,6 +117,44 @@
//!
//! Note that *none* of the try-runtime operations need unsafe RPCs.
//!
+//! ## Migration Best Practices
+//!
+//! One of the main use-cases of try-runtime is using it for testing storage migrations. The
+//! following points makes sure you can *effectively* test your migrations with try-runtime.
+//!
+//! #### Adding pre/post hooks
+//!
+//! One of the gems that come only in the `try-runtime` feature flag is the `pre_upgrade` and
+//! `post_upgrade` hooks for [`OnRuntimeUpgrade`]. This trait is implemented either inside the
+//! pallet, or manually in a runtime, to define a migration. In both cases, these functions can be
+//! added, given the right flag:
+//!
+//! ```ignore
+//! #[cfg(feature = try-runtime)]
+//! fn pre_upgrade() -> Result<(), &'static str> {}
+//!
+//! #[cfg(feature = try-runtime)]
+//! fn post_upgrade() -> Result<(), &'static str> {}
+//! ```
+//!
+//! (The pallet macro syntax will support this simply as a part of `#[pallet::hooks]`).
+//!
+//! These hooks allow you to execute some code, only within the `on-runtime-upgrade` command, before
+//! and after the migration. If any data needs to be temporarily stored between the pre/post
+//! migration hooks, [`OnRuntimeUpgradeHelpersExt`] can help with that.
+//!
+//! #### Logging
+//!
+//! It is super helpful to make sure your migration code uses logging (always with a `runtime` log
+//! target prefix, e.g. `runtime::balance`) and state exactly at which stage it is, and what it is
+//! doing.
+//!
+//! #### Guarding migrations
+//!
+//! Always make sure that any migration code is guarded either by [`StorageVersion`], or by some
+//! custom storage item, so that it is NEVER executed twice, even if the code lives in two
+//! consecutive runtimes.
+//!
//! ## Examples
//!
//! Run the migrations of the local runtime on the state of polkadot, from the polkadot repo where
--
GitLab