Skip to content

Commit 9547a52

Browse files
sam0x17sam0x17
authored
Merge pull request #343 from opentensor/fix/try-runtime
fix: try-runtime
2 parents 9051fed + a2110af commit 9547a52

File tree

7 files changed

+170
-37
lines changed

7 files changed

+170
-37
lines changed

‎.github/workflows/check-rust.yml‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,3 +352,54 @@ jobs:
352352

353353
- name: Check features
354354
run: zepter run check
355+
356+
check-finney-migrations:
357+
name: check finney migrations
358+
runs-on: ubuntu-22.04
359+
steps:
360+
- name: Checkout sources
361+
uses: actions/checkout@v3
362+
363+
- name: Run Try Runtime Checks
364+
uses: "paritytech/[email protected]"
365+
with:
366+
runtime-package: "node-subtensor-runtime"
367+
node-uri: "wss://entrypoint-finney.opentensor.ai:443"
368+
checks: "pre-and-post"
369+
extra-args: "--disable-spec-version-check --no-weight-warnings --disable-idempotency-checks"
370+
371+
# ----
372+
# We can enable devnet and finney migrations once Polkadot v1.0 is deployed to finney, after
373+
# which time all future migrations should be idempotent and won't start failing after the
374+
# upgrade is deployed.
375+
# ----
376+
# check-devnet-migrations:
377+
# name: check devnet migrations
378+
# runs-on: ubuntu-22.04
379+
# steps:
380+
# - name: Checkout sources
381+
# uses: actions/checkout@v3
382+
#
383+
# - name: Run Try Runtime Checks
384+
# uses: "paritytech/[email protected]"
385+
# with:
386+
# runtime-package: "node-subtensor-runtime"
387+
# node-uri: "wss://dev.chain.opentensor.ai:443"
388+
# checks: "pre-and-post"
389+
# extra-args: "--disable-spec-version-check --no-weight-warnings --disable-idempotency-checks"
390+
#
391+
# check-testnet-migrations:
392+
# name: check testnet migrations
393+
# runs-on: ubuntu-22.04
394+
# steps:
395+
# - name: Checkout sources
396+
# uses: actions/checkout@v3
397+
#
398+
# - name: Run Try Runtime Checks
399+
# uses: "paritytech/[email protected]"
400+
# with:
401+
# runtime-package: "node-subtensor-runtime"
402+
# node-uri: "wss://test.chain.opentensor.ai:443"
403+
# checks: "pre-and-post"
404+
# extra-args: "--disable-spec-version-check --no-weight-warnings --disable-idempotency-checks"
405+
#

‎Cargo.toml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,8 @@ panic = "unwind"
1313

1414
[profile.test]
1515
opt-level = 3
16+
17+
[profile.production]
18+
inherits = "release"
19+
lto = true
20+
codegen-units = 1

‎pallets/subtensor/src/lib.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ pub mod pallet {
7474

7575
// Tracks version for migrations. Should be monotonic with respect to the
7676
// order of migrations. (i.e. always increasing)
77-
const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);
77+
const STORAGE_VERSION: StorageVersion = StorageVersion::new(6);
7878

7979
#[pallet::pallet]
8080
#[pallet::without_storage_info]

‎runtime/src/lib.rs‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ mod migrations;
1010

1111
use codec::{Decode, Encode, MaxEncodedLen};
1212

