Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Feb 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions domains/pallets/messenger/src/fees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
29 changes: 14 additions & 15 deletions domains/pallets/messenger/src/messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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(
Expand All @@ -144,7 +152,6 @@ impl<T: Config> Pallet<T> {
channel_id,
nonce,
&msg_weight_tag,
&channel.fee,
endpoint_handler,
)
} else {
Expand Down Expand Up @@ -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) {
Expand All @@ -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)
}

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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,
Expand Down
12 changes: 9 additions & 3 deletions domains/pallets/messenger/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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]
Expand Down
Loading