From 043a008723563945ba4034c87e265dbbf52ef020 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Tue, 22 Dec 2020 09:22:07 +0300 Subject: [PATCH] add relayers_state param to the receive_messages_delivery_proof (#584) --- bridges/modules/message-lane/src/lib.rs | 93 +++++++++++++++++-- bridges/primitives/message-lane/src/lib.rs | 14 ++- .../messages-relay/src/message_lane_loop.rs | 1 + .../src/message_race_delivery.rs | 1 + .../relays/substrate/src/messages_target.rs | 5 +- .../src/millau_messages_to_rialto.rs | 3 +- .../src/rialto_messages_to_millau.rs | 3 +- 7 files changed, 109 insertions(+), 11 deletions(-) diff --git a/bridges/modules/message-lane/src/lib.rs b/bridges/modules/message-lane/src/lib.rs index 468df6867d..267ad33439 100644 --- a/bridges/modules/message-lane/src/lib.rs +++ b/bridges/modules/message-lane/src/lib.rs @@ -35,11 +35,12 @@ use crate::outbound_lane::{OutboundLane, OutboundLaneStorage}; use bp_message_lane::{ source_chain::{LaneMessageVerifier, MessageDeliveryAndDispatchPayment, TargetHeaderChain}, target_chain::{DispatchMessage, MessageDispatch, ProvedLaneMessages, ProvedMessages, SourceHeaderChain}, - InboundLaneData, LaneId, MessageData, MessageKey, MessageNonce, MessagePayload, OutboundLaneData, + total_unrewarded_messages, InboundLaneData, LaneId, MessageData, MessageKey, MessageNonce, MessagePayload, + OutboundLaneData, UnrewardedRelayersState, }; use codec::{Decode, Encode}; use frame_support::{ - decl_error, decl_event, decl_module, decl_storage, + decl_error, decl_event, decl_module, decl_storage, ensure, traits::Get, weights::{DispatchClass, Weight}, Parameter, StorageMap, @@ -156,6 +157,8 @@ decl_error! { InvalidMessagesDispatchWeight, /// Invalid messages delivery proof has been submitted. InvalidMessagesDeliveryProof, + /// The relayer has declared invalid unrewarded relayers state in the `receive_messages_delivery_proof` call. + InvalidUnrewardedRelayersState, } } @@ -408,7 +411,11 @@ decl_module! { /// Receive messages delivery proof from bridged chain. #[weight = 0] // TODO: update me (https://github.com/paritytech/parity-bridges-common/issues/78) - pub fn receive_messages_delivery_proof(origin, proof: MessagesDeliveryProofOf) -> DispatchResult { + pub fn receive_messages_delivery_proof( + origin, + proof: MessagesDeliveryProofOf, + relayers_state: UnrewardedRelayersState, + ) -> DispatchResult { ensure_operational::()?; let confirmation_relayer = ensure_signed(origin)?; @@ -421,6 +428,14 @@ decl_module! { Error::::InvalidMessagesDeliveryProof })?; + // verify that the relayer has declared correct `lane_data::relayers` state + // (we only care about total number of entries and messages, because this affects call weight) + ensure!( + total_unrewarded_messages(&lane_data.relayers) == relayers_state.total_messages + && lane_data.relayers.len() as MessageNonce == relayers_state.unrewarded_relayer_entries, + Error::::InvalidUnrewardedRelayersState + ); + // mark messages as delivered let mut lane = outbound_lane::(lane_id); let received_range = lane.confirm_delivery(lane_data.latest_received_nonce); @@ -501,6 +516,7 @@ impl, I: Instance> Module { bp_message_lane::UnrewardedRelayersState { unrewarded_relayer_entries: relayers.len() as _, messages_in_oldest_entry: relayers.front().map(|(begin, end, _)| 1 + end - begin).unwrap_or(0), + total_messages: total_unrewarded_messages(&relayers), } } } @@ -754,8 +770,9 @@ mod tests { InboundLaneData { latest_received_nonce: 1, ..Default::default() - } + }, )), + Default::default(), )); assert_eq!( @@ -863,8 +880,9 @@ mod tests { InboundLaneData { latest_received_nonce: 1, ..Default::default() - } + }, )), + Default::default(), ), Error::::Halted, ); @@ -955,6 +973,7 @@ mod tests { UnrewardedRelayersState { unrewarded_relayer_entries: 2, messages_in_oldest_entry: 1, + total_messages: 2, }, ); @@ -988,6 +1007,7 @@ mod tests { UnrewardedRelayersState { unrewarded_relayer_entries: 2, messages_in_oldest_entry: 1, + total_messages: 2, }, ); }); @@ -1065,6 +1085,11 @@ mod tests { ..Default::default() } )), + UnrewardedRelayersState { + unrewarded_relayer_entries: 1, + total_messages: 1, + ..Default::default() + }, )); assert!(TestMessageDeliveryAndDispatchPayment::is_reward_paid( TEST_RELAYER_A, @@ -1088,6 +1113,11 @@ mod tests { ..Default::default() } )), + UnrewardedRelayersState { + unrewarded_relayer_entries: 2, + total_messages: 2, + ..Default::default() + }, )); assert!(!TestMessageDeliveryAndDispatchPayment::is_reward_paid( TEST_RELAYER_A, @@ -1104,12 +1134,63 @@ mod tests { fn receive_messages_delivery_proof_rejects_invalid_proof() { run_test(|| { assert_noop!( - Module::::receive_messages_delivery_proof(Origin::signed(1), Err(()),), + Module::::receive_messages_delivery_proof(Origin::signed(1), Err(()), Default::default(),), Error::::InvalidMessagesDeliveryProof, ); }); } + #[test] + fn receive_messages_delivery_proof_rejects_proof_if_declared_relayers_state_is_invalid() { + run_test(|| { + // when number of relayers entires is invalid + assert_noop!( + Module::::receive_messages_delivery_proof( + Origin::signed(1), + Ok(( + TEST_LANE_ID, + InboundLaneData { + relayers: vec![(1, 1, TEST_RELAYER_A), (2, 2, TEST_RELAYER_B)] + .into_iter() + .collect(), + latest_received_nonce: 2, + ..Default::default() + } + )), + UnrewardedRelayersState { + unrewarded_relayer_entries: 1, + total_messages: 2, + ..Default::default() + }, + ), + Error::::InvalidUnrewardedRelayersState, + ); + + // when number of messages is invalid + assert_noop!( + Module::::receive_messages_delivery_proof( + Origin::signed(1), + Ok(( + TEST_LANE_ID, + InboundLaneData { + relayers: vec![(1, 1, TEST_RELAYER_A), (2, 2, TEST_RELAYER_B)] + .into_iter() + .collect(), + latest_received_nonce: 2, + ..Default::default() + } + )), + UnrewardedRelayersState { + unrewarded_relayer_entries: 2, + total_messages: 1, + ..Default::default() + }, + ), + Error::::InvalidUnrewardedRelayersState, + ); + }); + } + #[test] fn receive_messages_accepts_single_message_with_invalid_payload() { run_test(|| { diff --git a/bridges/primitives/message-lane/src/lib.rs b/bridges/primitives/message-lane/src/lib.rs index 86dea983d0..4edf936bff 100644 --- a/bridges/primitives/message-lane/src/lib.rs +++ b/bridges/primitives/message-lane/src/lib.rs @@ -103,13 +103,15 @@ impl Default for InboundLaneData { } /// Gist of `InboundLaneData::relayers` field used by runtime APIs. -#[derive(Clone, Encode, Decode, RuntimeDebug, PartialEq, Eq)] +#[derive(Clone, Default, Encode, Decode, RuntimeDebug, PartialEq, Eq)] pub struct UnrewardedRelayersState { /// Number of entries in the `InboundLaneData::relayers` set. pub unrewarded_relayer_entries: MessageNonce, /// Number of messages in the oldest entry of `InboundLaneData::relayers`. This is the /// minimal number of reward proofs required to push out this entry from the set. pub messages_in_oldest_entry: MessageNonce, + /// Total number of messages in the relayers vector. + pub total_messages: MessageNonce, } /// Outbound lane data. @@ -134,3 +136,13 @@ impl Default for OutboundLaneData { } } } + +/// Returns total number of messages in the `InboundLaneData::relayers` vector. +pub fn total_unrewarded_messages( + relayers: &VecDeque<(MessageNonce, MessageNonce, RelayerId)>, +) -> MessageNonce { + match (relayers.front(), relayers.back()) { + (Some((begin, _, _)), Some((_, end, _))) => end.checked_sub(*begin).and_then(|d| d.checked_add(1)).unwrap_or(0), + _ => 0, + } +} diff --git a/bridges/relays/messages-relay/src/message_lane_loop.rs b/bridges/relays/messages-relay/src/message_lane_loop.rs index c34323ace5..b60cbf3513 100644 --- a/bridges/relays/messages-relay/src/message_lane_loop.rs +++ b/bridges/relays/messages-relay/src/message_lane_loop.rs @@ -693,6 +693,7 @@ pub(crate) mod tests { UnrewardedRelayersState { unrewarded_relayer_entries: 0, messages_in_oldest_entry: 0, + total_messages: 0, }, )) } diff --git a/bridges/relays/messages-relay/src/message_race_delivery.rs b/bridges/relays/messages-relay/src/message_race_delivery.rs index f6933d9bd1..7724e9235d 100644 --- a/bridges/relays/messages-relay/src/message_race_delivery.rs +++ b/bridges/relays/messages-relay/src/message_race_delivery.rs @@ -497,6 +497,7 @@ mod tests { unrewarded_relayers: UnrewardedRelayersState { unrewarded_relayer_entries: 0, messages_in_oldest_entry: 0, + total_messages: 0, }, }, }), diff --git a/bridges/relays/substrate/src/messages_target.rs b/bridges/relays/substrate/src/messages_target.rs index 6b745cc7a8..8a16ecdd6e 100644 --- a/bridges/relays/substrate/src/messages_target.rs +++ b/bridges/relays/substrate/src/messages_target.rs @@ -37,7 +37,7 @@ use sp_trie::StorageProof; use std::ops::RangeInclusive; /// Message receiving proof returned by the target Substrate node. -pub type SubstrateMessagesReceivingProof = (HashOf, StorageProof, LaneId); +pub type SubstrateMessagesReceivingProof = (UnrewardedRelayersState, (HashOf, StorageProof, LaneId)); /// Substrate client as Substrate messages target. pub struct SubstrateMessagesTarget { @@ -156,12 +156,13 @@ where &self, id: TargetHeaderIdOf

, ) -> Result<(TargetHeaderIdOf

, P::MessagesReceivingProof), Self::Error> { + let (id, relayers_state) = self.unrewarded_relayers_state(id).await?; let proof = self .client .prove_messages_delivery(self.instance, self.lane_id, id.1) .await?; let proof = (id.1, proof, self.lane_id); - Ok((id, proof)) + Ok((id, (relayers_state, proof))) } async fn submit_messages_proof( diff --git a/bridges/relays/substrate/src/millau_messages_to_rialto.rs b/bridges/relays/substrate/src/millau_messages_to_rialto.rs index ebbb6a2bd5..b5d1db888e 100644 --- a/bridges/relays/substrate/src/millau_messages_to_rialto.rs +++ b/bridges/relays/substrate/src/millau_messages_to_rialto.rs @@ -59,9 +59,10 @@ impl SubstrateMessageLane for MillauMessagesToRialto { _generated_at_block: RialtoHeaderId, proof: ::MessagesReceivingProof, ) -> Result { + let (relayers_state, proof) = proof; let account_id = self.source_sign.signer.public().as_array_ref().clone().into(); let nonce = self.source_client.next_account_index(account_id).await?; - let call = millau_runtime::MessageLaneCall::receive_messages_delivery_proof(proof).into(); + let call = millau_runtime::MessageLaneCall::receive_messages_delivery_proof(proof, relayers_state).into(); let transaction = Millau::sign_transaction(&self.source_client, &self.source_sign.signer, nonce, call); Ok(transaction) } diff --git a/bridges/relays/substrate/src/rialto_messages_to_millau.rs b/bridges/relays/substrate/src/rialto_messages_to_millau.rs index 0e059f7cfd..22e56e312d 100644 --- a/bridges/relays/substrate/src/rialto_messages_to_millau.rs +++ b/bridges/relays/substrate/src/rialto_messages_to_millau.rs @@ -59,9 +59,10 @@ impl SubstrateMessageLane for RialtoMessagesToMillau { _generated_at_block: MillauHeaderId, proof: ::MessagesReceivingProof, ) -> Result { + let (relayers_state, proof) = proof; let account_id = self.source_sign.signer.public().as_array_ref().clone().into(); let nonce = self.source_client.next_account_index(account_id).await?; - let call = rialto_runtime::MessageLaneCall::receive_messages_delivery_proof(proof).into(); + let call = rialto_runtime::MessageLaneCall::receive_messages_delivery_proof(proof, relayers_state).into(); let transaction = Rialto::sign_transaction(&self.source_client, &self.source_sign.signer, nonce, call); Ok(transaction) }