From ec76490ddbbe4e83578ed773d0af9377bb3e001e Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Wed, 29 Jun 2022 16:34:58 +0300 Subject: [PATCH] Test pallet owner calls using macro Define macro that generates tests for set_owner() and set_operating_mode() in order to avoid duplicate code. Signed-off-by: Serban Iorga --- bridges/modules/grandpa/src/lib.rs | 103 +---------------------- bridges/modules/messages/Cargo.toml | 1 + bridges/modules/messages/src/lib.rs | 103 ++--------------------- bridges/primitives/test-utils/src/lib.rs | 89 ++++++++++++++++++++ 4 files changed, 100 insertions(+), 196 deletions(-) diff --git a/bridges/modules/grandpa/src/lib.rs b/bridges/modules/grandpa/src/lib.rs index c0b4d9d3ee..c75ccc2e80 100644 --- a/bridges/modules/grandpa/src/lib.rs +++ b/bridges/modules/grandpa/src/lib.rs @@ -600,8 +600,8 @@ mod tests { 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, + authority_list, generate_owned_bridge_module_tests, make_default_justification, + make_justification_for_header, JustificationGeneratorParams, ALICE, BOB, }; use codec::Encode; use frame_support::{ @@ -715,103 +715,6 @@ mod tests { }) } - #[test] - fn pallet_owner_may_change_owner() { - run_test(|| { - PalletOwner::::put(2); - - assert_ok!(Pallet::::set_owner(Origin::root(), Some(1))); - assert_noop!( - Pallet::::set_operating_mode( - Origin::signed(2), - BasicOperatingMode::Halted - ), - DispatchError::BadOrigin, - ); - assert_ok!(Pallet::::set_operating_mode( - Origin::root(), - BasicOperatingMode::Halted - )); - - assert_ok!(Pallet::::set_owner(Origin::signed(1), None)); - assert_noop!( - Pallet::::set_operating_mode( - Origin::signed(1), - BasicOperatingMode::Normal - ), - DispatchError::BadOrigin, - ); - assert_noop!( - Pallet::::set_operating_mode( - Origin::signed(2), - BasicOperatingMode::Normal - ), - DispatchError::BadOrigin, - ); - assert_ok!(Pallet::::set_operating_mode( - Origin::root(), - BasicOperatingMode::Normal - )); - }); - } - - #[test] - fn pallet_may_be_halted_by_root() { - run_test(|| { - assert_ok!(Pallet::::set_operating_mode( - Origin::root(), - BasicOperatingMode::Halted - )); - assert_ok!(Pallet::::set_operating_mode( - Origin::root(), - BasicOperatingMode::Normal - )); - }); - } - - #[test] - fn pallet_may_be_halted_by_owner() { - run_test(|| { - PalletOwner::::put(2); - - assert_ok!(Pallet::::set_operating_mode( - Origin::signed(2), - BasicOperatingMode::Halted - )); - assert_ok!(Pallet::::set_operating_mode( - Origin::signed(2), - BasicOperatingMode::Normal - )); - - assert_noop!( - Pallet::::set_operating_mode( - Origin::signed(1), - BasicOperatingMode::Halted - ), - DispatchError::BadOrigin, - ); - assert_noop!( - Pallet::::set_operating_mode( - Origin::signed(1), - BasicOperatingMode::Normal - ), - DispatchError::BadOrigin, - ); - - assert_ok!(Pallet::::set_operating_mode( - Origin::signed(2), - BasicOperatingMode::Halted - )); - assert_noop!( - Pallet::::set_operating_mode( - Origin::signed(1), - BasicOperatingMode::Normal - ), - DispatchError::BadOrigin, - ); - }); - } - #[test] fn pallet_rejects_transactions_if_halted() { run_test(|| { @@ -1184,4 +1087,6 @@ mod tests { bp_header_chain::storage_keys::best_finalized_key("Grandpa").0, ); } + + generate_owned_bridge_module_tests!(BasicOperatingMode::Normal, BasicOperatingMode::Halted); } diff --git a/bridges/modules/messages/Cargo.toml b/bridges/modules/messages/Cargo.toml index 47d41dba17..5f98523266 100644 --- a/bridges/modules/messages/Cargo.toml +++ b/bridges/modules/messages/Cargo.toml @@ -31,6 +31,7 @@ sp-std = { git = "https://github.com/paritytech/substrate", branch = "master", d [dev-dependencies] sp-io = { git = "https://github.com/paritytech/substrate", branch = "master" } pallet-balances = { git = "https://github.com/paritytech/substrate", branch = "master" } +bp-test-utils = { path = "../../primitives/test-utils" } [features] default = ["std"] diff --git a/bridges/modules/messages/src/lib.rs b/bridges/modules/messages/src/lib.rs index 03d73c88ea..a990101818 100644 --- a/bridges/modules/messages/src/lib.rs +++ b/bridges/modules/messages/src/lib.rs @@ -1120,6 +1120,7 @@ mod tests { REGULAR_PAYLOAD, TEST_LANE_ID, TEST_RELAYER_A, TEST_RELAYER_B, }; use bp_messages::{UnrewardedRelayer, UnrewardedRelayersState}; + use bp_test_utils::generate_owned_bridge_module_tests; use frame_support::{ assert_noop, assert_ok, storage::generator::{StorageMap, StorageValue}, @@ -1223,103 +1224,6 @@ mod tests { ); } - #[test] - fn pallet_owner_may_change_owner() { - run_test(|| { - PalletOwner::::put(2); - - assert_ok!(Pallet::::set_owner(Origin::root(), Some(1))); - assert_noop!( - Pallet::::set_operating_mode( - Origin::signed(2), - MessagesOperatingMode::Basic(BasicOperatingMode::Halted) - ), - DispatchError::BadOrigin, - ); - assert_ok!(Pallet::::set_operating_mode( - Origin::root(), - MessagesOperatingMode::Basic(BasicOperatingMode::Halted) - )); - - assert_ok!(Pallet::::set_owner(Origin::signed(1), None)); - assert_noop!( - Pallet::::set_operating_mode( - Origin::signed(1), - MessagesOperatingMode::Basic(BasicOperatingMode::Normal) - ), - DispatchError::BadOrigin, - ); - assert_noop!( - Pallet::::set_operating_mode( - Origin::signed(2), - MessagesOperatingMode::Basic(BasicOperatingMode::Normal) - ), - DispatchError::BadOrigin, - ); - assert_ok!(Pallet::::set_operating_mode( - Origin::root(), - MessagesOperatingMode::Basic(BasicOperatingMode::Normal) - )); - }); - } - - #[test] - fn pallet_may_be_halted_by_root() { - run_test(|| { - assert_ok!(Pallet::::set_operating_mode( - Origin::root(), - MessagesOperatingMode::Basic(BasicOperatingMode::Halted) - )); - assert_ok!(Pallet::::set_operating_mode( - Origin::root(), - MessagesOperatingMode::Basic(BasicOperatingMode::Normal) - )); - }); - } - - #[test] - fn pallet_may_be_halted_by_owner() { - run_test(|| { - PalletOwner::::put(2); - - assert_ok!(Pallet::::set_operating_mode( - Origin::signed(2), - MessagesOperatingMode::Basic(BasicOperatingMode::Halted) - )); - assert_ok!(Pallet::::set_operating_mode( - Origin::signed(2), - MessagesOperatingMode::Basic(BasicOperatingMode::Normal) - )); - - assert_noop!( - Pallet::::set_operating_mode( - Origin::signed(1), - MessagesOperatingMode::Basic(BasicOperatingMode::Halted) - ), - DispatchError::BadOrigin, - ); - assert_noop!( - Pallet::::set_operating_mode( - Origin::signed(1), - MessagesOperatingMode::Basic(BasicOperatingMode::Normal) - ), - DispatchError::BadOrigin, - ); - - assert_ok!(Pallet::::set_operating_mode( - Origin::signed(2), - MessagesOperatingMode::Basic(BasicOperatingMode::Halted) - )); - assert_noop!( - Pallet::::set_operating_mode( - Origin::signed(1), - MessagesOperatingMode::Basic(BasicOperatingMode::Normal) - ), - DispatchError::BadOrigin, - ); - }); - } - #[test] fn pallet_parameter_may_be_updated_by_root() { run_test(|| { @@ -2411,4 +2315,9 @@ mod tests { ); }); } + + generate_owned_bridge_module_tests!( + MessagesOperatingMode::Basic(BasicOperatingMode::Normal), + MessagesOperatingMode::Basic(BasicOperatingMode::Halted) + ); } diff --git a/bridges/primitives/test-utils/src/lib.rs b/bridges/primitives/test-utils/src/lib.rs index 38d9453c98..66dbe9e738 100644 --- a/bridges/primitives/test-utils/src/lib.rs +++ b/bridges/primitives/test-utils/src/lib.rs @@ -210,3 +210,92 @@ pub fn test_header(number: H::Number) -> H { pub fn header_id(index: u8) -> (H::Hash, H::Number) { (test_header::(index.into()).hash(), index.into()) } + +#[macro_export] +/// Adds methods for testing the `set_owner()` and `set_operating_mode()` for a pallet. +/// Some values are hardcoded like: +/// - `run_test()` +/// - `Pallet::` +/// - `PalletOwner::` +/// - `PalletOperatingMode::` +/// While this is not ideal, all the pallets use the same names, so it works for the moment. +/// We can revisit this in the future if anything changes. +macro_rules! generate_owned_bridge_module_tests { + ($normal_operating_mode: expr, $halted_operating_mode: expr) => { + #[test] + fn test_set_owner() { + run_test(|| { + PalletOwner::::put(1); + + // The root should be able to change the owner. + assert_ok!(Pallet::::set_owner(Origin::root(), Some(2))); + assert_eq!(PalletOwner::::get(), Some(2)); + + // The owner should be able to change the owner. + assert_ok!(Pallet::::set_owner(Origin::signed(2), Some(3))); + assert_eq!(PalletOwner::::get(), Some(3)); + + // Other users shouldn't be able to change the owner. + assert_noop!( + Pallet::::set_owner(Origin::signed(1), Some(4)), + DispatchError::BadOrigin + ); + assert_eq!(PalletOwner::::get(), Some(3)); + }); + } + + #[test] + fn test_set_operating_mode() { + run_test(|| { + PalletOwner::::put(1); + PalletOperatingMode::::put($normal_operating_mode); + + // The root should be able to halt the pallet. + assert_ok!(Pallet::::set_operating_mode( + Origin::root(), + $halted_operating_mode + )); + assert_eq!(PalletOperatingMode::::get(), $halted_operating_mode); + // The root should be able to resume the pallet. + assert_ok!(Pallet::::set_operating_mode( + Origin::root(), + $normal_operating_mode + )); + assert_eq!(PalletOperatingMode::::get(), $normal_operating_mode); + + // The owner should be able to halt the pallet. + assert_ok!(Pallet::::set_operating_mode( + Origin::signed(1), + $halted_operating_mode + )); + assert_eq!(PalletOperatingMode::::get(), $halted_operating_mode); + // The owner should be able to resume the pallet. + assert_ok!(Pallet::::set_operating_mode( + Origin::signed(1), + $normal_operating_mode + )); + assert_eq!(PalletOperatingMode::::get(), $normal_operating_mode); + + // Other users shouldn't be able to halt the pallet. + assert_noop!( + Pallet::::set_operating_mode( + Origin::signed(2), + $halted_operating_mode + ), + DispatchError::BadOrigin + ); + assert_eq!(PalletOperatingMode::::get(), $normal_operating_mode); + // Other users shouldn't be able to resume the pallet. + PalletOperatingMode::::put($halted_operating_mode); + assert_noop!( + Pallet::::set_operating_mode( + Origin::signed(2), + $normal_operating_mode + ), + DispatchError::BadOrigin + ); + assert_eq!(PalletOperatingMode::::get(), $halted_operating_mode); + }); + } + }; +}