diff --git a/bridges/primitives/runtime/src/messages.rs b/bridges/primitives/runtime/src/messages.rs index 053b44ff04..f6e04619c7 100644 --- a/bridges/primitives/runtime/src/messages.rs +++ b/bridges/primitives/runtime/src/messages.rs @@ -20,7 +20,7 @@ use codec::{Decode, Encode}; use frame_support::{weights::Weight, RuntimeDebug}; /// Where message dispatch fee is paid? -#[derive(Encode, Decode, RuntimeDebug, Clone, PartialEq, Eq)] +#[derive(Encode, Decode, RuntimeDebug, Clone, Copy, PartialEq, Eq)] pub enum DispatchFeePayment { /// The dispacth fee is paid at the source chain. AtSourceChain, diff --git a/bridges/relays/bin-substrate/src/messages_source.rs b/bridges/relays/bin-substrate/src/messages_source.rs index 8b460ea099..35c5b8da19 100644 --- a/bridges/relays/bin-substrate/src/messages_source.rs +++ b/bridges/relays/bin-substrate/src/messages_source.rs @@ -23,7 +23,7 @@ use crate::on_demand_headers::OnDemandHeadersRelay; use async_trait::async_trait; use bp_messages::{LaneId, MessageNonce}; -use bp_runtime::ChainId; +use bp_runtime::{messages::DispatchFeePayment, ChainId}; use bridge_runtime_common::messages::target::FromBridgedChainMessagesProof; use codec::{Decode, Encode}; use frame_support::{traits::Instance, weights::Weight}; @@ -351,6 +351,7 @@ fn make_message_details_map( size: details.size as _, // TODO: https://github.com/paritytech/parity-bridges-common/issues/997 reward: num_traits::Zero::zero(), + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, }, ); expected_nonce = details.nonce + 1; @@ -363,7 +364,6 @@ fn make_message_details_map( #[cfg(test)] mod tests { use super::*; - use bp_runtime::messages::DispatchFeePayment; fn message_details_from_rpc( nonces: RangeInclusive, @@ -390,7 +390,8 @@ mod tests { MessageDetails { dispatch_weight: 0, size: 0, - reward: 0 + reward: 0, + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, } ), ( @@ -398,7 +399,8 @@ mod tests { MessageDetails { dispatch_weight: 0, size: 0, - reward: 0 + reward: 0, + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, } ), ( @@ -406,7 +408,8 @@ mod tests { MessageDetails { dispatch_weight: 0, size: 0, - reward: 0 + reward: 0, + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, } ), ] @@ -425,7 +428,8 @@ mod tests { MessageDetails { dispatch_weight: 0, size: 0, - reward: 0 + reward: 0, + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, } ), ( @@ -433,7 +437,8 @@ mod tests { MessageDetails { dispatch_weight: 0, size: 0, - reward: 0 + reward: 0, + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, } ), ] diff --git a/bridges/relays/messages/Cargo.toml b/bridges/relays/messages/Cargo.toml index 943a3b7d9c..ea5d46845c 100644 --- a/bridges/relays/messages/Cargo.toml +++ b/bridges/relays/messages/Cargo.toml @@ -17,4 +17,5 @@ parking_lot = "0.11.0" # Bridge Dependencies bp-messages = { path = "../../primitives/messages" } +bp-runtime = { path = "../../primitives/runtime" } relay-utils = { path = "../utils" } diff --git a/bridges/relays/messages/src/message_lane_loop.rs b/bridges/relays/messages/src/message_lane_loop.rs index 3ae1cb8a03..c87c024f39 100644 --- a/bridges/relays/messages/src/message_lane_loop.rs +++ b/bridges/relays/messages/src/message_lane_loop.rs @@ -31,6 +31,7 @@ use crate::metrics::MessageLaneLoopMetrics; use async_trait::async_trait; use bp_messages::{LaneId, MessageNonce, UnrewardedRelayersState, Weight}; +use bp_runtime::messages::DispatchFeePayment; use futures::{channel::mpsc::unbounded, future::FutureExt, stream::StreamExt}; use relay_utils::{ interval, @@ -97,6 +98,8 @@ pub struct MessageDetails { pub size: u32, /// The relayer reward paid in the source chain tokens. pub reward: SourceChainBalance, + /// Where the fee for dispatching message is paid? + pub dispatch_fee_payment: DispatchFeePayment, } /// Messages details map. @@ -454,7 +457,7 @@ pub(crate) mod tests { } pub const CONFIRMATION_TRANSACTION_COST: TestSourceChainBalance = 1; - pub const DELIVERY_TRANSACTION_COST: TestSourceChainBalance = 1; + pub const BASE_MESSAGE_DELIVERY_TRANSACTION_COST: TestSourceChainBalance = 1; pub type TestSourceChainBalance = u64; pub type TestSourceHeaderId = HeaderId; @@ -590,6 +593,7 @@ pub(crate) mod tests { dispatch_weight: 1, size: 1, reward: 1, + dispatch_fee_payment: DispatchFeePayment::AtSourceChain, }, ) }) @@ -768,11 +772,13 @@ pub(crate) mod tests { async fn estimate_delivery_transaction_in_source_tokens( &self, - _nonces: RangeInclusive, - _total_dispatch_weight: Weight, + nonces: RangeInclusive, + total_dispatch_weight: Weight, total_size: u32, ) -> TestSourceChainBalance { - DELIVERY_TRANSACTION_COST * (total_size as TestSourceChainBalance) + BASE_MESSAGE_DELIVERY_TRANSACTION_COST * (nonces.end() - nonces.start() + 1) + + total_dispatch_weight + + total_size as TestSourceChainBalance } } diff --git a/bridges/relays/messages/src/message_race_delivery.rs b/bridges/relays/messages/src/message_race_delivery.rs index c4a5b235a5..bde09af706 100644 --- a/bridges/relays/messages/src/message_race_delivery.rs +++ b/bridges/relays/messages/src/message_race_delivery.rs @@ -28,6 +28,7 @@ use crate::metrics::MessageLaneLoopMetrics; use async_trait::async_trait; use bp_messages::{MessageNonce, UnrewardedRelayersState, Weight}; +use bp_runtime::messages::DispatchFeePayment; use futures::stream::FusedStream; use num_traits::{SaturatingAdd, Zero}; use relay_utils::FailedClient; @@ -560,6 +561,7 @@ async fn select_nonces_for_delivery_transaction( let mut soft_selected_count = 0; let mut selected_weight: Weight = 0; + let mut selected_unpaid_weight: Weight = 0; let mut selected_size: u32 = 0; let mut selected_count: MessageNonce = 0; @@ -627,6 +629,18 @@ async fn select_nonces_for_delivery_transaction( break; } + // If dispatch fee has been paid at the source chain, it means that it is **relayer** who's + // paying for dispatch at the target chain AND reward must cover this dispatch fee. + // + // If dispatch fee is paid at the target chain, it means that it'll be withdrawn from the + // dispatch origin account AND reward is not covering this fee. + // + // So in the latter case we're not adding the dispatch weight to the delivery transaction weight. + let new_selected_unpaid_weight = match details.dispatch_fee_payment { + DispatchFeePayment::AtSourceChain => selected_unpaid_weight.saturating_add(details.dispatch_weight), + DispatchFeePayment::AtTargetChain => selected_unpaid_weight, + }; + // now the message has passed all 'strong' checks, and we CAN deliver it. But do we WANT // to deliver it? It depends on the relayer strategy. match relayer_mode { @@ -637,7 +651,7 @@ async fn select_nonces_for_delivery_transaction( let delivery_transaction_cost = lane_target_client .estimate_delivery_transaction_in_source_tokens( 0..=(new_selected_count as MessageNonce - 1), - new_selected_weight, + new_selected_unpaid_weight, new_selected_size as u32, ) .await; @@ -685,6 +699,7 @@ async fn select_nonces_for_delivery_transaction( hard_selected_count = index + 1; selected_weight = new_selected_weight; + selected_unpaid_weight = new_selected_unpaid_weight; selected_size = new_selected_size; selected_count = new_selected_count; } @@ -740,13 +755,19 @@ mod tests { use crate::message_lane_loop::{ tests::{ header_id, TestMessageLane, TestMessagesProof, TestSourceChainBalance, TestSourceClient, - TestSourceHeaderId, TestTargetClient, TestTargetHeaderId, CONFIRMATION_TRANSACTION_COST, - DELIVERY_TRANSACTION_COST, + TestSourceHeaderId, TestTargetClient, TestTargetHeaderId, BASE_MESSAGE_DELIVERY_TRANSACTION_COST, + CONFIRMATION_TRANSACTION_COST, }, MessageDetails, }; + use bp_runtime::messages::DispatchFeePayment::*; - const DEFAULT_REWARD: TestSourceChainBalance = CONFIRMATION_TRANSACTION_COST + DELIVERY_TRANSACTION_COST; + const DEFAULT_DISPATCH_WEIGHT: Weight = 1; + const DEFAULT_SIZE: u32 = 1; + const DEFAULT_REWARD: TestSourceChainBalance = CONFIRMATION_TRANSACTION_COST + + BASE_MESSAGE_DELIVERY_TRANSACTION_COST + + DEFAULT_DISPATCH_WEIGHT + + (DEFAULT_SIZE as TestSourceChainBalance); type TestRaceState = RaceState; type TestStrategy = MessageDeliveryStrategy; @@ -755,6 +776,7 @@ mod tests { new_nonces: RangeInclusive, confirmed_nonce: MessageNonce, reward: TestSourceChainBalance, + dispatch_fee_payment: DispatchFeePayment, ) -> SourceClientNonces> { SourceClientNonces { new_nonces: new_nonces @@ -763,9 +785,10 @@ mod tests { ( nonce, MessageDetails { - dispatch_weight: 1, - size: 1, + dispatch_weight: DEFAULT_DISPATCH_WEIGHT, + size: DEFAULT_SIZE, reward, + dispatch_fee_payment, }, ) }) @@ -811,7 +834,7 @@ mod tests { race_strategy .strategy - .source_nonces_updated(header_id(1), source_nonces(20..=23, 19, DEFAULT_REWARD)); + .source_nonces_updated(header_id(1), source_nonces(20..=23, 19, DEFAULT_REWARD, AtSourceChain)); let target_nonces = TargetClientNonces { latest_nonce: 19, @@ -845,6 +868,7 @@ mod tests { dispatch_weight: idx, size: idx as _, reward: idx as _, + dispatch_fee_payment: AtSourceChain, }, ) }) @@ -1135,7 +1159,12 @@ mod tests { #[async_std::test] async fn no_losses_relayer_is_not_delivering_messages_if_cost_is_larger_than_reward() { let (mut state, mut strategy) = prepare_strategy(); - let nonces = source_nonces(24..=25, 19, DEFAULT_REWARD - DELIVERY_TRANSACTION_COST); + let nonces = source_nonces( + 24..=25, + 19, + DEFAULT_REWARD - BASE_MESSAGE_DELIVERY_TRANSACTION_COST, + AtSourceChain, + ); strategy.strategy.source_nonces_updated(header_id(2), nonces); state.best_finalized_source_header_id_at_best_target = Some(header_id(2)); strategy.relayer_mode = RelayerMode::NoLosses; @@ -1150,6 +1179,46 @@ mod tests { ); } + #[async_std::test] + async fn no_losses_relayer_is_delivering_unpaid_messages() { + async fn test_with_dispatch_fee_payment( + dispatch_fee_payment: DispatchFeePayment, + ) -> Option<(RangeInclusive, MessageProofParameters)> { + let (mut state, mut strategy) = prepare_strategy(); + let nonces = source_nonces( + 24..=24, + 19, + DEFAULT_REWARD - DEFAULT_DISPATCH_WEIGHT, + dispatch_fee_payment, + ); + strategy.strategy.source_nonces_updated(header_id(2), nonces); + state.best_finalized_source_header_id_at_best_target = Some(header_id(2)); + strategy.max_unrewarded_relayer_entries_at_target = 100; + strategy.max_unconfirmed_nonces_at_target = 100; + strategy.max_messages_in_single_batch = 100; + strategy.max_messages_weight_in_single_batch = 100; + strategy.max_messages_size_in_single_batch = 100; + strategy.relayer_mode = RelayerMode::NoLosses; + + // so now we have: + // - 20..=23 with reward = cost + // - 24..=24 with reward less than cost, but we're deducting `DEFAULT_DISPATCH_WEIGHT` from the + // cost, so it should be fine; + // => when MSG#24 fee is paid at the target chain, strategy shall select all 20..=24 + // => when MSG#25 fee is paid at the source chain, strategy shall only select 20..=23 + strategy.select_nonces_to_deliver(state).await + } + + assert_eq!( + test_with_dispatch_fee_payment(AtTargetChain).await, + Some(((20..=24), proof_parameters(false, 5))) + ); + assert_eq!( + test_with_dispatch_fee_payment(AtSourceChain).await, + Some(((20..=23), proof_parameters(false, 4))) + ); + } + #[async_std::test] async fn relayer_uses_flattened_view_of_the_source_queue_to_select_nonces() { // Real scenario that has happened on test deployments: @@ -1161,7 +1230,7 @@ mod tests { // This was happening because selector (`select_nonces_for_delivery_transaction`) has been called // for every `source_queue` entry separately without preserving any context. let (mut state, mut strategy) = prepare_strategy(); - let nonces = source_nonces(24..=25, 19, DEFAULT_REWARD - DELIVERY_TRANSACTION_COST); + let nonces = source_nonces(24..=25, 19, DEFAULT_REWARD, AtSourceChain); strategy.strategy.source_nonces_updated(header_id(2), nonces); strategy.max_unrewarded_relayer_entries_at_target = 100; strategy.max_unconfirmed_nonces_at_target = 100;