Update weight related parameters in relay (#662)

* update weight-related parameters in relay

* asserts + docs

Co-authored-by: Hernando Castano <castano.ha@gmail.com>

bridges/relays/substrate/Cargo.toml

@@ -29,6 +29,7 @@ headers-relay = { path = "../headers-relay" }
 messages-relay = { path = "../messages-relay" }
 millau-runtime = { path = "../../bin/millau/runtime" }
 pallet-bridge-call-dispatch = { path = "../../modules/call-dispatch" }
+pallet-message-lane = { path = "../../modules/message-lane" }
 pallet-substrate-bridge = { path = "../../modules/substrate" }
 relay-kusama-client = { path = "../kusama-client" }
 relay-millau-client = { path = "../millau-client" }

bridges/relays/substrate/src/messages_lane.rs

@@ -20,6 +20,7 @@ use crate::messages_target::SubstrateMessagesReceivingProof;
 use async_trait::async_trait;
 use bp_message_lane::MessageNonce;
 use codec::Encode;
+use frame_support::weights::Weight;
 use messages_relay::message_lane::{MessageLane, SourceHeaderIdOf, TargetHeaderIdOf};
 use relay_substrate_client::{BlockNumberOf, Chain, Client, Error as SubstrateError, HashOf};
 use relay_utils::BlockNumberBase;
@@ -117,3 +118,65 @@ where
 	type TargetHeaderNumber = BlockNumberOf<Target>;
 	type TargetHeaderHash = HashOf<Target>;
 }
+
+/// Returns maximal number of messages and their maximal cumulative dispatch weight, based
+/// on given chain parameters.
+pub fn select_delivery_transaction_limits<W: pallet_message_lane::WeightInfoExt>(
+	max_extrinsic_weight: Weight,
+	max_unconfirmed_messages_at_inbound_lane: MessageNonce,
+) -> (MessageNonce, Weight) {
+	// We may try to guess accurate value, based on maximal number of messages and per-message
+	// weight overhead, but the relay loop isn't using this info in a super-accurate way anyway.
+	// So just a rough guess: let's say 1/3 of max tx weight is for tx itself and the rest is
+	// for messages dispatch.
+
+	// Another thing to keep in mind is that our runtimes (when this code was written) accept
+	// messages with dispatch weight <= max_extrinsic_weight/2. So we can't reserve less than
+	// that for dispatch.
+
+	let weight_for_delivery_tx = max_extrinsic_weight / 3;
+	let weight_for_messages_dispatch = max_extrinsic_weight - weight_for_delivery_tx;
+
+	let delivery_tx_base_weight =
+		W::receive_messages_proof_overhead() + W::receive_messages_proof_outbound_lane_state_overhead();
+	let delivery_tx_weight_rest = weight_for_delivery_tx - delivery_tx_base_weight;
+	let max_number_of_messages = std::cmp::min(
+		delivery_tx_weight_rest / W::receive_messages_proof_messages_overhead(1),
+		max_unconfirmed_messages_at_inbound_lane,
+	);
+
+	assert!(
+		max_number_of_messages > 0,
+		"Relay should fit at least one message in every delivery transaction",
+	);
+	assert!(
+		weight_for_messages_dispatch >= max_extrinsic_weight / 2,
+		"Relay shall be able to deliver messages with dispatch weight = max_extrinsic_weight / 2",
+	);
+
+	(max_number_of_messages, weight_for_messages_dispatch)
+}
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+
+	type RialtoToMillauMessageLaneWeights = pallet_message_lane::weights::RialtoWeight<rialto_runtime::Runtime>;
+
+	#[test]
+	fn select_delivery_transaction_limits_works() {
+		let (max_count, max_weight) = select_delivery_transaction_limits::<RialtoToMillauMessageLaneWeights>(
+			bp_rialto::max_extrinsic_weight(),
+			bp_millau::MAX_UNREWARDED_RELAYER_ENTRIES_AT_INBOUND_LANE,
+		);
+		assert_eq!(
+			(max_count, max_weight),
+			// We don't actually care about these values, so feel free to update them whenever test
+			// fails. The only thing to do before that is to ensure that new values looks sane: i.e. weight
+			// reserved for messages dispatch allows dispatch of non-trivial messages.
+			//
+			// Any significant change in this values should attract additional attention.
+			(1024, 866_583_333_334),
+		);
+	}
+}

