Merge pull request #3395 from autonomys/xdm-fee-fix

Ensure to distribute the XDM fee regardless what the XDM execution result is

domains/pallets/messenger/src/fees.rs

@@ -7,7 +7,7 @@ use sp_messenger::endpoint::Endpoint;
 use sp_messenger::messages::{ChainId, ChannelId, FeeModel, MessageId, Nonce};
 use sp_messenger::OnXDMRewards;
 use sp_runtime::traits::CheckedAdd;
-use sp_runtime::DispatchResult;
+use sp_runtime::{DispatchResult, Saturating};
 
 impl<T: Config> Pallet<T> {
     /// Ensures the fees from the sender per FeeModel provided for a single request for a response.
@@ -58,15 +58,17 @@ impl<T: Config> Pallet<T> {
         message_id: (ChainId, MessageId),
         fee_model: &FeeModel<BalanceOf<T>>,
         endpoint: &Endpoint,
-    ) -> DispatchResult {
-        let handler = T::get_endpoint_handler(endpoint).ok_or(Error::<T>::NoMessageHandler)?;
-        let inbox_execution_fee = T::WeightToFee::weight_to_fee(&handler.message_weight());
-        let inbox_fee = inbox_execution_fee
-            .checked_add(&fee_model.relay_fee)
-            .ok_or(Error::<T>::BalanceOverflow)?;
+    ) {
+        let mut inbox_fee = fee_model.relay_fee;
+
+        // If the endpoint handler does not exist the message won't be handled thus it is okay
+        // to not add the execution fee in this case
+        if let Some(handler) = T::get_endpoint_handler(endpoint) {
+            let inbox_execution_fee = T::WeightToFee::weight_to_fee(&handler.message_weight());
+            inbox_fee = inbox_fee.saturating_add(inbox_execution_fee);
+        }
 
         InboxFee::<T>::insert(message_id, inbox_fee);
-        Ok(())
     }
 
     /// Rewards operators for executing an inbox message since src_chain signalled that responses are delivered.

domains/pallets/messenger/src/messages.rs

@@ -16,7 +16,7 @@ use frame_support::ensure;
 use scale_info::TypeInfo;
 use sp_messenger::endpoint::{EndpointHandler, EndpointRequest, EndpointResponse};
 use sp_messenger::messages::{
-    BlockMessageWithStorageKey, BlockMessagesWithStorageKey, ChainId, FeeModel, Message, MessageId,
+    BlockMessageWithStorageKey, BlockMessagesWithStorageKey, ChainId, Message, MessageId,
     MessageWeightTag, Payload, ProtocolMessageRequest, ProtocolMessageResponse, RequestResponse,
     VersionedPayload,
 };
@@ -136,6 +136,14 @@ impl<T: Config> Pallet<T> {
 
             // process incoming endpoint message.
             VersionedPayload::V0(Payload::Endpoint(RequestResponse::Request(req))) => {
+                // Firstly, store fees for inbox message execution regardless what the execution result is,
+                // since the fee is already charged from the sender of the src chain and processing of the
+                // XDM in this end is finished.
+                Self::store_fees_for_inbox_message(
+                    (dst_chain_id, (channel_id, nonce)),
+                    &channel.fee,
+                    &req.src_endpoint,
+                );
                 let response =
                     if let Some(endpoint_handler) = T::get_endpoint_handler(&req.dst_endpoint) {
                         Self::process_incoming_endpoint_message_req(
@@ -144,7 +152,6 @@ impl<T: Config> Pallet<T> {
                             channel_id,
                             nonce,
                             &msg_weight_tag,
-                            &channel.fee,
                             endpoint_handler,
                         )
                     } else {
@@ -228,7 +235,6 @@ impl<T: Config> Pallet<T> {
         channel_id: ChannelId,
         nonce: Nonce,
         msg_weight_tag: &MessageWeightTag,
-        fee: &FeeModel<BalanceOf<T>>,
         endpoint_handler: Box<dyn sp_messenger::endpoint::EndpointHandler<MessageId>>,
     ) -> EndpointResponse {
         if !ChainAllowlist::<T>::get().contains(&dst_chain_id) {
@@ -245,13 +251,6 @@ impl<T: Config> Pallet<T> {
             return Err(Error::<T>::InvalidChannelState.into());
         }
 
-        // store fees for inbox message execution
-        Self::store_fees_for_inbox_message(
-            (dst_chain_id, (channel_id, nonce)),
-            fee,
-            &req.src_endpoint,
-        )?;
-
         endpoint_handler.message(dst_chain_id, (channel_id, nonce), req)
     }
 
@@ -321,11 +320,7 @@ impl<T: Config> Pallet<T> {
             return Err(Error::<T>::WeightTagNotMatch.into());
         }
 
-        let resp = endpoint_handler.message_response(dst_chain_id, (channel_id, nonce), req, resp);
-
-        Self::reward_operators_for_outbox_execution(dst_chain_id, (channel_id, nonce));
-
-        resp
+        endpoint_handler.message_response(dst_chain_id, (channel_id, nonce), req, resp)
     }
 
     pub(crate) fn process_outbox_message_responses(
@@ -384,6 +379,10 @@ impl<T: Config> Pallet<T> {
                 VersionedPayload::V0(Payload::Endpoint(RequestResponse::Request(req))),
                 VersionedPayload::V0(Payload::Endpoint(RequestResponse::Response(resp))),
             ) => {
+                // Firstly, distribute the fees for outbox message execution regardless what the result is,
+                // since the fee is already charged from the sender and the processing of the XDM is finished.
+                Self::reward_operators_for_outbox_execution(dst_chain_id, (channel_id, nonce));
+
                 if let Some(endpoint_handler) = T::get_endpoint_handler(&req.dst_endpoint) {
                     Self::process_incoming_endpoint_message_response(
                         dst_chain_id,

domains/pallets/messenger/src/tests.rs

@@ -9,8 +9,8 @@ use crate::mock::{
 use crate::pallet::OutboxMessageCount;
 use crate::{
     ChainAllowlist, ChainAllowlistUpdate, Channel, ChannelId, ChannelState, Channels,
-    CloseChannelBy, Error, FeeModel, Inbox, InboxResponses, Nonce, Outbox, OutboxMessageResult,
-    OutboxResponses, Pallet, U256,
+    CloseChannelBy, Error, FeeModel, Inbox, InboxFee, InboxResponses, Nonce, Outbox,
+    OutboxMessageResult, OutboxResponses, Pallet, U256,
 };
 use frame_support::traits::fungible::Inspect;
 use frame_support::traits::tokens::{Fortitude, Preservation};
@@ -1068,7 +1068,13 @@ fn test_transport_funds_between_chains_if_dst_chain_disallows_after_message_is_s
     let post_response_balance =
         chain_a_test_ext.execute_with(|| chain_a::Balances::total_balance(&account_id));
     // The transferred fund should be refunded
-    assert_eq!(post_response_balance, post_transfer_balance + 500)
+    assert_eq!(post_response_balance, post_transfer_balance + 500);
+
+    // The relayer fee and inbox execution fee should be moved to the dst chain and
+    // distribute to the operator of the dst chain later
+    chain_b_test_ext.execute_with(|| {
+        assert_eq!(1, InboxFee::<chain_b::Runtime>::iter().count());
+    });
 }
 
 #[test]