Skip to content

Commit

Permalink
Unify the operating mode for bridge pallets (paritytech#1483)
Browse files Browse the repository at this point in the history 
Unify the operating mode for bridge pallets

- define the OperationMode trait and BasicOperatingMode enum
- use the OperationMode trait in all the bridge pallets
- use BasicOperatingMode instead of IsHalted for the Grandpa pallet
- use BasicOperatingMode as part of MessagesOperatingMode

Signed-off-by: Serban Iorga <serban@parity.io>
  • Loading branch information
@serban300
serban300 committed Apr 9, 2024
1 parent 69b9344 commit a0ee123
Show file tree
Hide file tree
Showing 11 changed files with 218 additions and 104 deletions.
3 changes: 2 additions & 1 deletion bridges/modules/grandpa/src/benchmarking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@

use crate::*;

use bp_runtime::BasicOperatingMode;
use bp_test_utils::{
accounts, make_justification_for_header, JustificationGeneratorParams, TEST_GRANDPA_ROUND,
TEST_GRANDPA_SET_ID,
Expand Down Expand Up @@ -84,7 +85,7 @@ fn prepare_benchmark_data<T: Config<I>, I: 'static>(
header: Box::new(bp_test_utils::test_header(Zero::zero())),
authority_list,
set_id: TEST_GRANDPA_SET_ID,
is_halted: false,
operating_mode: BasicOperatingMode::Normal,
};

bootstrap_bridge::<T, I>(init_data);
Expand Down
119 changes: 84 additions & 35 deletions bridges/modules/grandpa/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ pub type BridgedHeader<T, I> = HeaderOf<<T as Config<I>>::BridgedChain>;
#[frame_support::pallet]
pub mod pallet {
use super::*;
use bp_runtime::BasicOperatingMode;
use frame_support::pallet_prelude::*;
use frame_system::pallet_prelude::*;

Expand Down Expand Up @@ -119,14 +120,9 @@ pub mod pallet {

impl<T: Config<I>, I: 'static> OwnedBridgeModule<T> for Pallet<T, I> {
const LOG_TARGET: &'static str = "runtime::bridge-grandpa";
const OPERATING_MODE_KEY: &'static str = "IsHalted";
type OwnerStorage = PalletOwner<T, I>;
type OperatingMode = bool;
type OperatingModeStorage = IsHalted<T, I>;

fn is_halted() -> bool {
Self::OperatingModeStorage::get()
}
type OperatingMode = BasicOperatingMode;
type OperatingModeStorage = PalletOperatingMode<T, I>;
}

#[pallet::call]
Expand Down Expand Up @@ -237,8 +233,11 @@ pub mod pallet {
///
/// May only be called either by root, or by `PalletOwner`.
#[pallet::weight((T::DbWeight::get().reads_writes(1, 1), DispatchClass::Operational))]
pub fn set_operational(origin: OriginFor<T>, operational: bool) -> DispatchResult {
Self::set_operating_mode(origin, !operational)
pub fn set_operating_mode(
origin: OriginFor<T>,
operating_mode: BasicOperatingMode,
) -> DispatchResult {
<Self as OwnedBridgeModule<_>>::set_operating_mode(origin, operating_mode)
}
}

Expand Down Expand Up @@ -293,9 +292,12 @@ pub mod pallet {
pub type PalletOwner<T: Config<I>, I: 'static = ()> =
StorageValue<_, T::AccountId, OptionQuery>;

/// If true, all pallet transactions are failed immediately.
/// The current operating mode of the pallet.
///
/// Depending on the mode either all, or no transactions will be allowed.
#[pallet::storage]
pub type IsHalted<T: Config<I>, I: 'static = ()> = StorageValue<_, bool, ValueQuery>;
pub type PalletOperatingMode<T: Config<I>, I: 'static = ()> =
StorageValue<_, BasicOperatingMode, ValueQuery>;

#[pallet::genesis_config]
pub struct GenesisConfig<T: Config<I>, I: 'static = ()> {
Expand Down Expand Up @@ -324,7 +326,7 @@ pub mod pallet {
} else {
// Since the bridge hasn't been initialized we shouldn't allow anyone to perform
// transactions.
<IsHalted<T, I>>::put(true);
<PalletOperatingMode<T, I>>::put(BasicOperatingMode::Halted);
}
}
}
Expand Down Expand Up @@ -463,7 +465,8 @@ pub mod pallet {
pub(crate) fn initialize_bridge<T: Config<I>, I: 'static>(
init_params: super::InitializationData<BridgedHeader<T, I>>,
) {
let super::InitializationData { header, authority_list, set_id, is_halted } = init_params;
let super::InitializationData { header, authority_list, set_id, operating_mode } =
init_params;

let initial_hash = header.hash();
<InitialHash<T, I>>::put(initial_hash);
Expand All @@ -473,7 +476,7 @@ pub mod pallet {
let authority_set = bp_header_chain::AuthoritySet::new(authority_list, set_id);
<CurrentAuthoritySet<T, I>>::put(authority_set);

<IsHalted<T, I>>::put(is_halted);
<PalletOperatingMode<T, I>>::put(operating_mode);
}

#[cfg(feature = "runtime-benchmarks")]
Expand Down Expand Up @@ -576,14 +579,15 @@ pub fn initialize_for_benchmarks<T: Config<I>, I: 'static>(header: BridgedHeader
authority_list: sp_std::vec::Vec::new(), /* we don't verify any proofs in external
* benchmarks */
set_id: 0,
is_halted: false,
operating_mode: bp_runtime::BasicOperatingMode::Normal,
});
}

