Skip to content

Commit

Permalink
Merge pull request #641 from galacticcouncil/tx_pause_bounded_storage
Browse files Browse the repository at this point in the history
fix: transaction pause bounded storage
  • Loading branch information
mrq1911 authored Jul 27, 2023
2 parents 1b58960 + 136dbea commit 809d38e
Show file tree
Hide file tree
Showing 6 changed files with 244 additions and 14 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pallets/transaction-pause/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "pallet-transaction-pause"
version = "0.1.3"
version = "1.0.0"
authors = ["Acala Developers", "GalacticCouncil"]
edition = "2021"

Expand Down
34 changes: 28 additions & 6 deletions pallets/transaction-pause/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<u8, ConstU32<MAX_STR_LENGTH>>;

#[pallet::config]
pub trait Config: frame_system::Config {
type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;
Expand All @@ -57,6 +63,8 @@ pub mod pallet {
CannotPause,
/// invalid character encoding
InvalidCharacter,
/// pallet name or function name is too long
NameTooLong,
}

#[pallet::event]
Expand All @@ -79,10 +87,9 @@ pub mod pallet {
/// map (PalletNameBytes, FunctionNameBytes) => Option<()>
#[pallet::storage]
#[pallet::getter(fn paused_transactions)]
pub type PausedTransactions<T: Config> = StorageMap<_, Twox64Concat, (Vec<u8>, Vec<u8>), (), OptionQuery>;
pub type PausedTransactions<T: Config> = StorageMap<_, Twox64Concat, (BoundedName, BoundedName), (), OptionQuery>;

#[pallet::pallet]
#[pallet::without_storage_info]
pub struct Pallet<T>(_);

#[pallet::hooks]
Expand All @@ -95,14 +102,17 @@ pub mod pallet {
pub fn pause_transaction(origin: OriginFor<T>, pallet_name: Vec<u8>, function_name: Vec<u8>) -> DispatchResult {
T::UpdateOrigin::ensure_origin(origin)?;

let pallet_name_b = BoundedName::try_from(pallet_name.clone()).map_err(|_| Error::<T>::NameTooLong)?;
let function_name_b = BoundedName::try_from(function_name.clone()).map_err(|_| Error::<T>::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::<T>::InvalidCharacter)?;
ensure!(
pallet_name_string != <Self as PalletInfoAccess>::name(),
Error::<T>::CannotPause
);

PausedTransactions::<T>::mutate_exists((pallet_name.clone(), function_name.clone()), |maybe_paused| {
PausedTransactions::<T>::mutate_exists((pallet_name_b, function_name_b), |maybe_paused| {
if maybe_paused.is_none() {
*maybe_paused = Some(());
Self::deposit_event(Event::TransactionPaused {
Expand All @@ -122,7 +132,11 @@ pub mod pallet {
function_name: Vec<u8>,
) -> DispatchResult {
T::UpdateOrigin::ensure_origin(origin)?;
if PausedTransactions::<T>::take((&pallet_name, &function_name)).is_some() {

let pallet_name_b = BoundedName::try_from(pallet_name.clone()).map_err(|_| Error::<T>::NameTooLong)?;
let function_name_b = BoundedName::try_from(function_name.clone()).map_err(|_| Error::<T>::NameTooLong)?;

if PausedTransactions::<T>::take((&pallet_name_b, &function_name_b)).is_some() {
Self::deposit_event(Event::TransactionUnpaused {
pallet_name_bytes: pallet_name,
function_name_bytes: function_name,
Expand All @@ -133,7 +147,7 @@ pub mod pallet {
}
}

pub struct PausedTransactionFilter<T>(sp_std::marker::PhantomData<T>);
pub struct PausedTransactionFilter<T>(PhantomData<T>);
impl<T: Config> Contains<T::RuntimeCall> for PausedTransactionFilter<T>
where
<T as frame_system::Config>::RuntimeCall: GetCallMetadata,
Expand All @@ -143,6 +157,14 @@ where
function_name,
pallet_name,
} = call.get_call_metadata();
PausedTransactions::<T>::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::<T>::contains_key((pallet_name_b.unwrap_or_default(), function_name_b.unwrap_or_default()))
}
}
151 changes: 151 additions & 0 deletions pallets/transaction-pause/src/migration.rs
Original file line number Diff line number Diff line change
@@ -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<T: Config> = StorageMap<Pallet<T>, Twox64Concat, (Vec<u8>, Vec<u8>), (), OptionQuery>;
}
pub mod v1 {
use super::*;

pub struct Migration<T>(PhantomData<T>);

impl<T: Config> OnRuntimeUpgrade for Migration<T> {
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
assert_eq!(StorageVersion::get::<Pallet<T>>(), 0, "Storage version too high.");

let iter = v0::PausedTransactions::<T>::iter_keys();

log::info!(target: TARGET, "Transaction pause migration: PRE checks successful!");

Ok(iter.collect::<Vec<(Vec<u8>, Vec<u8>)>>().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::<T>::drain().collect::<Vec<_>>();
weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1));

for ((pallet_name, function_name), _) in status.into_iter() {
let pallet_name_b = BoundedVec::<u8, ConstU32<MAX_STR_LENGTH>>::try_from(pallet_name.clone());
let function_name_b = BoundedVec::<u8, ConstU32<MAX_STR_LENGTH>>::try_from(function_name.clone());

match (pallet_name_b, function_name_b) {
(Ok(pallet), Ok(function)) => {
crate::PausedTransactions::<T>::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::<Pallet<T>>();

weight.saturating_add(T::DbWeight::get().writes(1))
}

#[cfg(feature = "try-runtime")]
fn post_upgrade(state: Vec<u8>) -> Result<(), &'static str> {
assert_eq!(StorageVersion::get::<Pallet<T>>(), 1, "Unexpected storage version.");

let previous_state = <Vec<(Vec<u8>, Vec<u8>)> as codec::Decode>::decode(&mut state.as_slice()).unwrap();

let new_state = crate::PausedTransactions::<T>::iter_keys()
.map(|v| (v.0.into_inner(), v.1.into_inner()))
.collect::<Vec<(Vec<u8>, Vec<u8>)>>();

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::<Pallet<T>>(), 0);

v0::PausedTransactions::<T>::insert(
("first pallet".as_bytes().to_vec(), "first function".as_bytes().to_vec()),
(),
);
v0::PausedTransactions::<T>::insert(
(
"second pallet".as_bytes().to_vec(),
"second function".as_bytes().to_vec(),
),
(),
);

let state = v1::Migration::<T>::pre_upgrade().unwrap();
let _w = v1::Migration::<T>::on_runtime_upgrade();
v1::Migration::<T>::post_upgrade(state).unwrap();

assert_eq!(StorageVersion::get::<Pallet<T>>(), 1);

assert_eq!(
crate::PausedTransactions::<T>::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::<T>::get((
BoundedName::try_from("second pallet".as_bytes().to_vec()).unwrap(),
BoundedName::try_from("second function".as_bytes().to_vec()).unwrap()
)),
Some(())
);
});
}
}
50 changes: 46 additions & 4 deletions pallets/transaction-pause/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ const TOKENS_TRANSFER: &<Runtime as frame_system::Config>::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!(
Expand All @@ -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(
Expand All @@ -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(())
);

Expand All @@ -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::<Runtime>::NameTooLong
);

assert_noop!(
TransactionPause::pause_transaction(
RuntimeOrigin::signed(1),
b"Balances".to_vec(),
vec![1u8; (MAX_STR_LENGTH + 1) as usize],
),
Error::<Runtime>::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(
Expand All @@ -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(())
);

Expand All @@ -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::<Runtime>::NameTooLong
);

assert_noop!(
TransactionPause::unpause_transaction(
RuntimeOrigin::signed(1),
b"Balances".to_vec(),
vec![1u8; (MAX_STR_LENGTH + 1) as usize],
),
Error::<Runtime>::NameTooLong
);
});
}

Expand Down
Loading

0 comments on commit 809d38e

Please sign in to comment.