add relayers_state param to the receive_messages_delivery_proof (#584)

This commit is contained in:
Svyatoslav Nikolsky
2020-12-22 09:22:07 +03:00
committed by Bastian Köcher
parent 595481f02e
commit 043a008723
7 changed files with 109 additions and 11 deletions
+87 -6
View File
@@ -35,11 +35,12 @@ use crate::outbound_lane::{OutboundLane, OutboundLaneStorage};
use bp_message_lane::{ use bp_message_lane::{
source_chain::{LaneMessageVerifier, MessageDeliveryAndDispatchPayment, TargetHeaderChain}, source_chain::{LaneMessageVerifier, MessageDeliveryAndDispatchPayment, TargetHeaderChain},
target_chain::{DispatchMessage, MessageDispatch, ProvedLaneMessages, ProvedMessages, SourceHeaderChain}, 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 codec::{Decode, Encode};
use frame_support::{ use frame_support::{
decl_error, decl_event, decl_module, decl_storage, decl_error, decl_event, decl_module, decl_storage, ensure,
traits::Get, traits::Get,
weights::{DispatchClass, Weight}, weights::{DispatchClass, Weight},
Parameter, StorageMap, Parameter, StorageMap,
@@ -156,6 +157,8 @@ decl_error! {
InvalidMessagesDispatchWeight, InvalidMessagesDispatchWeight,
/// Invalid messages delivery proof has been submitted. /// Invalid messages delivery proof has been submitted.
InvalidMessagesDeliveryProof, 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. /// Receive messages delivery proof from bridged chain.
#[weight = 0] // TODO: update me (https://github.com/paritytech/parity-bridges-common/issues/78) #[weight = 0] // TODO: update me (https://github.com/paritytech/parity-bridges-common/issues/78)
pub fn receive_messages_delivery_proof(origin, proof: MessagesDeliveryProofOf<T, I>) -> DispatchResult { pub fn receive_messages_delivery_proof(
origin,
proof: MessagesDeliveryProofOf<T, I>,
relayers_state: UnrewardedRelayersState,
) -> DispatchResult {
ensure_operational::<T, I>()?; ensure_operational::<T, I>()?;
let confirmation_relayer = ensure_signed(origin)?; let confirmation_relayer = ensure_signed(origin)?;
@@ -421,6 +428,14 @@ decl_module! {
Error::<T, I>::InvalidMessagesDeliveryProof Error::<T, I>::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::<T, I>::InvalidUnrewardedRelayersState
);
// mark messages as delivered // mark messages as delivered
let mut lane = outbound_lane::<T, I>(lane_id); let mut lane = outbound_lane::<T, I>(lane_id);
let received_range = lane.confirm_delivery(lane_data.latest_received_nonce); let received_range = lane.confirm_delivery(lane_data.latest_received_nonce);
@@ -501,6 +516,7 @@ impl<T: Config<I>, I: Instance> Module<T, I> {
bp_message_lane::UnrewardedRelayersState { bp_message_lane::UnrewardedRelayersState {
unrewarded_relayer_entries: relayers.len() as _, unrewarded_relayer_entries: relayers.len() as _,
messages_in_oldest_entry: relayers.front().map(|(begin, end, _)| 1 + end - begin).unwrap_or(0), 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 { InboundLaneData {
latest_received_nonce: 1, latest_received_nonce: 1,
..Default::default() ..Default::default()
} },
)), )),
Default::default(),
)); ));
assert_eq!( assert_eq!(
@@ -863,8 +880,9 @@ mod tests {
InboundLaneData { InboundLaneData {
latest_received_nonce: 1, latest_received_nonce: 1,
..Default::default() ..Default::default()
} },
)), )),
Default::default(),
), ),
Error::<TestRuntime, DefaultInstance>::Halted, Error::<TestRuntime, DefaultInstance>::Halted,
); );
@@ -955,6 +973,7 @@ mod tests {
UnrewardedRelayersState { UnrewardedRelayersState {
unrewarded_relayer_entries: 2, unrewarded_relayer_entries: 2,
messages_in_oldest_entry: 1, messages_in_oldest_entry: 1,
total_messages: 2,
}, },
); );
@@ -988,6 +1007,7 @@ mod tests {
UnrewardedRelayersState { UnrewardedRelayersState {
unrewarded_relayer_entries: 2, unrewarded_relayer_entries: 2,
messages_in_oldest_entry: 1, messages_in_oldest_entry: 1,
total_messages: 2,
}, },
); );
}); });
@@ -1065,6 +1085,11 @@ mod tests {
..Default::default() ..Default::default()
} }
)), )),
UnrewardedRelayersState {
unrewarded_relayer_entries: 1,
total_messages: 1,
..Default::default()
},
)); ));
assert!(TestMessageDeliveryAndDispatchPayment::is_reward_paid( assert!(TestMessageDeliveryAndDispatchPayment::is_reward_paid(
TEST_RELAYER_A, TEST_RELAYER_A,
@@ -1088,6 +1113,11 @@ mod tests {
..Default::default() ..Default::default()
} }
)), )),
UnrewardedRelayersState {
unrewarded_relayer_entries: 2,
total_messages: 2,
..Default::default()
},
)); ));
assert!(!TestMessageDeliveryAndDispatchPayment::is_reward_paid( assert!(!TestMessageDeliveryAndDispatchPayment::is_reward_paid(
TEST_RELAYER_A, TEST_RELAYER_A,
@@ -1104,12 +1134,63 @@ mod tests {
fn receive_messages_delivery_proof_rejects_invalid_proof() { fn receive_messages_delivery_proof_rejects_invalid_proof() {
run_test(|| { run_test(|| {
assert_noop!( assert_noop!(
Module::<TestRuntime>::receive_messages_delivery_proof(Origin::signed(1), Err(()),), Module::<TestRuntime>::receive_messages_delivery_proof(Origin::signed(1), Err(()), Default::default(),),
Error::<TestRuntime, DefaultInstance>::InvalidMessagesDeliveryProof, Error::<TestRuntime, DefaultInstance>::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::<TestRuntime>::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::<TestRuntime, DefaultInstance>::InvalidUnrewardedRelayersState,
);
// when number of messages is invalid
assert_noop!(
Module::<TestRuntime>::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::<TestRuntime, DefaultInstance>::InvalidUnrewardedRelayersState,
);
});
}
#[test] #[test]
fn receive_messages_accepts_single_message_with_invalid_payload() { fn receive_messages_accepts_single_message_with_invalid_payload() {
run_test(|| { run_test(|| {
+13 -1
View File
@@ -103,13 +103,15 @@ impl<RelayerId> Default for InboundLaneData<RelayerId> {
} }
/// Gist of `InboundLaneData::relayers` field used by runtime APIs. /// 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 { pub struct UnrewardedRelayersState {
/// Number of entries in the `InboundLaneData::relayers` set. /// Number of entries in the `InboundLaneData::relayers` set.
pub unrewarded_relayer_entries: MessageNonce, pub unrewarded_relayer_entries: MessageNonce,
/// Number of messages in the oldest entry of `InboundLaneData::relayers`. This is the /// 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. /// minimal number of reward proofs required to push out this entry from the set.
pub messages_in_oldest_entry: MessageNonce, pub messages_in_oldest_entry: MessageNonce,
/// Total number of messages in the relayers vector.
pub total_messages: MessageNonce,
} }
/// Outbound lane data. /// 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<RelayerId>(
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,
}
}
@@ -693,6 +693,7 @@ pub(crate) mod tests {
UnrewardedRelayersState { UnrewardedRelayersState {
unrewarded_relayer_entries: 0, unrewarded_relayer_entries: 0,
messages_in_oldest_entry: 0, messages_in_oldest_entry: 0,
total_messages: 0,
}, },
)) ))
} }
@@ -497,6 +497,7 @@ mod tests {
unrewarded_relayers: UnrewardedRelayersState { unrewarded_relayers: UnrewardedRelayersState {
unrewarded_relayer_entries: 0, unrewarded_relayer_entries: 0,
messages_in_oldest_entry: 0, messages_in_oldest_entry: 0,
total_messages: 0,
}, },
}, },
}), }),
@@ -37,7 +37,7 @@ use sp_trie::StorageProof;
use std::ops::RangeInclusive; use std::ops::RangeInclusive;
/// Message receiving proof returned by the target Substrate node. /// Message receiving proof returned by the target Substrate node.
pub type SubstrateMessagesReceivingProof<C> = (HashOf<C>, StorageProof, LaneId); pub type SubstrateMessagesReceivingProof<C> = (UnrewardedRelayersState, (HashOf<C>, StorageProof, LaneId));
/// Substrate client as Substrate messages target. /// Substrate client as Substrate messages target.
pub struct SubstrateMessagesTarget<C: Chain, P> { pub struct SubstrateMessagesTarget<C: Chain, P> {
@@ -156,12 +156,13 @@ where
&self, &self,
id: TargetHeaderIdOf<P>, id: TargetHeaderIdOf<P>,
) -> Result<(TargetHeaderIdOf<P>, P::MessagesReceivingProof), Self::Error> { ) -> Result<(TargetHeaderIdOf<P>, P::MessagesReceivingProof), Self::Error> {
let (id, relayers_state) = self.unrewarded_relayers_state(id).await?;
let proof = self let proof = self
.client .client
.prove_messages_delivery(self.instance, self.lane_id, id.1) .prove_messages_delivery(self.instance, self.lane_id, id.1)
.await?; .await?;
let proof = (id.1, proof, self.lane_id); let proof = (id.1, proof, self.lane_id);
Ok((id, proof)) Ok((id, (relayers_state, proof)))
} }
async fn submit_messages_proof( async fn submit_messages_proof(
@@ -59,9 +59,10 @@ impl SubstrateMessageLane for MillauMessagesToRialto {
_generated_at_block: RialtoHeaderId, _generated_at_block: RialtoHeaderId,
proof: <Self as MessageLane>::MessagesReceivingProof, proof: <Self as MessageLane>::MessagesReceivingProof,
) -> Result<Self::SourceSignedTransaction, SubstrateError> { ) -> Result<Self::SourceSignedTransaction, SubstrateError> {
let (relayers_state, proof) = proof;
let account_id = self.source_sign.signer.public().as_array_ref().clone().into(); 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 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); let transaction = Millau::sign_transaction(&self.source_client, &self.source_sign.signer, nonce, call);
Ok(transaction) Ok(transaction)
} }
@@ -59,9 +59,10 @@ impl SubstrateMessageLane for RialtoMessagesToMillau {
_generated_at_block: MillauHeaderId, _generated_at_block: MillauHeaderId,
proof: <Self as MessageLane>::MessagesReceivingProof, proof: <Self as MessageLane>::MessagesReceivingProof,
) -> Result<Self::SourceSignedTransaction, SubstrateError> { ) -> Result<Self::SourceSignedTransaction, SubstrateError> {
let (relayers_state, proof) = proof;
let account_id = self.source_sign.signer.public().as_array_ref().clone().into(); 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 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); let transaction = Rialto::sign_transaction(&self.source_client, &self.source_sign.signer, nonce, call);
Ok(transaction) Ok(transaction)
} }