Test pallet order (#112)

* pallet order

* fmt

* adds test for bridge order

* revert unintended change

---------

Co-authored-by: claravanstaden <Cats 4 life!>

bridges/snowbridge/pallets/inbound-queue/src/mock.rs

@@ -88,7 +88,6 @@ impl pallet_balances::Config for Test {
 	type MaxFreezes = ();
 	type RuntimeHoldReason = ();
 	type RuntimeFreezeReason = ();
-	type MaxHolds = ();
 }
 
 parameter_types! {

bridges/snowbridge/pallets/system/src/mock.rs

@@ -136,7 +136,6 @@ impl pallet_balances::Config for Test {
 	type MaxFreezes = ();
 	type RuntimeHoldReason = ();
 	type RuntimeFreezeReason = ();
-	type MaxHolds = ();
 }
 
 impl pallet_xcm_origin::Config for Test {

bridges/snowbridge/runtime/test-common/src/lib.rs

@@ -13,9 +13,9 @@ use parachains_runtimes_test_utils::{
 };
 use snowbridge_core::{ChannelId, ParaId};
 use snowbridge_pallet_ethereum_client_fixtures::*;
-use sp_core::H160;
+use sp_core::{H160, U256};
 use sp_keyring::AccountKeyring::*;
-use sp_runtime::{traits::Header, AccountId32, SaturatedConversion, Saturating};
+use sp_runtime::{traits::Header, AccountId32, DigestItem, SaturatedConversion, Saturating};
 use xcm::{
 	latest::prelude::*,
 	v3::Error::{self, Barrier},
@@ -53,7 +53,8 @@ where
 		+ parachain_info::Config
 		+ pallet_collator_selection::Config
 		+ cumulus_pallet_parachain_system::Config
-		+ snowbridge_pallet_outbound_queue::Config,
+		+ snowbridge_pallet_outbound_queue::Config
+		+ pallet_timestamp::Config,
 	XcmConfig: xcm_executor::Config,
 {
 	let assethub_parachain_location = Location::new(1, Parachain(assethub_parachain_id));
@@ -125,7 +126,8 @@ pub fn send_transfer_token_message_success<Runtime, XcmConfig>(
 		+ pallet_message_queue::Config
 		+ cumulus_pallet_parachain_system::Config
 		+ snowbridge_pallet_outbound_queue::Config
-		+ snowbridge_pallet_system::Config,
+		+ snowbridge_pallet_system::Config
+		+ pallet_timestamp::Config,
 	XcmConfig: xcm_executor::Config,
 	ValidatorIdOf<Runtime>: From<AccountIdOf<Runtime>>,
 	<Runtime as frame_system::Config>::AccountId: From<sp_runtime::AccountId32> + AsRef<[u8]>,
@@ -193,12 +195,96 @@ pub fn send_transfer_token_message_success<Runtime, XcmConfig>(
 
 			let digest = included_head.digest();
 
-			//let digest = frame_system::Pallet::<Runtime>::digest();
 			let digest_items = digest.logs();
 			assert!(digest_items.len() == 1 && digest_items[0].as_other().is_some());
 		});
 }
 
+pub fn test_pallet_order_works<Runtime, XcmConfig, AllPalletsWithoutSystem>(
+	collator_session_key: CollatorSessionKeys<Runtime>,
+	runtime_para_id: u32,
+	assethub_parachain_id: u32,
+	weth_contract_address: H160,
+	destination_address: H160,
+	fee_amount: u128,
+	snowbridge_pallet_outbound_queue: Box<
+		dyn Fn(Vec<u8>) -> Option<snowbridge_pallet_outbound_queue::Event<Runtime>>,
+	>,
+) where
+	Runtime: frame_system::Config
+		+ pallet_balances::Config
+		+ pallet_session::Config
+		+ pallet_xcm::Config
+		+ parachain_info::Config
+		+ pallet_collator_selection::Config
+		+ pallet_message_queue::Config
+		+ cumulus_pallet_parachain_system::Config
+		+ snowbridge_pallet_outbound_queue::Config
+		+ snowbridge_pallet_system::Config
+		+ pallet_timestamp::Config,
+	XcmConfig: xcm_executor::Config,
+	AllPalletsWithoutSystem:
+		OnInitialize<BlockNumberFor<Runtime>> + OnFinalize<BlockNumberFor<Runtime>>,
+	ValidatorIdOf<Runtime>: From<AccountIdOf<Runtime>>,
+	<Runtime as frame_system::Config>::AccountId: From<sp_runtime::AccountId32> + AsRef<[u8]>,
+{
+	ExtBuilder::<Runtime>::default()
+		.with_collators(collator_session_key.collators())
+		.with_session_keys(collator_session_key.session_keys())
+		.with_para_id(runtime_para_id.into())
+		.with_tracing()
+		.build()
+		.execute_with(|| {
+			<snowbridge_pallet_system::Pallet<Runtime>>::initialize(
+				runtime_para_id.into(),
+				assethub_parachain_id.into(),
+			)
+			.unwrap();
+
+			// fund asset hub sovereign account enough so it can pay fees
+			initial_fund::<Runtime>(assethub_parachain_id, 5_000_000_000_000);
+
+			let outcome = send_transfer_token_message::<Runtime, XcmConfig>(
+				assethub_parachain_id,
+				weth_contract_address,
+				destination_address,
+				fee_amount,
+			);
+
+			assert_ok!(outcome.ensure_complete());
+
+			// check events
+			let mut events = <frame_system::Pallet<Runtime>>::events()
+				.into_iter()
+				.filter_map(|e| snowbridge_pallet_outbound_queue(e.event.encode()));
+			assert!(events.any(|e| matches!(
+				e,
+				snowbridge_pallet_outbound_queue::Event::MessageQueued { .. }
+			)));
+
+			let next_block_number: U256 = <frame_system::Pallet<Runtime>>::block_number()
+				.saturating_add(BlockNumberFor::<Runtime>::from(1u32))
+				.into();
+
+			let included_head =
+				RuntimeHelper::<Runtime, AllPalletsWithoutSystem>::run_to_block_with_finalize(
+					next_block_number.as_u32(),
+				);
+			let digest = included_head.digest();
+			let digest_items = digest.logs();
+
+			let mut found_outbound_digest = false;
+			for digest_item in digest_items {
+				match digest_item {
+					DigestItem::Other(_) => found_outbound_digest = true,
+					_ => {},
+				}
+			}
+
+			assert_eq!(found_outbound_digest, true);
+		});
+}
+
 pub fn send_unpaid_transfer_token_message<Runtime, XcmConfig>(
 	collator_session_key: CollatorSessionKeys<Runtime>,
 	runtime_para_id: u32,
@@ -213,7 +299,8 @@ pub fn send_unpaid_transfer_token_message<Runtime, XcmConfig>(
 		+ parachain_info::Config
 		+ pallet_collator_selection::Config
 		+ cumulus_pallet_parachain_system::Config
-		+ snowbridge_pallet_outbound_queue::Config,
+		+ snowbridge_pallet_outbound_queue::Config
+		+ pallet_timestamp::Config,
 	XcmConfig: xcm_executor::Config,
 	ValidatorIdOf<Runtime>: From<AccountIdOf<Runtime>>,
 {
@@ -301,7 +388,8 @@ pub fn send_transfer_token_message_failure<Runtime, XcmConfig>(
 		+ pallet_collator_selection::Config
 		+ cumulus_pallet_parachain_system::Config
 		+ snowbridge_pallet_outbound_queue::Config
-		+ snowbridge_pallet_system::Config,
+		+ snowbridge_pallet_system::Config
+		+ pallet_timestamp::Config,
 	XcmConfig: xcm_executor::Config,
 	ValidatorIdOf<Runtime>: From<AccountIdOf<Runtime>>,
 {
@@ -349,7 +437,8 @@ pub fn ethereum_extrinsic<Runtime>(
 		+ cumulus_pallet_parachain_system::Config
 		+ snowbridge_pallet_outbound_queue::Config
 		+ snowbridge_pallet_system::Config
-		+ snowbridge_pallet_ethereum_client::Config,
+		+ snowbridge_pallet_ethereum_client::Config
+		+ pallet_timestamp::Config,
 	ValidatorIdOf<Runtime>: From<AccountIdOf<Runtime>>,
 	<Runtime as pallet_utility::Config>::RuntimeCall:
 		From<snowbridge_pallet_ethereum_client::Call<Runtime>>,
@@ -430,7 +519,8 @@ pub fn ethereum_to_polkadot_message_extrinsics_work<Runtime>(
 		+ cumulus_pallet_parachain_system::Config
 		+ snowbridge_pallet_outbound_queue::Config
 		+ snowbridge_pallet_system::Config
-		+ snowbridge_pallet_ethereum_client::Config,
+		+ snowbridge_pallet_ethereum_client::Config
+		+ pallet_timestamp::Config,
 	ValidatorIdOf<Runtime>: From<AccountIdOf<Runtime>>,
 	<Runtime as pallet_utility::Config>::RuntimeCall:
 		From<snowbridge_pallet_ethereum_client::Call<Runtime>>,

cumulus/parachains/runtimes/assets/test-utils/Cargo.toml

@@ -18,6 +18,7 @@ frame-system = { path = "../../../../../substrate/frame/system", default-feature
 pallet-assets = { path = "../../../../../substrate/frame/assets", default-features = false }
 pallet-asset-conversion = { path = "../../../../../substrate/frame/asset-conversion", default-features = false }
 pallet-balances = { path = "../../../../../substrate/frame/balances", default-features = false }
+pallet-timestamp = { path = "../../../../../substrate/frame/timestamp", default-features = false }
 pallet-session = { path = "../../../../../substrate/frame/session", default-features = false }
 sp-io = { path = "../../../../../substrate/primitives/io", default-features = false }
 sp-runtime = { path = "../../../../../substrate/primitives/runtime", default-features = false }
@@ -61,6 +62,7 @@ std = [
 	"pallet-balances/std",
 	"pallet-collator-selection/std",
 	"pallet-session/std",
+	"pallet-timestamp/std",
 	"pallet-xcm-bridge-hub-router/std",
 	"pallet-xcm/std",
 	"parachain-info/std",

cumulus/parachains/runtimes/assets/test-utils/src/test_cases.rs

@@ -70,7 +70,8 @@ pub fn teleports_for_native_asset_works<
 		+ parachain_info::Config
 		+ pallet_collator_selection::Config
 		+ cumulus_pallet_parachain_system::Config
-		+ cumulus_pallet_xcmp_queue::Config,
+		+ cumulus_pallet_xcmp_queue::Config
+		+ pallet_timestamp::Config,
 	AllPalletsWithoutSystem:
 		OnInitialize<BlockNumberFor<Runtime>> + OnFinalize<BlockNumberFor<Runtime>>,
 	AccountIdOf<Runtime>: Into<[u8; 32]>,
@@ -350,7 +351,8 @@ pub fn teleports_for_foreign_assets_works<
 		+ pallet_collator_selection::Config
 		+ cumulus_pallet_parachain_system::Config
 		+ cumulus_pallet_xcmp_queue::Config
-		+ pallet_assets::Config<ForeignAssetsPalletInstance>,
+		+ pallet_assets::Config<ForeignAssetsPalletInstance>
+		+ pallet_timestamp::Config,
 	AllPalletsWithoutSystem:
 		OnInitialize<BlockNumberFor<Runtime>> + OnFinalize<BlockNumberFor<Runtime>>,
 	AccountIdOf<Runtime>: Into<[u8; 32]>,
@@ -701,7 +703,8 @@ pub fn asset_transactor_transfer_with_local_consensus_currency_works<Runtime, Xc
 		+ pallet_xcm::Config
 		+ parachain_info::Config
 		+ pallet_collator_selection::Config
-		+ cumulus_pallet_parachain_system::Config,
+		+ cumulus_pallet_parachain_system::Config
+		+ pallet_timestamp::Config,
 	AccountIdOf<Runtime>: Into<[u8; 32]>,
 	ValidatorIdOf<Runtime>: From<AccountIdOf<Runtime>>,
 	BalanceOf<Runtime>: From<Balance>,
@@ -826,7 +829,8 @@ pub fn asset_transactor_transfer_with_pallet_assets_instance_works<
 		+ parachain_info::Config
 		+ pallet_collator_selection::Config
 		+ cumulus_pallet_parachain_system::Config
-		+ pallet_assets::Config<AssetsPalletInstance>,
+		+ pallet_assets::Config<AssetsPalletInstance>
+		+ pallet_timestamp::Config,
 	AccountIdOf<Runtime>: Into<[u8; 32]>,
 	ValidatorIdOf<Runtime>: From<AccountIdOf<Runtime>>,
 	BalanceOf<Runtime>: From<Balance>,
@@ -1093,7 +1097,8 @@ pub fn create_and_manage_foreign_assets_for_local_consensus_parachain_assets_wor
 		+ parachain_info::Config
 		+ pallet_collator_selection::Config
 		+ cumulus_pallet_parachain_system::Config
-		+ pallet_assets::Config<ForeignAssetsPalletInstance>,
+		+ pallet_assets::Config<ForeignAssetsPalletInstance>
+		+ pallet_timestamp::Config,
 	AccountIdOf<Runtime>: Into<[u8; 32]>,
 	ValidatorIdOf<Runtime>: From<AccountIdOf<Runtime>>,
 	BalanceOf<Runtime>: From<Balance>,
@@ -1422,7 +1427,8 @@ pub fn reserve_transfer_native_asset_to_non_teleport_para_works<
 		+ parachain_info::Config
 		+ pallet_collator_selection::Config
 		+ cumulus_pallet_parachain_system::Config
-		+ cumulus_pallet_xcmp_queue::Config,
+		+ cumulus_pallet_xcmp_queue::Config
+		+ pallet_timestamp::Config,
 	AllPalletsWithoutSystem:
 		OnInitialize<BlockNumberFor<Runtime>> + OnFinalize<BlockNumberFor<Runtime>>,
 	AccountIdOf<Runtime>: Into<[u8; 32]>,

cumulus/parachains/runtimes/assets/test-utils/src/test_cases_over_bridge.rs

@@ -71,7 +71,8 @@ pub fn limited_reserve_transfer_assets_for_native_asset_works<
 		+ parachain_info::Config
 		+ pallet_collator_selection::Config
 		+ cumulus_pallet_parachain_system::Config
-		+ cumulus_pallet_xcmp_queue::Config,
+		+ cumulus_pallet_xcmp_queue::Config
+		+ pallet_timestamp::Config,
 	AllPalletsWithoutSystem:
 		OnInitialize<BlockNumberFor<Runtime>> + OnFinalize<BlockNumberFor<Runtime>>,
 	AccountIdOf<Runtime>: Into<[u8; 32]>,
@@ -347,7 +348,8 @@ pub fn receive_reserve_asset_deposited_from_different_consensus_works<
 		+ cumulus_pallet_parachain_system::Config
 		+ cumulus_pallet_xcmp_queue::Config
 		+ pallet_assets::Config<ForeignAssetsPalletInstance>
-		+ pallet_asset_conversion::Config,
+		+ pallet_asset_conversion::Config
+		+ pallet_timestamp::Config,
 	AllPalletsWithoutSystem:
 		OnInitialize<BlockNumberFor<Runtime>> + OnFinalize<BlockNumberFor<Runtime>>,
 	AccountIdOf<Runtime>: Into<[u8; 32]> + From<[u8; 32]>,
@@ -602,7 +604,8 @@ pub fn report_bridge_status_from_xcm_bridge_router_works<
 		+ pallet_collator_selection::Config
 		+ cumulus_pallet_parachain_system::Config
 		+ cumulus_pallet_xcmp_queue::Config
-		+ pallet_xcm_bridge_hub_router::Config<XcmBridgeHubRouterInstance>,
+		+ pallet_xcm_bridge_hub_router::Config<XcmBridgeHubRouterInstance>
+		+ pallet_timestamp::Config,
 	AllPalletsWithoutSystem:
 		OnInitialize<BlockNumberFor<Runtime>> + OnFinalize<BlockNumberFor<Runtime>>,
 	AccountIdOf<Runtime>: Into<[u8; 32]>,

cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs

@@ -732,7 +732,7 @@ construct_runtime!(
 
 		// Message Queue. Importantly, is registered last so that messages are processed after
 		// the `on_initialize` hooks of bridging pallets.
-		MessageQueue: pallet_message_queue = 250,
+		MessageQueue: pallet_message_queue = 175,
 	}
 );
 

cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/tests/snowbridge.rs

@@ -20,9 +20,9 @@ use bp_polkadot_core::Signature;
 use bridge_hub_rococo_runtime::{
 	bridge_to_bulletin_config::OnBridgeHubRococoRefundRococoBulletinMessages,
 	bridge_to_westend_config::OnBridgeHubRococoRefundBridgeHubWestendMessages,
-	xcm_config::XcmConfig, BridgeRejectObsoleteHeadersAndMessages, Executive,
-	MessageQueueServiceWeight, Runtime, RuntimeCall, RuntimeEvent, SessionKeys, SignedExtra,
-	UncheckedExtrinsic,
+	xcm_config::XcmConfig, AllPalletsWithoutSystem, BridgeRejectObsoleteHeadersAndMessages,
+	Executive, MessageQueueServiceWeight, Runtime, RuntimeCall, RuntimeEvent, SessionKeys,
+	SignedExtra, UncheckedExtrinsic,
 };
 use codec::{Decode, Encode};
 use cumulus_primitives_core::XcmError::{FailedToTransactAsset, NotHoldingFees};
@@ -135,6 +135,28 @@ fn ethereum_to_polkadot_message_extrinsics_work() {
 	);
 }
 
+#[test]
+pub fn test_pallet_order_works() {
+	snowbridge_runtime_test_common::test_pallet_order_works::<
+		Runtime,
+		XcmConfig,
+		AllPalletsWithoutSystem,
+	>(
+		collator_session_keys(),
+		1013,
+		1000,
+		H160::random(),
+		H160::random(),
+		DefaultBridgeHubEthereumBaseFee::get(),
+		Box::new(|runtime_event_encoded: Vec<u8>| {
+			match RuntimeEvent::decode(&mut &runtime_event_encoded[..]) {
+				Ok(RuntimeEvent::EthereumOutboundQueue(event)) => Some(event),
+				_ => None,
+			}
+		}),
+	)
+}
+
 fn construct_extrinsic(
 	sender: sp_keyring::AccountKeyring,
 	call: RuntimeCall,

cumulus/parachains/runtimes/bridge-hubs/test-utils/Cargo.toml

@@ -25,6 +25,7 @@ sp-std = { path = "../../../../../substrate/primitives/std", default-features =
 sp-tracing = { path = "../../../../../substrate/primitives/tracing" }
 pallet-balances = { path = "../../../../../substrate/frame/balances", default-features = false }
 pallet-utility = { path = "../../../../../substrate/frame/utility", default-features = false }
+pallet-timestamp = { path = "../../../../../substrate/frame/timestamp", default-features = false }
 
 # Cumulus
 asset-test-utils = { path = "../../assets/test-utils" }
@@ -73,6 +74,7 @@ std = [
 	"pallet-bridge-messages/std",
 	"pallet-bridge-parachains/std",
 	"pallet-bridge-relayers/std",
+	"pallet-timestamp/std",
 	"pallet-utility/std",
 	"parachains-common/std",
 	"parachains-runtimes-test-utils/std",

cumulus/parachains/runtimes/bridge-hubs/test-utils/src/test_cases/helpers.rs

@@ -197,7 +197,9 @@ where
 pub(crate) fn initialize_bridge_grandpa_pallet<Runtime, GPI>(
 	init_data: bp_header_chain::InitializationData<BridgedHeader<Runtime, GPI>>,
 ) where
-	Runtime: BridgeGrandpaConfig<GPI>,
+	Runtime: BridgeGrandpaConfig<GPI>
+		+ cumulus_pallet_parachain_system::Config
+		+ pallet_timestamp::Config,
 {
 	pallet_bridge_grandpa::Pallet::<Runtime, GPI>::initialize(
 		RuntimeHelper::<Runtime>::root_origin(),