#[cfg(test)]
mod tests {
use super::*;
use crate::mock::{run_test, test_header, Origin, TestHeader, TestNumber, TestRuntime};
use bp_runtime::BasicOperatingMode;
use bp_test_utils::{
authority_list, make_default_justification, make_justification_for_header,
JustificationGeneratorParams, ALICE, BOB,
Expand Down Expand Up @@ -611,7 +615,7 @@ mod tests {
header: Box::new(genesis),
authority_list: authority_list(),
set_id: 1,
is_halted: false,
operating_mode: BasicOperatingMode::Normal,
};

Pallet::<TestRuntime>::initialize(origin, init_data.clone()).map(|_| init_data)
Expand Down Expand Up @@ -685,7 +689,7 @@ mod tests {
CurrentAuthoritySet::<TestRuntime>::get().authorities,
init_data.authority_list
);
assert!(!IsHalted::<TestRuntime>::get());
assert_eq!(PalletOperatingMode::<TestRuntime>::get(), BasicOperatingMode::Normal);
})
}

Expand All @@ -707,29 +711,50 @@ mod tests {

assert_ok!(Pallet::<TestRuntime>::set_owner(Origin::root(), Some(1)));
assert_noop!(
Pallet::<TestRuntime>::set_operational(Origin::signed(2), false),
Pallet::<TestRuntime>::set_operating_mode(
Origin::signed(2),
BasicOperatingMode::Halted
),
DispatchError::BadOrigin,
);
assert_ok!(Pallet::<TestRuntime>::set_operational(Origin::root(), false));
assert_ok!(Pallet::<TestRuntime>::set_operating_mode(
Origin::root(),
BasicOperatingMode::Halted
));

assert_ok!(Pallet::<TestRuntime>::set_owner(Origin::signed(1), None));
assert_noop!(
Pallet::<TestRuntime>::set_operational(Origin::signed(1), true),
Pallet::<TestRuntime>::set_operating_mode(
Origin::signed(1),
BasicOperatingMode::Normal
),
DispatchError::BadOrigin,
);
assert_noop!(
Pallet::<TestRuntime>::set_operational(Origin::signed(2), true),
Pallet::<TestRuntime>::set_operating_mode(
Origin::signed(2),
BasicOperatingMode::Normal
),
DispatchError::BadOrigin,
);
assert_ok!(Pallet::<TestRuntime>::set_operational(Origin::root(), true));
assert_ok!(Pallet::<TestRuntime>::set_operating_mode(
Origin::root(),
BasicOperatingMode::Normal
));
});
}