13-
use migrations::account_data_migration;
13+
use migrations::{account_data_migration, init_storage_versions};
1414
use pallet_commitments::CanCommit;
1515
use pallet_grandpa::{
1616
fg_primitives, AuthorityId as GrandpaId, AuthorityList as GrandpaAuthorityList,
@@ -1150,7 +1150,12 @@ pub type SignedExtra = (
11501150
pallet_commitments::CommitmentsSignedExtension<Runtime>,
11511151
);
11521152

1153-
type Migrations = account_data_migration::Migration;
1153+
type Migrations = (
1154+
init_storage_versions::Migration,
1155+
account_data_migration::Migration,
1156+
pallet_multisig::migrations::v1::MigrateToV1<Runtime>,
1157+
pallet_preimage::migration::v1::Migration<Runtime>,
1158+
);
11541159

11551160
// Unchecked extrinsic type as expected by this runtime.
11561161
pub type UncheckedExtrinsic =

‎runtime/src/migrations/account_data_migration.rs‎

Lines changed: 79 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ use crate::*;
22
use frame_support::log;
33
use pallet_balances::ExtraFlags;
44

5+
#[cfg(feature = "try-runtime")]
6+
use sp_std::collections::btree_map::BTreeMap;
7+
58
mod prev {
69
use super::*;
710
use frame_support::{pallet_prelude::ValueQuery, storage_alias, Blake2_128Concat};
@@ -37,15 +40,27 @@ mod prev {
3740
>;
3841
}
3942

43+
#[cfg(feature = "try-runtime")]
44+
#[derive(Encode, Decode)]
45+
enum PreUpgradeInfo {
46+
MigrationAlreadyOccured,
47+
MigrationShouldRun(
48+
BTreeMap<AccountId, frame_system::AccountInfo<u32, pallet_balances::AccountData<Balance>>>,
49+
),
50+
}
51+
4052
const TARGET: &'static str = "runtime::account_data_migration";
4153
pub struct Migration;
4254
impl OnRuntimeUpgrade for Migration {
4355
/// Save pre-upgrade account ids to check are decodable post-upgrade.
4456
#[cfg(feature = "try-runtime")]
4557
fn pre_upgrade() -> Result<Vec<u8>, sp_runtime::TryRuntimeError> {
46-
use sp_std::collections::btree_map::BTreeMap;
47-
log::info!(target: TARGET, "pre-upgrade");
58+
// Skip migration if it already took place.
59+
if migration_already_occured() {
60+
return Ok(PreUpgradeInfo::MigrationAlreadyOccured.encode());
61+
}
4862

63+
log::info!(target: TARGET, "pre-upgrade");
4964
// Save the expected post-migration account state.
5065
let mut expected_account: BTreeMap<
5166
AccountId,
@@ -77,11 +92,20 @@ impl OnRuntimeUpgrade for Migration {
7792
expected_account.insert(acc_id, expected_acc);
7893
}
7994

80-
Ok(expected_account.encode())
95+
Ok(PreUpgradeInfo::MigrationShouldRun(expected_account).encode())
8196
}
8297

8398
/// Migrates Account storage to the new format, and calls `ensure_upgraded` for them.
8499
fn on_runtime_upgrade() -> Weight {
100+
// Skip migration if it already took place.
101+
if migration_already_occured() {
102+
log::warn!(
103+
target: TARGET,
104+
"Migration already completed and can be removed.",
105+
);
106+
return <Runtime as frame_system::Config>::DbWeight::get().reads_writes(0u64, 0u64);
107+
}
108+
85109
// Pull the storage in the previous format into memory
86110
let accounts = prev::Account::<Runtime>::iter().collect::<Vec<_>>();
87111
log::info!(target: TARGET, "Migrating {} accounts...", accounts.len());
@@ -119,42 +143,63 @@ impl OnRuntimeUpgrade for Migration {
119143
#[cfg(feature = "try-runtime")]
120144
fn post_upgrade(state: Vec<u8>) -> Result<(), sp_runtime::TryRuntimeError> {
121145
use frame_support::ensure;
122-
use sp_std::collections::btree_map::BTreeMap;
123146

124147
log::info!(target: TARGET, "Running post-upgrade...");
125148

126-
let expected_accounts: BTreeMap<
127-
AccountId,
128-
frame_system::AccountInfo<u32, pallet_balances::AccountData<Balance>>,
129-
> = Decode::decode(&mut &state[..]).expect("decoding state failed");
130-
131-
// Ensure the actual post-migration state matches the expected
132-
for (acc_id, acc) in frame_system::pallet::Account::<Runtime>::iter() {
133-
let expected = expected_accounts.get(&acc_id).expect("account not found");
134-
135-
// New system logic nukes the account if no providers or sufficients.
136-
if acc.providers > 0 || acc.sufficients > 0 {
137-
ensure!(acc.nonce == expected.nonce, "nonce mismatch");
138-
ensure!(acc.consumers == expected.consumers, "consumers mismatch");
139-
ensure!(acc.providers == expected.providers, "providers mismatch");
140-
ensure!(
141-
acc.sufficients == expected.sufficients,
142-
"sufficients mismatch"
143-
);
144-
ensure!(acc.data.free == expected.data.free, "data.free mismatch");
145-
ensure!(
146-
acc.data.reserved == expected.data.reserved,
147-
"data.reserved mismatch"
148-
);
149-
ensure!(
150-
acc.data.frozen == expected.data.frozen,
151-
"data.frozen mismatch"
152-
);
153-
ensure!(acc.data.flags == expected.data.flags, "data.flags mismatch");
149+
let pre_upgrade_data: PreUpgradeInfo =
150+
Decode::decode(&mut &state[..]).expect("decoding state failed");
151+
152+
match pre_upgrade_data {
153+
PreUpgradeInfo::MigrationAlreadyOccured => Ok(()),
154+
PreUpgradeInfo::MigrationShouldRun(expected_accounts) => {
155+
// Ensure the actual post-migration state matches the expected
156+
for (acc_id, acc) in frame_system::pallet::Account::<Runtime>::iter() {
157+
let expected = expected_accounts.get(&acc_id).expect("account not found");
158+
159+
// New system logic nukes the account if no providers or sufficients.
160+
if acc.providers > 0 || acc.sufficients > 0 {
161+
ensure!(acc.nonce == expected.nonce, "nonce mismatch");
162+
ensure!(acc.consumers == expected.consumers, "consumers mismatch");
163+
ensure!(acc.providers == expected.providers, "providers mismatch");
164+
ensure!(
165+
acc.sufficients == expected.sufficients,
166+
"sufficients mismatch"
167+
);
168+
ensure!(acc.data.free == expected.data.free, "data.free mismatch");
169+
ensure!(
170+
acc.data.reserved == expected.data.reserved,
171+
"data.reserved mismatch"
172+
);
173+
ensure!(
174+
acc.data.frozen == expected.data.frozen,
175+
"data.frozen mismatch"
176+
);
177+
ensure!(acc.data.flags == expected.data.flags, "data.flags mismatch");
178+
}
179+
}
180+
181+
log::info!(target: TARGET, "post-upgrade success ✅");
182+
Ok(())
154183
}
155184
}
185+
}
186+
}
156187

157-
log::info!(target: TARGET, "post-upgrade success ✅");
158-
Ok(())
188+
/// Check if the migration already took place. The migration is all-or-nothing, so if one
189+
/// account is migrated we know that the rest also must be migrated.
190+
///
191+
/// We can use the nonce to check if it's been migrated, because it being zero meant that
192+
/// the decode succeeded and this account has been migrated already.
193+
///
194+
/// Performance note: While this may appear poor for performance due to touching all keys,
195+
/// these keys will need to be touched anyway, so it's fine to just touch them loading them into
196+
/// the storage overlay here.
197+
fn migration_already_occured() -> bool {
198+
for (_, acc) in frame_system::pallet::Account::<Runtime>::iter() {
199+
if acc.nonce > 0 {
200+
return true;
201+
};
159202
}
203+
204+
false
160205
}

‎runtime/src/migrations/init_storage_versions.rs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
use crate::*;
2+
3+
/// Init the on-chain storage versions of pallets added to the runtime prior to this being an automatic process.
4+
pub struct Migration;
5+
6+
impl OnRuntimeUpgrade for Migration {
7+
fn on_runtime_upgrade() -> Weight {
8+
use frame_support::traits::GetStorageVersion;
9+
use frame_support::traits::StorageVersion;
10+
11+
if Triumvirate::on_chain_storage_version() == StorageVersion::new(0) {
12+
Triumvirate::current_storage_version().put::<Triumvirate>();
13+
}
14+
if TriumvirateMembers::on_chain_storage_version() == StorageVersion::new(0) {
15+
TriumvirateMembers::current_storage_version().put::<TriumvirateMembers>();
16+
}
17+
if SenateMembers::on_chain_storage_version() == StorageVersion::new(0) {
18+
SenateMembers::current_storage_version().put::<SenateMembers>();
19+
}
20+
if Scheduler::on_chain_storage_version() == StorageVersion::new(0) {
21+
Scheduler::current_storage_version().put::<Scheduler>();
22+
}
23+
24+
<Runtime as frame_system::Config>::DbWeight::get().reads_writes(4, 4)
25+
}
26+
}

‎runtime/src/migrations/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
pub mod account_data_migration;
2+
pub mod init_storage_versions;

0 commit comments

Comments
 (0)