From d05a98473dc933cfed9e5f59023efa2ec811f03c Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Tue, 28 Feb 2023 13:54:12 +0200 Subject: [PATCH] Refund messages confirmation tx (#1904) * Refund messages confirmation tx * Fixes --- bin/millau/runtime/src/lib.rs | 2 - bin/rialto-parachain/runtime/src/lib.rs | 1 - bin/rialto/runtime/src/lib.rs | 1 - bin/runtime-common/src/lib.rs | 3 +- bin/runtime-common/src/messages_call_ext.rs | 219 +++++++----- bin/runtime-common/src/mock.rs | 3 +- .../src/refund_relayer_extension.rs | 321 ++++++++++++++---- modules/relayers/src/payment_adapter.rs | 67 +--- 8 files changed, 407 insertions(+), 210 deletions(-) diff --git a/bin/millau/runtime/src/lib.rs b/bin/millau/runtime/src/lib.rs index fdbcddc0f3..3e46d6ba15 100644 --- a/bin/millau/runtime/src/lib.rs +++ b/bin/millau/runtime/src/lib.rs @@ -453,7 +453,6 @@ impl pallet_bridge_messages::Config for Runtime { Runtime, WithRialtoMessagesInstance, frame_support::traits::ConstU64<100_000>, - frame_support::traits::ConstU64<10_000>, >; type SourceHeaderChain = crate::rialto_messages::RialtoAsSourceHeaderChain; @@ -485,7 +484,6 @@ impl pallet_bridge_messages::Config for Run Runtime, WithRialtoParachainMessagesInstance, frame_support::traits::ConstU64<100_000>, - frame_support::traits::ConstU64<10_000>, >; type SourceHeaderChain = crate::rialto_parachain_messages::RialtoParachainAsSourceHeaderChain; diff --git a/bin/rialto-parachain/runtime/src/lib.rs b/bin/rialto-parachain/runtime/src/lib.rs index f2e408def3..593c7b2122 100644 --- a/bin/rialto-parachain/runtime/src/lib.rs +++ b/bin/rialto-parachain/runtime/src/lib.rs @@ -571,7 +571,6 @@ impl pallet_bridge_messages::Config for Runtime { Runtime, WithMillauMessagesInstance, frame_support::traits::ConstU128<100_000>, - frame_support::traits::ConstU128<100_000>, >; type SourceHeaderChain = crate::millau_messages::MillauAsSourceHeaderChain; diff --git a/bin/rialto/runtime/src/lib.rs b/bin/rialto/runtime/src/lib.rs index c090bb59b9..9873a6fd85 100644 --- a/bin/rialto/runtime/src/lib.rs +++ b/bin/rialto/runtime/src/lib.rs @@ -445,7 +445,6 @@ impl pallet_bridge_messages::Config for Runtime { Runtime, WithMillauMessagesInstance, frame_support::traits::ConstU128<100_000>, - frame_support::traits::ConstU128<100_000>, >; type SourceHeaderChain = crate::millau_messages::MillauAsSourceHeaderChain; diff --git a/bin/runtime-common/src/lib.rs b/bin/runtime-common/src/lib.rs index 32ea500db3..d9c8704907 100644 --- a/bin/runtime-common/src/lib.rs +++ b/bin/runtime-common/src/lib.rs @@ -77,8 +77,7 @@ where /// even honest relayers may lose their funds if there are multiple relays running and /// submitting the same messages/confirmations. fn validate(call: &T::RuntimeCall) -> TransactionValidity { - call.check_obsolete_receive_messages_proof()?; - call.check_obsolete_receive_messages_delivery_proof() + call.check_obsolete_call() } } diff --git a/bin/runtime-common/src/messages_call_ext.rs b/bin/runtime-common/src/messages_call_ext.rs index 740d17129c..588afad106 100644 --- a/bin/runtime-common/src/messages_call_ext.rs +++ b/bin/runtime-common/src/messages_call_ext.rs @@ -22,25 +22,57 @@ use frame_support::{dispatch::CallableCallFor, traits::IsSubType, RuntimeDebug}; use pallet_bridge_messages::{Config, Pallet}; use sp_runtime::transaction_validity::TransactionValidity; -/// Info about a `ReceiveMessagesProof` call which tries to update a single lane. +/// Generic info about a messages delivery/confirmation proof. #[derive(PartialEq, RuntimeDebug)] -pub struct ReceiveMessagesProofInfo { +pub struct BaseMessagesProofInfo { pub lane_id: LaneId, - pub best_proof_nonce: MessageNonce, + pub best_bundled_nonce: MessageNonce, pub best_stored_nonce: MessageNonce, } -/// Helper struct that provides methods for working with the `ReceiveMessagesProof` call. -pub struct ReceiveMessagesProofHelper, I: 'static> { +impl BaseMessagesProofInfo { + fn is_obsolete(&self) -> bool { + self.best_bundled_nonce <= self.best_stored_nonce + } +} + +/// Info about a `ReceiveMessagesProof` call which tries to update a single lane. +#[derive(PartialEq, RuntimeDebug)] +pub struct ReceiveMessagesProofInfo(pub BaseMessagesProofInfo); + +/// Info about a `ReceiveMessagesDeliveryProof` call which tries to update a single lane. +#[derive(PartialEq, RuntimeDebug)] +pub struct ReceiveMessagesDeliveryProofInfo(pub BaseMessagesProofInfo); + +/// Info about a `ReceiveMessagesProof` or a `ReceiveMessagesDeliveryProof` call +/// which tries to update a single lane. +#[derive(PartialEq, RuntimeDebug)] +pub enum CallInfo { + ReceiveMessagesProof(ReceiveMessagesProofInfo), + ReceiveMessagesDeliveryProof(ReceiveMessagesDeliveryProofInfo), +} + +/// Helper struct that provides methods for working with a call supported by `CallInfo`. +pub struct CallHelper, I: 'static> { pub _phantom_data: sp_std::marker::PhantomData<(T, I)>, } -impl, I: 'static> ReceiveMessagesProofHelper { - /// Check if the `ReceiveMessagesProof` call delivered at least some of the messages that - /// it contained. - pub fn was_partially_successful(info: &ReceiveMessagesProofInfo) -> bool { - let inbound_lane_data = pallet_bridge_messages::InboundLanes::::get(info.lane_id); - inbound_lane_data.last_delivered_nonce() > info.best_stored_nonce +impl, I: 'static> CallHelper { + /// Check if a call delivered proof/confirmation for at least some of the messages that it + /// contained. + pub fn was_partially_successful(info: &CallInfo) -> bool { + match info { + CallInfo::ReceiveMessagesProof(info) => { + let inbound_lane_data = + pallet_bridge_messages::InboundLanes::::get(info.0.lane_id); + inbound_lane_data.last_delivered_nonce() > info.0.best_stored_nonce + }, + CallInfo::ReceiveMessagesDeliveryProof(info) => { + let outbound_lane_data = + pallet_bridge_messages::OutboundLanes::::get(info.0.lane_id); + outbound_lane_data.latest_received_nonce > info.0.best_stored_nonce + }, + } } } @@ -51,17 +83,21 @@ pub trait MessagesCallSubType, I: 'static>: /// Create a new instance of `ReceiveMessagesProofInfo` from a `ReceiveMessagesProof` call. fn receive_messages_proof_info(&self) -> Option; - /// Create a new instance of `ReceiveMessagesProofInfo` from a `ReceiveMessagesProof` call, - /// if the call is for the provided lane. - fn receive_messages_proof_info_for(&self, lane_id: LaneId) -> Option; + /// Create a new instance of `ReceiveMessagesDeliveryProofInfo` from + /// a `ReceiveMessagesDeliveryProof` call. + fn receive_messages_delivery_proof_info(&self) -> Option; + + /// Create a new instance of `CallInfo` from a `ReceiveMessagesProof` + /// or a `ReceiveMessagesDeliveryProof` call. + fn call_info(&self) -> Option; - /// Check that a `ReceiveMessagesProof` call is trying to deliver at least some messages that - /// are better than the ones we know of. - fn check_obsolete_receive_messages_proof(&self) -> TransactionValidity; + /// Create a new instance of `CallInfo` from a `ReceiveMessagesProof` + /// or a `ReceiveMessagesDeliveryProof` call, if the call is for the provided lane. + fn call_info_for(&self, lane_id: LaneId) -> Option; - /// Check that a `ReceiveMessagesDeliveryProof` call is trying to deliver at least some message - /// confirmations that are better than the ones we know of. - fn check_obsolete_receive_messages_delivery_proof(&self) -> TransactionValidity; + /// Check that a `ReceiveMessagesProof` or a `ReceiveMessagesDeliveryProof` call is trying + /// to deliver/confirm at least some messages that are better than the ones we know of. + fn check_obsolete_call(&self) -> TransactionValidity; } impl< @@ -88,40 +124,17 @@ impl< { let inbound_lane_data = pallet_bridge_messages::InboundLanes::::get(proof.lane); - return Some(ReceiveMessagesProofInfo { + return Some(ReceiveMessagesProofInfo(BaseMessagesProofInfo { lane_id: proof.lane, - best_proof_nonce: proof.nonces_end, + best_bundled_nonce: proof.nonces_end, best_stored_nonce: inbound_lane_data.last_delivered_nonce(), - }) + })) } None } - fn receive_messages_proof_info_for(&self, lane_id: LaneId) -> Option { - self.receive_messages_proof_info().filter(|info| info.lane_id == lane_id) - } - - fn check_obsolete_receive_messages_proof(&self) -> TransactionValidity { - if let Some(proof_info) = self.receive_messages_proof_info() { - if proof_info.best_proof_nonce <= proof_info.best_stored_nonce { - log::trace!( - target: pallet_bridge_messages::LOG_TARGET, - "Rejecting obsolete messages delivery transaction: \ - lane {:?}, bundled {:?}, best {:?}", - proof_info.lane_id, - proof_info.best_proof_nonce, - proof_info.best_stored_nonce, - ); - - return sp_runtime::transaction_validity::InvalidTransaction::Stale.into() - } - } - - Ok(sp_runtime::transaction_validity::ValidTransaction::default()) - } - - fn check_obsolete_receive_messages_delivery_proof(&self) -> TransactionValidity { + fn receive_messages_delivery_proof_info(&self) -> Option { if let Some(pallet_bridge_messages::Call::::receive_messages_delivery_proof { ref proof, ref relayers_state, @@ -129,18 +142,62 @@ impl< }) = self.is_sub_type() { let outbound_lane_data = pallet_bridge_messages::OutboundLanes::::get(proof.lane); - if relayers_state.last_delivered_nonce <= outbound_lane_data.latest_received_nonce { + + return Some(ReceiveMessagesDeliveryProofInfo(BaseMessagesProofInfo { + lane_id: proof.lane, + best_bundled_nonce: relayers_state.last_delivered_nonce, + best_stored_nonce: outbound_lane_data.latest_received_nonce, + })) + } + + None + } + + fn call_info(&self) -> Option { + if let Some(info) = self.receive_messages_proof_info() { + return Some(CallInfo::ReceiveMessagesProof(info)) + } + + if let Some(info) = self.receive_messages_delivery_proof_info() { + return Some(CallInfo::ReceiveMessagesDeliveryProof(info)) + } + + None + } + + fn call_info_for(&self, lane_id: LaneId) -> Option { + self.call_info().filter(|info| { + let actual_lane_id = match info { + CallInfo::ReceiveMessagesProof(info) => info.0.lane_id, + CallInfo::ReceiveMessagesDeliveryProof(info) => info.0.lane_id, + }; + actual_lane_id == lane_id + }) + } + + fn check_obsolete_call(&self) -> TransactionValidity { + match self.call_info() { + Some(CallInfo::ReceiveMessagesProof(proof_info)) if proof_info.0.is_obsolete() => { log::trace!( target: pallet_bridge_messages::LOG_TARGET, - "Rejecting obsolete messages confirmation transaction: \ - lane {:?}, bundled {:?}, best {:?}", - proof.lane, - relayers_state.last_delivered_nonce, - outbound_lane_data.latest_received_nonce, + "Rejecting obsolete messages delivery transaction: {:?}", + proof_info ); return sp_runtime::transaction_validity::InvalidTransaction::Stale.into() - } + }, + Some(CallInfo::ReceiveMessagesDeliveryProof(proof_info)) + if proof_info.0.is_obsolete() => + { + log::trace!( + target: pallet_bridge_messages::LOG_TARGET, + "Rejecting obsolete messages confirmation transaction: {:?}", + proof_info, + ); + + return sp_runtime::transaction_validity::InvalidTransaction::Stale.into() + }, + _ => {}, } Ok(sp_runtime::transaction_validity::ValidTransaction::default()) @@ -153,8 +210,8 @@ mod tests { messages::{ source::FromBridgedChainMessagesDeliveryProof, target::FromBridgedChainMessagesProof, }, + messages_call_ext::MessagesCallSubType, mock::{TestRuntime, ThisChainRuntimeCall}, - BridgeRuntimeFilterCall, }; use bp_messages::UnrewardedRelayersState; @@ -169,22 +226,21 @@ mod tests { nonces_start: bp_messages::MessageNonce, nonces_end: bp_messages::MessageNonce, ) -> bool { - pallet_bridge_messages::Pallet::::validate( - &ThisChainRuntimeCall::BridgeMessages( - pallet_bridge_messages::Call::::receive_messages_proof { - relayer_id_at_bridged_chain: 42, - messages_count: (nonces_end - nonces_start + 1) as u32, - dispatch_weight: frame_support::weights::Weight::zero(), - proof: FromBridgedChainMessagesProof { - bridged_header_hash: Default::default(), - storage_proof: vec![], - lane: bp_messages::LaneId([0, 0, 0, 0]), - nonces_start, - nonces_end, - }, + ThisChainRuntimeCall::BridgeMessages( + pallet_bridge_messages::Call::::receive_messages_proof { + relayer_id_at_bridged_chain: 42, + messages_count: (nonces_end - nonces_start + 1) as u32, + dispatch_weight: frame_support::weights::Weight::zero(), + proof: FromBridgedChainMessagesProof { + bridged_header_hash: Default::default(), + storage_proof: vec![], + lane: bp_messages::LaneId([0, 0, 0, 0]), + nonces_start, + nonces_end, }, - ), + }, ) + .check_obsolete_call() .is_ok() } @@ -230,21 +286,20 @@ mod tests { } fn validate_message_confirmation(last_delivered_nonce: bp_messages::MessageNonce) -> bool { - pallet_bridge_messages::Pallet::::validate( - &ThisChainRuntimeCall::BridgeMessages( - pallet_bridge_messages::Call::::receive_messages_delivery_proof { - proof: FromBridgedChainMessagesDeliveryProof { - bridged_header_hash: Default::default(), - storage_proof: Vec::new(), - lane: bp_messages::LaneId([0, 0, 0, 0]), - }, - relayers_state: UnrewardedRelayersState { - last_delivered_nonce, - ..Default::default() - }, + ThisChainRuntimeCall::BridgeMessages( + pallet_bridge_messages::Call::::receive_messages_delivery_proof { + proof: FromBridgedChainMessagesDeliveryProof { + bridged_header_hash: Default::default(), + storage_proof: Vec::new(), + lane: bp_messages::LaneId([0, 0, 0, 0]), + }, + relayers_state: UnrewardedRelayersState { + last_delivered_nonce, + ..Default::default() }, - ), + }, ) + .check_obsolete_call() .is_ok() } diff --git a/bin/runtime-common/src/mock.rs b/bin/runtime-common/src/mock.rs index 29c540bc66..c2cd8e9ba8 100644 --- a/bin/runtime-common/src/mock.rs +++ b/bin/runtime-common/src/mock.rs @@ -230,8 +230,7 @@ impl pallet_bridge_messages::Config for TestRuntime { type DeliveryConfirmationPayments = pallet_bridge_relayers::DeliveryConfirmationPaymentsAdapter< TestRuntime, (), - frame_support::traits::ConstU64<100_000>, - frame_support::traits::ConstU64<10_000>, + ConstU64<100_000>, >; type SourceHeaderChain = SourceHeaderChainAdapter; diff --git a/bin/runtime-common/src/refund_relayer_extension.rs b/bin/runtime-common/src/refund_relayer_extension.rs index aa03082e9f..0a61f4fdd0 100644 --- a/bin/runtime-common/src/refund_relayer_extension.rs +++ b/bin/runtime-common/src/refund_relayer_extension.rs @@ -20,7 +20,7 @@ //! (parachain or relay chain). use crate::messages_call_ext::{ - MessagesCallSubType, ReceiveMessagesProofHelper, ReceiveMessagesProofInfo, + CallHelper as MessagesCallHelper, CallInfo as MessagesCallInfo, MessagesCallSubType, }; use bp_messages::LaneId; use bp_relayers::{RewardsAccountOwner, RewardsAccountParams}; @@ -143,23 +143,23 @@ pub struct PreDispatchData { /// Type of the call that the extension recognizes. #[derive(RuntimeDebugNoBound, PartialEq)] pub enum CallInfo { - /// Relay chain finality + parachain finality + message delivery calls. - AllFinalityAndDelivery( + /// Relay chain finality + parachain finality + message delivery/confirmation calls. + AllFinalityAndMsgs( SubmitFinalityProofInfo, SubmitParachainHeadsInfo, - ReceiveMessagesProofInfo, + MessagesCallInfo, ), - /// Parachain finality + message delivery calls. - ParachainFinalityAndDelivery(SubmitParachainHeadsInfo, ReceiveMessagesProofInfo), - /// Standalone message delivery call. - Delivery(ReceiveMessagesProofInfo), + /// Parachain finality + message delivery/confirmation calls. + ParachainFinalityAndMsgs(SubmitParachainHeadsInfo, MessagesCallInfo), + /// Standalone message delivery/confirmation call. + Msgs(MessagesCallInfo), } impl CallInfo { /// Returns the pre-dispatch `finality_target` sent to the `SubmitFinalityProof` call. fn submit_finality_proof_info(&self) -> Option> { match *self { - Self::AllFinalityAndDelivery(info, _, _) => Some(info), + Self::AllFinalityAndMsgs(info, _, _) => Some(info), _ => None, } } @@ -167,18 +167,18 @@ impl CallInfo { /// Returns the pre-dispatch `SubmitParachainHeadsInfo`. fn submit_parachain_heads_info(&self) -> Option<&SubmitParachainHeadsInfo> { match self { - Self::AllFinalityAndDelivery(_, info, _) => Some(info), - Self::ParachainFinalityAndDelivery(info, _) => Some(info), + Self::AllFinalityAndMsgs(_, info, _) => Some(info), + Self::ParachainFinalityAndMsgs(info, _) => Some(info), _ => None, } } /// Returns the pre-dispatch `ReceiveMessagesProofInfo`. - fn receive_messages_proof_info(&self) -> &ReceiveMessagesProofInfo { + fn messages_call_info(&self) -> &MessagesCallInfo { match self { - Self::AllFinalityAndDelivery(_, _, info) => info, - Self::ParachainFinalityAndDelivery(_, info) => info, - Self::Delivery(info) => info, + Self::AllFinalityAndMsgs(_, _, info) => info, + Self::ParachainFinalityAndMsgs(_, info) => info, + Self::Msgs(info) => info, } } } @@ -269,7 +269,7 @@ where for nested_call in calls { nested_call.check_obsolete_submit_finality_proof()?; nested_call.check_obsolete_submit_parachain_heads()?; - nested_call.check_obsolete_receive_messages_proof()?; + nested_call.check_obsolete_call()?; } } @@ -290,18 +290,16 @@ where let parse_call = || { let mut calls = self.expand_call(call)?.into_iter(); match calls.len() { - 3 => Some(CallInfo::AllFinalityAndDelivery( + 3 => Some(CallInfo::AllFinalityAndMsgs( calls.next()?.submit_finality_proof_info()?, calls.next()?.submit_parachain_heads_info_for(Para::Id::get())?, - calls.next()?.receive_messages_proof_info_for(Msgs::Id::get())?, + calls.next()?.call_info_for(Msgs::Id::get())?, )), - 2 => Some(CallInfo::ParachainFinalityAndDelivery( + 2 => Some(CallInfo::ParachainFinalityAndMsgs( calls.next()?.submit_parachain_heads_info_for(Para::Id::get())?, - calls.next()?.receive_messages_proof_info_for(Msgs::Id::get())?, - )), - 1 => Some(CallInfo::Delivery( - calls.next()?.receive_messages_proof_info_for(Msgs::Id::get())?, + calls.next()?.call_info_for(Msgs::Id::get())?, )), + 1 => Some(CallInfo::Msgs(calls.next()?.call_info_for(Msgs::Id::get())?)), _ => None, } }; @@ -374,10 +372,9 @@ where // Check if the `ReceiveMessagesProof` call delivered at least some of the messages that // it contained. If this happens, we consider the transaction "helpful" and refund it. - let msgs_proof_info = call_info.receive_messages_proof_info(); - if !ReceiveMessagesProofHelper::::was_partially_successful( - msgs_proof_info, - ) { + let msgs_call_info = call_info.messages_call_info(); + if !MessagesCallHelper::::was_partially_successful(msgs_call_info) + { return Ok(()) } @@ -398,11 +395,15 @@ where let refund = Refund::compute_refund(info, &post_info, post_info_len, tip); // finally - register refund in relayers pallet + let rewards_account_owner = match msgs_call_info { + MessagesCallInfo::ReceiveMessagesProof(_) => RewardsAccountOwner::ThisChain, + MessagesCallInfo::ReceiveMessagesDeliveryProof(_) => RewardsAccountOwner::BridgedChain, + }; RelayersPallet::::register_relayer_reward( RewardsAccountParams::new( Msgs::Id::get(), Runtime::BridgedChainId::get(), - RewardsAccountOwner::ThisChain, + rewards_account_owner, ), &relayer, refund, @@ -425,8 +426,16 @@ where #[cfg(test)] mod tests { use super::*; - use crate::{messages::target::FromBridgedChainMessagesProof, mock::*}; - use bp_messages::{InboundLaneData, MessageNonce}; + use crate::{ + messages::{ + source::FromBridgedChainMessagesDeliveryProof, target::FromBridgedChainMessagesProof, + }, + messages_call_ext::{ + BaseMessagesProofInfo, ReceiveMessagesDeliveryProofInfo, ReceiveMessagesProofInfo, + }, + mock::*, + }; + use bp_messages::{InboundLaneData, MessageNonce, OutboundLaneData, UnrewardedRelayersState}; use bp_parachains::{BestParaHeadHash, ParaInfo}; use bp_polkadot_core::parachains::{ParaHash, ParaHeadsProof, ParaId}; use bp_runtime::HeaderId; @@ -442,7 +451,8 @@ mod tests { parameter_types! { TestParachain: u32 = 1000; pub TestLaneId: LaneId = TEST_LANE_ID; - pub DirectedTestLaneId: RewardsAccountParams = RewardsAccountParams::new(TEST_LANE_ID, TEST_BRIDGED_CHAIN_ID, RewardsAccountOwner::ThisChain); + pub MsgProofsRewardsAccount: RewardsAccountParams = RewardsAccountParams::new(TEST_LANE_ID, TEST_BRIDGED_CHAIN_ID, RewardsAccountOwner::ThisChain); + pub MsgDeliveryProofsRewardsAccount: RewardsAccountParams = RewardsAccountParams::new(TEST_LANE_ID, TEST_BRIDGED_CHAIN_ID, RewardsAccountOwner::BridgedChain); } bp_runtime::generate_static_str_provider!(TestExtension); @@ -466,7 +476,7 @@ mod tests { best_relay_header_number: RelayBlockNumber, parachain_head_at_relay_header_number: RelayBlockNumber, parachain_head_hash: ParaHash, - best_delivered_message: MessageNonce, + best_message: MessageNonce, ) { let authorities = test_keyring().into_iter().map(|(a, w)| (a.into(), w)).collect(); let best_relay_header = HeaderId(best_relay_header_number, RelayBlockHash::default()); @@ -486,9 +496,13 @@ mod tests { pallet_bridge_parachains::ParasInfo::::insert(para_id, para_info); let lane_id = TestLaneId::get(); - let lane_data = - InboundLaneData { last_confirmed_nonce: best_delivered_message, ..Default::default() }; - pallet_bridge_messages::InboundLanes::::insert(lane_id, lane_data); + let in_lane_data = + InboundLaneData { last_confirmed_nonce: best_message, ..Default::default() }; + pallet_bridge_messages::InboundLanes::::insert(lane_id, in_lane_data); + + let out_lane_data = + OutboundLaneData { latest_received_nonce: best_message, ..Default::default() }; + pallet_bridge_messages::OutboundLanes::::insert(lane_id, out_lane_data); } fn submit_relay_header_call(relay_header_number: RelayBlockNumber) -> RuntimeCall { @@ -532,6 +546,20 @@ mod tests { }) } + fn message_confirmation_call(best_message: MessageNonce) -> RuntimeCall { + RuntimeCall::BridgeMessages(MessagesCall::receive_messages_delivery_proof { + proof: FromBridgedChainMessagesDeliveryProof { + bridged_header_hash: Default::default(), + storage_proof: vec![], + lane: TestLaneId::get(), + }, + relayers_state: UnrewardedRelayersState { + last_delivered_nonce: best_message, + ..Default::default() + }, + }) + } + fn parachain_finality_and_delivery_batch_call( parachain_head_at_relay_header_number: RelayBlockNumber, best_message: MessageNonce, @@ -544,6 +572,18 @@ mod tests { }) } + fn parachain_finality_and_confirmation_batch_call( + parachain_head_at_relay_header_number: RelayBlockNumber, + best_message: MessageNonce, + ) -> RuntimeCall { + RuntimeCall::Utility(UtilityCall::batch_all { + calls: vec![ + submit_parachain_head_call(parachain_head_at_relay_header_number), + message_confirmation_call(best_message), + ], + }) + } + fn all_finality_and_delivery_batch_call( relay_header_number: RelayBlockNumber, parachain_head_at_relay_header_number: RelayBlockNumber, @@ -558,10 +598,24 @@ mod tests { }) } + fn all_finality_and_confirmation_batch_call( + relay_header_number: RelayBlockNumber, + parachain_head_at_relay_header_number: RelayBlockNumber, + best_message: MessageNonce, + ) -> RuntimeCall { + RuntimeCall::Utility(UtilityCall::batch_all { + calls: vec![ + submit_relay_header_call(relay_header_number), + submit_parachain_head_call(parachain_head_at_relay_header_number), + message_confirmation_call(best_message), + ], + }) + } + fn all_finality_pre_dispatch_data() -> PreDispatchData { PreDispatchData { relayer: relayer_account_at_this_chain(), - call_info: CallInfo::AllFinalityAndDelivery( + call_info: CallInfo::AllFinalityAndMsgs( SubmitFinalityProofInfo { block_number: 200, extra_weight: Weight::zero(), @@ -572,11 +626,38 @@ mod tests { para_id: ParaId(TestParachain::get()), para_head_hash: [1u8; 32].into(), }, - ReceiveMessagesProofInfo { - lane_id: TEST_LANE_ID, - best_proof_nonce: 200, - best_stored_nonce: 100, + MessagesCallInfo::ReceiveMessagesProof(ReceiveMessagesProofInfo( + BaseMessagesProofInfo { + lane_id: TEST_LANE_ID, + best_bundled_nonce: 200, + best_stored_nonce: 100, + }, + )), + ), + } + } + + fn all_finality_confirmation_pre_dispatch_data() -> PreDispatchData { + PreDispatchData { + relayer: relayer_account_at_this_chain(), + call_info: CallInfo::AllFinalityAndMsgs( + SubmitFinalityProofInfo { + block_number: 200, + extra_weight: Weight::zero(), + extra_size: 0, }, + SubmitParachainHeadsInfo { + at_relay_block_number: 200, + para_id: ParaId(TestParachain::get()), + para_head_hash: [1u8; 32].into(), + }, + MessagesCallInfo::ReceiveMessagesDeliveryProof(ReceiveMessagesDeliveryProofInfo( + BaseMessagesProofInfo { + lane_id: TEST_LANE_ID, + best_bundled_nonce: 200, + best_stored_nonce: 100, + }, + )), ), } } @@ -584,17 +665,39 @@ mod tests { fn parachain_finality_pre_dispatch_data() -> PreDispatchData { PreDispatchData { relayer: relayer_account_at_this_chain(), - call_info: CallInfo::ParachainFinalityAndDelivery( + call_info: CallInfo::ParachainFinalityAndMsgs( SubmitParachainHeadsInfo { at_relay_block_number: 200, para_id: ParaId(TestParachain::get()), para_head_hash: [1u8; 32].into(), }, - ReceiveMessagesProofInfo { - lane_id: TEST_LANE_ID, - best_proof_nonce: 200, - best_stored_nonce: 100, + MessagesCallInfo::ReceiveMessagesProof(ReceiveMessagesProofInfo( + BaseMessagesProofInfo { + lane_id: TEST_LANE_ID, + best_bundled_nonce: 200, + best_stored_nonce: 100, + }, + )), + ), + } + } + + fn parachain_finality_confirmation_pre_dispatch_data() -> PreDispatchData { + PreDispatchData { + relayer: relayer_account_at_this_chain(), + call_info: CallInfo::ParachainFinalityAndMsgs( + SubmitParachainHeadsInfo { + at_relay_block_number: 200, + para_id: ParaId(TestParachain::get()), + para_head_hash: [1u8; 32].into(), }, + MessagesCallInfo::ReceiveMessagesDeliveryProof(ReceiveMessagesDeliveryProofInfo( + BaseMessagesProofInfo { + lane_id: TEST_LANE_ID, + best_bundled_nonce: 200, + best_stored_nonce: 100, + }, + )), ), } } @@ -602,11 +705,26 @@ mod tests { fn delivery_pre_dispatch_data() -> PreDispatchData { PreDispatchData { relayer: relayer_account_at_this_chain(), - call_info: CallInfo::Delivery(ReceiveMessagesProofInfo { - lane_id: TEST_LANE_ID, - best_proof_nonce: 200, - best_stored_nonce: 100, - }), + call_info: CallInfo::Msgs(MessagesCallInfo::ReceiveMessagesProof( + ReceiveMessagesProofInfo(BaseMessagesProofInfo { + lane_id: TEST_LANE_ID, + best_bundled_nonce: 200, + best_stored_nonce: 100, + }), + )), + } + } + + fn confirmation_pre_dispatch_data() -> PreDispatchData { + PreDispatchData { + relayer: relayer_account_at_this_chain(), + call_info: CallInfo::Msgs(MessagesCallInfo::ReceiveMessagesDeliveryProof( + ReceiveMessagesDeliveryProofInfo(BaseMessagesProofInfo { + lane_id: TEST_LANE_ID, + best_bundled_nonce: 200, + best_stored_nonce: 100, + }), + )), } } @@ -669,16 +787,28 @@ mod tests { initialize_environment(100, 100, Default::default(), 100); assert_eq!(run_validate(message_delivery_call(200)), Ok(ValidTransaction::default()),); + assert_eq!( + run_validate(message_confirmation_call(200)), + Ok(ValidTransaction::default()), + ); assert_eq!( run_validate(parachain_finality_and_delivery_batch_call(200, 200)), Ok(ValidTransaction::default()), ); + assert_eq!( + run_validate(parachain_finality_and_confirmation_batch_call(200, 200)), + Ok(ValidTransaction::default()), + ); assert_eq!( run_validate(all_finality_and_delivery_batch_call(200, 200, 200)), Ok(ValidTransaction::default()), ); + assert_eq!( + run_validate(all_finality_and_confirmation_batch_call(200, 200, 200)), + Ok(ValidTransaction::default()), + ); }); } @@ -708,17 +838,15 @@ mod tests { run_pre_dispatch(all_finality_and_delivery_batch_call(101, 100, 200)), Err(TransactionValidityError::Invalid(InvalidTransaction::Stale)), ); - assert_eq!( - run_pre_dispatch(parachain_finality_and_delivery_batch_call(100, 200)), + run_validate(all_finality_and_delivery_batch_call(101, 100, 200)), Err(TransactionValidityError::Invalid(InvalidTransaction::Stale)), ); assert_eq!( - run_validate(all_finality_and_delivery_batch_call(101, 100, 200)), + run_pre_dispatch(parachain_finality_and_delivery_batch_call(100, 200)), Err(TransactionValidityError::Invalid(InvalidTransaction::Stale)), ); - assert_eq!( run_validate(parachain_finality_and_delivery_batch_call(100, 200)), Err(TransactionValidityError::Invalid(InvalidTransaction::Stale)), @@ -735,9 +863,8 @@ mod tests { run_pre_dispatch(all_finality_and_delivery_batch_call(200, 200, 100)), Err(TransactionValidityError::Invalid(InvalidTransaction::Stale)), ); - assert_eq!( - run_pre_dispatch(parachain_finality_and_delivery_batch_call(200, 100)), + run_pre_dispatch(all_finality_and_confirmation_batch_call(200, 200, 100)), Err(TransactionValidityError::Invalid(InvalidTransaction::Stale)), ); @@ -745,11 +872,28 @@ mod tests { run_validate(all_finality_and_delivery_batch_call(200, 200, 100)), Err(TransactionValidityError::Invalid(InvalidTransaction::Stale)), ); + assert_eq!( + run_validate(all_finality_and_confirmation_batch_call(200, 200, 100)), + Err(TransactionValidityError::Invalid(InvalidTransaction::Stale)), + ); + + assert_eq!( + run_pre_dispatch(parachain_finality_and_delivery_batch_call(200, 100)), + Err(TransactionValidityError::Invalid(InvalidTransaction::Stale)), + ); + assert_eq!( + run_pre_dispatch(parachain_finality_and_confirmation_batch_call(200, 100)), + Err(TransactionValidityError::Invalid(InvalidTransaction::Stale)), + ); assert_eq!( run_validate(parachain_finality_and_delivery_batch_call(200, 100)), Err(TransactionValidityError::Invalid(InvalidTransaction::Stale)), ); + assert_eq!( + run_validate(parachain_finality_and_confirmation_batch_call(200, 100)), + Err(TransactionValidityError::Invalid(InvalidTransaction::Stale)), + ); }); } @@ -762,6 +906,10 @@ mod tests { run_pre_dispatch(all_finality_and_delivery_batch_call(200, 200, 200)), Ok(Some(all_finality_pre_dispatch_data())), ); + assert_eq!( + run_pre_dispatch(all_finality_and_confirmation_batch_call(200, 200, 200)), + Ok(Some(all_finality_confirmation_pre_dispatch_data())), + ); }); } @@ -774,6 +922,10 @@ mod tests { run_pre_dispatch(parachain_finality_and_delivery_batch_call(200, 200)), Ok(Some(parachain_finality_pre_dispatch_data())), ); + assert_eq!( + run_pre_dispatch(parachain_finality_and_confirmation_batch_call(200, 200)), + Ok(Some(parachain_finality_confirmation_pre_dispatch_data())), + ); }); } @@ -801,7 +953,7 @@ mod tests { } #[test] - fn pre_dispatch_parses_message_delivery_transaction() { + fn pre_dispatch_parses_message_transaction() { run_test(|| { initialize_environment(100, 100, Default::default(), 100); @@ -809,6 +961,10 @@ mod tests { run_pre_dispatch(message_delivery_call(200)), Ok(Some(delivery_pre_dispatch_data())), ); + assert_eq!( + run_pre_dispatch(message_confirmation_call(200)), + Ok(Some(confirmation_pre_dispatch_data())), + ); }); } @@ -862,6 +1018,16 @@ mod tests { Ok(()) )); assert_storage_noop!(run_post_dispatch(Some(delivery_pre_dispatch_data()), Ok(()))); + + assert_storage_noop!(run_post_dispatch( + Some(all_finality_confirmation_pre_dispatch_data()), + Ok(()) + )); + assert_storage_noop!(run_post_dispatch( + Some(parachain_finality_confirmation_pre_dispatch_data()), + Ok(()) + )); + assert_storage_noop!(run_post_dispatch(Some(confirmation_pre_dispatch_data()), Ok(()))); }); } @@ -882,7 +1048,7 @@ mod tests { assert_eq!( RelayersPallet::::relayer_reward( relayer_account_at_this_chain(), - DirectedTestLaneId::get() + MsgProofsRewardsAccount::get() ), Some(regular_reward), ); @@ -890,7 +1056,7 @@ mod tests { // now repeat the same with size+weight refund: we expect smaller reward let mut pre_dispatch_data = all_finality_pre_dispatch_data(); match pre_dispatch_data.call_info { - CallInfo::AllFinalityAndDelivery(ref mut info, ..) => { + CallInfo::AllFinalityAndMsgs(ref mut info, ..) => { info.extra_weight.set_ref_time( frame_support::weights::constants::WEIGHT_REF_TIME_PER_SECOND, ); @@ -901,7 +1067,7 @@ mod tests { run_post_dispatch(Some(pre_dispatch_data), Ok(())); let reward_after_two_calls = RelayersPallet::::relayer_reward( relayer_account_at_this_chain(), - DirectedTestLaneId::get(), + MsgProofsRewardsAccount::get(), ) .unwrap(); assert!( @@ -922,7 +1088,16 @@ mod tests { assert_eq!( RelayersPallet::::relayer_reward( relayer_account_at_this_chain(), - DirectedTestLaneId::get() + MsgProofsRewardsAccount::get() + ), + Some(expected_reward()), + ); + + run_post_dispatch(Some(all_finality_confirmation_pre_dispatch_data()), Ok(())); + assert_eq!( + RelayersPallet::::relayer_reward( + relayer_account_at_this_chain(), + MsgDeliveryProofsRewardsAccount::get() ), Some(expected_reward()), ); @@ -938,7 +1113,16 @@ mod tests { assert_eq!( RelayersPallet::::relayer_reward( relayer_account_at_this_chain(), - DirectedTestLaneId::get() + MsgProofsRewardsAccount::get() + ), + Some(expected_reward()), + ); + + run_post_dispatch(Some(parachain_finality_confirmation_pre_dispatch_data()), Ok(())); + assert_eq!( + RelayersPallet::::relayer_reward( + relayer_account_at_this_chain(), + MsgDeliveryProofsRewardsAccount::get() ), Some(expected_reward()), ); @@ -946,7 +1130,7 @@ mod tests { } #[test] - fn post_dispatch_refunds_relayer_in_message_delivery_transaction() { + fn post_dispatch_refunds_relayer_in_message_transaction() { run_test(|| { initialize_environment(200, 200, Default::default(), 200); @@ -954,7 +1138,16 @@ mod tests { assert_eq!( RelayersPallet::::relayer_reward( relayer_account_at_this_chain(), - DirectedTestLaneId::get() + MsgProofsRewardsAccount::get() + ), + Some(expected_reward()), + ); + + run_post_dispatch(Some(confirmation_pre_dispatch_data()), Ok(())); + assert_eq!( + RelayersPallet::::relayer_reward( + relayer_account_at_this_chain(), + MsgDeliveryProofsRewardsAccount::get() ), Some(expected_reward()), ); diff --git a/modules/relayers/src/payment_adapter.rs b/modules/relayers/src/payment_adapter.rs index cfd07cd6cc..2752044958 100644 --- a/modules/relayers/src/payment_adapter.rs +++ b/modules/relayers/src/payment_adapter.rs @@ -24,22 +24,21 @@ use bp_messages::{ }; use bp_relayers::{RewardsAccountOwner, RewardsAccountParams}; use frame_support::{sp_runtime::SaturatedConversion, traits::Get}; -use sp_arithmetic::traits::{Saturating, UniqueSaturatedFrom, Zero}; +use sp_arithmetic::traits::{Saturating, Zero}; use sp_std::{collections::vec_deque::VecDeque, marker::PhantomData, ops::RangeInclusive}; /// Adapter that allows relayers pallet to be used as a delivery+dispatch payment mechanism /// for the messages pallet. -pub struct DeliveryConfirmationPaymentsAdapter( - PhantomData<(T, MI, DeliveryReward, ConfirmationReward)>, +pub struct DeliveryConfirmationPaymentsAdapter( + PhantomData<(T, MI, DeliveryReward)>, ); -impl DeliveryConfirmationPayments - for DeliveryConfirmationPaymentsAdapter +impl DeliveryConfirmationPayments + for DeliveryConfirmationPaymentsAdapter where T: Config + pallet_bridge_messages::Config, MI: 'static, DeliveryReward: Get, - ConfirmationReward: Get, { type Error = &'static str; @@ -61,7 +60,6 @@ where RewardsAccountOwner::BridgedChain, ), DeliveryReward::get(), - ConfirmationReward::get(), ); } } @@ -72,32 +70,17 @@ fn register_relayers_rewards( relayers_rewards: RelayersRewards, lane_id: RewardsAccountParams, delivery_fee: T::Reward, - confirmation_fee: T::Reward, ) { // reward every relayer except `confirmation_relayer` let mut confirmation_relayer_reward = T::Reward::zero(); for (relayer, messages) in relayers_rewards { // sane runtime configurations guarantee that the number of messages will be below // `u32::MAX` - let mut relayer_reward = - T::Reward::unique_saturated_from(messages).saturating_mul(delivery_fee); + let relayer_reward = T::Reward::saturated_from(messages).saturating_mul(delivery_fee); if relayer != *confirmation_relayer { - // If delivery confirmation is submitted by other relayer, let's deduct confirmation fee - // from relayer reward. - // - // If confirmation fee has been increased (or if it was the only component of message - // fee), then messages relayer may receive zero reward. - let mut confirmation_reward = - T::Reward::saturated_from(messages).saturating_mul(confirmation_fee); - confirmation_reward = sp_std::cmp::min(confirmation_reward, relayer_reward); - relayer_reward = relayer_reward.saturating_sub(confirmation_reward); - confirmation_relayer_reward = - confirmation_relayer_reward.saturating_add(confirmation_reward); Pallet::::register_relayer_reward(lane_id, &relayer, relayer_reward); } else { - // If delivery confirmation is submitted by this relayer, let's add confirmation fee - // from other relayers to this relayer reward. confirmation_relayer_reward = confirmation_relayer_reward.saturating_add(relayer_reward); } @@ -132,69 +115,41 @@ mod tests { relayers_rewards(), TEST_REWARDS_ACCOUNT_PARAMS, 50, - 10, ); assert_eq!( RelayerRewards::::get(RELAYER_1, TEST_REWARDS_ACCOUNT_PARAMS), - Some(80) + Some(100) ); assert_eq!( RelayerRewards::::get(RELAYER_2, TEST_REWARDS_ACCOUNT_PARAMS), - Some(170) + Some(150) ); }); } #[test] - fn confirmation_relayer_is_rewarded_if_it_has_not_delivered_any_delivered_messages() { + fn confirmation_relayer_is_not_rewarded_if_it_has_not_delivered_any_messages() { run_test(|| { register_relayers_rewards::( &RELAYER_3, relayers_rewards(), TEST_REWARDS_ACCOUNT_PARAMS, 50, - 10, ); assert_eq!( RelayerRewards::::get(RELAYER_1, TEST_REWARDS_ACCOUNT_PARAMS), - Some(80) + Some(100) ); assert_eq!( RelayerRewards::::get(RELAYER_2, TEST_REWARDS_ACCOUNT_PARAMS), - Some(120) + Some(150) ); assert_eq!( RelayerRewards::::get(RELAYER_3, TEST_REWARDS_ACCOUNT_PARAMS), - Some(50) - ); - }); - } - - #[test] - fn only_confirmation_relayer_is_rewarded_if_confirmation_fee_has_significantly_increased() { - run_test(|| { - register_relayers_rewards::( - &RELAYER_3, - relayers_rewards(), - TEST_REWARDS_ACCOUNT_PARAMS, - 50, - 1000, - ); - - assert_eq!( - RelayerRewards::::get(RELAYER_1, TEST_REWARDS_ACCOUNT_PARAMS), None ); - assert_eq!( - RelayerRewards::::get(RELAYER_2, TEST_REWARDS_ACCOUNT_PARAMS), - None - ); - assert_eq!( - RelayerRewards::::get(RELAYER_3, TEST_REWARDS_ACCOUNT_PARAMS), - Some(250) - ); }); } }