Skip to content

Commit 631de29

Browse files
joepetrowskigeorgepisaltuggwpez
authored andcommitted
Fix Stuck Collator Funds (#4229)
Fixes #4206 In #1340 one of the storage types was changed from `Candidates` to `CandidateList`. Since the actual key includes the hash of this value, all of the candidates stored here are (a) "missing" and (b) unable to unreserve their candidacy bond. This migration kills the storage values and refunds the deposit held for each candidate. --------- Signed-off-by: georgepisaltu <[email protected]> Co-authored-by: georgepisaltu <[email protected]> Co-authored-by: Oliver Tale-Yazdi <[email protected]> Co-authored-by: georgepisaltu <[email protected]>
1 parent 34a81d7 commit 631de29

File tree

14 files changed

+394
-18
lines changed

14 files changed

+394
-18
lines changed

cumulus/pallets/collator-selection/Cargo.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ sp-staking = { path = "../../../substrate/primitives/staking", default-features
2424
frame-support = { path = "../../../substrate/frame/support", default-features = false, version = "26.0.0" }
2525
frame-system = { path = "../../../substrate/frame/system", default-features = false, version = "26.0.0" }
2626
pallet-authorship = { path = "../../../substrate/frame/authorship", default-features = false, version = "26.0.0" }
27+
pallet-balances = { path = "../../../substrate/frame/balances", default-features = false, version = "26.0.0" }
2728
pallet-session = { path = "../../../substrate/frame/session", default-features = false, version = "26.0.0" }
2829

2930
frame-benchmarking = { path = "../../../substrate/frame/benchmarking", default-features = false, optional = true, version = "26.0.0" }
@@ -35,7 +36,6 @@ sp-tracing = { path = "../../../substrate/primitives/tracing" }
3536
sp-runtime = { path = "../../../substrate/primitives/runtime" }
3637
pallet-timestamp = { path = "../../../substrate/frame/timestamp" }
3738
sp-consensus-aura = { path = "../../../substrate/primitives/consensus/aura" }
38-
pallet-balances = { path = "../../../substrate/frame/balances" }
3939
pallet-aura = { path = "../../../substrate/frame/aura" }
4040

4141
[features]
@@ -54,6 +54,7 @@ std = [
5454
"frame-system/std",
5555
"log/std",
5656
"pallet-authorship/std",
57+
"pallet-balances/std",
5758
"pallet-session/std",
5859
"rand/std",
5960
"scale-info/std",

cumulus/pallets/collator-selection/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,8 @@ pub mod pallet {
118118
use sp_staking::SessionIndex;
119119
use sp_std::vec::Vec;
120120

121-
/// The current storage version.
122-
const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);
121+
/// The in-code storage version.
122+
const STORAGE_VERSION: StorageVersion = StorageVersion::new(2);
123123

124124
type BalanceOf<T> =
125125
<<T as Config>::Currency as Currency<<T as SystemConfig>::AccountId>>::Balance;

cumulus/pallets/collator-selection/src/migration.rs

Lines changed: 232 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,107 @@
1717
//! A module that is responsible for migration of storage for Collator Selection.
1818
1919
use super::*;
20-
use frame_support::traits::OnRuntimeUpgrade;
20+
use frame_support::traits::{OnRuntimeUpgrade, UncheckedOnRuntimeUpgrade};
2121
use log;
2222

23+
/// Migrate to v2. Should have been part of <https://github.com/paritytech/polkadot-sdk/pull/1340>.
24+
pub mod v2 {
25+
use super::*;
26+
use frame_support::{
27+
pallet_prelude::*,
28+
storage_alias,
29+
traits::{Currency, ReservableCurrency},
30+
};
31+
use sp_runtime::traits::{Saturating, Zero};
32+
#[cfg(feature = "try-runtime")]
33+
use sp_std::vec::Vec;
34+
35+
/// [`UncheckedMigrationToV2`] wrapped in a
36+
/// [`VersionedMigration`](frame_support::migrations::VersionedMigration), ensuring the
37+
/// migration is only performed when on-chain version is 1.
38+
pub type MigrationToV2<T> = frame_support::migrations::VersionedMigration<
39+
1,
40+
2,
41+
UncheckedMigrationToV2<T>,
42+
Pallet<T>,
43+
<T as frame_system::Config>::DbWeight,
44+
>;
45+
46+
#[storage_alias]
47+
pub type Candidates<T: Config> = StorageValue<
48+
Pallet<T>,
49+
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>,
50+
ValueQuery,
51+
>;
52+
53+
/// Migrate to V2.
54+
pub struct UncheckedMigrationToV2<T>(sp_std::marker::PhantomData<T>);
55+
impl<T: Config + pallet_balances::Config> UncheckedOnRuntimeUpgrade for UncheckedMigrationToV2<T> {
56+
fn on_runtime_upgrade() -> Weight {
57+
let mut weight = Weight::zero();
58+
let mut count: u64 = 0;
59+
// candidates who exist under the old `Candidates` key
60+
let candidates = Candidates::<T>::take();
61+
62+
// New candidates who have registered since the upgrade. Under normal circumstances,
63+
// this should not exist because the migration should be applied when the upgrade
64+
// happens. But in Polkadot/Kusama we messed this up, and people registered under
65+
// `CandidateList` while their funds were locked in `Candidates`.
66+
let new_candidate_list = CandidateList::<T>::get();
67+
if new_candidate_list.len().is_zero() {
68+
// The new list is empty, so this is essentially being applied correctly. We just
69+
// put the candidates into the new storage item.
70+
CandidateList::<T>::put(&candidates);
71+
// 1 write for the new list
72+
weight.saturating_accrue(T::DbWeight::get().reads_writes(0, 1));
73+
} else {
74+
// Oops, the runtime upgraded without the migration. There are new candidates in
75+
// `CandidateList`. So, let's just refund the old ones and assume they have already
76+
// started participating in the new system.
77+
for candidate in candidates {
78+
let err = T::Currency::unreserve(&candidate.who, candidate.deposit);
79+
if err > Zero::zero() {
80+
log::error!(
81+
target: LOG_TARGET,
82+
"{:?} balance was unable to be unreserved from {:?}",
83+
err, &candidate.who,
84+
);
85+
}
86+
count.saturating_inc();
87+
}
88+
weight.saturating_accrue(
89+
<<T as pallet_balances::Config>::WeightInfo as pallet_balances::WeightInfo>::force_unreserve().saturating_mul(count.into()),
90+
);
91+
}
92+
93+
log::info!(
94+
target: LOG_TARGET,
95+
"Unreserved locked bond of {} candidates, upgraded storage to version 2",
96+
count,
97+
);
98+
99+
weight.saturating_accrue(T::DbWeight::get().reads_writes(3, 2));
100+
weight
101+
}
102+
103+
#[cfg(feature = "try-runtime")]
104+
fn pre_upgrade() -> Result<Vec<u8>, sp_runtime::DispatchError> {
105+
let number_of_candidates = Candidates::<T>::get().to_vec().len();
106+
Ok((number_of_candidates as u32).encode())
107+
}
108+
109+
#[cfg(feature = "try-runtime")]
110+
fn post_upgrade(_number_of_candidates: Vec<u8>) -> Result<(), sp_runtime::DispatchError> {
111+
let new_number_of_candidates = Candidates::<T>::get().to_vec().len();
112+
assert_eq!(
113+
new_number_of_candidates, 0 as usize,
114+
"after migration, the candidates map should be empty"
115+
);
116+
Ok(())
117+
}
118+
}
119+
}
120+
23121
/// Version 1 Migration
24122
/// This migration ensures that any existing `Invulnerables` storage lists are sorted.
25123
pub mod v1 {
@@ -90,3 +188,136 @@ pub mod v1 {
90188
}
91189
}
92190
}
191+
192+
#[cfg(all(feature = "try-runtime", test))]
193+
mod tests {
194+
use super::*;
195+
use crate::{
196+
migration::v2::Candidates,
197+
mock::{new_test_ext, Balances, Test},
198+
};
199+
use frame_support::{
200+
traits::{Currency, ReservableCurrency, StorageVersion},
201+
BoundedVec,
202+
};
203+
use sp_runtime::traits::ConstU32;
204+
205+
#[test]
206+
fn migrate_to_v2_with_new_candidates() {
207+
new_test_ext().execute_with(|| {
208+
let storage_version = StorageVersion::new(1);
209+
storage_version.put::<Pallet<Test>>();
210+
211+
let one = 1u64;
212+
let two = 2u64;
213+
let three = 3u64;
214+
let deposit = 10u64;
215+
216+
// Set balance to 100
217+
Balances::make_free_balance_be(&one, 100u64);
218+
Balances::make_free_balance_be(&two, 100u64);
219+
Balances::make_free_balance_be(&three, 100u64);
220+
221+
// Reservations: 10 for the "old" candidacy and 10 for the "new"
222+
Balances::reserve(&one, 10u64).unwrap(); // old
223+
Balances::reserve(&two, 20u64).unwrap(); // old + new
224+
Balances::reserve(&three, 10u64).unwrap(); // new
225+
226+
// Candidate info
227+
let candidate_one = CandidateInfo { who: one, deposit };
228+
let candidate_two = CandidateInfo { who: two, deposit };
229+
let candidate_three = CandidateInfo { who: three, deposit };
230+
231+
// Storage lists
232+
let bounded_candidates =
233+
BoundedVec::<CandidateInfo<u64, u64>, ConstU32<20>>::try_from(vec![
234+
candidate_one.clone(),
235+
candidate_two.clone(),
236+
])
237+
.expect("it works");
238+
let bounded_candidate_list =
239+
BoundedVec::<CandidateInfo<u64, u64>, ConstU32<20>>::try_from(vec![
240+
candidate_two.clone(),
241+
candidate_three.clone(),
242+
])
243+
.expect("it works");
244+
245+
// Set storage
246+
Candidates::<Test>::put(bounded_candidates);
247+
CandidateList::<Test>::put(bounded_candidate_list.clone());
248+
249+
// Sanity check
250+
assert_eq!(Balances::free_balance(one), 90);
251+
assert_eq!(Balances::free_balance(two), 80);
252+
assert_eq!(Balances::free_balance(three), 90);
253+
254+
// Run migration
255+
v2::MigrationToV2::<Test>::on_runtime_upgrade();
256+
257+
let new_storage_version = StorageVersion::get::<Pallet<Test>>();
258+
assert_eq!(new_storage_version, 2);
259+
260+
// 10 should have been unreserved from the old candidacy
261+
assert_eq!(Balances::free_balance(one), 100);
262+
assert_eq!(Balances::free_balance(two), 90);
263+
assert_eq!(Balances::free_balance(three), 90);
264+
// The storage item should be gone
265+
assert!(Candidates::<Test>::get().is_empty());
266+
// The new storage item should be preserved
267+
assert_eq!(CandidateList::<Test>::get(), bounded_candidate_list);
268+
});
269+
}
270+
271+
#[test]
272+
fn migrate_to_v2_without_new_candidates() {
273+
new_test_ext().execute_with(|| {
274+
let storage_version = StorageVersion::new(1);
275+
storage_version.put::<Pallet<Test>>();
276+
277+
let one = 1u64;
278+
let two = 2u64;
279+
let deposit = 10u64;
280+
281+
// Set balance to 100
282+
Balances::make_free_balance_be(&one, 100u64);
283+
Balances::make_free_balance_be(&two, 100u64);
284+
285+
// Reservations
286+
Balances::reserve(&one, 10u64).unwrap(); // old
287+
Balances::reserve(&two, 10u64).unwrap(); // old
288+
289+
// Candidate info
290+
let candidate_one = CandidateInfo { who: one, deposit };
291+
let candidate_two = CandidateInfo { who: two, deposit };
292+
293+
// Storage lists
294+
let bounded_candidates =
295+
BoundedVec::<CandidateInfo<u64, u64>, ConstU32<20>>::try_from(vec![
296+
candidate_one.clone(),
297+
candidate_two.clone(),
298+
])
299+
.expect("it works");
300+
301+
// Set storage
302+
Candidates::<Test>::put(bounded_candidates.clone());
303+
304+
// Sanity check
305+
assert_eq!(Balances::free_balance(one), 90);
306+
assert_eq!(Balances::free_balance(two), 90);
307+
308+
// Run migration
309+
v2::MigrationToV2::<Test>::on_runtime_upgrade();
310+
311+
let new_storage_version = StorageVersion::get::<Pallet<Test>>();
312+
assert_eq!(new_storage_version, 2);
313+
314+
// Nothing changes deposit-wise
315+
assert_eq!(Balances::free_balance(one), 90);
316+
assert_eq!(Balances::free_balance(two), 90);
317+
// The storage item should be gone
318+
assert!(Candidates::<Test>::get().is_empty());
319+
// The new storage item should have the info now
320+
assert_eq!(CandidateList::<Test>::get(), bounded_candidates);
321+
});
322+
}
323+
}

cumulus/parachain-template/runtime/src/lib.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,13 @@ pub type SignedExtra = (
115115
pub type UncheckedExtrinsic =
116116
generic::UncheckedExtrinsic<Address, RuntimeCall, Signature, SignedExtra>;
117117

118+
/// Migrations to apply on runtime upgrade.
119+
pub type Migrations = (
120+
pallet_collator_selection::migration::v2::MigrationToV2<Runtime>,
121+
// permanent
122+
pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
123+
);
124+
118125
/// Executive: handles dispatch to the various modules.
119126
pub type Executive = frame_executive::Executive<
120127
Runtime,

cumulus/parachains/runtimes/assets/asset-hub-rococo/src/lib.rs

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -959,7 +959,67 @@ pub type SignedExtra = (
959959
pub type UncheckedExtrinsic =
960960
generic::UncheckedExtrinsic<Address, RuntimeCall, Signature, SignedExtra>;
961961
/// Migrations to apply on runtime upgrade.
962-
pub type Migrations = (pallet_collator_selection::migration::v1::MigrateToV1<Runtime>,);
962+
#[allow(deprecated)]
963+
pub type Migrations = (
964+
InitStorageVersions,
965+
// unreleased
966+
cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4<Runtime>,
967+
pallet_collator_selection::migration::v2::MigrationToV2<Runtime>,
968+
// permanent
969+
pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
970+
);
971+
972+
/// Migration to initialize storage versions for pallets added after genesis.
973+
///
974+
/// This is now done automatically (see <https://github.com/paritytech/polkadot-sdk/pull/1297>),
975+
/// but some pallets had made it in and had storage set in them for this parachain before it was
976+
/// merged.
977+
pub struct InitStorageVersions;
978+
979+
impl frame_support::traits::OnRuntimeUpgrade for InitStorageVersions {
980+
fn on_runtime_upgrade() -> Weight {
981+
use frame_support::traits::{GetStorageVersion, StorageVersion};
982+
983+
let mut writes = 0;
984+
985+
if PolkadotXcm::on_chain_storage_version() == StorageVersion::new(0) {
986+
PolkadotXcm::in_code_storage_version().put::<PolkadotXcm>();
987+
writes.saturating_inc();
988+
}
989+
990+
if Multisig::on_chain_storage_version() == StorageVersion::new(0) {
991+
Multisig::in_code_storage_version().put::<Multisig>();
992+
writes.saturating_inc();
993+
}
994+
995+
if Assets::on_chain_storage_version() == StorageVersion::new(0) {
996+
Assets::in_code_storage_version().put::<Assets>();
997+
writes.saturating_inc();
998+
}
999+
1000+
if Uniques::on_chain_storage_version() == StorageVersion::new(0) {
1001+
Uniques::in_code_storage_version().put::<Uniques>();
1002+
writes.saturating_inc();
1003+
}
1004+
1005+
if Nfts::on_chain_storage_version() == StorageVersion::new(0) {
1006+
Nfts::in_code_storage_version().put::<Nfts>();
1007+
writes.saturating_inc();
1008+
}
1009+
1010+
if ForeignAssets::on_chain_storage_version() == StorageVersion::new(0) {
1011+
ForeignAssets::in_code_storage_version().put::<ForeignAssets>();
1012+
writes.saturating_inc();
1013+
}
1014+
1015+
if PoolAssets::on_chain_storage_version() == StorageVersion::new(0) {
1016+
PoolAssets::in_code_storage_version().put::<PoolAssets>();
1017+
writes.saturating_inc();
1018+
}
1019+
1020+
<Runtime as frame_system::Config>::DbWeight::get().reads_writes(7, writes)
1021+
}
1022+
}
9631023

9641024
/// Executive: handles dispatch to the various modules.
9651025
pub type Executive = frame_executive::Executive<

cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -939,7 +939,7 @@ pub type Migrations = (
939939
// v9420
940940
pallet_nfts::migration::v1::MigrateToV1<Runtime>,
941941
// unreleased
942-
pallet_collator_selection::migration::v1::MigrateToV1<Runtime>,
942+
pallet_collator_selection::migration::v2::MigrationToV2<Runtime>,
943943
// unreleased
944944
migrations::NativeAssetParents0ToParents1Migration<Runtime>,
945945
// unreleased

cumulus/parachains/runtimes/bridge-hubs/bridge-hub-kusama/src/lib.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,11 @@ pub type UncheckedExtrinsic =
110110

111111
/// Migrations to apply on runtime upgrade.
112112
pub type Migrations = (
113-
// unreleased
114-
pallet_collator_selection::migration::v1::MigrateToV1<Runtime>,
113+
pallet_collator_selection::migration::v2::MigrationToV2<Runtime>,
114+
cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4<Runtime>,
115+
pallet_broker::migration::MigrateV0ToV1<Runtime>,
116+
// permanent
117+
pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
115118
);
116119

117120
/// Executive: handles dispatch to the various modules.

cumulus/parachains/runtimes/bridge-hubs/bridge-hub-polkadot/src/lib.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,9 @@ pub type UncheckedExtrinsic =
110110

111111
/// Migrations to apply on runtime upgrade.
112112
pub type Migrations = (
113-
// unreleased
114-
pallet_collator_selection::migration::v1::MigrateToV1<Runtime>,
113+
pallet_collator_selection::migration::v2::MigrationToV2<Runtime>,
114+
// permanent
115+
pallet_xcm::migration::MigrateToLatestXcmVersion<Runtime>,
115116
);
116117

117118
/// Executive: handles dispatch to the various modules.

0 commit comments

Comments
 (0)