diff --git a/Cargo.lock b/Cargo.lock index 9082a39f4..62e192956 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7460,7 +7460,7 @@ dependencies = [ [[package]] name = "pallet-transaction-pause" -version = "0.1.3" +version = "1.0.0" dependencies = [ "frame-benchmarking", "frame-support", diff --git a/pallets/transaction-pause/Cargo.toml b/pallets/transaction-pause/Cargo.toml index 382099b93..0756c989b 100644 --- a/pallets/transaction-pause/Cargo.toml +++ b/pallets/transaction-pause/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pallet-transaction-pause" -version = "0.1.3" +version = "1.0.0" authors = ["Acala Developers", "GalacticCouncil"] edition = "2021" diff --git a/pallets/transaction-pause/src/lib.rs b/pallets/transaction-pause/src/lib.rs index fc97f987b..7f154d279 100644 --- a/pallets/transaction-pause/src/lib.rs +++ b/pallets/transaction-pause/src/lib.rs @@ -23,12 +23,14 @@ use frame_support::{ dispatch::{CallMetadata, GetCallMetadata}, pallet_prelude::*, traits::{Contains, PalletInfoAccess}, + BoundedVec, }; use frame_system::pallet_prelude::*; use sp_runtime::DispatchResult; use sp_std::{prelude::*, vec::Vec}; mod benchmarking; +pub mod migration; mod mock; mod tests; pub mod weights; @@ -40,6 +42,10 @@ pub use weights::WeightInfo; pub mod pallet { use super::*; + // max length of a pallet name or function name + pub const MAX_STR_LENGTH: u32 = 40; + pub type BoundedName = BoundedVec>; + #[pallet::config] pub trait Config: frame_system::Config { type RuntimeEvent: From> + IsType<::RuntimeEvent>; @@ -57,6 +63,8 @@ pub mod pallet { CannotPause, /// invalid character encoding InvalidCharacter, + /// pallet name or function name is too long + NameTooLong, } #[pallet::event] @@ -79,10 +87,9 @@ pub mod pallet { /// map (PalletNameBytes, FunctionNameBytes) => Option<()> #[pallet::storage] #[pallet::getter(fn paused_transactions)] - pub type PausedTransactions = StorageMap<_, Twox64Concat, (Vec, Vec), (), OptionQuery>; + pub type PausedTransactions = StorageMap<_, Twox64Concat, (BoundedName, BoundedName), (), OptionQuery>; #[pallet::pallet] - #[pallet::without_storage_info] pub struct Pallet(_); #[pallet::hooks] @@ -95,6 +102,9 @@ pub mod pallet { pub fn pause_transaction(origin: OriginFor, pallet_name: Vec, function_name: Vec) -> DispatchResult { T::UpdateOrigin::ensure_origin(origin)?; + let pallet_name_b = BoundedName::try_from(pallet_name.clone()).map_err(|_| Error::::NameTooLong)?; + let function_name_b = BoundedName::try_from(function_name.clone()).map_err(|_| Error::::NameTooLong)?; + // not allowed to pause calls of this pallet to ensure safe let pallet_name_string = sp_std::str::from_utf8(&pallet_name).map_err(|_| Error::::InvalidCharacter)?; ensure!( @@ -102,7 +112,7 @@ pub mod pallet { Error::::CannotPause ); - PausedTransactions::::mutate_exists((pallet_name.clone(), function_name.clone()), |maybe_paused| { + PausedTransactions::::mutate_exists((pallet_name_b, function_name_b), |maybe_paused| { if maybe_paused.is_none() { *maybe_paused = Some(()); Self::deposit_event(Event::TransactionPaused { @@ -122,7 +132,11 @@ pub mod pallet { function_name: Vec, ) -> DispatchResult { T::UpdateOrigin::ensure_origin(origin)?; - if PausedTransactions::::take((&pallet_name, &function_name)).is_some() { + + let pallet_name_b = BoundedName::try_from(pallet_name.clone()).map_err(|_| Error::::NameTooLong)?; + let function_name_b = BoundedName::try_from(function_name.clone()).map_err(|_| Error::::NameTooLong)?; + + if PausedTransactions::::take((&pallet_name_b, &function_name_b)).is_some() { Self::deposit_event(Event::TransactionUnpaused { pallet_name_bytes: pallet_name, function_name_bytes: function_name, @@ -133,7 +147,7 @@ pub mod pallet { } } -pub struct PausedTransactionFilter(sp_std::marker::PhantomData); +pub struct PausedTransactionFilter(PhantomData); impl Contains for PausedTransactionFilter where ::RuntimeCall: GetCallMetadata, @@ -143,6 +157,14 @@ where function_name, pallet_name, } = call.get_call_metadata(); - PausedTransactions::::contains_key((pallet_name.as_bytes(), function_name.as_bytes())) + + let pallet_name_b = BoundedName::try_from(pallet_name.as_bytes().to_vec()); + let function_name_b = BoundedName::try_from(function_name.as_bytes().to_vec()); + if pallet_name_b.is_err() || function_name_b.is_err() { + return false; + } + + // it's safe to call unwrap here thanks to the test above + PausedTransactions::::contains_key((pallet_name_b.unwrap_or_default(), function_name_b.unwrap_or_default())) } } diff --git a/pallets/transaction-pause/src/migration.rs b/pallets/transaction-pause/src/migration.rs new file mode 100644 index 000000000..569da8a5a --- /dev/null +++ b/pallets/transaction-pause/src/migration.rs @@ -0,0 +1,151 @@ +// This file is part of pallet-transaction-pause + +// Copyright (C) 2020-2023 Intergalactic, Limited (GIB). +// 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::*; +use frame_support::{ + log, storage_alias, + traits::{Get, OnRuntimeUpgrade, StorageVersion}, + weights::Weight, +}; + +/// The log target. +const TARGET: &str = "runtime::transaction-pause::migration::v1"; + +pub mod v0 { + use super::*; + use sp_std::vec::Vec; + + #[storage_alias] + pub type PausedTransactions = StorageMap, Twox64Concat, (Vec, Vec), (), OptionQuery>; +} +pub mod v1 { + use super::*; + + pub struct Migration(PhantomData); + + impl OnRuntimeUpgrade for Migration { + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, &'static str> { + assert_eq!(StorageVersion::get::>(), 0, "Storage version too high."); + + let iter = v0::PausedTransactions::::iter_keys(); + + log::info!(target: TARGET, "Transaction pause migration: PRE checks successful!"); + + Ok(iter.collect::, Vec)>>().encode()) + } + + fn on_runtime_upgrade() -> Weight { + log::info!(target: TARGET, "Running migration to v1 for Transaction pause"); + + let mut weight = Weight::zero(); + + let status = v0::PausedTransactions::::drain().collect::>(); + weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1)); + + for ((pallet_name, function_name), _) in status.into_iter() { + let pallet_name_b = BoundedVec::>::try_from(pallet_name.clone()); + let function_name_b = BoundedVec::>::try_from(function_name.clone()); + + match (pallet_name_b, function_name_b) { + (Ok(pallet), Ok(function)) => { + crate::PausedTransactions::::insert((pallet, function), ()); + weight.saturating_accrue(T::DbWeight::get().writes(1)); + } + _ => log::info!( + target: TARGET, + "Value not migrated because BoundedVec exceeds its limit: {:?}", + (pallet_name, function_name) + ), + }; + } + + StorageVersion::new(1).put::>(); + + weight.saturating_add(T::DbWeight::get().writes(1)) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(state: Vec) -> Result<(), &'static str> { + assert_eq!(StorageVersion::get::>(), 1, "Unexpected storage version."); + + let previous_state = , Vec)> as codec::Decode>::decode(&mut state.as_slice()).unwrap(); + + let new_state = crate::PausedTransactions::::iter_keys() + .map(|v| (v.0.into_inner(), v.1.into_inner())) + .collect::, Vec)>>(); + + for old_entry in previous_state.iter() { + assert!( + new_state.contains(old_entry), + "Migrated storage entries don't match the entries prior migration!" + ); + } + + log::info!(target: TARGET, "Transaction pause migration: POST checks successful!"); + + Ok(()) + } + } +} + +#[cfg(test)] +#[cfg(feature = "try-runtime")] +mod test { + use super::*; + use crate::mock::{Runtime as T, *}; + + #[test] + fn migration_works() { + ExtBuilder::default().build().execute_with(|| { + assert_eq!(StorageVersion::get::>(), 0); + + v0::PausedTransactions::::insert( + ("first pallet".as_bytes().to_vec(), "first function".as_bytes().to_vec()), + (), + ); + v0::PausedTransactions::::insert( + ( + "second pallet".as_bytes().to_vec(), + "second function".as_bytes().to_vec(), + ), + (), + ); + + let state = v1::Migration::::pre_upgrade().unwrap(); + let _w = v1::Migration::::on_runtime_upgrade(); + v1::Migration::::post_upgrade(state).unwrap(); + + assert_eq!(StorageVersion::get::>(), 1); + + assert_eq!( + crate::PausedTransactions::::get(( + BoundedName::try_from("first pallet".as_bytes().to_vec()).unwrap(), + BoundedName::try_from("first function".as_bytes().to_vec()).unwrap() + )), + Some(()) + ); + assert_eq!( + crate::PausedTransactions::::get(( + BoundedName::try_from("second pallet".as_bytes().to_vec()).unwrap(), + BoundedName::try_from("second function".as_bytes().to_vec()).unwrap() + )), + Some(()) + ); + }); + } +} diff --git a/pallets/transaction-pause/src/tests.rs b/pallets/transaction-pause/src/tests.rs index 8460ae572..184f190b2 100644 --- a/pallets/transaction-pause/src/tests.rs +++ b/pallets/transaction-pause/src/tests.rs @@ -37,6 +37,9 @@ const TOKENS_TRANSFER: &::RuntimeCall = #[test] fn pause_transaction_work() { ExtBuilder::default().build().execute_with(|| { + let balances_b_str = BoundedName::try_from(b"Balances".to_vec()).unwrap(); + let transfer_b_str = BoundedName::try_from(b"transfer".to_vec()).unwrap(); + System::set_block_number(1); assert_noop!( @@ -45,7 +48,7 @@ fn pause_transaction_work() { ); assert_eq!( - TransactionPause::paused_transactions((b"Balances".to_vec(), b"transfer".to_vec())), + TransactionPause::paused_transactions((balances_b_str.clone(), transfer_b_str.clone())), None ); assert_ok!(TransactionPause::pause_transaction( @@ -58,7 +61,7 @@ fn pause_transaction_work() { function_name_bytes: b"transfer".to_vec(), })); assert_eq!( - TransactionPause::paused_transactions((b"Balances".to_vec(), b"transfer".to_vec())), + TransactionPause::paused_transactions((balances_b_str, transfer_b_str)), Some(()) ); @@ -83,12 +86,33 @@ fn pause_transaction_work() { b"OtherPallet".to_vec(), b"pause_transaction".to_vec() )); + + assert_noop!( + TransactionPause::pause_transaction( + RuntimeOrigin::signed(1), + vec![1u8; (MAX_STR_LENGTH + 1) as usize], + b"transfer".to_vec() + ), + Error::::NameTooLong + ); + + assert_noop!( + TransactionPause::pause_transaction( + RuntimeOrigin::signed(1), + b"Balances".to_vec(), + vec![1u8; (MAX_STR_LENGTH + 1) as usize], + ), + Error::::NameTooLong + ); }); } #[test] fn unpause_transaction_work() { ExtBuilder::default().build().execute_with(|| { + let balances_b_str = BoundedName::try_from(b"Balances".to_vec()).unwrap(); + let transfer_b_str = BoundedName::try_from(b"transfer".to_vec()).unwrap(); + System::set_block_number(1); assert_ok!(TransactionPause::pause_transaction( @@ -97,7 +121,7 @@ fn unpause_transaction_work() { b"transfer".to_vec() )); assert_eq!( - TransactionPause::paused_transactions((b"Balances".to_vec(), b"transfer".to_vec())), + TransactionPause::paused_transactions((balances_b_str.clone(), transfer_b_str.clone())), Some(()) ); @@ -116,9 +140,27 @@ fn unpause_transaction_work() { function_name_bytes: b"transfer".to_vec(), })); assert_eq!( - TransactionPause::paused_transactions((b"Balances".to_vec(), b"transfer".to_vec())), + TransactionPause::paused_transactions((balances_b_str, transfer_b_str)), None ); + + assert_noop!( + TransactionPause::unpause_transaction( + RuntimeOrigin::signed(1), + vec![1u8; (MAX_STR_LENGTH + 1) as usize], + b"transfer".to_vec() + ), + Error::::NameTooLong + ); + + assert_noop!( + TransactionPause::unpause_transaction( + RuntimeOrigin::signed(1), + b"Balances".to_vec(), + vec![1u8; (MAX_STR_LENGTH + 1) as usize], + ), + Error::::NameTooLong + ); }); } diff --git a/runtime/hydradx/src/migrations.rs b/runtime/hydradx/src/migrations.rs index cbc236046..020cbfcd1 100644 --- a/runtime/hydradx/src/migrations.rs +++ b/runtime/hydradx/src/migrations.rs @@ -11,22 +11,33 @@ pub struct OnRuntimeUpgradeMigration; impl OnRuntimeUpgrade for OnRuntimeUpgradeMigration { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, &'static str> { + log::info!("PreMigrate Transaction Pause Pallet start"); + let tx_pause_state = pallet_transaction_pause::migration::v1::Migration::::pre_upgrade()?; + log::info!("PreMigrate Transaction Pause Pallet end"); + log::info!("PreMigrate Collator Rewards Pallet start"); pallet_collator_rewards::migration::v1::pre_migrate::(); log::info!("PreMigrate Collator Rewards Pallet end"); + log::info!("PreMigrate Genesis History Pallet start"); pallet_genesis_history::migration::v1::pre_migrate::(); log::info!("PreMigrate Genesis History Pallet end"); - Ok(vec![]) + Ok(tx_pause_state) } fn on_runtime_upgrade() -> Weight { let mut weight: Weight = Weight::zero(); + log::info!("Migrate Transaction Pause Pallet to v1 start"); + weight = + weight.saturating_add(pallet_transaction_pause::migration::v1::Migration::::on_runtime_upgrade()); + log::info!("Migrate Transaction Pause Pallet to v1 end"); + log::info!("Migrate Collator Rewards Pallet to v1 start"); weight = weight.saturating_add(pallet_collator_rewards::migration::v1::migrate::()); log::info!("Migrate Collator Rewards Pallet to v1 end"); + log::info!("Migrate Genesis History Pallet to v1 start"); weight = weight.saturating_add(pallet_genesis_history::migration::v1::migrate::()); log::info!("Migrate Genesis History Pallet to v1 end"); @@ -35,7 +46,11 @@ impl OnRuntimeUpgrade for OnRuntimeUpgradeMigration { } #[cfg(feature = "try-runtime")] - fn post_upgrade(_state: Vec) -> Result<(), &'static str> { + fn post_upgrade(state: Vec) -> Result<(), &'static str> { + log::info!("PostMigrate Transaction Pause Pallet start"); + pallet_transaction_pause::migration::v1::Migration::::post_upgrade(state)?; + log::info!("PostMigrate Transaction Pause Pallet end"); + log::info!("PostMigrate Collator Rewards Pallet start"); pallet_collator_rewards::migration::v1::post_migrate::(); log::info!("PostMigrate Collator Rewards Pallet end");