Merge pull request #3356 from autonomys/xdm_max_outgoing

XDM: ensure max outgoing messages per channel is atleast one and ensure to return open channel that can still accept messages

Author: vedhavyas

Cargo.lock

@@ -34,7 +34,7 @@ use core::num::NonZeroU64;
 use domain_runtime_primitives::opaque::Header as DomainHeader;
 use domain_runtime_primitives::{
     maximum_domain_block_weight, AccountIdConverter, BlockNumber as DomainNumber,
-    Hash as DomainHash,
+    Hash as DomainHash, MAX_OUTGOING_MESSAGES,
 };
 use frame_support::genesis_builder_helper::{build_state, get_preset};
 use frame_support::inherent::ProvideInherent;
@@ -650,8 +650,12 @@ parameter_types! {
     pub const ChannelInitReservePortion: Perbill = Perbill::from_percent(20);
     // TODO update the fee model
     pub const ChannelFeeModel: FeeModel<Balance> = FeeModel{relay_fee: SSC};
+    pub const MaxOutgoingMessages: u32 = MAX_OUTGOING_MESSAGES;
 }
 
+// ensure the max outgoing messages is not 0.
+const_assert!(MaxOutgoingMessages::get() >= 1);
+
 pub struct DomainRegistration;
 impl sp_messenger::DomainRegistration for DomainRegistration {
     fn is_domain_registered(domain_id: DomainId) -> bool {
@@ -684,6 +688,7 @@ impl pallet_messenger::Config for Runtime {
     type ChannelInitReservePortion = ChannelInitReservePortion;
     type DomainRegistration = DomainRegistration;
     type ChannelFeeModel = ChannelFeeModel;
+    type MaxOutgoingMessages = MaxOutgoingMessages;
 }
 
 impl<C> frame_system::offchain::SendTransactionTypes<C> for Runtime

crates/subspace-runtime/src/lib.rs

@@ -3788,9 +3788,6 @@ async fn test_domain_sudo_calls() {
         .construct_and_send_extrinsic(evm_domain_test_runtime::RuntimeCall::Messenger(
             pallet_messenger::Call::initiate_channel {
                 dst_chain_id: ChainId::Consensus,
-                params: pallet_messenger::InitiateChannelParams {
-                    max_outgoing_messages: 100,
-                },
             },
         ))
         .await
@@ -3931,9 +3928,6 @@ async fn test_xdm_between_consensus_and_domain_should_work() {
         .construct_and_send_extrinsic(evm_domain_test_runtime::RuntimeCall::Messenger(
             pallet_messenger::Call::initiate_channel {
                 dst_chain_id: ChainId::Consensus,
-                params: pallet_messenger::InitiateChannelParams {
-                    max_outgoing_messages: 100,
-                },
             },
         ))
         .await
@@ -4144,9 +4138,6 @@ async fn test_xdm_between_domains_should_work() {
     bob.construct_and_send_extrinsic(auto_id_domain_test_runtime::RuntimeCall::Messenger(
         pallet_messenger::Call::initiate_channel {
             dst_chain_id: ChainId::Domain(EVM_DOMAIN_ID),
-            params: pallet_messenger::InitiateChannelParams {
-                max_outgoing_messages: 100,
-            },
         },
     ))
     .await
@@ -4298,9 +4289,6 @@ async fn test_unordered_cross_domains_message_should_work() {
         .construct_and_send_extrinsic(evm_domain_test_runtime::RuntimeCall::Messenger(
             pallet_messenger::Call::initiate_channel {
                 dst_chain_id: ChainId::Consensus,
-                params: pallet_messenger::InitiateChannelParams {
-                    max_outgoing_messages: 100,
-                },
             },
         ))
         .await
@@ -5434,9 +5422,6 @@ async fn test_xdm_false_invalid_fraud_proof() {
         .construct_and_send_extrinsic(evm_domain_test_runtime::RuntimeCall::Messenger(
             pallet_messenger::Call::initiate_channel {
                 dst_chain_id: ChainId::Consensus,
-                params: pallet_messenger::InitiateChannelParams {
-                    max_outgoing_messages: 100,
-                },
             },
         ))
         .await
@@ -5632,9 +5617,6 @@ async fn test_stale_fork_xdm_true_invalid_fraud_proof() {
         .construct_and_send_extrinsic(evm_domain_test_runtime::RuntimeCall::Messenger(
             pallet_messenger::Call::initiate_channel {
                 dst_chain_id: ChainId::Consensus,
-                params: pallet_messenger::InitiateChannelParams {
-                    max_outgoing_messages: 100,
-                },
             },
         ))
         .await

domains/client/domain-operator/src/tests.rs

@@ -33,9 +33,6 @@ mod benchmarks {
     fn initiate_channel() {
         let dst_chain_id: ChainId = u32::MAX.into();
         assert_ne!(T::SelfChainId::get(), dst_chain_id);
-        let channel_params = InitiateChannelParams {
-            max_outgoing_messages: 100,
-        };
         let channel_id = NextChannelId::<T>::get(dst_chain_id);
         let account = account("account", 0, 0);
         T::Currency::set_balance(
@@ -47,11 +44,7 @@ mod benchmarks {
         ChainAllowlist::<T>::put(list);
 
         #[extrinsic_call]
-        _(
-            RawOrigin::Signed(account.clone()),
-            dst_chain_id,
-            channel_params,
-        );
+        _(RawOrigin::Signed(account.clone()), dst_chain_id);
 
         let channel = Channels::<T>::get(dst_chain_id, channel_id).expect("channel should exist");
         assert_eq!(channel.state, ChannelState::Initiated);
@@ -203,6 +196,9 @@ mod benchmarks {
             last_delivered_message_response_nonce: None,
         };
         Outbox::<T>::insert((dst_chain_id, channel_id, next_outbox_nonce), req_msg);
+        OutboxMessageCount::<T>::mutate((dst_chain_id, channel_id), |count| {
+            *count = count.saturating_add(1u32);
+        });
         // Insert a dummy response message which will be handled during the `relay_message_response` call
         let resp_msg: Message<BalanceOf<T>> = Message {
             src_chain_id: dst_chain_id,

domains/pallets/messenger/src/benchmarking.rs

@@ -98,12 +98,6 @@ pub(crate) enum CloseChannelBy<AccountId> {
     Sudo,
 }
 
-/// Parameters for a new channel between two chains.
-#[derive(Default, Debug, Encode, Decode, Clone, Eq, PartialEq, TypeInfo, Copy)]
-pub struct InitiateChannelParams {
-    pub max_outgoing_messages: u32,
-}
-
 /// Hold identifier trait for messenger specific balance holds
 pub trait HoldIdentifier<T: Config> {
     fn messenger_channel() -> FungibleHoldId<T>;
@@ -114,8 +108,8 @@ mod pallet {
     use crate::weights::WeightInfo;
     use crate::{
         BalanceOf, ChainAllowlistUpdate, Channel, ChannelId, ChannelState, CloseChannelBy,
-        FeeModel, HoldIdentifier, InitiateChannelParams, Nonce, OutboxMessageResult, StateRootOf,
-        ValidatedRelayMessage, U256,
+        FeeModel, HoldIdentifier, Nonce, OutboxMessageResult, StateRootOf, ValidatedRelayMessage,
+        U256,
     };
     #[cfg(not(feature = "std"))]
     use alloc::boxed::Box;
@@ -191,6 +185,8 @@ mod pallet {
         type DomainRegistration: DomainRegistration;
         /// Channels fee model
         type ChannelFeeModel: Get<FeeModel<BalanceOf<Self>>>;
+        /// Maximum outgoing messages from a given channel
+        type MaxOutgoingMessages: Get<u32>;
     }
 
     /// Pallet messenger used to communicate between chains and other blockchains.
@@ -243,25 +239,21 @@ mod pallet {
     /// Used by the dst_chains to verify the message response.
     #[pallet::storage]
     #[pallet::getter(fn inbox_responses)]
-    pub(super) type InboxResponses<T: Config> = CountedStorageMap<
-        _,
-        Identity,
-        (ChainId, ChannelId, Nonce),
-        Message<BalanceOf<T>>,
-        OptionQuery,
-    >;
+    pub(super) type InboxResponses<T: Config> =
+        StorageMap<_, Identity, (ChainId, ChannelId, Nonce), Message<BalanceOf<T>>, OptionQuery>;
 
     /// Stores the outgoing messages that are awaiting message responses from the dst_chain.
     /// Messages are processed in the outbox nonce order of chain's channel.
     #[pallet::storage]
     #[pallet::getter(fn outbox)]
-    pub(super) type Outbox<T: Config> = CountedStorageMap<
-        _,
-        Identity,
-        (ChainId, ChannelId, Nonce),
-        Message<BalanceOf<T>>,
-        OptionQuery,
-    >;
+    pub(super) type Outbox<T: Config> =
+        StorageMap<_, Identity, (ChainId, ChannelId, Nonce), Message<BalanceOf<T>>, OptionQuery>;
+
+    /// Stores the outgoing messages count that are awaiting message responses from the dst_chain.
+    #[pallet::storage]
+    #[pallet::getter(fn outbox_message_count)]
+    pub(super) type OutboxMessageCount<T: Config> =
+        StorageMap<_, Identity, (ChainId, ChannelId), u32, ValueQuery>;
 
     /// A temporary storage for storing decoded outbox response message between `pre_dispatch_relay_message_response`
     /// and `relay_message_response`.
@@ -545,6 +537,15 @@ mod pallet {
 
         /// Invalid channel reserve fee
         InvalidChannelReserveFee,
+
+        /// Invalid max outgoing messages
+        InvalidMaxOutgoingMessages,
+
+        /// Message count overflow
+        MessageCountOverflow,
+
+        /// Message count underflow
+        MessageCountUnderflow,
     }
 
     #[pallet::call]
@@ -554,11 +555,7 @@ mod pallet {
         /// Channel is set to initiated and do not accept or receive any messages.
         #[pallet::call_index(0)]
         #[pallet::weight(T::WeightInfo::initiate_channel())]
-        pub fn initiate_channel(
-            origin: OriginFor<T>,
-            dst_chain_id: ChainId,
-            params: InitiateChannelParams,
-        ) -> DispatchResult {
+        pub fn initiate_channel(origin: OriginFor<T>, dst_chain_id: ChainId) -> DispatchResult {
             let owner = ensure_signed(origin)?;
 
             // reserve channel open fees
@@ -575,7 +572,7 @@ mod pallet {
 
             // initiate the channel config
             let channel_open_params = ChannelOpenParams {
-                max_outgoing_messages: params.max_outgoing_messages,
+                max_outgoing_messages: T::MaxOutgoingMessages::get(),
                 fee_model: T::ChannelFeeModel::get(),
             };
             let channel_id = Self::do_init_channel(
@@ -907,8 +904,11 @@ mod pallet {
             // loop through channels in descending order until open channel is found.
             // we always prefer latest opened channel.
             while let Some(channel_id) = next_channel_id.checked_sub(ChannelId::one()) {
+                let message_count = OutboxMessageCount::<T>::get((dst_chain_id, channel_id));
                 if let Some(channel) = Channels::<T>::get(dst_chain_id, channel_id) {
-                    if channel.state == ChannelState::Open {
+                    if channel.state == ChannelState::Open
+                        && message_count < channel.max_outgoing_messages
+                    {
                         return Some((channel_id, channel.fee));
                     }
                 }
@@ -1006,6 +1006,12 @@ mod pallet {
                 Error::<T>::InvalidChain,
             );
 
+            // ensure max outgoing messages is at least 1
+            ensure!(
+                init_params.max_outgoing_messages >= 1u32,
+                Error::<T>::InvalidMaxOutgoingMessages
+            );
+
             // If the channel owner is in this chain then the channel reserve fee
             // must not be empty
             ensure!(

domains/pallets/messenger/src/lib.rs

@@ -1,7 +1,7 @@
 #[cfg(not(feature = "std"))]
 extern crate alloc;
 
-use crate::pallet::{ChainAllowlist, UpdatedChannels};
+use crate::pallet::{ChainAllowlist, OutboxMessageCount, UpdatedChannels};
 use crate::{
     BalanceOf, ChannelId, ChannelState, Channels, CloseChannelBy, Config, Error, Event,
     InboxResponses, MessageWeightTags as MessageWeightTagStore, Nonce, Outbox, OutboxMessageResult,
@@ -52,7 +52,7 @@ impl<T: Config> Pallet<T> {
             |maybe_channel| -> Result<Nonce, DispatchError> {
                 let channel = maybe_channel.as_mut().ok_or(Error::<T>::MissingChannel)?;
                 // check if the outbox is full
-                let count = Outbox::<T>::count();
+                let count = OutboxMessageCount::<T>::get((dst_chain_id, channel_id));
                 ensure!(
                     count < channel.max_outgoing_messages,
                     Error::<T>::OutboxFull
@@ -72,6 +72,15 @@ impl<T: Config> Pallet<T> {
                         .latest_response_received_message_nonce,
                 };
                 Outbox::<T>::insert((dst_chain_id, channel_id, next_outbox_nonce), msg);
+                OutboxMessageCount::<T>::try_mutate(
+                    (dst_chain_id, channel_id),
+                    |count| -> Result<(), DispatchError> {
+                        *count = count
+                            .checked_add(1u32)
+                            .ok_or(Error::<T>::MessageCountOverflow)?;
+                        Ok(())
+                    },
+                )?;
 
                 // update channel state
                 channel.next_outbox_nonce = next_outbox_nonce
@@ -340,6 +349,16 @@ impl<T: Config> Pallet<T> {
         let req_msg = Outbox::<T>::take((dst_chain_id, channel_id, nonce))
             .ok_or(Error::<T>::MissingMessage)?;
 
+        OutboxMessageCount::<T>::try_mutate(
+            (dst_chain_id, channel_id),
+            |count| -> Result<(), DispatchError> {
+                *count = count
+                    .checked_sub(1u32)
+                    .ok_or(Error::<T>::MessageCountUnderflow)?;
+                Ok(())
+            },
+        )?;
+
         // clear out box message weight tag
         MessageWeightTagStore::<T>::mutate(|maybe_messages| {
             let mut messages = maybe_messages.as_mut().cloned().unwrap_or_default();

domains/pallets/messenger/src/messages.rs

@@ -60,6 +60,7 @@ macro_rules! impl_runtime {
             pub const ChannelReserveFee: Balance = 10;
             pub const ChannelInitReservePortion: Perbill = Perbill::from_percent(20);
             pub const ChannelFeeModel: FeeModel<Balance> = FeeModel{relay_fee: 1};
+            pub const MaxOutgoingMessages: u32 = 25;
         }
 
         #[derive(
@@ -102,6 +103,7 @@ macro_rules! impl_runtime {
             type HoldIdentifier = MockHoldIdentifer;
             type DomainRegistration = DomainRegistration;
             type ChannelFeeModel = ChannelFeeModel;
+            type MaxOutgoingMessages = MaxOutgoingMessages;
             /// function to fetch endpoint response handler by Endpoint.
             fn get_endpoint_handler(
                 #[allow(unused_variables)] endpoint: &Endpoint,