send_message weight now depends on message size (paritytech#603)

* `send_message` weight now depends on message size

* fix tests

* Update modules/message-lane/src/benchmarking.rs

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

* Update modules/message-lane/src/benchmarking.rs

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

bridges/bin/rialto/runtime/src/lib.rs

@@ -813,6 +813,8 @@ impl_runtime_apis! {
 				}
 			}
 
+			use crate::millau_messages::{ToMillauMessagePayload, WithMillauMessageBridge};
+			use bridge_runtime_common::messages;
 			use pallet_message_lane::benchmarking::{
 				Module as MessageLaneBench,
 				Config as MessageLaneConfig,
@@ -822,6 +824,10 @@ impl_runtime_apis! {
 			};
 
 			impl MessageLaneConfig<WithMillauMessageLaneInstance> for Runtime {
+				fn maximal_message_size() -> u32 {
+					messages::source::maximal_message_size::<WithMillauMessageBridge>()
+				}
+
 				fn bridged_relayer_id() -> Self::InboundRelayer {
 					Default::default()
 				}
@@ -840,24 +846,14 @@ impl_runtime_apis! {
 				fn prepare_outbound_message(
 					params: MessageLaneMessageParams<Self::AccountId>,
 				) -> (millau_messages::ToMillauMessagePayload, Balance) {
-					use crate::millau_messages::{ToMillauMessagePayload, WithMillauMessageBridge};
-					use bridge_runtime_common::messages;
-					use pallet_message_lane::benchmarking::WORST_MESSAGE_SIZE_FACTOR;
-
-					let max_message_size = messages::source::maximal_message_size::<WithMillauMessageBridge>();
-					let message_size = match params.size_factor {
-						0 => 1,
-						factor => max_message_size / WORST_MESSAGE_SIZE_FACTOR
-							* sp_std::cmp::min(factor, WORST_MESSAGE_SIZE_FACTOR),
-					};
-					let message_payload = vec![0; message_size as usize];
+					let message_payload = vec![0; params.size as usize];
 					let dispatch_origin = pallet_bridge_call_dispatch::CallOrigin::SourceAccount(
 						params.sender_account,
 					);
 
 					let message = ToMillauMessagePayload {
 						spec_version: 0,
-						weight: message_size as _,
+						weight: params.size as _,
 						origin: dispatch_origin,
 						call: message_payload,
 					};

bridges/modules/call-dispatch/src/lib.rs

@@ -25,7 +25,7 @@
 #![warn(missing_docs)]
 
 use bp_message_dispatch::{MessageDispatch, Weight};
-use bp_runtime::{derive_account_id, InstanceId, SourceAccount};
+use bp_runtime::{derive_account_id, InstanceId, Size, SourceAccount};
 use codec::{Decode, Encode};
 use frame_support::{
 	decl_event, decl_module, decl_storage,
@@ -104,6 +104,14 @@ pub struct MessagePayload<SourceChainAccountId, TargetChainAccountPublic, Target
 	pub call: Call,
 }
 
+impl<SourceChainAccountId, TargetChainAccountPublic, TargetChainSignature> Size
+	for MessagePayload<SourceChainAccountId, TargetChainAccountPublic, TargetChainSignature, Vec<u8>>
+{
+	fn size_hint(&self) -> u32 {
+		self.call.len() as _
+	}
+}
+
 /// The module configuration trait.
 pub trait Config<I = DefaultInstance>: frame_system::Config {
 	/// The overarching event type.

bridges/modules/message-lane/src/benchmarking.rs

@@ -27,8 +27,6 @@ use frame_support::{traits::Get, weights::Weight};
 use frame_system::RawOrigin;
 use sp_std::{collections::btree_map::BTreeMap, convert::TryInto, ops::RangeInclusive, prelude::*};
 
-/// Message crafted with this size factor should be the largest possible message.
-pub const WORST_MESSAGE_SIZE_FACTOR: u32 = 1000;
 /// Fee paid by submitter for single message delivery.
 const MESSAGE_FEE: u32 = 1_000_000;
 
@@ -39,10 +37,8 @@ pub struct Module<T: Config<I>, I: crate::Instance>(crate::Module<T, I>);
 
 /// Benchmark-specific message parameters.
 pub struct MessageParams<ThisAccountId> {
-	/// Size factor of the message payload. Message payload grows with every factor
-	/// increment. Zero is the smallest possible message and the `WORST_MESSAGE_SIZE_FACTOR` is
-	/// largest possible message.
-	pub size_factor: u32,
+	/// Size of the message payload.
+	pub size: u32,
 	/// Message sender account.
 	pub sender_account: ThisAccountId,
 }
@@ -67,6 +63,8 @@ pub struct MessageDeliveryProofParams<ThisChainAccountId> {
 
 /// Trait that must be implemented by runtime.
 pub trait Config<I: Instance>: crate::Config<I> {
+	/// Get maximal size of the message payload.
+	fn maximal_message_size() -> u32;
 	/// Return id of relayer account at the bridged chain.
 	fn bridged_relayer_id() -> Self::InboundRelayer;
 	/// Return balance of given account.
@@ -101,10 +99,12 @@ benchmarks_instance! {
 	// * outbound lane already has state, so it needs to be read and decoded;
 	// * relayers fund account does not exists (in practice it needs to exist in production environment);
 	// * maximal number of messages is being pruned during the call;
-	// * message size is maximal for the target chain.
+	// * message size is minimal for the target chain.
 	//
-	// Results of this benchmark may be directly used in the `send_message`.
-	send_message_worst_case {
+	// Result of this benchmark is used as a base weight for `send_message` call. Then the 'message weight'
+	// (estimated using `send_half_maximal_message_worst_case` and `send_maximal_message_worst_case`) is
+	// added.
+	send_minimal_message_worst_case {
 		let lane_id = bench_lane_id();
 		let sender = account("sender", 0, SEED);
 		T::endow_account(&sender);
@@ -116,7 +116,81 @@ benchmarks_instance! {
 		confirm_message_delivery::<T, I>(T::MaxMessagesToPruneAtOnce::get());
 
 		let (payload, fee) = T::prepare_outbound_message(MessageParams {
-			size_factor: WORST_MESSAGE_SIZE_FACTOR,
+			size: 0,
+			sender_account: sender.clone(),
+		});
+	}: send_message(RawOrigin::Signed(sender), lane_id, payload, fee)
+	verify {
+		assert_eq!(
+			crate::Module::<T, I>::outbound_latest_generated_nonce(bench_lane_id()),
+			T::MaxMessagesToPruneAtOnce::get() + 1,
+		);
+	}
+
+	// Benchmark `send_message` extrinsic with the worst possible conditions:
+	// * outbound lane already has state, so it needs to be read and decoded;
+	// * relayers fund account does not exists (in practice it needs to exist in production environment);
+	// * maximal number of messages is being pruned during the call;
+	// * message size is 1KB.
+	//
+	// With single KB of message size, the weight of the call is increased (roughly) by
+	// `(send_16_kb_message_worst_case - send_1_kb_message_worst_case) / 15`.
+	send_1_kb_message_worst_case {
+		let lane_id = bench_lane_id();
+		let sender = account("sender", 0, SEED);
+		T::endow_account(&sender);
+
+		// 'send' messages that are to be pruned when our message is sent
+		for _nonce in 1..=T::MaxMessagesToPruneAtOnce::get() {
+			send_regular_message::<T, I>();
+		}
+		confirm_message_delivery::<T, I>(T::MaxMessagesToPruneAtOnce::get());
+
+		let size = 1024;
+		assert!(
+			T::maximal_message_size() > size,
+			"This benchmark can only be used with runtime that accepts 1KB messages",
+		);
+
+		let (payload, fee) = T::prepare_outbound_message(MessageParams {
+			size,
+			sender_account: sender.clone(),
+		});
+	}: send_message(RawOrigin::Signed(sender), lane_id, payload, fee)
+	verify {
+		assert_eq!(
+			crate::Module::<T, I>::outbound_latest_generated_nonce(bench_lane_id()),
+			T::MaxMessagesToPruneAtOnce::get() + 1,
+		);
+	}
+
+	// Benchmark `send_message` extrinsic with the worst possible conditions:
+	// * outbound lane already has state, so it needs to be read and decoded;
+	// * relayers fund account does not exists (in practice it needs to exist in production environment);
+	// * maximal number of messages is being pruned during the call;
+	// * message size is 16KB.
+	//
+	// With single KB of message size, the weight of the call is increased (roughly) by
+	// `(send_16_kb_message_worst_case - send_1_kb_message_worst_case) / 15`.
+	send_16_kb_message_worst_case {
+		let lane_id = bench_lane_id();
+		let sender = account("sender", 0, SEED);
+		T::endow_account(&sender);
+
+		// 'send' messages that are to be pruned when our message is sent
+		for _nonce in 1..=T::MaxMessagesToPruneAtOnce::get() {
+			send_regular_message::<T, I>();
+		}
+		confirm_message_delivery::<T, I>(T::MaxMessagesToPruneAtOnce::get());
+
+		let size = 16 * 1024;
+		assert!(
+			T::maximal_message_size() > size,
+			"This benchmark can only be used with runtime that accepts 16KB messages",
+		);
+
+		let (payload, fee) = T::prepare_outbound_message(MessageParams {
+			size,
 			sender_account: sender.clone(),
 		});
 	}: send_message(RawOrigin::Signed(sender), lane_id, payload, fee)
@@ -340,6 +414,38 @@ benchmarks_instance! {
 	// Benchmarks for manual checks.
 	//
 
+	// Benchmark `send_message` extrinsic with following conditions:
+	// * outbound lane already has state, so it needs to be read and decoded;
+	// * relayers fund account does not exists (in practice it needs to exist in production environment);
+	// * maximal number of messages is being pruned during the call;
+	// * message size varies from minimal to maximal for the target chain.
+	//
+	// Results of this benchmark may be used to check how message size affects `send_message` performance.
+	send_messages_of_various_lengths {
+		let i in 0..T::maximal_message_size().try_into().unwrap_or_default();
+
+		let lane_id = bench_lane_id();
+		let sender = account("sender", 0, SEED);
+		T::endow_account(&sender);
+
+		// 'send' messages that are to be pruned when our message is sent
+		for _nonce in 1..=T::MaxMessagesToPruneAtOnce::get() {
+			send_regular_message::<T, I>();
+		}
+		confirm_message_delivery::<T, I>(T::MaxMessagesToPruneAtOnce::get());
+
+		let (payload, fee) = T::prepare_outbound_message(MessageParams {
+			size: i as _,
+			sender_account: sender.clone(),
+		});
+	}: send_message(RawOrigin::Signed(sender), lane_id, payload, fee)
+	verify {
+		assert_eq!(
+			crate::Module::<T, I>::outbound_latest_generated_nonce(bench_lane_id()),
+			T::MaxMessagesToPruneAtOnce::get() + 1,
+		);
+	}
+
 	// Benchmark `receive_messages_proof` extrinsic with multiple minimal-weight messages and following conditions:
 	// * proof does not include outbound lane state proof;
 	// * inbound lane already has state, so it needs to be read and decoded;

bridges/modules/message-lane/src/lib.rs

@@ -39,14 +39,14 @@ pub use crate::weights_ext::{ensure_weights_are_correct, WeightInfoExt};
 
 use crate::inbound_lane::{InboundLane, InboundLaneStorage};
 use crate::outbound_lane::{OutboundLane, OutboundLaneStorage};
-use crate::weights::WeightInfo;
 
 use bp_message_lane::{
 	source_chain::{LaneMessageVerifier, MessageDeliveryAndDispatchPayment, TargetHeaderChain},
 	target_chain::{DispatchMessage, MessageDispatch, ProvedLaneMessages, ProvedMessages, SourceHeaderChain},
 	total_unrewarded_messages, InboundLaneData, LaneId, MessageData, MessageKey, MessageNonce, MessagePayload,
 	OutboundLaneData, UnrewardedRelayersState,
 };
+use bp_runtime::Size;
 use codec::{Decode, Encode};
 use frame_support::{
 	decl_error, decl_event, decl_module, decl_storage, ensure,
@@ -104,7 +104,7 @@ pub trait Config<I = DefaultInstance>: frame_system::Config {
 	type MaxUnconfirmedMessagesAtInboundLane: Get<MessageNonce>;
 
 	/// Payload type of outbound messages. This payload is dispatched on the bridged chain.
-	type OutboundPayload: Parameter;
+	type OutboundPayload: Parameter + Size;
 	/// Message fee type of outbound messages. This fee is paid on this chain.
 	type OutboundMessageFee: From<u32> + Parameter + SaturatingAdd + Zero;
 
@@ -261,10 +261,9 @@ decl_module! {
 		}
 
 		/// Send message over lane.
-		///
-		/// The weight of the call assumes that the largest possible message is sent in
-		/// worst possible environment.
-		#[weight = T::WeightInfo::send_message_worst_case()]
+		#[weight = T::WeightInfo::send_message_overhead()
+			.saturating_add(T::WeightInfo::send_message_size_overhead(Size::size_hint(payload)))
+		]
 		pub fn send_message(
 			origin,
 			lane_id: LaneId,

bridges/modules/message-lane/src/mock.rs

@@ -21,6 +21,7 @@ use bp_message_lane::{
 	target_chain::{DispatchMessage, MessageDispatch, ProvedLaneMessages, ProvedMessages, SourceHeaderChain},
 	InboundLaneData, LaneId, Message, MessageData, MessageKey, MessageNonce,
 };
+use bp_runtime::Size;
 use codec::{Decode, Encode};
 use frame_support::{impl_outer_event, impl_outer_origin, parameter_types, weights::Weight};
 use sp_core::H256;
@@ -32,7 +33,8 @@ use sp_runtime::{
 use std::collections::BTreeMap;
 
 pub type AccountId = u64;
-pub type TestPayload = (u64, Weight);
+#[derive(Decode, Encode, Clone, Debug, PartialEq, Eq)]
+pub struct TestPayload(pub u64, pub Weight);
 pub type TestMessageFee = u64;
 pub type TestRelayer = u64;
 
@@ -123,6 +125,12 @@ impl Config for TestRuntime {
 	type MessageDispatch = TestMessageDispatch;
 }
 
+impl Size for TestPayload {
+	fn size_hint(&self) -> u32 {
+		16
+	}
+}
+
 /// Account id of test relayer.
 pub const TEST_RELAYER_A: AccountId = 100;
 
@@ -139,10 +147,10 @@ pub const TEST_ERROR: &str = "Test error";
 pub const TEST_LANE_ID: LaneId = [0, 0, 0, 1];
 
 /// Regular message payload.
-pub const REGULAR_PAYLOAD: TestPayload = (0, 50);
+pub const REGULAR_PAYLOAD: TestPayload = TestPayload(0, 50);
 
 /// Payload that is rejected by `TestTargetHeaderChain`.
-pub const PAYLOAD_REJECTED_BY_TARGET_CHAIN: TestPayload = (1, 50);
+pub const PAYLOAD_REJECTED_BY_TARGET_CHAIN: TestPayload = TestPayload(1, 50);
 
 /// Vec of proved messages, grouped by lane.
 pub type MessagesByLaneVec = Vec<(LaneId, ProvedLaneMessages<Message<TestMessageFee>>)>;