#[test]
fn pallet_may_be_halted_by_root() {
run_test(|| {
assert_ok!(Pallet::<TestRuntime>::set_operational(Origin::root(), false));
assert_ok!(Pallet::<TestRuntime>::set_operational(Origin::root(), true));
assert_ok!(Pallet::<TestRuntime>::set_operating_mode(
Origin::root(),
BasicOperatingMode::Halted
));
assert_ok!(Pallet::<TestRuntime>::set_operating_mode(
Origin::root(),
BasicOperatingMode::Normal
));
});
}

Expand All @@ -738,21 +763,39 @@ mod tests {
run_test(|| {
PalletOwner::<TestRuntime>::put(2);

assert_ok!(Pallet::<TestRuntime>::set_operational(Origin::signed(2), false));
assert_ok!(Pallet::<TestRuntime>::set_operational(Origin::signed(2), true));
assert_ok!(Pallet::<TestRuntime>::set_operating_mode(
Origin::signed(2),
BasicOperatingMode::Halted
));
assert_ok!(Pallet::<TestRuntime>::set_operating_mode(
Origin::signed(2),
BasicOperatingMode::Normal
));

assert_noop!(
Pallet::<TestRuntime>::set_operational(Origin::signed(1), false),
Pallet::<TestRuntime>::set_operating_mode(
Origin::signed(1),
BasicOperatingMode::Halted
),
DispatchError::BadOrigin,
);
assert_noop!(
Pallet::<TestRuntime>::set_operational(Origin::signed(1), true),
Pallet::<TestRuntime>::set_operating_mode(
Origin::signed(1),
BasicOperatingMode::Normal
),
DispatchError::BadOrigin,
);

assert_ok!(Pallet::<TestRuntime>::set_operational(Origin::signed(2), false));
assert_ok!(Pallet::<TestRuntime>::set_operating_mode(
Origin::signed(2),
BasicOperatingMode::Halted
));
assert_noop!(
Pallet::<TestRuntime>::set_operational(Origin::signed(1), true),
Pallet::<TestRuntime>::set_operating_mode(
Origin::signed(1),
BasicOperatingMode::Normal
),
DispatchError::BadOrigin,
);
});
Expand All @@ -763,13 +806,19 @@ mod tests {
run_test(|| {
initialize_substrate_bridge();

assert_ok!(Pallet::<TestRuntime>::set_operational(Origin::root(), false));
assert_ok!(Pallet::<TestRuntime>::set_operating_mode(
Origin::root(),
BasicOperatingMode::Halted
));
assert_noop!(
submit_finality_proof(1),
Error::<TestRuntime>::BridgeModule(bp_runtime::OwnedBridgeModuleError::Halted)
);

assert_ok!(Pallet::<TestRuntime>::set_operational(Origin::root(), true));
assert_ok!(Pallet::<TestRuntime>::set_operating_mode(
Origin::root(),
BasicOperatingMode::Normal
));
assert_ok!(submit_finality_proof(1));
})
}
Expand Down Expand Up @@ -851,7 +900,7 @@ mod tests {
header: Box::new(genesis),
authority_list: invalid_authority_list,
set_id: 1,
is_halted: false,
operating_mode: BasicOperatingMode::Normal,
};

assert_ok!(Pallet::<TestRuntime>::initialize(Origin::root(), init_data));
Expand Down Expand Up @@ -1115,8 +1164,8 @@ mod tests {
#[test]
fn storage_keys_computed_properly() {
assert_eq!(
IsHalted::<TestRuntime>::storage_value_final_key().to_vec(),
bp_header_chain::storage_keys::is_halted_key("Grandpa").0,
PalletOperatingMode::<TestRuntime>::storage_value_final_key().to_vec(),
bp_header_chain::storage_keys::pallet_operating_mode_key("Grandpa").0,
);

assert_eq!(
Expand Down
Loading

0 comments on commit a0ee123

Please sign in to comment.