Limit number of messages in commitment queues (paritytech#329)

* Limit number of messages in commitment queues * Set commitment queue limit to 20, try_mutate -> decode_len + append
Snowfork · Apr 1, 2021 · 35991c7 · 35991c7
1 parent 49a5b17
commit 35991c7
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 11 deletions.
diff --git a/parachain/pallets/commitments/src/lib.rs b/parachain/pallets/commitments/src/lib.rs
@@ -2,9 +2,10 @@
 
 use sp_std::prelude::*;
 use frame_support::{
-	decl_module, decl_storage, decl_event, decl_error,
+	decl_module, decl_storage, decl_event, decl_error, ensure,
 	weights::Weight,
 	dispatch::DispatchResult,
+	traits::Get,
 };
 use sp_io::offchain_index;
 use sp_core::{H160, H256, RuntimeDebug};
@@ -55,6 +56,9 @@ pub trait Config: frame_system::Config {
 	type Hashing: Hash<Output = H256>;
 
 	type Event: From<Event> + Into<<Self as frame_system::Config>::Event>;
+
+	/// Max number of messages that can be queued and committed in one go for a given channel.
+	type MaxMessagesPerCommit: Get<usize>;
 }
 
 decl_storage! {
@@ -74,7 +78,10 @@ decl_event! {
 }
 
 decl_error! {
-	pub enum Error for Module<T: Config> {}
+	pub enum Error for Module<T: Config> {
+		/// No more messages can be queued for the channel during this commit cycle.
+		QueueSizeLimitReached,
+	}
 }
 
 decl_module! {
@@ -150,16 +157,19 @@ impl<T: Config> Module<T> {
 impl<T: Config> MessageCommitment for Module<T> {
 
 	// Add a message for eventual inclusion in a commitment
-	// TODO (Security): Limit number of messages per commitment
-	//   https://github.com/Snowfork/polkadot-ethereum/issues/226
 	fn add(channel_id: ChannelId, target: H160, nonce: u64, payload: &[u8]) -> DispatchResult {
+		ensure!(
+			MessageQueues::decode_len(channel_id).unwrap_or(0) < T::MaxMessagesPerCommit::get(),
+			Error::<T>::QueueSizeLimitReached,
+		);
+
 		MessageQueues::append(
 			channel_id,
 			Message {
 				target,
 				nonce,
-				payload: payload.to_vec()
-			}
+				payload: payload.to_vec(),
+			},
 		);
 		Ok(())
 	}

diff --git a/parachain/pallets/commitments/src/mock.rs b/parachain/pallets/commitments/src/mock.rs
@@ -52,12 +52,14 @@ impl frame_system::Config for Test {
 
 parameter_types! {
 	pub const CommitInterval: u64 = 5;
+	pub const MaxMessagesPerCommit: usize = 2;
 }
 
 impl commitments::Config for Test {
 	const INDEXING_PREFIX: &'static [u8] = b"commitment";
 	type Event = Event;
 	type Hashing = Keccak256;
+	type MaxMessagesPerCommit = MaxMessagesPerCommit;
 }
 
 // Build genesis storage according to the mock runtime.

diff --git a/parachain/pallets/commitments/src/tests.rs b/parachain/pallets/commitments/src/tests.rs
@@ -1,13 +1,11 @@
-use crate::mock::{new_test_ext, System, Commitments};
+use crate::mock::{new_test_ext, System, Commitments, Test};
 
-use crate::{Message, MessageQueues};
+use crate::{self as commitments, Error, Message, MessageQueues};
 
 use sp_runtime::DigestItem;
 use sp_core::H160;
 
-use frame_support::traits::OnInitialize;
-
-use frame_support::storage::StorageMap;
+use frame_support::{assert_err, storage::StorageMap, traits::OnInitialize};
 
 use artemis_core::{ChannelId, MessageCommitment};
 
@@ -60,3 +58,18 @@ fn test_add_message() {
 
 	});
 }
+
+#[test]
+fn test_add_message_exceeds_limit() {
+	new_test_ext().execute_with(|| {
+		let max_messages = <Test as commitments::Config>::MaxMessagesPerCommit::get();
+		(0..max_messages).for_each(
+			|_| Commitments::add(ChannelId::Basic, CONTRACT_A, 0, &vec![0, 1, 2]).unwrap()
+		);
+
+		assert_err!(
+			Commitments::add(ChannelId::Basic, CONTRACT_A, 0, &vec![0, 1, 2]),
+			Error::<Test>::QueueSizeLimitReached,
+		);
+	})
+}
diff --git a/parachain/runtime/local/src/lib.rs b/parachain/runtime/local/src/lib.rs
@@ -441,12 +441,14 @@ impl verifier_lightclient::Config for Runtime {
 
 parameter_types! {
 	pub const CommitInterval: BlockNumber = 5;
+	pub const MaxMessagesPerCommit: usize = 20;
 }
 
 impl commitments::Config for Runtime {
 	const INDEXING_PREFIX: &'static [u8] = b"commitment";
 	type Event = Event;
 	type Hashing = Keccak256;
+	type MaxMessagesPerCommit = MaxMessagesPerCommit;
 }
 
 impl assets::Config for Runtime {

diff --git a/parachain/runtime/rococo/src/lib.rs b/parachain/runtime/rococo/src/lib.rs
@@ -443,12 +443,14 @@ impl verifier_lightclient::Config for Runtime {
 
 parameter_types! {
 	pub const CommitInterval: BlockNumber = 5;
+	pub const MaxMessagesPerCommit: usize = 20;
 }
 
 impl commitments::Config for Runtime {
 	const INDEXING_PREFIX: &'static [u8] = b"commitment";
 	type Event = Event;
 	type Hashing = Keccak256;
+	type MaxMessagesPerCommit = MaxMessagesPerCommit;
 }
 
 impl assets::Config for Runtime {

diff --git a/parachain/runtime/snowbridge/src/lib.rs b/parachain/runtime/snowbridge/src/lib.rs
@@ -443,12 +443,14 @@ impl verifier_lightclient::Config for Runtime {
 
 parameter_types! {
 	pub const CommitInterval: BlockNumber = 5;
+	pub const MaxMessagesPerCommit: usize = 20;
 }
 
 impl commitments::Config for Runtime {
 	const INDEXING_PREFIX: &'static [u8] = b"commitment";
 	type Event = Event;
 	type Hashing = Keccak256;
+	type MaxMessagesPerCommit = MaxMessagesPerCommit;
 }
 
 impl assets::Config for Runtime {