Skip to content

Commit

Permalink
Backport 4229 Into v1.7.0 (#4288)
Browse files Browse the repository at this point in the history
Backport of #4229 into the v1.7.0 release in order to support dependent
runtimes. This version of the SDK doesn't have
`UncheckedOnRuntimeUpgrade`, so I do it the "old way".
  • Loading branch information
joepetrowski authored and bkchr committed Jun 6, 2024
1 parent 31b1975 commit c00ca35
Show file tree
Hide file tree
Showing 4 changed files with 244 additions and 2 deletions.
3 changes: 2 additions & 1 deletion cumulus/pallets/collator-selection/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ frame-support = { path = "../../../substrate/frame/support", default-features =
frame-system = { path = "../../../substrate/frame/system", default-features = false }
pallet-authorship = { path = "../../../substrate/frame/authorship", default-features = false }
pallet-session = { path = "../../../substrate/frame/session", default-features = false }
pallet-balances = { path = "../../../substrate/frame/balances", default-features = false }

frame-benchmarking = { path = "../../../substrate/frame/benchmarking", default-features = false, optional = true }

Expand All @@ -38,7 +39,6 @@ sp-tracing = { path = "../../../substrate/primitives/tracing" }
sp-runtime = { path = "../../../substrate/primitives/runtime" }
pallet-timestamp = { path = "../../../substrate/frame/timestamp" }
sp-consensus-aura = { path = "../../../substrate/primitives/consensus/aura" }
pallet-balances = { path = "../../../substrate/frame/balances" }
pallet-aura = { path = "../../../substrate/frame/aura" }

[features]
Expand All @@ -59,6 +59,7 @@ std = [
"frame-system/std",
"log/std",
"pallet-authorship/std",
"pallet-balances/std",
"pallet-session/std",
"rand/std",
"scale-info/std",
Expand Down
2 changes: 1 addition & 1 deletion cumulus/pallets/collator-selection/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ pub mod pallet {
use sp_std::vec::Vec;

/// The current storage version.
const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);
const STORAGE_VERSION: StorageVersion = StorageVersion::new(2);

type BalanceOf<T> =
<<T as Config>::Currency as Currency<<T as SystemConfig>::AccountId>>::Balance;
Expand Down
231 changes: 231 additions & 0 deletions cumulus/pallets/collator-selection/src/migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,104 @@ use super::*;
use frame_support::traits::OnRuntimeUpgrade;
use log;

/// Migrate to v2. Should have been part of <https://github.com/paritytech/polkadot-sdk/pull/1340>.
pub mod v2 {
use super::*;
use frame_support::{
pallet_prelude::*,
storage_alias,
traits::{Currency, ReservableCurrency},
};
use sp_runtime::traits::{Saturating, Zero};
#[cfg(feature = "try-runtime")]
use sp_std::vec::Vec;

#[storage_alias]
pub type Candidates<T: Config> = StorageValue<
Pallet<T>,
BoundedVec<CandidateInfo<<T as frame_system::Config>::AccountId, <<T as Config>::Currency as Currency<<T as frame_system::Config>::AccountId>>::Balance>, <T as Config>::MaxCandidates>,
ValueQuery,
>;

/// Migrate to V2.
pub struct MigrationToV2<T>(sp_std::marker::PhantomData<T>);
impl<T: Config + pallet_balances::Config> OnRuntimeUpgrade for MigrationToV2<T> {
fn on_runtime_upgrade() -> Weight {
let on_chain_version = Pallet::<T>::on_chain_storage_version();
if on_chain_version == 1 {
let mut weight = Weight::zero();
let mut count: u64 = 0;
// candidates who exist under the old `Candidates` key
let candidates = Candidates::<T>::take();

// New candidates who have registered since the upgrade. Under normal circumstances,
// this should not exist because the migration should be applied when the upgrade
// happens. But in Polkadot/Kusama we messed this up, and people registered under
// `CandidateList` while their funds were locked in `Candidates`.
let new_candidate_list = CandidateList::<T>::get();
if new_candidate_list.len().is_zero() {
// The new list is empty, so this is essentially being applied correctly. We
// just put the candidates into the new storage item.
CandidateList::<T>::put(&candidates);
// 1 write for the new list
weight.saturating_accrue(T::DbWeight::get().reads_writes(0, 1));
} else {
// Oops, the runtime upgraded without the migration. There are new candidates in
// `CandidateList`. So, let's just refund the old ones and assume they have
// already started participating in the new system.
for candidate in candidates {
let err = T::Currency::unreserve(&candidate.who, candidate.deposit);
if err > Zero::zero() {
log::error!(
target: LOG_TARGET,
"{:?} balance was unable to be unreserved from {:?}",
err, &candidate.who,
);
}
count.saturating_inc();
}
weight.saturating_accrue(
<<T as pallet_balances::Config>::WeightInfo as pallet_balances::WeightInfo>::force_unreserve().saturating_mul(count.into()),
);
}

StorageVersion::new(2).put::<Pallet<T>>();

log::info!(
target: LOG_TARGET,
"Unreserved locked bond of {} candidates, upgraded storage to version 2",
count,
);

weight.saturating_accrue(T::DbWeight::get().reads_writes(3, 2));
weight
} else {
log::info!(
target: LOG_TARGET,
"Migration did not execute. This probably should be removed"
);
T::DbWeight::get().reads(1)
}
}

#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, sp_runtime::DispatchError> {
let number_of_candidates = Candidates::<T>::get().to_vec().len();
Ok((number_of_candidates as u32).encode())
}

#[cfg(feature = "try-runtime")]
fn post_upgrade(_number_of_candidates: Vec<u8>) -> Result<(), sp_runtime::DispatchError> {
let new_number_of_candidates = Candidates::<T>::get().to_vec().len();
assert_eq!(
new_number_of_candidates, 0 as usize,
"after migration, the candidates map should be empty"
);
Ok(())
}
}
}

/// Version 1 Migration
/// This migration ensures that any existing `Invulnerables` storage lists are sorted.
pub mod v1 {
Expand Down Expand Up @@ -90,3 +188,136 @@ pub mod v1 {
}
}
}

#[cfg(all(feature = "try-runtime", test))]
mod tests {
use super::*;
use crate::{
migration::v2::Candidates,
mock::{new_test_ext, Balances, Test},
};
use frame_support::{
traits::{Currency, ReservableCurrency, StorageVersion},
BoundedVec,
};
use sp_runtime::traits::ConstU32;

#[test]
fn migrate_to_v2_with_new_candidates() {
new_test_ext().execute_with(|| {
let storage_version = StorageVersion::new(1);
storage_version.put::<Pallet<Test>>();

let one = 1u64;
let two = 2u64;
let three = 3u64;
let deposit = 10u64;

// Set balance to 100
Balances::make_free_balance_be(&one, 100u64);
Balances::make_free_balance_be(&two, 100u64);
Balances::make_free_balance_be(&three, 100u64);

// Reservations: 10 for the "old" candidacy and 10 for the "new"
Balances::reserve(&one, 10u64).unwrap(); // old
Balances::reserve(&two, 20u64).unwrap(); // old + new
Balances::reserve(&three, 10u64).unwrap(); // new

// Candidate info
let candidate_one = CandidateInfo { who: one, deposit };
let candidate_two = CandidateInfo { who: two, deposit };
let candidate_three = CandidateInfo { who: three, deposit };

// Storage lists
let bounded_candidates =
BoundedVec::<CandidateInfo<u64, u64>, ConstU32<20>>::try_from(vec![
candidate_one.clone(),
candidate_two.clone(),
])
.expect("it works");
let bounded_candidate_list =
BoundedVec::<CandidateInfo<u64, u64>, ConstU32<20>>::try_from(vec![
candidate_two.clone(),
candidate_three.clone(),
])
.expect("it works");

// Set storage
Candidates::<Test>::put(bounded_candidates);
CandidateList::<Test>::put(bounded_candidate_list.clone());

// Sanity check
assert_eq!(Balances::free_balance(one), 90);
assert_eq!(Balances::free_balance(two), 80);
assert_eq!(Balances::free_balance(three), 90);

// Run migration
v2::MigrationToV2::<Test>::on_runtime_upgrade();

let new_storage_version = StorageVersion::get::<Pallet<Test>>();
assert_eq!(new_storage_version, 2);

// 10 should have been unreserved from the old candidacy
assert_eq!(Balances::free_balance(one), 100);
assert_eq!(Balances::free_balance(two), 90);
assert_eq!(Balances::free_balance(three), 90);
// The storage item should be gone
assert!(Candidates::<Test>::get().is_empty());
// The new storage item should be preserved
assert_eq!(CandidateList::<Test>::get(), bounded_candidate_list);
});
}

#[test]
fn migrate_to_v2_without_new_candidates() {
new_test_ext().execute_with(|| {
let storage_version = StorageVersion::new(1);
storage_version.put::<Pallet<Test>>();

let one = 1u64;
let two = 2u64;
let deposit = 10u64;

// Set balance to 100
Balances::make_free_balance_be(&one, 100u64);
Balances::make_free_balance_be(&two, 100u64);

// Reservations
Balances::reserve(&one, 10u64).unwrap(); // old
Balances::reserve(&two, 10u64).unwrap(); // old

// Candidate info
let candidate_one = CandidateInfo { who: one, deposit };
let candidate_two = CandidateInfo { who: two, deposit };

// Storage lists
let bounded_candidates =
BoundedVec::<CandidateInfo<u64, u64>, ConstU32<20>>::try_from(vec![
candidate_one.clone(),
candidate_two.clone(),
])
.expect("it works");

// Set storage
Candidates::<Test>::put(bounded_candidates.clone());

// Sanity check
assert_eq!(Balances::free_balance(one), 90);
assert_eq!(Balances::free_balance(two), 90);

// Run migration
v2::MigrationToV2::<Test>::on_runtime_upgrade();

let new_storage_version = StorageVersion::get::<Pallet<Test>>();
assert_eq!(new_storage_version, 2);

// Nothing changes deposit-wise
assert_eq!(Balances::free_balance(one), 90);
assert_eq!(Balances::free_balance(two), 90);
// The storage item should be gone
assert!(Candidates::<Test>::get().is_empty());
// The new storage item should have the info now
assert_eq!(CandidateList::<Test>::get(), bounded_candidates);
});
}
}
10 changes: 10 additions & 0 deletions prdoc/1.7.0/pr_4288.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
title: "Fix Stuck Collator Funds"

doc:
- audience: Runtime Dev
description: |
Fixes stuck collator funds by providing a migration that should have been in PR 1340.

crates:
- name: pallet-collator-selection
bump: patch

0 comments on commit c00ca35

Please sign in to comment.