From 3d56dc1215a5d5381364d0d9c82f94bfe03c348f Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 3 Feb 2025 22:47:34 +0100 Subject: [PATCH 1/8] [AHM] Vesting Signed-off-by: Oliver Tale-Yazdi --- Cargo.lock | 2 + pallets/ah-migrator/Cargo.toml | 1 + pallets/ah-migrator/src/lib.rs | 24 ++++++-- pallets/ah-migrator/src/vesting.rs | 61 +++++++++++++++++++ pallets/rc-migrator/Cargo.toml | 1 + pallets/rc-migrator/src/lib.rs | 39 ++++++++++++ pallets/rc-migrator/src/types.rs | 12 ++-- pallets/rc-migrator/src/vesting.rs | 98 ++++++++++++++++++++++++++++++ 8 files changed, 228 insertions(+), 10 deletions(-) create mode 100644 pallets/ah-migrator/src/vesting.rs create mode 100644 pallets/rc-migrator/src/vesting.rs diff --git a/Cargo.lock b/Cargo.lock index e3ebd0a536..af321b6c50 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7662,6 +7662,7 @@ dependencies = [ "pallet-scheduler", "pallet-staking", "pallet-state-trie-migration", + "pallet-vesting", "parity-scale-codec", "polkadot-parachain-primitives", "polkadot-runtime-common", @@ -8893,6 +8894,7 @@ dependencies = [ "pallet-referenda", "pallet-scheduler", "pallet-staking", + "pallet-vesting", "parity-scale-codec", "polkadot-parachain-primitives", "polkadot-runtime-common", diff --git a/pallets/ah-migrator/Cargo.toml b/pallets/ah-migrator/Cargo.toml index c7260084ba..8f70d12423 100644 --- a/pallets/ah-migrator/Cargo.toml +++ b/pallets/ah-migrator/Cargo.toml @@ -25,6 +25,7 @@ pallet-referenda = { workspace = true } pallet-scheduler = { workspace = true } pallet-staking = { workspace = true } pallet-state-trie-migration = { workspace = true } +pallet-vesting = { workspace = true } polkadot-parachain-primitives = { workspace = true } polkadot-runtime-common = { workspace = true } runtime-parachains = { workspace = true } diff --git a/pallets/ah-migrator/src/lib.rs b/pallets/ah-migrator/src/lib.rs index 304d51ad80..7eb283e92c 100644 --- a/pallets/ah-migrator/src/lib.rs +++ b/pallets/ah-migrator/src/lib.rs @@ -41,6 +41,7 @@ pub mod referenda; pub mod scheduler; pub mod staking; pub mod types; +pub mod vesting; pub use pallet::*; @@ -66,6 +67,7 @@ use pallet_rc_migrator::{ fast_unstake::{FastUnstakeMigrator, RcFastUnstakeMessage}, nom_pools::*, }, + vesting::RcVestingSchedule, }; use pallet_referenda::TrackIdOf; use polkadot_runtime_common::claims as pallet_claims; @@ -92,6 +94,7 @@ type RcAccountFor = RcAccount< pub enum PalletEventName { FastUnstake, BagsList, + Vesting, } #[frame_support::pallet(dev_mode)] @@ -113,6 +116,7 @@ pub mod pallet { + pallet_fast_unstake::Config + pallet_bags_list::Config + pallet_scheduler::Config + + pallet_vesting::Config { /// The overarching event type. type RuntimeEvent: From> + IsType<::RuntimeEvent>; @@ -432,6 +436,16 @@ pub mod pallet { } #[pallet::call_index(8)] + pub fn receive_vesting_schedules( + origin: OriginFor, + schedules: Vec>, + ) -> DispatchResult { + ensure_root(origin)?; + + Self::do_receive_vesting_schedules(schedules).map_err(Into::into) + } + + #[pallet::call_index(9)] pub fn receive_fast_unstake_messages( origin: OriginFor, messages: Vec>, @@ -442,7 +456,7 @@ pub mod pallet { } /// Receive referendum counts, deciding counts, votes for the track queue. - #[pallet::call_index(9)] + #[pallet::call_index(10)] pub fn receive_referenda_values( origin: OriginFor, referendum_count: u32, @@ -458,7 +472,7 @@ pub mod pallet { } /// Receive referendums from the Relay Chain. - #[pallet::call_index(10)] + #[pallet::call_index(11)] pub fn receive_referendums( origin: OriginFor, referendums: Vec<(u32, RcReferendumInfoOf)>, @@ -468,7 +482,7 @@ pub mod pallet { Self::do_receive_referendums(referendums).map_err(Into::into) } - #[pallet::call_index(11)] + #[pallet::call_index(12)] pub fn receive_claims( origin: OriginFor, messages: Vec>, @@ -478,7 +492,7 @@ pub mod pallet { Self::do_receive_claims(messages).map_err(Into::into) } - #[pallet::call_index(12)] + #[pallet::call_index(13)] pub fn receive_bags_list_messages( origin: OriginFor, messages: Vec>, @@ -488,7 +502,7 @@ pub mod pallet { Self::do_receive_bags_list_messages(messages).map_err(Into::into) } - #[pallet::call_index(13)] + #[pallet::call_index(14)] pub fn receive_scheduler_messages( origin: OriginFor, messages: Vec>, diff --git a/pallets/ah-migrator/src/vesting.rs b/pallets/ah-migrator/src/vesting.rs new file mode 100644 index 0000000000..7528e2fa14 --- /dev/null +++ b/pallets/ah-migrator/src/vesting.rs @@ -0,0 +1,61 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use super::*; + +impl Pallet { + pub fn do_receive_vesting_schedules( + messages: Vec>, + ) -> Result<(), Error> { + log::info!(target: LOG_TARGET, "Integrating {} vesting schedules", messages.len()); + Self::deposit_event(Event::BatchReceived { + pallet: PalletEventName::Vesting, + count: messages.len() as u32, + }); + let (mut count_good, mut count_bad) = (0, 0); + + for message in messages { + match Self::do_process_vesting_schedule(message) { + Ok(()) => count_good += 1, + Err(e) => { + count_bad += 1; + log::error!(target: LOG_TARGET, "Error while integrating vesting: {:?}", e); + }, + } + } + + Self::deposit_event(Event::BatchProcessed { + pallet: PalletEventName::Vesting, + count_good, + count_bad, + }); + + Ok(()) + } + + pub fn do_process_vesting_schedule(message: RcVestingSchedule) -> Result<(), Error> { + if pallet_vesting::Vesting::::contains_key(&message.who) { + defensive!("Vesting schedule for {:?} already exists", message.who); + return Err(Error::::InsertConflict); + } + + pallet_vesting::Vesting::::insert(&message.who, message.schedules); + log::debug!(target: LOG_TARGET, "Integrated vesting schedule for {:?}", message.who); + + Ok(()) + } +} diff --git a/pallets/rc-migrator/Cargo.toml b/pallets/rc-migrator/Cargo.toml index 1a3804449f..4e899427db 100644 --- a/pallets/rc-migrator/Cargo.toml +++ b/pallets/rc-migrator/Cargo.toml @@ -28,6 +28,7 @@ pallet-multisig = { workspace = true } pallet-preimage = { workspace = true } pallet-fast-unstake = { workspace = true } pallet-referenda = { workspace = true } +pallet-vesting = { workspace = true } pallet-nomination-pools = { workspace = true } polkadot-runtime-common = { workspace = true } runtime-parachains = { workspace = true } diff --git a/pallets/rc-migrator/src/lib.rs b/pallets/rc-migrator/src/lib.rs index 4dc8e7b660..c11a436bb6 100644 --- a/pallets/rc-migrator/src/lib.rs +++ b/pallets/rc-migrator/src/lib.rs @@ -39,6 +39,7 @@ pub mod proxy; pub mod referenda; pub mod staking; pub mod types; +pub mod vesting; mod weights; pub use pallet::*; pub mod scheduler; @@ -77,6 +78,7 @@ use staking::{ }; use storage::TransactionOutcome; use types::{AhWeightInfo, PalletMigration}; +use vesting::VestingMigrator; use weights::WeightInfo; use xcm::prelude::*; @@ -175,6 +177,12 @@ pub enum MigrationStage { }, NomPoolsMigrationDone, + VestingMigrationInit, + VestingMigrationOngoing { + next_key: Option, + }, + VestingMigrationDone, + FastUnstakeMigrationInit, FastUnstakeMigrationOngoing { next_key: Option>, @@ -260,6 +268,7 @@ pub mod pallet { + pallet_fast_unstake::Config + pallet_bags_list::Config + pallet_scheduler::Config + + pallet_vesting::Config { /// The overarching event type. type RuntimeEvent: From> + IsType<::RuntimeEvent>; @@ -655,6 +664,36 @@ pub mod pallet { } }, MigrationStage::NomPoolsMigrationDone => { + Self::transition(MigrationStage::VestingMigrationInit); + }, + + MigrationStage::VestingMigrationInit => { + Self::transition(MigrationStage::VestingMigrationOngoing { next_key: None }); + }, + MigrationStage::VestingMigrationOngoing { next_key } => { + let res = with_transaction_opaque_err::, Error, _>(|| { + match VestingMigrator::::migrate_many(next_key, &mut weight_counter) { + Ok(last_key) => TransactionOutcome::Commit(Ok(last_key)), + Err(e) => TransactionOutcome::Rollback(Err(e)), + } + }) + .expect("Always returning Ok; qed"); + + match res { + Ok(None) => { + Self::transition(MigrationStage::VestingMigrationDone); + }, + Ok(Some(next_key)) => { + Self::transition(MigrationStage::VestingMigrationOngoing { + next_key: Some(next_key), + }); + }, + e => { + defensive!("Error while migrating vesting: {:?}", e); + }, + } + }, + MigrationStage::VestingMigrationDone => { Self::transition(MigrationStage::FastUnstakeMigrationInit); }, MigrationStage::FastUnstakeMigrationInit => { diff --git a/pallets/rc-migrator/src/types.rs b/pallets/rc-migrator/src/types.rs index d2b4f78a58..a4a62cb3bb 100644 --- a/pallets/rc-migrator/src/types.rs +++ b/pallets/rc-migrator/src/types.rs @@ -48,20 +48,22 @@ pub enum AhMigratorCall { #[codec(index = 7)] ReceiveNomPoolsMessages { messages: Vec> }, #[codec(index = 8)] - ReceiveFastUnstakeMessages { messages: Vec> }, + ReceiveVestingSchedules { messages: Vec> }, #[codec(index = 9)] + ReceiveFastUnstakeMessages { messages: Vec> }, + #[codec(index = 10)] ReceiveReferendaValues { referendum_count: u32, deciding_count: Vec<(TrackIdOf, u32)>, track_queue: Vec<(TrackIdOf, Vec<(u32, u128)>)>, }, - #[codec(index = 10)] - ReceiveReferendums { referendums: Vec<(u32, ReferendumInfoOf)> }, #[codec(index = 11)] - ReceiveClaimsMessages { messages: Vec> }, + ReceiveReferendums { referendums: Vec<(u32, ReferendumInfoOf)> }, #[codec(index = 12)] - ReceiveBagsListMessages { messages: Vec> }, + ReceiveClaimsMessages { messages: Vec> }, #[codec(index = 13)] + ReceiveBagsListMessages { messages: Vec> }, + #[codec(index = 14)] ReceiveSchedulerMessages { messages: Vec> }, } diff --git a/pallets/rc-migrator/src/vesting.rs b/pallets/rc-migrator/src/vesting.rs new file mode 100644 index 0000000000..34470ed952 --- /dev/null +++ b/pallets/rc-migrator/src/vesting.rs @@ -0,0 +1,98 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use crate::*; +use frame_support::traits::Currency; +use pallet_vesting::MaxVestingSchedulesGet; + +pub type BalanceOf = <::Currency as Currency< + ::AccountId, +>>::Balance; + +#[derive( + Encode, Decode, CloneNoBound, PartialEqNoBound, EqNoBound, TypeInfo, MaxEncodedLen, DebugNoBound, +)] +#[codec(mel_bound(T: pallet_vesting::Config))] +#[scale_info(skip_type_params(T))] +pub struct RcVestingSchedule { + pub who: ::AccountId, + pub schedules: BoundedVec< + pallet_vesting::VestingInfo, BlockNumberFor>, + MaxVestingSchedulesGet, + >, +} + +pub struct VestingMigrator { + _phantom: PhantomData, +} + +impl PalletMigration for VestingMigrator { + type Key = T::AccountId; + type Error = Error; + + fn migrate_many( + current_key: Option, + weight_counter: &mut WeightMeter, + ) -> Result, Self::Error> { + let mut inner_key = current_key; + let mut messages = Vec::new(); + + loop { + if weight_counter + .try_consume(::DbWeight::get().reads_writes(1, 1)) + .is_err() + { + if messages.is_empty() { + return Err(Error::OutOfWeight); + } else { + break; + } + } + if messages.len() > 10_000 { + log::warn!("Weight allowed very big batch, stopping"); + break; + } + + let mut iter = match inner_key { + Some(who) => pallet_vesting::Vesting::::iter_from( + pallet_vesting::Vesting::::hashed_key_for(who), + ), + None => pallet_vesting::Vesting::::iter(), + }; + + match iter.next() { + Some((who, schedules)) => { + messages.push(RcVestingSchedule { who: who.clone(), schedules }); + log::debug!(target: LOG_TARGET, "Migrating vesting schedules for {:?}", who); + inner_key = Some(who); + }, + None => { + inner_key = None; + break; + }, + } + } + + if !messages.is_empty() { + Pallet::::send_chunked_xcm(messages, |messages| { + types::AhMigratorCall::ReceiveVestingSchedules { messages } + })?; + } + + Ok(inner_key) + } +} From c1bd66601feae8b0c8c420909ed6bf5e4a8e64fa Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 3 Feb 2025 22:59:01 +0100 Subject: [PATCH 2/8] handle truncatening Signed-off-by: Oliver Tale-Yazdi --- pallets/ah-migrator/src/vesting.rs | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/pallets/ah-migrator/src/vesting.rs b/pallets/ah-migrator/src/vesting.rs index 7528e2fa14..fde4228a8b 100644 --- a/pallets/ah-migrator/src/vesting.rs +++ b/pallets/ah-migrator/src/vesting.rs @@ -48,13 +48,27 @@ impl Pallet { } pub fn do_process_vesting_schedule(message: RcVestingSchedule) -> Result<(), Error> { - if pallet_vesting::Vesting::::contains_key(&message.who) { - defensive!("Vesting schedule for {:?} already exists", message.who); - return Err(Error::::InsertConflict); + let mut schedules = pallet_vesting::Vesting::::get(&message.who).unwrap_or_default(); + + if !schedules.is_empty() { + log::warn!(target: LOG_TARGET, "Merging with existing vesting schedule for {:?}", message.who); } - pallet_vesting::Vesting::::insert(&message.who, message.schedules); - log::debug!(target: LOG_TARGET, "Integrated vesting schedule for {:?}", message.who); + let all_schedules = + schedules.into_iter().chain(message.schedules.into_iter()).collect::>(); + let bounded_schedule = BoundedVec::<_, _>::truncate_from(all_schedules); + let truncated = all_schedules.len() - bounded_schedule.len(); + + if truncated == 0 { + pallet_vesting::Vesting::::insert(&message.who, bounded_schedule); + log::debug!(target: LOG_TARGET, "Integrated vesting schedule for {:?}", message.who); + } else { + log::error!(target: LOG_TARGET, "Truncated {} vesting schedules for {:?}", truncated, message.who); + + // TODO what do? should we create a storage item and insert the truncated ones? + // Nobody seems to use the Vesting pallet on AH yet, but we cannot be sure that there + // won't be a truncatenation. + } Ok(()) } From d6dd9f4d90f93c9ce858dcdbd29c22e04ce37805 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Mon, 3 Feb 2025 23:16:48 +0100 Subject: [PATCH 3/8] cleanup Signed-off-by: Oliver Tale-Yazdi --- pallets/ah-migrator/src/vesting.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/pallets/ah-migrator/src/vesting.rs b/pallets/ah-migrator/src/vesting.rs index fde4228a8b..99cc0d3213 100644 --- a/pallets/ah-migrator/src/vesting.rs +++ b/pallets/ah-migrator/src/vesting.rs @@ -54,9 +54,12 @@ impl Pallet { log::warn!(target: LOG_TARGET, "Merging with existing vesting schedule for {:?}", message.who); } - let all_schedules = - schedules.into_iter().chain(message.schedules.into_iter()).collect::>(); - let bounded_schedule = BoundedVec::<_, _>::truncate_from(all_schedules); + let all_schedules = schedules + .clone() + .into_iter() + .chain(message.schedules.into_iter()) + .collect::>(); + let bounded_schedule = BoundedVec::<_, _>::truncate_from(all_schedules.clone()); let truncated = all_schedules.len() - bounded_schedule.len(); if truncated == 0 { @@ -64,6 +67,7 @@ impl Pallet { log::debug!(target: LOG_TARGET, "Integrated vesting schedule for {:?}", message.who); } else { log::error!(target: LOG_TARGET, "Truncated {} vesting schedules for {:?}", truncated, message.who); + defensive!("Truncated vesting schedules for {:?}", message.who); // TODO what do? should we create a storage item and insert the truncated ones? // Nobody seems to use the Vesting pallet on AH yet, but we cannot be sure that there From 523d2e9e3cf148ba88341ecda882fc0057e605bc Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 4 Feb 2025 19:52:26 +0100 Subject: [PATCH 4/8] fix Signed-off-by: Oliver Tale-Yazdi --- pallets/ah-migrator/src/lib.rs | 20 ++++++++ pallets/ah-migrator/src/vesting.rs | 81 ++++++++++++++++++++++++------ 2 files changed, 87 insertions(+), 14 deletions(-) diff --git a/pallets/ah-migrator/src/lib.rs b/pallets/ah-migrator/src/lib.rs index 7eb283e92c..944815f30b 100644 --- a/pallets/ah-migrator/src/lib.rs +++ b/pallets/ah-migrator/src/lib.rs @@ -185,6 +185,11 @@ pub mod pallet { FailedToConvertCall, /// Failed to bound a call. FailedToBoundCall, + /// Failed to merge RC and AH vesting schedules. + /// + /// See the Event `FailedToMergeVestingSchedules` for more info. + FailedToMergeVestingSchedules, + Unreachable, } #[pallet::event] @@ -333,6 +338,21 @@ pub mod pallet { /// How many scheduler messages failed to integrate. count_bad: u32, }, + /// Should not happen. Manual intervention by the Fellowship required. + /// + /// Can happen when existing AH and incoming RC vesting schedules have more combined + /// entries than allowed. This triggers the merging logic which has henceforth failed + /// with the given inner pallet-vesting error. + FailedToMergeVestingSchedules { + /// The account that failed to merge the schedules. + who: AccountId32, + /// The first schedule index that failed to merge. + schedule1: u32, + /// The second schedule index that failed to merge. + schedule2: u32, + /// The index of the pallet-vesting error that occurred. + pallet_vesting_error_index: Option, + }, } #[pallet::pallet] diff --git a/pallets/ah-migrator/src/vesting.rs b/pallets/ah-migrator/src/vesting.rs index 99cc0d3213..32a08273ed 100644 --- a/pallets/ah-migrator/src/vesting.rs +++ b/pallets/ah-migrator/src/vesting.rs @@ -47,33 +47,86 @@ impl Pallet { Ok(()) } + /// Integrate vesting schedules. + /// + /// May merges them with pre-existing schedules if there is not enough space. pub fn do_process_vesting_schedule(message: RcVestingSchedule) -> Result<(), Error> { - let mut schedules = pallet_vesting::Vesting::::get(&message.who).unwrap_or_default(); + let ah_schedules = pallet_vesting::Vesting::::get(&message.who).unwrap_or_default(); - if !schedules.is_empty() { + if !ah_schedules.is_empty() { log::warn!(target: LOG_TARGET, "Merging with existing vesting schedule for {:?}", message.who); } - let all_schedules = schedules - .clone() + let all_schedules = ah_schedules .into_iter() .chain(message.schedules.into_iter()) .collect::>(); - let bounded_schedule = BoundedVec::<_, _>::truncate_from(all_schedules.clone()); - let truncated = all_schedules.len() - bounded_schedule.len(); + let (bounded, truncated) = all_schedules + .split_at(T::MAX_VESTING_SCHEDULES.min(all_schedules.len() as u32) as usize); + let bounded_schedules = BoundedVec::<_, _>::truncate_from(bounded.to_vec()); - if truncated == 0 { - pallet_vesting::Vesting::::insert(&message.who, bounded_schedule); + if truncated.is_empty() { + pallet_vesting::Vesting::::insert(&message.who, bounded_schedules); log::debug!(target: LOG_TARGET, "Integrated vesting schedule for {:?}", message.who); - } else { - log::error!(target: LOG_TARGET, "Truncated {} vesting schedules for {:?}", truncated, message.who); - defensive!("Truncated vesting schedules for {:?}", message.who); + return Ok(()); + } + + log::error!(target: LOG_TARGET, "Truncated {} vesting schedules for {:?}", truncated.len(), message.who); + pallet_vesting::Vesting::::insert(&message.who, &bounded_schedules); + + for truncate in truncated { + let len = pallet_vesting::Vesting::::get(&message.who).unwrap_or_default().len(); + let last_index = len.checked_sub(1).ok_or(Error::::Unreachable)? as u32; + let second_last_index = len.checked_sub(2).ok_or(Error::::Unreachable)? as u32; + Self::merge_schedules(message.who.clone(), second_last_index, last_index)?; - // TODO what do? should we create a storage item and insert the truncated ones? - // Nobody seems to use the Vesting pallet on AH yet, but we cannot be sure that there - // won't be a truncatenation. + // Now we have at least one slot free that we can use to insert into: + defensive_assert!( + pallet_vesting::Vesting::::get(&message.who).unwrap_or_default().len() < + bounded_schedules.len() + ); + // Insert the new schedule into the free slot: + let mut schedules = pallet_vesting::Vesting::::get(&message.who).unwrap_or_default(); + sc hedules.try_push(*truncate).map_err(|_| Error::::Unreachable)?; + pallet_vesting::Vesting::::insert(&message.who, &schedules); } Ok(()) } + + /// Merges two vesting schedules. + /// + /// This function makes use of an pallet-vesting call, which is not entirely clean, but our best + /// bet since otherwise it requires big code duplication. However, nobody is currently using the + /// Vesting pallet on AH, and I recon it unlikely that someone would create a lot of vesting + /// schedules just to have them merged (which they can also do on their own). + pub fn merge_schedules( + who: T::AccountId, + schedule1_index: u32, + schedule2_index: u32, + ) -> Result<(), Error> { + // Pretend to be the account: + let origin: ::RuntimeOrigin = + frame_system::RawOrigin::Signed(who.clone()).into(); + + // NOTE: the pallet macro should already add a transaction, but am not 100% sure: + let res = frame_support::storage::transactional::with_storage_layer(|| { + pallet_vesting::Pallet::::merge_schedules(origin, schedule1_index, schedule2_index) + }); + + let Err(err) = res else { + return Ok(()); + }; + + defensive!("Failed to merge vesting schedules: {:?}", err); + // This is an important error, so we emit an event to monitor if it happens in production: + let err_index = err.using_encoded(|e| e.get(0).copied()); + Self::deposit_event(Event::FailedToMergeVestingSchedules { + who, + schedule1: schedule1_index, + schedule2: schedule2_index, + pallet_vesting_error_index: err_index, + }); + Err(Error::::FailedToMergeVestingSchedules) + } } From 9c2aeb225354bbff383b3b39a829604571a71fe1 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 4 Feb 2025 20:49:10 +0100 Subject: [PATCH 5/8] set storage version Signed-off-by: Oliver Tale-Yazdi --- pallets/ah-migrator/src/vesting.rs | 17 ++++++++++++++++- pallets/rc-migrator/src/vesting.rs | 4 +--- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/pallets/ah-migrator/src/vesting.rs b/pallets/ah-migrator/src/vesting.rs index 32a08273ed..5a2be1b786 100644 --- a/pallets/ah-migrator/src/vesting.rs +++ b/pallets/ah-migrator/src/vesting.rs @@ -21,6 +21,7 @@ impl Pallet { pub fn do_receive_vesting_schedules( messages: Vec>, ) -> Result<(), Error> { + alias::StorageVersion::::put(alias::Releases::V1); log::info!(target: LOG_TARGET, "Integrating {} vesting schedules", messages.len()); Self::deposit_event(Event::BatchReceived { pallet: PalletEventName::Vesting, @@ -87,7 +88,7 @@ impl Pallet { ); // Insert the new schedule into the free slot: let mut schedules = pallet_vesting::Vesting::::get(&message.who).unwrap_or_default(); - sc hedules.try_push(*truncate).map_err(|_| Error::::Unreachable)?; + schedules.try_push(*truncate).map_err(|_| Error::::Unreachable)?; pallet_vesting::Vesting::::insert(&message.who, &schedules); } @@ -130,3 +131,17 @@ impl Pallet { Err(Error::::FailedToMergeVestingSchedules) } } + +pub mod alias { + use super::*; + + #[frame_support::storage_alias(pallet_name)] + pub type StorageVersion = StorageValue, Releases, ValueQuery>; + + #[derive(Encode, Decode, Clone, Copy, PartialEq, Eq, RuntimeDebug, MaxEncodedLen, Default, TypeInfo)] + pub enum Releases { + #[default] + V0, + V1, + } +} diff --git a/pallets/rc-migrator/src/vesting.rs b/pallets/rc-migrator/src/vesting.rs index 34470ed952..efe09049ed 100644 --- a/pallets/rc-migrator/src/vesting.rs +++ b/pallets/rc-migrator/src/vesting.rs @@ -68,9 +68,7 @@ impl PalletMigration for VestingMigrator { } let mut iter = match inner_key { - Some(who) => pallet_vesting::Vesting::::iter_from( - pallet_vesting::Vesting::::hashed_key_for(who), - ), + Some(who) => pallet_vesting::Vesting::::iter_from_key(who), None => pallet_vesting::Vesting::::iter(), }; From 2eee99f3a4c1ac87d4ab2159126902e8914d1eb0 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 4 Feb 2025 21:41:31 +0100 Subject: [PATCH 6/8] doc Signed-off-by: Oliver Tale-Yazdi --- pallets/rc-migrator/src/vesting.md | 36 ++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 pallets/rc-migrator/src/vesting.md diff --git a/pallets/rc-migrator/src/vesting.md b/pallets/rc-migrator/src/vesting.md new file mode 100644 index 0000000000..eb6cf87916 --- /dev/null +++ b/pallets/rc-migrator/src/vesting.md @@ -0,0 +1,36 @@ +# Pallet Vesting + +Pallet vesting has one storage map to hold the vesting schedules and one storage value to track the +current version of the pallet. The version can be easily migrated, but for the schedules it is a bit difficult. + +## Storage: Vesting + +The vesting schedules are already measured in relay blocks, as can be seen +[here](https://github.com/polkadot-fellows/runtimes/blob/b613b54d94af5f4702533a56c6260651a14bdccb/system-parachains/asset-hubs/asset-hub-polkadot/src/lib.rs#L297). +This means that we can just integrate the existing schedules. The only possibly issue is when there +are lots of pre-existing schedules. The maximal number of schedules is 28; both on Relay and AH. +We cannot use the merge functionality of the vesting pallet since that can be used as an attack +vector: anyone can send 28 vested transfers with very large unlock duration and low amount to force +all other schedules to adapt this long unlock period. This would reduce the rewards per block, which +is bad. +For now, we are writing all colliding AH schedules into a storage item for manual inspection later. +It could still happen that unmalicious users will have more than 28 schedules, but as nobody has +used the vested transfers on AH yet. + +Q: Maybe we should disable vested transfers with the next runtime upgrade on AH. + +## Storage: StorageVersion + +The vesting pallet is not using the proper FRAME version tracking; rather, it tracks its version in +the `StorageVersion` value. It does this incorrectly though, with Asset Hub reporting version 0 +instead of 1. We ignore and correct this by writing 1 to the storage. + + +## User Impact + +This affects users that have vesting schedules on the Relay chain or on Asset Hub. There exists a +risk that the number of total schedules exceeds 28, which means that they will not fit into the +storage anymore. + +We then prioritize the schedules from AH and pause and stash all schedules that do not fit (up to +28). From 57ad7adc9ca735a3db87c2543885455bc2afdf9d Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 11 Feb 2025 13:29:14 +0100 Subject: [PATCH 7/8] cleanup Signed-off-by: Oliver Tale-Yazdi --- Cargo.lock | 4 +- pallets/ah-migrator/src/lib.rs | 14 +++--- pallets/ah-migrator/src/vesting.rs | 80 ++++-------------------------- pallets/rc-migrator/src/lib.rs | 1 + pallets/rc-migrator/src/types.rs | 8 +-- 5 files changed, 22 insertions(+), 85 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ea1de9ad10..182cbc6898 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7668,8 +7668,8 @@ dependencies = [ "pallet-scheduler", "pallet-staking", "pallet-state-trie-migration", - "pallet-vesting", "pallet-treasury", + "pallet-vesting", "parity-scale-codec", "polkadot-parachain-primitives", "polkadot-runtime-common", @@ -8905,8 +8905,8 @@ dependencies = [ "pallet-referenda", "pallet-scheduler", "pallet-staking", - "pallet-vesting", "pallet-treasury", + "pallet-vesting", "parity-scale-codec", "polkadot-parachain-primitives", "polkadot-runtime-common", diff --git a/pallets/ah-migrator/src/lib.rs b/pallets/ah-migrator/src/lib.rs index c516455245..c56c513bfe 100644 --- a/pallets/ah-migrator/src/lib.rs +++ b/pallets/ah-migrator/src/lib.rs @@ -199,10 +199,8 @@ pub mod pallet { FailedToConvertCall, /// Failed to bound a call. FailedToBoundCall, - /// Failed to merge RC and AH vesting schedules. - /// - /// See the Event `FailedToMergeVestingSchedules` for more info. - FailedToMergeVestingSchedules, + /// Failed to integrate a vesting schedule. + FailedToIntegrateVestingSchedule, Unreachable, } @@ -565,7 +563,7 @@ pub mod pallet { Self::do_receive_scheduler_messages(messages).map_err(Into::into) } - #[pallet::call_index(14)] + #[pallet::call_index(15)] pub fn receive_indices( origin: OriginFor, indices: Vec>, @@ -575,7 +573,7 @@ pub mod pallet { Self::do_receive_indices(indices).map_err(Into::into) } - #[pallet::call_index(15)] + #[pallet::call_index(16)] pub fn receive_conviction_voting_messages( origin: OriginFor, messages: Vec>, @@ -585,7 +583,7 @@ pub mod pallet { Self::do_receive_conviction_voting_messages(messages).map_err(Into::into) } - #[pallet::call_index(16)] + #[pallet::call_index(17)] pub fn receive_bounties_messages( origin: OriginFor, messages: Vec>, @@ -595,7 +593,7 @@ pub mod pallet { Self::do_receive_bounties_messages(messages).map_err(Into::into) } - #[pallet::call_index(17)] + #[pallet::call_index(18)] pub fn receive_asset_rates( origin: OriginFor, rates: Vec<(::AssetKind, FixedU128)>, diff --git a/pallets/ah-migrator/src/vesting.rs b/pallets/ah-migrator/src/vesting.rs index 1dfd7b2ca3..5e77d03024 100644 --- a/pallets/ah-migrator/src/vesting.rs +++ b/pallets/ah-migrator/src/vesting.rs @@ -49,87 +49,25 @@ impl Pallet { } /// Integrate vesting schedules. - /// - /// May merges them with pre-existing schedules if there is not enough space. pub fn do_process_vesting_schedule(message: RcVestingSchedule) -> Result<(), Error> { - let ah_schedules = pallet_vesting::Vesting::::get(&message.who).unwrap_or_default(); + let mut ah_schedules = pallet_vesting::Vesting::::get(&message.who).unwrap_or_default(); if !ah_schedules.is_empty() { - log::warn!(target: LOG_TARGET, "Merging with existing vesting schedule for {:?}", message.who); + defensive!("We disabled vesting, looks like someone used it. Manually verify this and then remove this defensive assert."); } - let all_schedules = ah_schedules - .into_iter() - .chain(message.schedules.into_iter()) - .collect::>(); - let (bounded, truncated) = all_schedules - .split_at(T::MAX_VESTING_SCHEDULES.min(all_schedules.len() as u32) as usize); - let bounded_schedules = BoundedVec::<_, _>::truncate_from(bounded.to_vec()); - - if truncated.is_empty() { - pallet_vesting::Vesting::::insert(&message.who, bounded_schedules); - log::debug!(target: LOG_TARGET, "Integrated vesting schedule for {:?}", message.who); - return Ok(()); + for schedule in message.schedules { + ah_schedules + .try_push(schedule) + .defensive() + .map_err(|_| Error::::FailedToIntegrateVestingSchedule)?; } - log::error!(target: LOG_TARGET, "Truncated {} vesting schedules for {:?}", truncated.len(), message.who); - pallet_vesting::Vesting::::insert(&message.who, &bounded_schedules); - - for truncate in truncated { - let len = pallet_vesting::Vesting::::get(&message.who).unwrap_or_default().len(); - let last_index = len.checked_sub(1).ok_or(Error::::Unreachable)? as u32; - let second_last_index = len.checked_sub(2).ok_or(Error::::Unreachable)? as u32; - Self::merge_schedules(message.who.clone(), second_last_index, last_index)?; - - // Now we have at least one slot free that we can use to insert into: - defensive_assert!( - pallet_vesting::Vesting::::get(&message.who).unwrap_or_default().len() < - bounded_schedules.len() - ); - // Insert the new schedule into the free slot: - let mut schedules = pallet_vesting::Vesting::::get(&message.who).unwrap_or_default(); - schedules.try_push(*truncate).map_err(|_| Error::::Unreachable)?; - pallet_vesting::Vesting::::insert(&message.who, &schedules); - } + pallet_vesting::Vesting::::insert(&message.who, &ah_schedules); + log::warn!(target: LOG_TARGET, "Integrated vesting schedule for {:?}, len {}", message.who, ah_schedules.len()); Ok(()) } - - /// Merges two vesting schedules. - /// - /// This function makes use of an pallet-vesting call, which is not entirely clean, but our best - /// bet since otherwise it requires big code duplication. However, nobody is currently using the - /// Vesting pallet on AH, and I recon it unlikely that someone would create a lot of vesting - /// schedules just to have them merged (which they can also do on their own). - pub fn merge_schedules( - who: T::AccountId, - schedule1_index: u32, - schedule2_index: u32, - ) -> Result<(), Error> { - // Pretend to be the account: - let origin: ::RuntimeOrigin = - frame_system::RawOrigin::Signed(who.clone()).into(); - - // NOTE: the pallet macro should already add a transaction, but am not 100% sure: - let res = frame_support::storage::transactional::with_storage_layer(|| { - pallet_vesting::Pallet::::merge_schedules(origin, schedule1_index, schedule2_index) - }); - - let Err(err) = res else { - return Ok(()); - }; - - defensive!("Failed to merge vesting schedules: {:?}", err); - // This is an important error, so we emit an event to monitor if it happens in production: - let err_index = err.using_encoded(|e| e.get(0).copied()); - Self::deposit_event(Event::FailedToMergeVestingSchedules { - who, - schedule1: schedule1_index, - schedule2: schedule2_index, - pallet_vesting_error_index: err_index, - }); - Err(Error::::FailedToMergeVestingSchedules) - } } pub mod alias { diff --git a/pallets/rc-migrator/src/lib.rs b/pallets/rc-migrator/src/lib.rs index b10a0aa5c9..f3f414b3ef 100644 --- a/pallets/rc-migrator/src/lib.rs +++ b/pallets/rc-migrator/src/lib.rs @@ -264,6 +264,7 @@ impl Result { Ok(match s { + "skip-accounts" => MigrationStage::AccountsMigrationDone, "preimage" => MigrationStage::PreimageMigrationInit, "referenda" => MigrationStage::ReferendaMigrationInit, "multisig" => MigrationStage::MultisigMigrationInit, diff --git a/pallets/rc-migrator/src/types.rs b/pallets/rc-migrator/src/types.rs index f04419a279..98bb8e2e99 100644 --- a/pallets/rc-migrator/src/types.rs +++ b/pallets/rc-migrator/src/types.rs @@ -66,15 +66,15 @@ pub enum AhMigratorCall { ReceiveBagsListMessages { messages: Vec> }, #[codec(index = 14)] ReceiveSchedulerMessages { messages: Vec> }, - #[codec(index = 14)] - ReceiveIndices { indices: Vec> }, #[codec(index = 15)] + ReceiveIndices { indices: Vec> }, + #[codec(index = 16)] ReceiveConvictionVotingMessages { messages: Vec>, }, - #[codec(index = 16)] - ReceiveBountiesMessages { messages: Vec> }, #[codec(index = 17)] + ReceiveBountiesMessages { messages: Vec> }, + #[codec(index = 18)] ReceiveAssetRates { asset_rates: Vec<(::AssetKind, FixedU128)> }, } From 2ccd4cbce41a681e6326f65148b7b5c03838c4e6 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 11 Feb 2025 13:29:25 +0100 Subject: [PATCH 8/8] fmt Signed-off-by: Oliver Tale-Yazdi --- pallets/ah-migrator/Cargo.toml | 3 +++ pallets/rc-migrator/Cargo.toml | 3 +++ 2 files changed, 6 insertions(+) diff --git a/pallets/ah-migrator/Cargo.toml b/pallets/ah-migrator/Cargo.toml index 507a971035..93ada94d27 100644 --- a/pallets/ah-migrator/Cargo.toml +++ b/pallets/ah-migrator/Cargo.toml @@ -70,6 +70,7 @@ std = [ "pallet-staking/std", "pallet-state-trie-migration/std", "pallet-treasury/std", + "pallet-vesting/std", "polkadot-parachain-primitives/std", "polkadot-runtime-common/std", "runtime-parachains/std", @@ -102,6 +103,7 @@ runtime-benchmarks = [ "pallet-staking/runtime-benchmarks", "pallet-state-trie-migration/runtime-benchmarks", "pallet-treasury/runtime-benchmarks", + "pallet-vesting/runtime-benchmarks", "polkadot-parachain-primitives/runtime-benchmarks", "polkadot-runtime-common/runtime-benchmarks", "runtime-parachains/runtime-benchmarks", @@ -127,6 +129,7 @@ try-runtime = [ "pallet-staking/try-runtime", "pallet-state-trie-migration/try-runtime", "pallet-treasury/try-runtime", + "pallet-vesting/try-runtime", "polkadot-runtime-common/try-runtime", "runtime-parachains/try-runtime", "sp-runtime/try-runtime", diff --git a/pallets/rc-migrator/Cargo.toml b/pallets/rc-migrator/Cargo.toml index 247d45fca7..566fca5528 100644 --- a/pallets/rc-migrator/Cargo.toml +++ b/pallets/rc-migrator/Cargo.toml @@ -65,6 +65,7 @@ std = [ "pallet-scheduler/std", "pallet-staking/std", "pallet-treasury/std", + "pallet-vesting/std", "polkadot-parachain-primitives/std", "polkadot-runtime-common/std", "runtime-parachains/std", @@ -96,6 +97,7 @@ runtime-benchmarks = [ "pallet-scheduler/runtime-benchmarks", "pallet-staking/runtime-benchmarks", "pallet-treasury/runtime-benchmarks", + "pallet-vesting/runtime-benchmarks", "polkadot-parachain-primitives/runtime-benchmarks", "polkadot-runtime-common/runtime-benchmarks", "runtime-parachains/runtime-benchmarks", @@ -120,6 +122,7 @@ try-runtime = [ "pallet-scheduler/try-runtime", "pallet-staking/try-runtime", "pallet-treasury/try-runtime", + "pallet-vesting/try-runtime", "polkadot-runtime-common/try-runtime", "runtime-parachains/try-runtime", "sp-runtime/try-runtime",