bridges/relays/substrate/src/millau_messages_to_rialto.rs

@@ -16,7 +16,7 @@
 
 //! Millau-to-Rialto messages sync entrypoint.
 
-use crate::messages_lane::{SubstrateMessageLane, SubstrateMessageLaneToSubstrate};
+use crate::messages_lane::{select_delivery_transaction_limits, SubstrateMessageLane, SubstrateMessageLaneToSubstrate};
 use crate::messages_source::SubstrateMessagesSource;
 use crate::messages_target::SubstrateMessagesTarget;
 use crate::{MillauClient, RialtoClient};
@@ -123,13 +123,12 @@ pub fn run(
 		lane.relayer_id_at_source,
 	);
 
-	// TODO: these two parameters need to be updated after https://github.com/paritytech/parity-bridges-common/issues/78
+	// TODO: use Millau weights after https://github.com/paritytech/parity-bridges-common/issues/390
-	// the rough idea is to reserve some portion (1/3?) of max extrinsic weight for delivery tx overhead + messages
+	let (max_messages_in_single_batch, max_messages_weight_in_single_batch) =
-	// overhead
+		select_delivery_transaction_limits::<pallet_message_lane::weights::RialtoWeight<millau_runtime::Runtime>>(
-	// this must be tuned mostly with `max_messages_in_single_batch`, but `max_messages_weight_in_single_batch` also
+			bp_millau::max_extrinsic_weight(),
-	// needs to be updated (subtract tx overhead)
+			bp_rialto::MAX_UNREWARDED_RELAYER_ENTRIES_AT_INBOUND_LANE,
-	let max_messages_in_single_batch = 1024;
+		);
-	let max_messages_weight_in_single_batch = bp_rialto::max_extrinsic_weight();
 
 	messages_relay::message_lane_loop::run(
 		messages_relay::message_lane_loop::Params {

bridges/relays/substrate/src/rialto_messages_to_millau.rs

@@ -16,7 +16,7 @@
 
 //! Rialto-to-Millau messages sync entrypoint.
 
-use crate::messages_lane::{SubstrateMessageLane, SubstrateMessageLaneToSubstrate};
+use crate::messages_lane::{select_delivery_transaction_limits, SubstrateMessageLane, SubstrateMessageLaneToSubstrate};
 use crate::messages_source::SubstrateMessagesSource;
 use crate::messages_target::SubstrateMessagesTarget;
 use crate::{MillauClient, RialtoClient};
@@ -123,13 +123,11 @@ pub fn run(
 		lane.relayer_id_at_source,
 	);
 
-	// TODO: these two parameters need to be updated after https://github.com/paritytech/parity-bridges-common/issues/78
+	let (max_messages_in_single_batch, max_messages_weight_in_single_batch) =
-	// the rough idea is to reserve some portion (1/3?) of max extrinsic weight for delivery tx overhead + messages
+		select_delivery_transaction_limits::<pallet_message_lane::weights::RialtoWeight<rialto_runtime::Runtime>>(
-	// overhead
+			bp_rialto::max_extrinsic_weight(),
-	// this must be tuned mostly with `max_messages_in_single_batch`, but `max_messages_weight_in_single_batch` also
+			bp_millau::MAX_UNREWARDED_RELAYER_ENTRIES_AT_INBOUND_LANE,
-	// needs to be updated (subtract tx overhead)
+		);
-	let max_messages_in_single_batch = 1024;
-	let max_messages_weight_in_single_batch = bp_rialto::max_extrinsic_weight();
 
 	messages_relay::message_lane_loop::run(
 		messages_relay::message_lane_loop::Params {