Fix xcmp message processing condition. (#510)

* Fix xcmp message processing condition.

* add a very simple test

* Update Cargo.lock

* remove comment

* remove comment

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
Co-authored-by: Alexander Popiak <alexander.popiak@gmail.com>

cumulus/Cargo.lock

@@ -1783,16 +1783,21 @@ dependencies = [
 name = "cumulus-pallet-xcmp-queue"
 version = "0.1.0"
 dependencies = [
+ "cumulus-pallet-parachain-system",
  "cumulus-primitives-core",
  "frame-support",
  "frame-system",
  "log",
+ "pallet-balances",
  "parity-scale-codec",
  "rand 0.8.3",
  "rand_chacha 0.3.1",
+ "sp-core",
+ "sp-io",
  "sp-runtime",
  "sp-std",
  "xcm",
+ "xcm-builder",
  "xcm-executor",
 ]
 

cumulus/pallets/xcmp-queue/Cargo.toml

@@ -24,6 +24,13 @@ xcm-executor = { git = "https://github.com/paritytech/polkadot", default-feature
 # Cumulus Dependencies
 cumulus-primitives-core = { path = "../../primitives/core", default-features = false }
 
+[dev-dependencies]
+sp-core = { git = 'https://github.com/paritytech/substrate', branch = "master", version = '3.0.0' }
+sp-io = { git = 'https://github.com/paritytech/substrate', branch = "master", version = '3.0.0' }
+cumulus-pallet-parachain-system = { path = "../parachain-system" }
+xcm-builder = { git = "https://github.com/paritytech/polkadot", branch = "master" }
+pallet-balances = { git = 'https://github.com/paritytech/substrate', branch = "master", version = '3.0.0' }
+
 [features]
 default = [ "std" ]
 std = [

cumulus/pallets/xcmp-queue/src/lib.rs

@@ -25,6 +25,12 @@
 
 #![cfg_attr(not(feature = "std"), no_std)]
 
+#[cfg(test)]
+mod mock;
+
+#[cfg(test)]
+mod tests;
+
 use codec::{Decode, Encode};
 use cumulus_primitives_core::{
 	relay_chain::BlockNumber as RelayBlockNumber, ChannelStatus, GetChannelInfo, MessageSendError,
@@ -203,7 +209,7 @@ pub enum ChannelSignal {
 }
 
 /// The aggregate XCMP message format.
-#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Encode, Decode)]
+#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Encode, Decode, RuntimeDebug)]
 pub enum XcmpMessageFormat {
 	/// Encoded `VersionedXcm` messages, all concatenated.
 	ConcatenatedVersionedXcm,
@@ -552,7 +558,7 @@ impl<T: Config> Pallet<T> {
 			// If there are more and we're making progress, we process them after we've given the
 			// other channels a look in. If we've still not unlocked all weight, then we set them
 			// up for processing a second time anyway.
-			if !status[index].2.is_empty() && weight_processed > 0 || weight_available != max_weight
+			if !status[index].2.is_empty() && (weight_processed > 0 || weight_available != max_weight)
 			{
 				if shuffle_index + 1 == shuffled.len() {
 					// Only this queue left. Just run around this loop once more.

cumulus/pallets/xcmp-queue/src/mock.rs

@@ -0,0 +1,163 @@
+// Copyright (C) 2021 Parity Technologies (UK) Ltd.
+// 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 crate as xcmp_queue;
+use sp_core::H256;
+use frame_support::parameter_types;
+use sp_runtime::{
+	traits::{BlakeTwo256, IdentityLookup},
+	testing::{Header},
+};
+use xcm_builder::{
+	FixedWeightBounds, IsConcrete, LocationInverter, NativeAsset, CurrencyAdapter,
+	ParentIsDefault,
+};
+
+type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic<Test>;
+type Block = frame_system::mocking::MockBlock<Test>;
+
+// Configure a mock runtime to test the pallet.
+frame_support::construct_runtime!(
+	pub enum Test where
+		Block = Block,
+		NodeBlock = Block,
+		UncheckedExtrinsic = UncheckedExtrinsic,
+	{
+		System: frame_system::{Pallet, Call, Config, Storage, Event<T>},
+		Balances: pallet_balances::{Pallet, Call, Storage, Config<T>, Event<T>},
+		ParachainSystem: cumulus_pallet_parachain_system::{Pallet, Call, Config, Storage, Inherent, Event<T>},
+		XcmpQueue: xcmp_queue::{Pallet, Call, Storage, Event<T>},
+	}
+);
+
+parameter_types! {
+	pub const BlockHashCount: u64 = 250;
+	pub const SS58Prefix: u8 = 42;
+}
+
+type AccountId = u64;
+
+impl frame_system::Config for Test {
+	type BaseCallFilter = ();
+	type BlockWeights = ();
+	type BlockLength = ();
+	type DbWeight = ();
+	type Origin = Origin;
+	type Call = Call;
+	type Index = u64;
+	type BlockNumber = u64;
+	type Hash = H256;
+	type Hashing = BlakeTwo256;
+	type AccountId = AccountId;
+	type Lookup = IdentityLookup<Self::AccountId>;
+	type Header = Header;
+	type Event = Event;
+	type BlockHashCount = BlockHashCount;
+	type Version = ();
+	type PalletInfo = PalletInfo;
+	type AccountData = pallet_balances::AccountData<u64>;
+	type OnNewAccount = ();
+	type OnKilledAccount = ();
+	type SystemWeightInfo = ();
+	type SS58Prefix = SS58Prefix;
+	type OnSetCode = cumulus_pallet_parachain_system::ParachainSetCode<Test>;
+}
+
+parameter_types! {
+	pub const ExistentialDeposit: u64 = 5;
+	pub const MaxReserves: u32 = 50;
+}
+
+impl pallet_balances::Config for Test {
+	type Balance = u64;
+	type Event = Event;
+	type DustRemoval = ();
+	type ExistentialDeposit = ExistentialDeposit;
+	type AccountStore = System;
+	type WeightInfo = ();
+	type MaxLocks = ();
+	type MaxReserves = MaxReserves;
+	type ReserveIdentifier = [u8; 8];
+}
+
+impl cumulus_pallet_parachain_system::Config for Test {
+	type Event = Event;
+	type OnValidationData = ();
+	type SelfParaId = ();
+	type OutboundXcmpMessageSource = XcmpQueue;
+	type DmpMessageHandler = ();
+	type ReservedDmpWeight = ();
+	type XcmpMessageHandler = XcmpQueue;
+	type ReservedXcmpWeight = ();
+}
+
+parameter_types! {
+	pub const RelayChain: MultiLocation = MultiLocation::X1(Junction::Parent);
+	pub Ancestry: MultiLocation = MultiLocation::X1(
+		Junction::Parachain(1u32.into())
+	);
+	pub UnitWeightCost: Weight = 1_000_000;
+}
+
+/// Means for transacting assets on this chain.
+pub type LocalAssetTransactor = CurrencyAdapter<
+	// Use this currency:
+	Balances,
+	// Use this currency when it is a fungible asset matching the given location or name:
+	IsConcrete<RelayChain>,
+	// Do a simple punn to convert an AccountId32 MultiLocation into a native chain account ID:
+	LocationToAccountId,
+	// Our chain's account ID type (we can't get away without mentioning it explicitly):
+	AccountId,
+	// We don't track any teleports.
+	(),
+>;
+
+pub type LocationToAccountId = (
+	ParentIsDefault<AccountId>,
+);
+
+pub struct XcmConfig;
+impl xcm_executor::Config for XcmConfig {
+	type Call = Call;
+	type XcmSender = XcmRouter;
+	// How to withdraw and deposit an asset.
+	type AssetTransactor = LocalAssetTransactor;
+	type OriginConverter = ();
+	type IsReserve = NativeAsset;
+	type IsTeleporter = NativeAsset;
+	type LocationInverter = LocationInverter<Ancestry>;
+	type Barrier = ();
+	type Weigher = FixedWeightBounds<UnitWeightCost, Call>;
+	type Trader = ();
+	type ResponseHandler = ();
+}
+
+pub type XcmRouter = (
+	// XCMP to communicate with the sibling chains.
+	XcmpQueue,
+);
+
+impl Config for Test {
+	type Event = Event;
+	type XcmExecutor = xcm_executor::XcmExecutor<XcmConfig>;
+	type ChannelInfo = ParachainSystem;
+}
+
+pub fn new_test_ext() -> sp_io::TestExternalities {
+	let t = frame_system::GenesisConfig::default().build_storage::<Test>().unwrap();
+	t.into()
+}

cumulus/pallets/xcmp-queue/src/tests.rs

@@ -0,0 +1,38 @@
+// Copyright (C) 2021 Parity Technologies (UK) Ltd.
+// 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 mock::{new_test_ext, XcmpQueue};
+use cumulus_primitives_core::XcmpMessageHandler;
+
+#[test]
+fn one_message_does_not_panic() {
+	new_test_ext().execute_with(|| {
+		let message_format = XcmpMessageFormat::ConcatenatedVersionedXcm.encode();
+		let messages = vec![
+			(
+				Default::default(),
+				1u32.into(),
+				message_format.as_slice(),
+			),
+		];
+
+		// This shouldn't cause a panic
+		XcmpQueue::handle_xcmp_messages(
+			messages.into_iter(),
+			Weight::max_value(),
+		);
+	})
+}