From 19f9c8ffdbc91b0763fbc930f749fae63d81882a Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Fri, 4 Aug 2023 16:14:36 +0300 Subject: [PATCH] remove message sender origin (#2322) --- bin/millau/runtime/src/rialto_messages.rs | 7 -- .../runtime/src/rialto_parachain_messages.rs | 7 -- .../runtime/src/millau_messages.rs | 6 -- bin/rialto/runtime/src/millau_messages.rs | 7 -- bin/runtime-common/src/messages.rs | 10 +-- .../src/messages_xcm_extension.rs | 13 +-- modules/messages/src/lib.rs | 87 +++++-------------- modules/messages/src/mock.rs | 3 +- primitives/messages/src/source_chain.rs | 22 ++--- 9 files changed, 34 insertions(+), 128 deletions(-) diff --git a/bin/millau/runtime/src/rialto_messages.rs b/bin/millau/runtime/src/rialto_messages.rs index 04084e0b9520c..a9a80fdf7d715 100644 --- a/bin/millau/runtime/src/rialto_messages.rs +++ b/bin/millau/runtime/src/rialto_messages.rs @@ -27,7 +27,6 @@ use bridge_runtime_common::{ }; use frame_support::{parameter_types, weights::Weight, RuntimeDebug}; use pallet_bridge_relayers::WeightInfoExt as _; -use xcm::latest::prelude::*; use xcm_builder::HaulBlobExporter; /// Default lane that is used to send messages to Rialto. @@ -124,12 +123,6 @@ pub struct ToRialtoXcmBlobHauler; impl XcmBlobHauler for ToRialtoXcmBlobHauler { type MessageSender = pallet_bridge_messages::Pallet; - type MessageSenderOrigin = RuntimeOrigin; - - fn message_sender_origin() -> RuntimeOrigin { - pallet_xcm::Origin::from(MultiLocation::new(1, crate::xcm_config::UniversalLocation::get())) - .into() - } fn xcm_lane() -> LaneId { XCM_LANE diff --git a/bin/millau/runtime/src/rialto_parachain_messages.rs b/bin/millau/runtime/src/rialto_parachain_messages.rs index 1050f500fe8c1..a8540b705d754 100644 --- a/bin/millau/runtime/src/rialto_parachain_messages.rs +++ b/bin/millau/runtime/src/rialto_parachain_messages.rs @@ -29,7 +29,6 @@ use bridge_runtime_common::{ }; use frame_support::{parameter_types, weights::Weight, RuntimeDebug}; use pallet_bridge_relayers::WeightInfoExt as _; -use xcm::latest::prelude::*; use xcm_builder::HaulBlobExporter; /// Default lane that is used to send messages to Rialto parachain. @@ -125,12 +124,6 @@ pub struct ToRialtoParachainXcmBlobHauler; impl XcmBlobHauler for ToRialtoParachainXcmBlobHauler { type MessageSender = pallet_bridge_messages::Pallet; - type MessageSenderOrigin = RuntimeOrigin; - - fn message_sender_origin() -> RuntimeOrigin { - pallet_xcm::Origin::from(MultiLocation::new(1, crate::xcm_config::UniversalLocation::get())) - .into() - } fn xcm_lane() -> LaneId { XCM_LANE diff --git a/bin/rialto-parachain/runtime/src/millau_messages.rs b/bin/rialto-parachain/runtime/src/millau_messages.rs index 68c9cbbec6011..666b1f0a8412e 100644 --- a/bin/rialto-parachain/runtime/src/millau_messages.rs +++ b/bin/rialto-parachain/runtime/src/millau_messages.rs @@ -29,7 +29,6 @@ use bridge_runtime_common::{ messages_xcm_extension::{XcmBlobHauler, XcmBlobHaulerAdapter}, }; use frame_support::{parameter_types, weights::Weight, RuntimeDebug}; -use xcm::latest::prelude::*; use xcm_builder::HaulBlobExporter; /// Default lane that is used to send messages to Millau. @@ -124,11 +123,6 @@ pub struct ToMillauXcmBlobHauler; impl XcmBlobHauler for ToMillauXcmBlobHauler { type MessageSender = pallet_bridge_messages::Pallet; - type MessageSenderOrigin = RuntimeOrigin; - - fn message_sender_origin() -> RuntimeOrigin { - pallet_xcm::Origin::from(MultiLocation::new(1, crate::UniversalLocation::get())).into() - } fn xcm_lane() -> LaneId { XCM_LANE diff --git a/bin/rialto/runtime/src/millau_messages.rs b/bin/rialto/runtime/src/millau_messages.rs index a99243cc3e6a9..357a5d7f02f09 100644 --- a/bin/rialto/runtime/src/millau_messages.rs +++ b/bin/rialto/runtime/src/millau_messages.rs @@ -26,7 +26,6 @@ use bridge_runtime_common::{ messages_xcm_extension::{XcmBlobHauler, XcmBlobHaulerAdapter}, }; use frame_support::{parameter_types, weights::Weight, RuntimeDebug}; -use xcm::latest::prelude::*; use xcm_builder::HaulBlobExporter; /// Lane that is used for XCM messages exchange. @@ -123,12 +122,6 @@ pub struct ToMillauXcmBlobHauler; impl XcmBlobHauler for ToMillauXcmBlobHauler { type MessageSender = pallet_bridge_messages::Pallet; - type MessageSenderOrigin = RuntimeOrigin; - - fn message_sender_origin() -> RuntimeOrigin { - pallet_xcm::Origin::from(MultiLocation::new(1, crate::xcm_config::UniversalLocation::get())) - .into() - } fn xcm_lane() -> LaneId { XCM_LANE diff --git a/bin/runtime-common/src/messages.rs b/bin/runtime-common/src/messages.rs index e6c7deb98a2ca..316c4d3cad927 100644 --- a/bin/runtime-common/src/messages.rs +++ b/bin/runtime-common/src/messages.rs @@ -72,8 +72,6 @@ pub type HasherOf = bp_runtime::HasherOf>; pub type AccountIdOf = bp_runtime::AccountIdOf>; /// Type of balances that is used on the chain. pub type BalanceOf = bp_runtime::BalanceOf>; -/// Type of origin that is used on the chain. -pub type OriginOf = ::RuntimeOrigin; /// Sub-module that is declaring types required for processing This -> Bridged chain messages. pub mod source { @@ -138,17 +136,11 @@ pub mod source { #[derive(RuntimeDebug)] pub struct FromThisChainMessageVerifier(PhantomData); - impl LaneMessageVerifier>, FromThisChainMessagePayload> - for FromThisChainMessageVerifier + impl LaneMessageVerifier for FromThisChainMessageVerifier where B: MessageBridge, - // matches requirements from the `frame_system::Config::Origin` - OriginOf>: Clone - + Into>>, OriginOf>>>, - AccountIdOf>: PartialEq + Clone, { fn verify_message( - _submitter: &OriginOf>, _lane: &LaneId, _lane_outbound_data: &OutboundLaneData, _payload: &FromThisChainMessagePayload, diff --git a/bin/runtime-common/src/messages_xcm_extension.rs b/bin/runtime-common/src/messages_xcm_extension.rs index 96fdf1d501826..0317d745c15cf 100644 --- a/bin/runtime-common/src/messages_xcm_extension.rs +++ b/bin/runtime-common/src/messages_xcm_extension.rs @@ -110,12 +110,7 @@ impl MessageDispat /// one side, where on the other it can be dispatched by [`XcmBlobMessageDispatch`]. pub trait XcmBlobHauler { /// Runtime message sender adapter. - type MessageSender: MessagesBridge; - - /// Runtime message sender origin, which is used by [`Self::MessageSender`]. - type MessageSenderOrigin; - /// Our location within the Consensus Universe. - fn message_sender_origin() -> Self::MessageSenderOrigin; + type MessageSender: MessagesBridge; /// Return message lane (as "point-to-point link") used to deliver XCM messages. fn xcm_lane() -> LaneId; @@ -124,12 +119,10 @@ pub trait XcmBlobHauler { /// XCM bridge adapter which connects [`XcmBlobHauler`] with [`XcmBlobHauler::MessageSender`] and /// makes sure that XCM blob is sent to the [`pallet_bridge_messages`] queue to be relayed. pub struct XcmBlobHaulerAdapter(sp_std::marker::PhantomData); -impl> HaulBlob - for XcmBlobHaulerAdapter -{ +impl HaulBlob for XcmBlobHaulerAdapter { fn haul_blob(blob: sp_std::prelude::Vec) -> Result<(), HaulBlobError> { let lane = H::xcm_lane(); - H::MessageSender::send_message(H::message_sender_origin(), lane, blob) + H::MessageSender::send_message(lane, blob) .map(|artifacts| (lane, artifacts.nonce).using_encoded(sp_io::hashing::blake2_256)) .map(|result| { log::info!( diff --git a/modules/messages/src/lib.rs b/modules/messages/src/lib.rs index 3bfd5bc7e98f7..91ab3c965b93c 100644 --- a/modules/messages/src/lib.rs +++ b/modules/messages/src/lib.rs @@ -153,7 +153,7 @@ pub mod pallet { /// Target header chain. type TargetHeaderChain: TargetHeaderChain; /// Message payload verifier. - type LaneMessageVerifier: LaneMessageVerifier; + type LaneMessageVerifier: LaneMessageVerifier; /// Delivery confirmation payments. type DeliveryConfirmationPayments: DeliveryConfirmationPayments; @@ -643,8 +643,7 @@ pub mod pallet { } } -impl bp_messages::source_chain::MessagesBridge - for Pallet +impl bp_messages::source_chain::MessagesBridge for Pallet where T: Config, I: 'static, @@ -652,17 +651,15 @@ where type Error = sp_runtime::DispatchErrorWithPostInfo; fn send_message( - sender: T::RuntimeOrigin, lane: LaneId, message: T::OutboundPayload, ) -> Result { - crate::send_message::(sender, lane, message) + crate::send_message::(lane, message) } } /// Function that actually sends message. fn send_message, I: 'static>( - submitter: T::RuntimeOrigin, lane_id: LaneId, payload: T::OutboundPayload, ) -> sp_std::result::Result< @@ -688,18 +685,16 @@ fn send_message, I: 'static>( // now let's enforce any additional lane rules let mut lane = outbound_lane::(lane_id); - T::LaneMessageVerifier::verify_message(&submitter, &lane_id, &lane.data(), &payload).map_err( - |err| { - log::trace!( - target: LOG_TARGET, - "Message to lane {:?} is rejected by lane verifier: {:?}", - lane_id, - err, - ); + T::LaneMessageVerifier::verify_message(&lane_id, &lane.data(), &payload).map_err(|err| { + log::trace!( + target: LOG_TARGET, + "Message to lane {:?} is rejected by lane verifier: {:?}", + lane_id, + err, + ); - Error::::MessageRejectedByLaneVerifier(err) - }, - )?; + Error::::MessageRejectedByLaneVerifier(err) + })?; // finally, save message in outbound storage and emit event let encoded_payload = payload.encode(); @@ -915,7 +910,7 @@ mod tests { let message_nonce = outbound_lane::(TEST_LANE_ID).data().latest_generated_nonce + 1; - send_message::(RuntimeOrigin::signed(1), TEST_LANE_ID, REGULAR_PAYLOAD) + send_message::(TEST_LANE_ID, REGULAR_PAYLOAD) .expect("send_message has failed"); // check event with assigned nonce @@ -982,11 +977,7 @@ mod tests { )); assert_noop!( - send_message::( - RuntimeOrigin::signed(1), - TEST_LANE_ID, - REGULAR_PAYLOAD, - ), + send_message::(TEST_LANE_ID, REGULAR_PAYLOAD,), Error::::NotOperatingNormally, ); @@ -1036,11 +1027,7 @@ mod tests { ); assert_noop!( - send_message::( - RuntimeOrigin::signed(1), - TEST_LANE_ID, - REGULAR_PAYLOAD, - ), + send_message::(TEST_LANE_ID, REGULAR_PAYLOAD,), Error::::NotOperatingNormally, ); @@ -1090,11 +1077,7 @@ mod tests { .extra .extend_from_slice(&[0u8; MAX_OUTBOUND_PAYLOAD_SIZE as usize]); assert_noop!( - send_message::( - RuntimeOrigin::signed(1), - TEST_LANE_ID, - message_payload.clone(), - ), + send_message::(TEST_LANE_ID, message_payload.clone(),), Error::::MessageRejectedByPallet( VerificationError::MessageTooLarge ), @@ -1105,11 +1088,7 @@ mod tests { message_payload.extra.pop(); } assert_eq!(message_payload.encoded_size() as u32, MAX_OUTBOUND_PAYLOAD_SIZE); - assert_ok!(send_message::( - RuntimeOrigin::signed(1), - TEST_LANE_ID, - message_payload, - ),); + assert_ok!(send_message::(TEST_LANE_ID, message_payload,),); }) } @@ -1118,11 +1097,7 @@ mod tests { run_test(|| { // messages with this payload are rejected by target chain verifier assert_noop!( - send_message::( - RuntimeOrigin::signed(1), - TEST_LANE_ID, - PAYLOAD_REJECTED_BY_TARGET_CHAIN, - ), + send_message::(TEST_LANE_ID, PAYLOAD_REJECTED_BY_TARGET_CHAIN,), Error::::MessageRejectedByChainVerifier(VerificationError::Other( mock::TEST_ERROR )), @@ -1137,7 +1112,7 @@ mod tests { let mut message = REGULAR_PAYLOAD; message.reject_by_lane_verifier = true; assert_noop!( - send_message::(RuntimeOrigin::signed(1), TEST_LANE_ID, message,), + send_message::(TEST_LANE_ID, message,), Error::::MessageRejectedByLaneVerifier(VerificationError::Other( mock::TEST_ERROR )), @@ -1293,16 +1268,8 @@ mod tests { #[test] fn receive_messages_delivery_proof_rewards_relayers() { run_test(|| { - assert_ok!(send_message::( - RuntimeOrigin::signed(1), - TEST_LANE_ID, - REGULAR_PAYLOAD, - )); - assert_ok!(send_message::( - RuntimeOrigin::signed(1), - TEST_LANE_ID, - REGULAR_PAYLOAD, - )); + assert_ok!(send_message::(TEST_LANE_ID, REGULAR_PAYLOAD,)); + assert_ok!(send_message::(TEST_LANE_ID, REGULAR_PAYLOAD,)); // this reports delivery of message 1 => reward is paid to TEST_RELAYER_A let single_message_delivery_proof = TestMessagesDeliveryProof(Ok(( @@ -1899,11 +1866,7 @@ mod tests { send_regular_message(); receive_messages_delivery_proof(); // send + receive confirmation for lane 2 - assert_ok!(send_message::( - RuntimeOrigin::signed(1), - TEST_LANE_ID_2, - REGULAR_PAYLOAD, - )); + assert_ok!(send_message::(TEST_LANE_ID_2, REGULAR_PAYLOAD,)); assert_ok!(Pallet::::receive_messages_delivery_proof( RuntimeOrigin::signed(1), TestMessagesDeliveryProof(Ok(( @@ -1979,11 +1942,7 @@ mod tests { fn outbound_message_from_unconfigured_lane_is_rejected() { run_test(|| { assert_noop!( - send_message::( - RuntimeOrigin::signed(1), - TEST_LANE_ID_3, - REGULAR_PAYLOAD, - ), + send_message::(TEST_LANE_ID_3, REGULAR_PAYLOAD,), Error::::InactiveOutboundLane, ); }); diff --git a/modules/messages/src/mock.rs b/modules/messages/src/mock.rs index 62bc76c5e010b..83752523efb9b 100644 --- a/modules/messages/src/mock.rs +++ b/modules/messages/src/mock.rs @@ -310,9 +310,8 @@ impl TargetHeaderChain for TestTargetHeaderChain { #[derive(Debug, Default)] pub struct TestLaneMessageVerifier; -impl LaneMessageVerifier for TestLaneMessageVerifier { +impl LaneMessageVerifier for TestLaneMessageVerifier { fn verify_message( - _submitter: &RuntimeOrigin, _lane: &LaneId, _lane_outbound_data: &OutboundLaneData, payload: &TestPayload, diff --git a/primitives/messages/src/source_chain.rs b/primitives/messages/src/source_chain.rs index f3c50b84a09ad..09f6396ad7421 100644 --- a/primitives/messages/src/source_chain.rs +++ b/primitives/messages/src/source_chain.rs @@ -71,11 +71,10 @@ pub trait TargetHeaderChain { /// Lane3 until some block, ...), then it may be built using this verifier. /// /// Any fee requirements should also be enforced here. -pub trait LaneMessageVerifier { +pub trait LaneMessageVerifier { /// Verify message payload and return Ok(()) if message is valid and allowed to be sent over the /// lane. fn verify_message( - submitter: &SenderOrigin, lane: &LaneId, outbound_data: &OutboundLaneData, payload: &Payload, @@ -124,32 +123,24 @@ pub struct SendMessageArtifacts { } /// Messages bridge API to be used from other pallets. -pub trait MessagesBridge { +pub trait MessagesBridge { /// Error type. type Error: Debug; /// Send message over the bridge. /// /// Returns unique message nonce or error if send has failed. - fn send_message( - sender: SenderOrigin, - lane: LaneId, - message: Payload, - ) -> Result; + fn send_message(lane: LaneId, message: Payload) -> Result; } /// Bridge that does nothing when message is being sent. #[derive(Eq, RuntimeDebug, PartialEq)] pub struct NoopMessagesBridge; -impl MessagesBridge for NoopMessagesBridge { +impl MessagesBridge for NoopMessagesBridge { type Error = &'static str; - fn send_message( - _sender: SenderOrigin, - _lane: LaneId, - _message: Payload, - ) -> Result { + fn send_message(_lane: LaneId, _message: Payload) -> Result { Ok(SendMessageArtifacts { nonce: 0 }) } } @@ -176,9 +167,8 @@ impl TargetHeaderChain for ForbidOutboun } } -impl LaneMessageVerifier for ForbidOutboundMessages { +impl LaneMessageVerifier for ForbidOutboundMessages { fn verify_message( - _submitter: &SenderOrigin, _lane: &LaneId, _outbound_data: &OutboundLaneData, _payload: &Payload,