From ae0148e62e02dcc02e9260b8663eabd6e6eee929 Mon Sep 17 00:00:00 2001 From: vedhavyas Date: Thu, 6 Oct 2022 11:23:52 +0200 Subject: [PATCH 1/8] refactor tests to easily define multiple domains --- crates/pallet-messenger/src/lib.rs | 12 ++ crates/pallet-messenger/src/messages.rs | 6 + crates/pallet-messenger/src/mock.rs | 171 +++++++++++++----------- crates/pallet-messenger/src/tests.rs | 55 ++++---- 4 files changed, 141 insertions(+), 103 deletions(-) diff --git a/crates/pallet-messenger/src/lib.rs b/crates/pallet-messenger/src/lib.rs index d60b32c59a..432de2a04e 100644 --- a/crates/pallet-messenger/src/lib.rs +++ b/crates/pallet-messenger/src/lib.rs @@ -89,6 +89,7 @@ mod pallet { Channel, ChannelId, ChannelState, InitiateChannelParams, Nonce, StateRootOf, U256, }; use frame_support::pallet_prelude::*; + use frame_support::transactional; use frame_system::pallet_prelude::*; use sp_core::storage::StorageKey; use sp_messenger::SystemDomainTracker; @@ -204,6 +205,15 @@ mod pallet { channel_id: ChannelId, nonce: Nonce, }, + + /// Emits when a message response is available for Inbox message. + InboxMessageResponse { + /// Destination domain ID. + domain_id: T::DomainId, + /// Channel Is + channel_id: ChannelId, + nonce: Nonce, + }, } type Tag = (DomainId, ChannelId, Nonce); @@ -327,6 +337,7 @@ mod pallet { /// Channel is set to initiated and do not accept or receive any messages. /// Only a root user can create the channel. #[pallet::weight((10_000, Pays::No))] + #[transactional] pub fn initiate_channel( origin: OriginFor, dst_domain_id: T::DomainId, @@ -355,6 +366,7 @@ mod pallet { /// Channel is set to Closed and do not accept or receive any messages. /// Only a root user can close an open channel. #[pallet::weight((10_000, Pays::No))] + #[transactional] pub fn close_channel( origin: OriginFor, domain_id: T::DomainId, diff --git a/crates/pallet-messenger/src/messages.rs b/crates/pallet-messenger/src/messages.rs index a1e91f10fa..979dc75d99 100644 --- a/crates/pallet-messenger/src/messages.rs +++ b/crates/pallet-messenger/src/messages.rs @@ -150,6 +150,12 @@ impl Pallet { }, ); + Self::deposit_event(Event::InboxMessageResponse { + domain_id: dst_domain_id, + channel_id, + nonce: next_inbox_nonce, + }); + next_inbox_nonce = next_inbox_nonce .checked_add(Nonce::one()) .ok_or(DispatchError::Arithmetic(ArithmeticError::Overflow))?; diff --git a/crates/pallet-messenger/src/mock.rs b/crates/pallet-messenger/src/mock.rs index b67689b144..c9743b30b9 100644 --- a/crates/pallet-messenger/src/mock.rs +++ b/crates/pallet-messenger/src/mock.rs @@ -1,69 +1,107 @@ -use crate::{ChannelId, Channels}; -use frame_support::parameter_types; +use crate::{ChannelId, Channels, Config}; use frame_support::storage::generator::StorageDoubleMap; -use frame_support::traits::{ConstU16, ConstU32, ConstU64}; use sp_core::storage::StorageKey; use sp_core::H256; -use sp_runtime::testing::Header; -use sp_runtime::traits::{BlakeTwo256, IdentityLookup}; use sp_state_machine::backend::Backend; use sp_state_machine::{prove_read, InMemoryBackend}; -use sp_std::vec::Vec; use sp_trie::StorageProof; -type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; -type Block = frame_system::mocking::MockBlock; - -frame_support::construct_runtime!( - pub struct Test where - Block = Block, - NodeBlock = Block, - UncheckedExtrinsic = UncheckedExtrinsic, - { - System: frame_system::{Pallet, Call, Config, Storage, Event}, - SystemDomainTracker: mock_system_domain_tracker::{Pallet, Storage}, - Messenger: crate::{Pallet, Call, Event} - } -); - -impl frame_system::Config for Test { - type BaseCallFilter = frame_support::traits::Everything; - type BlockWeights = (); - type BlockLength = (); - type DbWeight = (); - type Origin = Origin; - type Call = Call; - type Index = u64; - type BlockNumber = u64; - type Hash = H256; - type Hashing = BlakeTwo256; - type AccountId = u64; - type Lookup = IdentityLookup; - type Header = Header; - type Event = Event; - type BlockHashCount = ConstU64<250>; - type Version = (); - type PalletInfo = PalletInfo; - type AccountData = (); - type OnNewAccount = (); - type OnKilledAccount = (); - type SystemWeightInfo = (); - type SS58Prefix = ConstU16<42>; - type OnSetCode = (); - type MaxConsumers = ConstU32<16>; +pub(crate) type DomainId = u64; + +macro_rules! impl_runtime { + ($runtime:ty, $domain_id:literal) => { + use crate::mock::{mock_system_domain_tracker, DomainId}; + use frame_support::parameter_types; + use sp_core::H256; + use sp_runtime::testing::Header; + use sp_runtime::traits::{BlakeTwo256, ConstU16, ConstU32, ConstU64, IdentityLookup}; + use sp_std::vec::Vec; + + type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; + type Block = frame_system::mocking::MockBlock; + + frame_support::construct_runtime!( + pub struct Runtime where + Block = Block, + NodeBlock = Block, + UncheckedExtrinsic = UncheckedExtrinsic, + { + System: frame_system::{Pallet, Call, Config, Storage, Event}, + SystemDomainTracker: mock_system_domain_tracker::{Pallet, Storage}, + Messenger: crate::{Pallet, Call, Event} + } + ); + + + impl frame_system::Config for $runtime { + type BaseCallFilter = frame_support::traits::Everything; + type BlockWeights = (); + type BlockLength = (); + type DbWeight = (); + type Origin = Origin; + type Call = Call; + type Index = u64; + type BlockNumber = u64; + type Hash = H256; + type Hashing = BlakeTwo256; + type AccountId = u64; + type Lookup = IdentityLookup; + type Header = Header; + type Event = Event; + type BlockHashCount = ConstU64<250>; + type Version = (); + type PalletInfo = PalletInfo; + type AccountData = (); + type OnNewAccount = (); + type OnKilledAccount = (); + type SystemWeightInfo = (); + type SS58Prefix = ConstU16<42>; + type OnSetCode = (); + type MaxConsumers = ConstU32<16>; + } + + parameter_types! { + pub const ExistentialDeposit: u64 = 1; + } + + impl mock_system_domain_tracker::Config for $runtime {} + + parameter_types! { + pub const SelfDomainId: DomainId = $domain_id; + } + + impl crate::Config for $runtime { + type Event = Event; + type DomainId = DomainId; + type SelfDomainId = SelfDomainId; + type SystemDomainTracker = SystemDomainTracker; + } + + pub fn new_test_ext() -> sp_io::TestExternalities { + let t = frame_system::GenesisConfig::default() + .build_storage::() + .unwrap(); + + let mut t: sp_io::TestExternalities = t.into(); + t.execute_with(|| System::set_block_number(1)); + t + } + }; } -parameter_types! { - pub const ExistentialDeposit: u64 = 1; +pub(crate) mod domain_a { + impl_runtime!(Runtime, 1); } -pub(crate) type DomainId = u64; +pub(crate) mod domain_b { + impl_runtime!(Runtime, 2); +} #[frame_support::pallet] -mod mock_system_domain_tracker { +pub(crate) mod mock_system_domain_tracker { use frame_support::pallet_prelude::*; use sp_core::H256; - use sp_messenger::SystemDomainTracker; + use sp_messenger::SystemDomainTracker as SystemDomainTrackerT; #[pallet::config] pub trait Config: frame_system::Config {} @@ -77,36 +115,13 @@ mod mock_system_domain_tracker { #[pallet::storage] pub(super) type StateRoot = StorageValue<_, H256, ValueQuery>; - impl SystemDomainTracker for Pallet { + impl SystemDomainTrackerT for Pallet { fn latest_state_roots() -> Vec { vec![StateRoot::::get()] } } } -impl mock_system_domain_tracker::Config for Test {} - -parameter_types! { - pub const SelfDomainId: DomainId = 0; -} - -impl crate::Config for Test { - type Event = Event; - type DomainId = DomainId; - type SelfDomainId = SelfDomainId; - type SystemDomainTracker = SystemDomainTracker; -} - -pub fn new_test_ext() -> sp_io::TestExternalities { - let t = frame_system::GenesisConfig::default() - .build_storage::() - .unwrap(); - - let mut t: sp_io::TestExternalities = t.into(); - t.execute_with(|| System::set_block_number(1)); - t -} - fn storage_proof_for_key( backend: InMemoryBackend, key: StorageKey, @@ -117,12 +132,12 @@ fn storage_proof_for_key( (root, proof) } -pub(crate) fn storage_proof_of_channels( +pub(crate) fn storage_proof_of_channels( backend: InMemoryBackend, - domain_id: DomainId, + domain_id: T::DomainId, channel_id: ChannelId, ) -> (H256, StorageKey, StorageProof) { - let key = Channels::::storage_double_map_final_key(domain_id, channel_id); + let key = Channels::::storage_double_map_final_key(domain_id, channel_id); let storage_key = StorageKey(key); let (root, proof) = storage_proof_for_key(backend, storage_key.clone()); (root, storage_key, proof) diff --git a/crates/pallet-messenger/src/tests.rs b/crates/pallet-messenger/src/tests.rs index eb3ba2c7fe..68e7980a33 100644 --- a/crates/pallet-messenger/src/tests.rs +++ b/crates/pallet-messenger/src/tests.rs @@ -1,5 +1,8 @@ use crate::messages::{Payload, ProtocolMessageRequest, RequestResponse, VersionedPayload}; -use crate::mock::{new_test_ext, DomainId, Event, Messenger, Origin, System, Test}; +use crate::mock::domain_a::{ + new_test_ext as new_domain_a_ext, Event, Messenger, Origin, Runtime, System, +}; +use crate::mock::DomainId; use crate::verification::{Proof, StorageProofVerifier, VerificationError}; use crate::{ Channel, ChannelId, ChannelState, Channels, Error, InitiateChannelParams, Nonce, Outbox, U256, @@ -18,10 +21,12 @@ fn create_channel(domain_id: DomainId, channel_id: ChannelId) { params, )); - System::assert_has_event(Event::Messenger(crate::Event::::ChannelInitiated { - domain_id, - channel_id, - })); + System::assert_has_event(Event::Messenger( + crate::Event::::ChannelInitiated { + domain_id, + channel_id, + }, + )); assert_eq!( Messenger::next_channel_id(domain_id), channel_id.checked_add(U256::one()).unwrap() @@ -32,8 +37,8 @@ fn create_channel(domain_id: DomainId, channel_id: ChannelId) { assert_eq!(channel.next_inbox_nonce, Nonce::zero()); assert_eq!(channel.next_outbox_nonce, Nonce::one()); assert_eq!(channel.latest_response_received_message_nonce, None); - assert_eq!(Outbox::::count(), 1); - let msg = Outbox::::get((domain_id, channel_id, Nonce::zero())).unwrap(); + assert_eq!(Outbox::::count(), 1); + let msg = Outbox::::get((domain_id, channel_id, Nonce::zero())).unwrap(); assert_eq!(msg.dst_domain_id, domain_id); assert_eq!(msg.channel_id, channel_id); assert_eq!( @@ -43,7 +48,7 @@ fn create_channel(domain_id: DomainId, channel_id: ChannelId) { ))) ); - System::assert_last_event(Event::Messenger(crate::Event::::OutboxMessage { + System::assert_last_event(Event::Messenger(crate::Event::::OutboxMessage { domain_id, channel_id, nonce: Nonce::zero(), @@ -52,7 +57,7 @@ fn create_channel(domain_id: DomainId, channel_id: ChannelId) { #[test] fn test_initiate_channel() { - new_test_ext().execute_with(|| { + new_domain_a_ext().execute_with(|| { let domain_id = 1; let channel_id = U256::zero(); create_channel(domain_id, channel_id) @@ -61,32 +66,32 @@ fn test_initiate_channel() { #[test] fn test_close_missing_channel() { - new_test_ext().execute_with(|| { + new_domain_a_ext().execute_with(|| { let domain_id = 1; let channel_id = U256::zero(); assert_err!( Messenger::close_channel(Origin::root(), domain_id, channel_id,), - Error::::MissingChannel + Error::::MissingChannel ); }); } #[test] fn test_close_not_open_channel() { - new_test_ext().execute_with(|| { + new_domain_a_ext().execute_with(|| { let domain_id = 1; let channel_id = U256::zero(); create_channel(domain_id, channel_id); assert_err!( Messenger::close_channel(Origin::root(), domain_id, channel_id,), - Error::::InvalidChannelState + Error::::InvalidChannelState ); }); } #[test] fn test_close_open_channel() { - new_test_ext().execute_with(|| { + new_domain_a_ext().execute_with(|| { let domain_id = 1; let channel_id = U256::zero(); create_channel(domain_id, channel_id); @@ -94,7 +99,7 @@ fn test_close_open_channel() { let channel = Messenger::channels(domain_id, channel_id).unwrap(); assert_eq!(channel.state, ChannelState::Open); - System::assert_has_event(Event::Messenger(crate::Event::::ChannelOpen { + System::assert_has_event(Event::Messenger(crate::Event::::ChannelOpen { domain_id, channel_id, })); @@ -107,12 +112,12 @@ fn test_close_open_channel() { let channel = Messenger::channels(domain_id, channel_id).unwrap(); assert_eq!(channel.state, ChannelState::Closed); - System::assert_has_event(Event::Messenger(crate::Event::::ChannelClosed { + System::assert_has_event(Event::Messenger(crate::Event::::ChannelClosed { domain_id, channel_id, })); - let msg = Outbox::::get((domain_id, channel_id, Nonce::one())).unwrap(); + let msg = Outbox::::get((domain_id, channel_id, Nonce::one())).unwrap(); assert_eq!(msg.dst_domain_id, domain_id); assert_eq!(msg.channel_id, channel_id); assert_eq!( @@ -122,7 +127,7 @@ fn test_close_open_channel() { ))) ); - System::assert_last_event(Event::Messenger(crate::Event::::OutboxMessage { + System::assert_last_event(Event::Messenger(crate::Event::::OutboxMessage { domain_id, channel_id, nonce: Nonce::one(), @@ -132,7 +137,7 @@ fn test_close_open_channel() { #[test] fn test_storage_proof_verification_invalid() { - let mut t = new_test_ext(); + let mut t = new_domain_a_ext(); let domain_id = 1; let channel_id = U256::zero(); t.execute_with(|| { @@ -141,7 +146,7 @@ fn test_storage_proof_verification_invalid() { }); let (_, _, storage_proof) = - crate::mock::storage_proof_of_channels(t.as_backend(), domain_id, channel_id); + crate::mock::storage_proof_of_channels::(t.as_backend(), domain_id, channel_id); let proof = Proof { state_root: Default::default(), message_proof: storage_proof, @@ -153,7 +158,7 @@ fn test_storage_proof_verification_invalid() { #[test] fn test_storage_proof_verification_missing_value() { - let mut t = new_test_ext(); + let mut t = new_domain_a_ext(); let domain_id = 1; let channel_id = U256::zero(); t.execute_with(|| { @@ -162,7 +167,7 @@ fn test_storage_proof_verification_missing_value() { }); let (state_root, storage_key, storage_proof) = - crate::mock::storage_proof_of_channels(t.as_backend(), domain_id, U256::one()); + crate::mock::storage_proof_of_channels::(t.as_backend(), domain_id, U256::one()); let proof = Proof { state_root, message_proof: storage_proof, @@ -174,18 +179,18 @@ fn test_storage_proof_verification_missing_value() { #[test] fn test_storage_proof_verification() { - let mut t = new_test_ext(); + let mut t = new_domain_a_ext(); let domain_id = 1; let channel_id = U256::zero(); let mut expected_channel = None; t.execute_with(|| { create_channel(domain_id, channel_id); assert_ok!(Messenger::do_open_channel(domain_id, channel_id)); - expected_channel = Channels::::get(domain_id, channel_id); + expected_channel = Channels::::get(domain_id, channel_id); }); let (state_root, storage_key, storage_proof) = - crate::mock::storage_proof_of_channels(t.as_backend(), domain_id, channel_id); + crate::mock::storage_proof_of_channels::(t.as_backend(), domain_id, channel_id); let proof = Proof { state_root, message_proof: storage_proof, From e6d3cd020383c7920bc625b05bd76ac0474a5dc1 Mon Sep 17 00:00:00 2001 From: vedhavyas Date: Thu, 6 Oct 2022 15:23:05 +0200 Subject: [PATCH 2/8] ability to receive outbox message responses --- crates/pallet-messenger/src/lib.rs | 198 ++++++++------ crates/pallet-messenger/src/messages.rs | 120 +++++++-- crates/pallet-messenger/src/mock.rs | 54 +++- crates/pallet-messenger/src/tests.rs | 326 +++++++++++++++++++++--- 4 files changed, 566 insertions(+), 132 deletions(-) diff --git a/crates/pallet-messenger/src/lib.rs b/crates/pallet-messenger/src/lib.rs index 432de2a04e..129e67e071 100644 --- a/crates/pallet-messenger/src/lib.rs +++ b/crates/pallet-messenger/src/lib.rs @@ -140,7 +140,7 @@ mod pallet { /// Stores the message responses of the incoming processed responses. /// Used by the dst_domain to verify the message response. #[pallet::storage] - #[pallet::getter(fn inbox_message_responses)] + #[pallet::getter(fn inbox_responses)] pub(super) type InboxMessageResponses = CountedStorageMap< _, Identity, @@ -162,7 +162,7 @@ mod pallet { >; #[pallet::storage] - #[pallet::getter(fn outbox_message_responses)] + #[pallet::getter(fn outbox_responses)] pub(super) type OutboxMessageResponses = CountedStorageMap< _, Identity, @@ -236,77 +236,12 @@ mod pallet { /// Validate unsigned call to this module. fn validate_unsigned(_source: TransactionSource, call: &Self::Call) -> TransactionValidity { - // Firstly let's check that we call the right function. - if let Call::relay_message { msg: bundled_msg } = call { - // fetch state roots from System domain tracker - let state_roots = T::SystemDomainTracker::latest_state_roots(); - if !state_roots.contains(&bundled_msg.proof.state_root) { - return InvalidTransaction::BadProof.into(); + match call { + Call::relay_message { msg: xdm } => Self::do_validate_relay_message(xdm), + Call::relay_message_response { msg: xdm } => { + Self::do_validate_relay_message_response(xdm) } - - // channel should be either already be created or match the next channelId for domain. - let next_channel_id = NextChannelId::::get(bundled_msg.dst_domain_id); - ensure!( - bundled_msg.channel_id <= next_channel_id, - InvalidTransaction::Call - ); - - // verify nonce - let mut should_init_channel = false; - let next_nonce = - match Channels::::get(bundled_msg.src_domain_id, bundled_msg.channel_id) { - None => { - // if there is no channel config, this must the Channel open request. - // ensure nonce is 0 - should_init_channel = true; - Nonce::zero() - } - Some(channel) => channel.next_inbox_nonce, - }; - // nonce should be either be next or in future. - ensure!( - bundled_msg.nonce >= next_nonce, - InvalidTransaction::BadProof - ); - - // derive the key as stored on the src_domain. - let key = Outbox::::hashed_key_for(( - T::SelfDomainId::get(), - bundled_msg.channel_id, - next_nonce, - )); - - // verify, decode, and store the message - let msg = StorageProofVerifier::::verify_and_get_value::< - Message, - >(bundled_msg.proof.clone(), StorageKey(key)) - .map_err(|_| TransactionValidityError::Invalid(InvalidTransaction::BadProof))?; - - if should_init_channel { - if let VersionedPayload::V0(Payload::Protocol(RequestResponse::Request( - ProtocolMessageRequest::ChannelOpen(params), - ))) = msg.payload - { - Self::do_init_channel(msg.src_domain_id, params) - .map_err(|_| InvalidTransaction::Call)?; - } else { - return InvalidTransaction::Call.into(); - } - } - - let provides_tag = (msg.dst_domain_id, msg.channel_id, next_nonce); - Inbox::::insert( - ( - bundled_msg.src_domain_id, - bundled_msg.channel_id, - next_nonce, - ), - msg, - ); - - unsigned_validity::("MessengerInbox", provides_tag) - } else { - InvalidTransaction::Call.into() + _ => InvalidTransaction::Call.into(), } } } @@ -328,6 +263,9 @@ mod pallet { /// Emits when the message verification failed. MessageVerification(VerificationError), + + /// Emits when there is no message available for the given nonce. + MissingMessage, } #[pallet::call] @@ -393,7 +331,18 @@ mod pallet { msg: CrossDomainMessage>, ) -> DispatchResult { ensure_none(origin)?; - Self::process_inbox_messages(msg.src_domain_id, msg.nonce)?; + Self::process_inbox_messages(msg.src_domain_id, msg.channel_id)?; + Ok(()) + } + + /// Receives a response from the dst_domain for a message in Outbox. + #[pallet::weight((10_000, Pays::No))] + pub fn relay_message_response( + origin: OriginFor, + msg: CrossDomainMessage>, + ) -> DispatchResult { + ensure_none(origin)?; + Self::process_outbox_message_responses(msg.src_domain_id, msg.channel_id)?; Ok(()) } } @@ -477,5 +426,108 @@ mod pallet { }); Ok(channel_id) } + + pub(crate) fn do_validate_relay_message( + xdm: &CrossDomainMessage>, + ) -> TransactionValidity { + let mut should_init_channel = false; + let next_nonce = match Channels::::get(xdm.src_domain_id, xdm.channel_id) { + None => { + // if there is no channel config, this must the Channel open request. + // ensure nonce is 0 + should_init_channel = true; + Nonce::zero() + } + Some(channel) => channel.next_inbox_nonce, + }; + + // derive the key as stored on the src_domain. + let key = StorageKey(Outbox::::hashed_key_for(( + T::SelfDomainId::get(), + xdm.channel_id, + xdm.nonce, + ))); + + // verify, decode, and store the message + let msg = Self::do_verify_xdm(next_nonce, key, xdm)?; + + if should_init_channel { + if let VersionedPayload::V0(Payload::Protocol(RequestResponse::Request( + ProtocolMessageRequest::ChannelOpen(params), + ))) = msg.payload + { + Self::do_init_channel(msg.src_domain_id, params) + .map_err(|_| InvalidTransaction::Call)?; + } else { + return InvalidTransaction::Call.into(); + } + } + + let provides_tag = (msg.dst_domain_id, msg.channel_id, msg.nonce); + Inbox::::insert((xdm.src_domain_id, xdm.channel_id, msg.nonce), msg); + unsigned_validity::("MessengerInbox", provides_tag) + } + + pub(crate) fn do_validate_relay_message_response( + xdm: &CrossDomainMessage>, + ) -> TransactionValidity { + // channel should be open and message should be present in outbox + let next_nonce = match Channels::::get(xdm.src_domain_id, xdm.channel_id) + .and_then(|channel| channel.latest_response_received_message_nonce) + { + // this is the first message response. next nonce is 0 + None => Some(Nonce::zero()), + Some(last_nonce) => last_nonce.checked_add(Nonce::one()), + } + .ok_or(TransactionValidityError::Invalid(InvalidTransaction::Call))?; + + // derive the key as stored on the src_domain. + let key = StorageKey(InboxMessageResponses::::hashed_key_for(( + T::SelfDomainId::get(), + xdm.channel_id, + xdm.nonce, + ))); + + // verify, decode, and store the message + let msg = Self::do_verify_xdm(next_nonce, key, xdm)?; + + let provides_tag = (msg.dst_domain_id, msg.channel_id, xdm.nonce); + OutboxMessageResponses::::insert( + (xdm.src_domain_id, xdm.channel_id, xdm.nonce), + msg, + ); + + unsigned_validity::("MessengerOutboxResponse", provides_tag) + } + + pub(crate) fn do_verify_xdm( + next_nonce: Nonce, + storage_key: StorageKey, + xdm: &CrossDomainMessage>, + ) -> Result, TransactionValidityError> { + // fetch state roots from System domain tracker + let state_roots = T::SystemDomainTracker::latest_state_roots(); + if !state_roots.contains(&xdm.proof.state_root) { + return Err(TransactionValidityError::Invalid( + InvalidTransaction::BadProof, + )); + } + + // channel should be either already be created or match the next channelId for domain. + let next_channel_id = NextChannelId::::get(xdm.dst_domain_id); + ensure!(xdm.channel_id <= next_channel_id, InvalidTransaction::Call); + + // verify nonce + // nonce should be either be next or in future. + ensure!(xdm.nonce >= next_nonce, InvalidTransaction::BadProof); + + // verify, decode, and store the message + let msg = StorageProofVerifier::::verify_and_get_value::< + Message, + >(xdm.proof.clone(), storage_key) + .map_err(|_| TransactionValidityError::Invalid(InvalidTransaction::BadProof))?; + + Ok(msg) + } } } diff --git a/crates/pallet-messenger/src/messages.rs b/crates/pallet-messenger/src/messages.rs index 979dc75d99..00747f5001 100644 --- a/crates/pallet-messenger/src/messages.rs +++ b/crates/pallet-messenger/src/messages.rs @@ -1,7 +1,7 @@ use crate::verification::Proof; use crate::{ ChannelId, Channels, Config, Error, Event, Inbox, InboxMessageResponses, InitiateChannelParams, - Nonce, Outbox, Pallet, + Nonce, Outbox, OutboxMessageResponses, Pallet, }; use codec::{Decode, Encode}; use frame_support::ensure; @@ -20,7 +20,7 @@ pub enum ProtocolMessageRequest { /// Defines protocol requests performed on domains. #[derive(Debug, Encode, Decode, Clone, Eq, PartialEq, TypeInfo)] -pub struct ProtocolMessageResponse(Result<(), DispatchError>); +pub struct ProtocolMessageResponse(pub Result<(), DispatchError>); /// Protocol message that encompasses request or its response. #[derive(Debug, Encode, Decode, Clone, Eq, PartialEq, TypeInfo)] @@ -132,9 +132,12 @@ impl Pallet { let mut messages_processed = 0; while let Some(msg) = Inbox::::take((dst_domain_id, channel_id, next_inbox_nonce)) { let response = match msg.payload { - VersionedPayload::V0(Payload::Protocol(msg)) => { - Self::process_incoming_protocol_message(dst_domain_id, channel_id, msg) - } + VersionedPayload::V0(Payload::Protocol(msg)) => match msg { + RequestResponse::Request(req) => { + Self::process_incoming_protocol_message_req(dst_domain_id, channel_id, req) + } + RequestResponse::Response(_) => Err(Error::::InvalidMessagePayload.into()), + }, }; InboxMessageResponses::::insert( @@ -177,21 +180,104 @@ impl Pallet { Ok(()) } - fn process_incoming_protocol_message( + fn process_incoming_protocol_message_req( domain_id: T::DomainId, channel_id: ChannelId, - req_resp: RequestResponse, + req: ProtocolMessageRequest, ) -> Result<(), DispatchError> { - match req_resp { - RequestResponse::Request(req) => match req { - ProtocolMessageRequest::ChannelOpen(_) => { - Self::do_open_channel(domain_id, channel_id) - } - ProtocolMessageRequest::ChannelClose => { - Self::do_close_channel(domain_id, channel_id) - } - }, - RequestResponse::Response(_) => Err(Error::::InvalidMessagePayload.into()), + match req { + ProtocolMessageRequest::ChannelOpen(_) => Self::do_open_channel(domain_id, channel_id), + ProtocolMessageRequest::ChannelClose => Self::do_close_channel(domain_id, channel_id), + } + } + + fn process_incoming_protocol_message_response( + domain_id: T::DomainId, + channel_id: ChannelId, + req: ProtocolMessageRequest, + resp: ProtocolMessageResponse, + ) -> DispatchResult { + match (req, resp.0) { + // channel open request is accepted by dst_domain. + // open channel on our end. + (ProtocolMessageRequest::ChannelOpen(_), Ok(_)) => { + Self::do_open_channel(domain_id, channel_id) + } + + // for rest of the branches we dont care about the outcome and return Ok + // for channel close request, we do not care about the response as channel is already closed. + // for channel open request and request is rejected, channel is left in init state and no new messages are accepted. + _ => Ok(()), } } + + pub(crate) fn process_outbox_message_responses( + dst_domain_id: T::DomainId, + channel_id: ChannelId, + ) -> DispatchResult { + // fetch the next message response nonce to process + // starts with nonce 0 + let mut last_message_response_nonce = Channels::::get(dst_domain_id, channel_id) + .ok_or(Error::::MissingChannel)? + .latest_response_received_message_nonce; + + let mut next_message_response_nonce = last_message_response_nonce + .and_then(|nonce| nonce.checked_add(Nonce::one())) + .unwrap_or(Nonce::zero()); + + // TODO(ved): maybe a bound of number of message responses to process in a single call? + let mut messages_processed = 0; + while let Some(msg) = OutboxMessageResponses::::take(( + dst_domain_id, + channel_id, + next_message_response_nonce, + )) { + match msg.payload { + VersionedPayload::V0(Payload::Protocol(msg)) => match msg { + RequestResponse::Response(resp) => { + if let VersionedPayload::V0(Payload::Protocol(RequestResponse::Request( + req, + ))) = Outbox::::take(( + dst_domain_id, + channel_id, + next_message_response_nonce, + )) + .ok_or(Error::::MissingMessage)? + .payload + { + Self::process_incoming_protocol_message_response( + dst_domain_id, + channel_id, + req, + resp, + ) + } else { + Err(Error::::InvalidMessagePayload.into()) + } + } + RequestResponse::Request(_) => Err(Error::::InvalidMessagePayload.into()), + }, + }?; + + last_message_response_nonce = Some(next_message_response_nonce); + next_message_response_nonce = next_message_response_nonce + .checked_add(Nonce::one()) + .ok_or(DispatchError::Arithmetic(ArithmeticError::Overflow))?; + messages_processed += 1; + } + + if messages_processed > 0 { + Channels::::mutate( + dst_domain_id, + channel_id, + |maybe_channel| -> DispatchResult { + let channel = maybe_channel.as_mut().ok_or(Error::::MissingChannel)?; + channel.latest_response_received_message_nonce = last_message_response_nonce; + Ok(()) + }, + )?; + } + + Ok(()) + } } diff --git a/crates/pallet-messenger/src/mock.rs b/crates/pallet-messenger/src/mock.rs index c9743b30b9..0f683c911c 100644 --- a/crates/pallet-messenger/src/mock.rs +++ b/crates/pallet-messenger/src/mock.rs @@ -1,16 +1,18 @@ -use crate::{ChannelId, Channels, Config}; +use crate::{ChannelId, Channels, Config, InboxMessageResponses, Nonce, Outbox, StateRootOf}; use frame_support::storage::generator::StorageDoubleMap; use sp_core::storage::StorageKey; -use sp_core::H256; +use sp_runtime::traits::BlakeTwo256; use sp_state_machine::backend::Backend; use sp_state_machine::{prove_read, InMemoryBackend}; use sp_trie::StorageProof; pub(crate) type DomainId = u64; +pub type TestExternalities = sp_state_machine::TestExternalities; + macro_rules! impl_runtime { ($runtime:ty, $domain_id:literal) => { - use crate::mock::{mock_system_domain_tracker, DomainId}; + use crate::mock::{mock_system_domain_tracker, DomainId,TestExternalities}; use frame_support::parameter_types; use sp_core::H256; use sp_runtime::testing::Header; @@ -77,12 +79,12 @@ macro_rules! impl_runtime { type SystemDomainTracker = SystemDomainTracker; } - pub fn new_test_ext() -> sp_io::TestExternalities { + pub fn new_test_ext() -> TestExternalities { let t = frame_system::GenesisConfig::default() .build_storage::() .unwrap(); - let mut t: sp_io::TestExternalities = t.into(); + let mut t: TestExternalities = t.into(); t.execute_with(|| System::set_block_number(1)); t } @@ -120,12 +122,18 @@ pub(crate) mod mock_system_domain_tracker { vec![StateRoot::::get()] } } + + impl Pallet { + pub fn set_state_root(state_root: H256) { + StateRoot::::put(state_root) + } + } } -fn storage_proof_for_key( - backend: InMemoryBackend, +fn storage_proof_for_key( + backend: InMemoryBackend, key: StorageKey, -) -> (H256, StorageProof) { +) -> (StateRootOf, StorageProof) { let state_version = sp_runtime::StateVersion::default(); let root = backend.storage_root(std::iter::empty(), state_version).0; let proof = StorageProof::new(prove_read(backend, &[key]).unwrap().iter_nodes()); @@ -133,12 +141,36 @@ fn storage_proof_for_key( } pub(crate) fn storage_proof_of_channels( - backend: InMemoryBackend, + backend: InMemoryBackend, domain_id: T::DomainId, channel_id: ChannelId, -) -> (H256, StorageKey, StorageProof) { +) -> (StateRootOf, StorageKey, StorageProof) { let key = Channels::::storage_double_map_final_key(domain_id, channel_id); let storage_key = StorageKey(key); - let (root, proof) = storage_proof_for_key(backend, storage_key.clone()); + let (root, proof) = storage_proof_for_key::(backend, storage_key.clone()); + (root, storage_key, proof) +} + +pub(crate) fn storage_proof_of_outbox_messages( + backend: InMemoryBackend, + domain_id: T::DomainId, + channel_id: ChannelId, + nonce: Nonce, +) -> (StateRootOf, StorageKey, StorageProof) { + let key = Outbox::::hashed_key_for((domain_id, channel_id, nonce)); + let storage_key = StorageKey(key); + let (root, proof) = storage_proof_for_key::(backend, storage_key.clone()); + (root, storage_key, proof) +} + +pub(crate) fn storage_proof_of_inbox_message_responses( + backend: InMemoryBackend, + domain_id: T::DomainId, + channel_id: ChannelId, + nonce: Nonce, +) -> (StateRootOf, StorageKey, StorageProof) { + let key = InboxMessageResponses::::hashed_key_for((domain_id, channel_id, nonce)); + let storage_key = StorageKey(key); + let (root, proof) = storage_proof_for_key::(backend, storage_key.clone()); (root, storage_key, proof) } diff --git a/crates/pallet-messenger/src/tests.rs b/crates/pallet-messenger/src/tests.rs index 68e7980a33..7bedaa2bc7 100644 --- a/crates/pallet-messenger/src/tests.rs +++ b/crates/pallet-messenger/src/tests.rs @@ -1,8 +1,14 @@ -use crate::messages::{Payload, ProtocolMessageRequest, RequestResponse, VersionedPayload}; +use crate::messages::{ + CrossDomainMessage, Message, Payload, ProtocolMessageRequest, ProtocolMessageResponse, + RequestResponse, VersionedPayload, +}; use crate::mock::domain_a::{ new_test_ext as new_domain_a_ext, Event, Messenger, Origin, Runtime, System, }; -use crate::mock::DomainId; +use crate::mock::{ + domain_a, domain_b, storage_proof_of_inbox_message_responses, storage_proof_of_outbox_messages, + DomainId, TestExternalities, +}; use crate::verification::{Proof, StorageProofVerifier, VerificationError}; use crate::{ Channel, ChannelId, ChannelState, Channels, Error, InitiateChannelParams, Nonce, Outbox, U256, @@ -10,6 +16,8 @@ use crate::{ use frame_support::{assert_err, assert_ok}; use sp_core::storage::StorageKey; use sp_core::Blake2Hasher; +use sp_runtime::traits::ValidateUnsigned; +use sp_runtime::transaction_validity::TransactionSource; fn create_channel(domain_id: DomainId, channel_id: ChannelId) { let params = InitiateChannelParams { @@ -55,6 +63,37 @@ fn create_channel(domain_id: DomainId, channel_id: ChannelId) { })); } +fn close_channel(domain_id: DomainId, channel_id: ChannelId) { + assert_ok!(Messenger::close_channel( + Origin::root(), + domain_id, + channel_id, + )); + + let channel = Messenger::channels(domain_id, channel_id).unwrap(); + assert_eq!(channel.state, ChannelState::Closed); + System::assert_has_event(Event::Messenger(crate::Event::::ChannelClosed { + domain_id, + channel_id, + })); + + let msg = Outbox::::get((domain_id, channel_id, Nonce::one())).unwrap(); + assert_eq!(msg.dst_domain_id, domain_id); + assert_eq!(msg.channel_id, channel_id); + assert_eq!( + msg.payload, + VersionedPayload::V0(Payload::Protocol(RequestResponse::Request( + ProtocolMessageRequest::ChannelClose + ))) + ); + + System::assert_last_event(Event::Messenger(crate::Event::::OutboxMessage { + domain_id, + channel_id, + nonce: Nonce::one(), + })); +} + #[test] fn test_initiate_channel() { new_domain_a_ext().execute_with(|| { @@ -95,8 +134,9 @@ fn test_close_open_channel() { let domain_id = 1; let channel_id = U256::zero(); create_channel(domain_id, channel_id); - assert_ok!(Messenger::do_open_channel(domain_id, channel_id)); + // open channel + assert_ok!(Messenger::do_open_channel(domain_id, channel_id)); let channel = Messenger::channels(domain_id, channel_id).unwrap(); assert_eq!(channel.state, ChannelState::Open); System::assert_has_event(Event::Messenger(crate::Event::::ChannelOpen { @@ -104,34 +144,8 @@ fn test_close_open_channel() { channel_id, })); - assert_ok!(Messenger::close_channel( - Origin::root(), - domain_id, - channel_id, - )); - - let channel = Messenger::channels(domain_id, channel_id).unwrap(); - assert_eq!(channel.state, ChannelState::Closed); - System::assert_has_event(Event::Messenger(crate::Event::::ChannelClosed { - domain_id, - channel_id, - })); - - let msg = Outbox::::get((domain_id, channel_id, Nonce::one())).unwrap(); - assert_eq!(msg.dst_domain_id, domain_id); - assert_eq!(msg.channel_id, channel_id); - assert_eq!( - msg.payload, - VersionedPayload::V0(Payload::Protocol(RequestResponse::Request( - ProtocolMessageRequest::ChannelClose - ))) - ); - - System::assert_last_event(Event::Messenger(crate::Event::::OutboxMessage { - domain_id, - channel_id, - nonce: Nonce::one(), - })); + // close channel + close_channel(domain_id, channel_id) }); } @@ -201,3 +215,253 @@ fn test_storage_proof_verification() { assert!(res.is_ok()); assert_eq!(res.unwrap(), expected_channel.unwrap()) } + +fn open_channel_between_domains( + domain_a_test_ext: &mut TestExternalities, + domain_b_test_ext: &mut TestExternalities, +) -> ChannelId { + let domain_a_id = domain_a::SelfDomainId::get(); + let domain_b_id = domain_b::SelfDomainId::get(); + + // initiate channel open on domain_a + let channel_id = domain_a_test_ext.execute_with(|| -> ChannelId { + let channel_id = U256::zero(); + create_channel(domain_b_id, channel_id); + channel_id + }); + + channel_relay_request_and_response( + domain_a_test_ext, + domain_b_test_ext, + channel_id, + Nonce::zero(), + ); + + // check channel state be open on domain_b + domain_b_test_ext.execute_with(|| { + let channel = domain_b::Messenger::channels(domain_a_id, channel_id).unwrap(); + assert_eq!(channel.state, ChannelState::Open); + domain_b::System::assert_has_event(domain_b::Event::Messenger(crate::Event::< + domain_b::Runtime, + >::ChannelInitiated { + domain_id: domain_a_id, + channel_id, + })); + domain_b::System::assert_has_event(domain_b::Event::Messenger(crate::Event::< + domain_b::Runtime, + >::ChannelOpen { + domain_id: domain_a_id, + channel_id, + })); + }); + + // check channel state be open on domain_a + domain_a_test_ext.execute_with(|| { + let channel = domain_a::Messenger::channels(domain_b_id, channel_id).unwrap(); + assert_eq!(channel.state, ChannelState::Open); + assert_eq!( + channel.latest_response_received_message_nonce, + Some(Nonce::zero()) + ); + assert_eq!(channel.next_inbox_nonce, Nonce::zero()); + assert_eq!(channel.next_outbox_nonce, Nonce::one()); + domain_a::System::assert_has_event(domain_a::Event::Messenger(crate::Event::< + domain_a::Runtime, + >::ChannelOpen { + domain_id: domain_b_id, + channel_id, + })); + }); + + channel_id +} + +fn close_channel_between_domains( + domain_a_test_ext: &mut TestExternalities, + domain_b_test_ext: &mut TestExternalities, + channel_id: ChannelId, +) { + let domain_a_id = domain_a::SelfDomainId::get(); + let domain_b_id = domain_b::SelfDomainId::get(); + + // initiate channel close on domain_a + domain_a_test_ext.execute_with(|| { + close_channel(domain_b_id, channel_id); + }); + + channel_relay_request_and_response( + domain_a_test_ext, + domain_b_test_ext, + channel_id, + Nonce::one(), + ); + + // check channel state be close on domain_b + domain_b_test_ext.execute_with(|| { + let channel = domain_b::Messenger::channels(domain_a_id, channel_id).unwrap(); + assert_eq!(channel.state, ChannelState::Closed); + domain_b::System::assert_has_event(domain_b::Event::Messenger(crate::Event::< + domain_b::Runtime, + >::ChannelClosed { + domain_id: domain_a_id, + channel_id, + })); + }); + + // check channel state be closed on domain_a + domain_a_test_ext.execute_with(|| { + let channel = domain_a::Messenger::channels(domain_b_id, channel_id).unwrap(); + assert_eq!(channel.state, ChannelState::Closed); + assert_eq!( + channel.latest_response_received_message_nonce, + Some(Nonce::one()) + ); + assert_eq!(channel.next_inbox_nonce, Nonce::zero()); + assert_eq!( + channel.next_outbox_nonce, + Nonce::one().checked_add(Nonce::one()).unwrap() + ); + domain_a::System::assert_has_event(domain_a::Event::Messenger(crate::Event::< + domain_a::Runtime, + >::ChannelClosed { + domain_id: domain_b_id, + channel_id, + })); + }) +} + +fn channel_relay_request_and_response( + domain_a_test_ext: &mut TestExternalities, + domain_b_test_ext: &mut TestExternalities, + channel_id: ChannelId, + nonce: Nonce, +) { + let domain_a_id = domain_a::SelfDomainId::get(); + let domain_b_id = domain_b::SelfDomainId::get(); + + // relay message to domain_b + let (state_root, _key, message_proof) = storage_proof_of_outbox_messages::( + domain_a_test_ext.as_backend(), + domain_b_id, + channel_id, + nonce, + ); + + let xdm = CrossDomainMessage { + src_domain_id: domain_a_id, + dst_domain_id: domain_b_id, + channel_id, + nonce, + proof: Proof { + state_root, + message_proof, + }, + }; + domain_b_test_ext.execute_with(|| { + // set state root + domain_b::SystemDomainTracker::set_state_root(xdm.proof.state_root); + + // validate the message + let pre_check = crate::Pallet::::validate_unsigned( + TransactionSource::Local, + &crate::Call::relay_message { msg: xdm.clone() }, + ); + assert_ok!(pre_check); + + // process inbox message + let result = domain_b::Messenger::relay_message(domain_b::Origin::none(), xdm); + assert_ok!(result); + + domain_b::System::assert_has_event(domain_b::Event::Messenger(crate::Event::< + domain_b::Runtime, + >::InboxMessageResponse { + domain_id: domain_a_id, + channel_id, + nonce, + })); + + let response = + domain_b::Messenger::inbox_responses((domain_a_id, channel_id, nonce)).unwrap(); + assert_eq!( + response, + Message { + src_domain_id: domain_b_id, + dst_domain_id: domain_a_id, + channel_id, + nonce, + payload: VersionedPayload::V0(Payload::Protocol(RequestResponse::Response( + ProtocolMessageResponse(Ok(())) + ))) + } + ); + assert_eq!( + domain_a::Messenger::inbox((domain_b_id, channel_id, nonce)), + None + ); + }); + + // relay message response to domain_a + let (state_root, _key, message_proof) = + storage_proof_of_inbox_message_responses::( + domain_b_test_ext.as_backend(), + domain_a_id, + channel_id, + nonce, + ); + + let xdm = CrossDomainMessage { + src_domain_id: domain_b_id, + dst_domain_id: domain_a_id, + channel_id, + nonce, + proof: Proof { + state_root, + message_proof, + }, + }; + domain_a_test_ext.execute_with(|| { + domain_a::SystemDomainTracker::set_state_root(xdm.proof.state_root); + + // validate message response + let pre_check = crate::Pallet::::validate_unsigned( + TransactionSource::Local, + &crate::Call::relay_message_response { msg: xdm.clone() }, + ); + assert_ok!(pre_check); + + // process outbox message response + let result = domain_a::Messenger::relay_message_response(domain_a::Origin::none(), xdm); + assert_ok!(result); + + // outbox message and message response should not exists + assert_eq!( + domain_a::Messenger::outbox((domain_b_id, channel_id, nonce)), + None + ); + assert_eq!( + domain_a::Messenger::outbox_responses((domain_b_id, channel_id, nonce)), + None + ); + }) +} + +#[test] +fn test_open_channel_between_domains() { + let mut domain_a_test_ext = domain_a::new_test_ext(); + let mut domain_b_test_ext = domain_b::new_test_ext(); + // open channel between domain_a and domain_b + // domain_a initiates the channel open + open_channel_between_domains(&mut domain_a_test_ext, &mut domain_b_test_ext); +} + +#[test] +fn test_close_channel_between_domains() { + let mut domain_a_test_ext = domain_a::new_test_ext(); + let mut domain_b_test_ext = domain_b::new_test_ext(); + // open channel between domain_a and domain_b + // domain_a initiates the channel open + let channel_id = open_channel_between_domains(&mut domain_a_test_ext, &mut domain_b_test_ext); + + // close open channel + close_channel_between_domains(&mut domain_a_test_ext, &mut domain_b_test_ext, channel_id) +} From 92c09b7814806a61ae9e97d9877aa426991f60dc Mon Sep 17 00:00:00 2001 From: vedhavyas Date: Thu, 6 Oct 2022 16:33:19 +0200 Subject: [PATCH 3/8] clean up messages responses state that are delivered --- crates/pallet-messenger/src/lib.rs | 11 +++--- crates/pallet-messenger/src/messages.rs | 45 ++++++++++++++++++++----- crates/pallet-messenger/src/mock.rs | 4 +-- crates/pallet-messenger/src/tests.rs | 37 +++++++++++++++++--- 4 files changed, 75 insertions(+), 22 deletions(-) diff --git a/crates/pallet-messenger/src/lib.rs b/crates/pallet-messenger/src/lib.rs index 129e67e071..c6e3184d7d 100644 --- a/crates/pallet-messenger/src/lib.rs +++ b/crates/pallet-messenger/src/lib.rs @@ -141,7 +141,7 @@ mod pallet { /// Used by the dst_domain to verify the message response. #[pallet::storage] #[pallet::getter(fn inbox_responses)] - pub(super) type InboxMessageResponses = CountedStorageMap< + pub(super) type InboxResponses = CountedStorageMap< _, Identity, (T::DomainId, ChannelId, Nonce), @@ -163,7 +163,7 @@ mod pallet { #[pallet::storage] #[pallet::getter(fn outbox_responses)] - pub(super) type OutboxMessageResponses = CountedStorageMap< + pub(super) type OutboxResponses = CountedStorageMap< _, Identity, (T::DomainId, ChannelId, Nonce), @@ -482,7 +482,7 @@ mod pallet { .ok_or(TransactionValidityError::Invalid(InvalidTransaction::Call))?; // derive the key as stored on the src_domain. - let key = StorageKey(InboxMessageResponses::::hashed_key_for(( + let key = StorageKey(InboxResponses::::hashed_key_for(( T::SelfDomainId::get(), xdm.channel_id, xdm.nonce, @@ -492,10 +492,7 @@ mod pallet { let msg = Self::do_verify_xdm(next_nonce, key, xdm)?; let provides_tag = (msg.dst_domain_id, msg.channel_id, xdm.nonce); - OutboxMessageResponses::::insert( - (xdm.src_domain_id, xdm.channel_id, xdm.nonce), - msg, - ); + OutboxResponses::::insert((xdm.src_domain_id, xdm.channel_id, xdm.nonce), msg); unsigned_validity::("MessengerOutboxResponse", provides_tag) } diff --git a/crates/pallet-messenger/src/messages.rs b/crates/pallet-messenger/src/messages.rs index 00747f5001..8ebc5eb914 100644 --- a/crates/pallet-messenger/src/messages.rs +++ b/crates/pallet-messenger/src/messages.rs @@ -1,7 +1,7 @@ use crate::verification::Proof; use crate::{ - ChannelId, Channels, Config, Error, Event, Inbox, InboxMessageResponses, InitiateChannelParams, - Nonce, Outbox, OutboxMessageResponses, Pallet, + ChannelId, Channels, Config, Error, Event, Inbox, InboxResponses, InitiateChannelParams, Nonce, + Outbox, OutboxResponses, Pallet, }; use codec::{Decode, Encode}; use frame_support::ensure; @@ -55,6 +55,8 @@ pub struct Message { pub nonce: Nonce, /// Payload of the message pub payload: VersionedPayload, + /// Last delivered message response nonce on src_domain. + pub last_delivered_message_response_nonce: Option, } /// Cross Domain message contains Message and its proof on src_domain. @@ -101,6 +103,8 @@ impl Pallet { channel_id, nonce: next_outbox_nonce, payload, + last_delivered_message_response_nonce: channel + .latest_response_received_message_nonce, }; Outbox::::insert((dst_domain_id, channel_id, next_outbox_nonce), msg); @@ -120,6 +124,24 @@ impl Pallet { ) } + /// Removes messages responses from Inbox responses as the src_domain signalled that responses are delivered. + /// all the messages with nonce <= latest_confirmed_nonce are deleted. + fn clean_delivered_message_responses( + dst_domain_id: T::DomainId, + channel_id: ChannelId, + latest_confirmed_nonce: Option, + ) { + let mut current_nonce = latest_confirmed_nonce; + while let Some(nonce) = current_nonce { + // fail if we have cleared all the messages + if InboxResponses::::take((dst_domain_id, channel_id, nonce)).is_none() { + return; + } + + current_nonce = nonce.checked_sub(Nonce::one()) + } + } + pub(crate) fn process_inbox_messages( dst_domain_id: T::DomainId, channel_id: ChannelId, @@ -140,7 +162,7 @@ impl Pallet { }, }; - InboxMessageResponses::::insert( + InboxResponses::::insert( (dst_domain_id, channel_id, next_inbox_nonce), Message { src_domain_id: T::SelfDomainId::get(), @@ -150,6 +172,8 @@ impl Pallet { payload: VersionedPayload::V0(Payload::Protocol(RequestResponse::Response( ProtocolMessageResponse(response), ))), + // this nonce is not considered in response context. + last_delivered_message_response_nonce: None, }, ); @@ -163,6 +187,13 @@ impl Pallet { .checked_add(Nonce::one()) .ok_or(DispatchError::Arithmetic(ArithmeticError::Overflow))?; messages_processed += 1; + + // clean any delivered inbox responses + Self::clean_delivered_message_responses( + dst_domain_id, + channel_id, + msg.last_delivered_message_response_nonce, + ) } if messages_processed > 0 { @@ -227,11 +258,9 @@ impl Pallet { // TODO(ved): maybe a bound of number of message responses to process in a single call? let mut messages_processed = 0; - while let Some(msg) = OutboxMessageResponses::::take(( - dst_domain_id, - channel_id, - next_message_response_nonce, - )) { + while let Some(msg) = + OutboxResponses::::take((dst_domain_id, channel_id, next_message_response_nonce)) + { match msg.payload { VersionedPayload::V0(Payload::Protocol(msg)) => match msg { RequestResponse::Response(resp) => { diff --git a/crates/pallet-messenger/src/mock.rs b/crates/pallet-messenger/src/mock.rs index 0f683c911c..9a269a0c9a 100644 --- a/crates/pallet-messenger/src/mock.rs +++ b/crates/pallet-messenger/src/mock.rs @@ -1,4 +1,4 @@ -use crate::{ChannelId, Channels, Config, InboxMessageResponses, Nonce, Outbox, StateRootOf}; +use crate::{ChannelId, Channels, Config, InboxResponses, Nonce, Outbox, StateRootOf}; use frame_support::storage::generator::StorageDoubleMap; use sp_core::storage::StorageKey; use sp_runtime::traits::BlakeTwo256; @@ -169,7 +169,7 @@ pub(crate) fn storage_proof_of_inbox_message_responses( channel_id: ChannelId, nonce: Nonce, ) -> (StateRootOf, StorageKey, StorageProof) { - let key = InboxMessageResponses::::hashed_key_for((domain_id, channel_id, nonce)); + let key = InboxResponses::::hashed_key_for((domain_id, channel_id, nonce)); let storage_key = StorageKey(key); let (root, proof) = storage_proof_for_key::(backend, storage_key.clone()); (root, storage_key, proof) diff --git a/crates/pallet-messenger/src/tests.rs b/crates/pallet-messenger/src/tests.rs index 7bedaa2bc7..9e004e58c8 100644 --- a/crates/pallet-messenger/src/tests.rs +++ b/crates/pallet-messenger/src/tests.rs @@ -11,7 +11,8 @@ use crate::mock::{ }; use crate::verification::{Proof, StorageProofVerifier, VerificationError}; use crate::{ - Channel, ChannelId, ChannelState, Channels, Error, InitiateChannelParams, Nonce, Outbox, U256, + Channel, ChannelId, ChannelState, Channels, Error, Inbox, InboxResponses, + InitiateChannelParams, Nonce, Outbox, OutboxResponses, U256, }; use frame_support::{assert_err, assert_ok}; use sp_core::storage::StorageKey; @@ -63,7 +64,7 @@ fn create_channel(domain_id: DomainId, channel_id: ChannelId) { })); } -fn close_channel(domain_id: DomainId, channel_id: ChannelId) { +fn close_channel(domain_id: DomainId, channel_id: ChannelId, last_delivered_nonce: Option) { assert_ok!(Messenger::close_channel( Origin::root(), domain_id, @@ -80,6 +81,10 @@ fn close_channel(domain_id: DomainId, channel_id: ChannelId) { let msg = Outbox::::get((domain_id, channel_id, Nonce::one())).unwrap(); assert_eq!(msg.dst_domain_id, domain_id); assert_eq!(msg.channel_id, channel_id); + assert_eq!( + msg.last_delivered_message_response_nonce, + last_delivered_nonce + ); assert_eq!( msg.payload, VersionedPayload::V0(Payload::Protocol(RequestResponse::Request( @@ -145,7 +150,7 @@ fn test_close_open_channel() { })); // close channel - close_channel(domain_id, channel_id) + close_channel(domain_id, channel_id, None) }); } @@ -286,7 +291,7 @@ fn close_channel_between_domains( // initiate channel close on domain_a domain_a_test_ext.execute_with(|| { - close_channel(domain_b_id, channel_id); + close_channel(domain_b_id, channel_id, Some(Nonce::zero())); }); channel_relay_request_and_response( @@ -306,6 +311,21 @@ fn close_channel_between_domains( domain_id: domain_a_id, channel_id, })); + + assert_eq!(channel.latest_response_received_message_nonce, None); + assert_eq!( + channel.next_inbox_nonce, + Nonce::one().checked_add(Nonce::one()).unwrap() + ); + assert_eq!(channel.next_outbox_nonce, Nonce::zero()); + + // Outbox, Outbox responses, Inbox, InboxResponses must be empty + assert_eq!(Outbox::::count(), 0); + assert_eq!(OutboxResponses::::count(), 0); + assert_eq!(Inbox::::count(), 0); + + // latest inbox message response is cleared on next message + assert_eq!(InboxResponses::::count(), 1); }); // check channel state be closed on domain_a @@ -327,6 +347,12 @@ fn close_channel_between_domains( domain_id: domain_b_id, channel_id, })); + + // Outbox, Outbox responses, Inbox, InboxResponses must be empty + assert_eq!(Outbox::::count(), 0); + assert_eq!(OutboxResponses::::count(), 0); + assert_eq!(Inbox::::count(), 0); + assert_eq!(InboxResponses::::count(), 0); }) } @@ -391,7 +417,8 @@ fn channel_relay_request_and_response( nonce, payload: VersionedPayload::V0(Payload::Protocol(RequestResponse::Response( ProtocolMessageResponse(Ok(())) - ))) + ))), + last_delivered_message_response_nonce: None } ); assert_eq!( From 3491f246f3f9b2a86eb2f3971b116d38bdfaa11a Mon Sep 17 00:00:00 2001 From: vedhavyas Date: Fri, 7 Oct 2022 15:46:08 +0200 Subject: [PATCH 4/8] define endpoints and traits for message passing and integrate endpoint message passing --- Cargo.lock | 5 + crates/pallet-messenger/src/lib.rs | 121 ++++++++++++++++++++--- crates/pallet-messenger/src/messages.rs | 125 ++++++++++++++++-------- crates/pallet-messenger/src/mock.rs | 31 +++++- crates/pallet-messenger/src/tests.rs | 119 +++++++++++++++++++--- crates/sp-messenger/Cargo.toml | 10 +- crates/sp-messenger/src/endpoint.rs | 47 +++++++++ crates/sp-messenger/src/lib.rs | 2 + 8 files changed, 386 insertions(+), 74 deletions(-) create mode 100644 crates/sp-messenger/src/endpoint.rs diff --git a/Cargo.lock b/Cargo.lock index dc89f03e91..fe7c9a6925 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7585,6 +7585,11 @@ dependencies = [ [[package]] name = "sp-messenger" version = "0.1.0" +dependencies = [ + "parity-scale-codec", + "scale-info", + "sp-runtime", +] [[package]] name = "sp-objects" diff --git a/crates/pallet-messenger/src/lib.rs b/crates/pallet-messenger/src/lib.rs index c6e3184d7d..408686d9c0 100644 --- a/crates/pallet-messenger/src/lib.rs +++ b/crates/pallet-messenger/src/lib.rs @@ -18,8 +18,6 @@ #![cfg_attr(not(feature = "std"), no_std)] #![forbid(unsafe_code)] #![warn(rust_2018_idioms, missing_debug_implementations)] -// TODO(ved): remove once all the types and traits are connected -#![allow(dead_code)] #[cfg(test)] mod mock; @@ -34,6 +32,7 @@ pub use pallet::*; use scale_info::TypeInfo; use sp_core::U256; use sp_runtime::traits::Hash; +use sp_runtime::DispatchError; /// State of a channel. #[derive(Default, Debug, Encode, Decode, Clone, Eq, PartialEq, TypeInfo)] @@ -76,6 +75,14 @@ pub struct InitiateChannelParams { max_outgoing_messages: u32, } +#[derive(Debug, Encode, Decode, Clone, Eq, PartialEq, TypeInfo, Copy)] +pub enum OutboxMessageStatus { + /// Message response handler returned Ok. + Ok, + /// Message response handler failed with Err. + Err(DispatchError), +} + pub(crate) type StateRootOf = <::Hashing as Hash>::Output; #[frame_support::pallet] @@ -86,13 +93,15 @@ mod pallet { }; use crate::verification::{StorageProofVerifier, VerificationError}; use crate::{ - Channel, ChannelId, ChannelState, InitiateChannelParams, Nonce, StateRootOf, U256, + Channel, ChannelId, ChannelState, InitiateChannelParams, Nonce, OutboxMessageStatus, + StateRootOf, U256, }; use frame_support::pallet_prelude::*; use frame_support::transactional; use frame_system::pallet_prelude::*; use sp_core::storage::StorageKey; - use sp_messenger::SystemDomainTracker; + use sp_messenger::endpoint::{Endpoint, EndpointHandler, EndpointRequest, Sender}; + use sp_messenger::SystemDomainTracker as SystemDomainTrackerT; use sp_runtime::ArithmeticError; #[pallet::config] @@ -103,7 +112,11 @@ mod pallet { /// Gets the domain_id that is treated as src_domain for outgoing messages. type SelfDomainId: Get; /// System domain tracker. - type SystemDomainTracker: SystemDomainTracker>; + type SystemDomainTracker: SystemDomainTrackerT>; + /// function to fetch endpoint response handler by Endpoint. + fn get_endpoint_response_handler( + endpoint: &Endpoint, + ) -> Option>>; } /// Pallet messenger used to communicate between domains and other blockchains. @@ -206,6 +219,21 @@ mod pallet { nonce: Nonce, }, + /// Emits when a handler returns an error after handling the message response. + OutboxMessageStatus { + domain_id: T::DomainId, + channel_id: ChannelId, + nonce: Nonce, + status: OutboxMessageStatus, + }, + + /// Emits when a new inbox message is validated and added to Inbox. + InboxMessage { + domain_id: T::DomainId, + channel_id: ChannelId, + nonce: Nonce, + }, + /// Emits when a message response is available for Inbox message. InboxMessageResponse { /// Destination domain ID. @@ -255,6 +283,12 @@ mod pallet { /// Emits when the said channel is not in an open state. InvalidChannelState, + /// Emits when there are no open channels for a domain + NoOpenChannel, + + /// Emits when there are not message handler with given endpoint ID. + NoMessageHandler, + /// Emits when the outbox is full for a channel. OutboxFull, @@ -347,7 +381,40 @@ mod pallet { } } + impl Sender for Pallet { + fn send_message(dst_domain_id: T::DomainId, req: EndpointRequest) -> DispatchResult { + let channel_id = Self::get_open_channel_for_domain(dst_domain_id) + .ok_or(Error::::NoOpenChannel)?; + Self::new_outbox_message( + T::SelfDomainId::get(), + dst_domain_id, + channel_id, + VersionedPayload::V0(Payload::Endpoint(RequestResponse::Request(req))), + )?; + Ok(()) + } + } + impl Pallet { + /// Returns the last open channel for a given domain. + fn get_open_channel_for_domain(dst_domain_id: T::DomainId) -> Option { + let mut next_channel_id = NextChannelId::::get(dst_domain_id); + + // loop through channels in descending order until open channel is found. + // we always prefer latest opened channel. + while let Some(channel_id) = next_channel_id.checked_sub(ChannelId::one()) { + if let Some(channel) = Channels::::get(dst_domain_id, channel_id) { + if channel.state == ChannelState::Open { + return Some(channel_id); + } + } + + next_channel_id = channel_id + } + + None + } + /// Opens an initiated channel. pub(crate) fn do_open_channel( domain_id: T::DomainId, @@ -434,11 +501,18 @@ mod pallet { let next_nonce = match Channels::::get(xdm.src_domain_id, xdm.channel_id) { None => { // if there is no channel config, this must the Channel open request. - // ensure nonce is 0 + // so nonce is 0 should_init_channel = true; Nonce::zero() } - Some(channel) => channel.next_inbox_nonce, + Some(channel) => { + // Ensure channel is ready to receive messages + ensure!( + channel.state == ChannelState::Open, + InvalidTransaction::Call + ); + channel.next_inbox_nonce + } }; // derive the key as stored on the src_domain. @@ -465,6 +539,11 @@ mod pallet { let provides_tag = (msg.dst_domain_id, msg.channel_id, msg.nonce); Inbox::::insert((xdm.src_domain_id, xdm.channel_id, msg.nonce), msg); + Self::deposit_event(Event::InboxMessage { + domain_id: xdm.src_domain_id, + channel_id: xdm.channel_id, + nonce: xdm.nonce, + }); unsigned_validity::("MessengerInbox", provides_tag) } @@ -472,12 +551,24 @@ mod pallet { xdm: &CrossDomainMessage>, ) -> TransactionValidity { // channel should be open and message should be present in outbox - let next_nonce = match Channels::::get(xdm.src_domain_id, xdm.channel_id) - .and_then(|channel| channel.latest_response_received_message_nonce) - { - // this is the first message response. next nonce is 0 - None => Some(Nonce::zero()), - Some(last_nonce) => last_nonce.checked_add(Nonce::one()), + let next_nonce = match Channels::::get(xdm.src_domain_id, xdm.channel_id) { + // unknown channel. return + None => return Err(InvalidTransaction::Call.into()), + // verify if channel can receive messages + Some(channel) => { + match channel.latest_response_received_message_nonce { + None => { + // this is the first message response. + // ensure channel is in init state + ensure!( + channel.state == ChannelState::Initiated, + InvalidTransaction::Call + ); + Some(Nonce::zero()) + } + Some(last_nonce) => last_nonce.checked_add(Nonce::one()), + } + } } .ok_or(TransactionValidityError::Invalid(InvalidTransaction::Call))?; @@ -487,10 +578,8 @@ mod pallet { xdm.channel_id, xdm.nonce, ))); - // verify, decode, and store the message let msg = Self::do_verify_xdm(next_nonce, key, xdm)?; - let provides_tag = (msg.dst_domain_id, msg.channel_id, xdm.nonce); OutboxResponses::::insert((xdm.src_domain_id, xdm.channel_id, xdm.nonce), msg); @@ -518,7 +607,7 @@ mod pallet { // nonce should be either be next or in future. ensure!(xdm.nonce >= next_nonce, InvalidTransaction::BadProof); - // verify, decode, and store the message + // verify and decode the message let msg = StorageProofVerifier::::verify_and_get_value::< Message, >(xdm.proof.clone(), storage_key) diff --git a/crates/pallet-messenger/src/messages.rs b/crates/pallet-messenger/src/messages.rs index 8ebc5eb914..0f758c78e4 100644 --- a/crates/pallet-messenger/src/messages.rs +++ b/crates/pallet-messenger/src/messages.rs @@ -1,11 +1,12 @@ use crate::verification::Proof; use crate::{ ChannelId, Channels, Config, Error, Event, Inbox, InboxResponses, InitiateChannelParams, Nonce, - Outbox, OutboxResponses, Pallet, + Outbox, OutboxMessageStatus, OutboxResponses, Pallet, }; use codec::{Decode, Encode}; use frame_support::ensure; use scale_info::TypeInfo; +use sp_messenger::endpoint::{EndpointRequest, EndpointResponse}; use sp_runtime::traits::Get; use sp_runtime::{ArithmeticError, DispatchError, DispatchResult}; @@ -19,8 +20,7 @@ pub enum ProtocolMessageRequest { } /// Defines protocol requests performed on domains. -#[derive(Debug, Encode, Decode, Clone, Eq, PartialEq, TypeInfo)] -pub struct ProtocolMessageResponse(pub Result<(), DispatchError>); +pub type ProtocolMessageResponse = Result<(), DispatchError>; /// Protocol message that encompasses request or its response. #[derive(Debug, Encode, Decode, Clone, Eq, PartialEq, TypeInfo)] @@ -32,8 +32,10 @@ pub enum RequestResponse { /// Payload of the message #[derive(Debug, Encode, Decode, Clone, Eq, PartialEq, TypeInfo)] pub enum Payload { - /// Protocol specific payload. + /// Protocol message. Protocol(RequestResponse), + /// Endpoint message. + Endpoint(RequestResponse), } /// Versioned message payload @@ -142,6 +144,7 @@ impl Pallet { } } + /// Process the incoming messages from given domain_id and channel_id. pub(crate) fn process_inbox_messages( dst_domain_id: T::DomainId, channel_id: ChannelId, @@ -154,11 +157,34 @@ impl Pallet { let mut messages_processed = 0; while let Some(msg) = Inbox::::take((dst_domain_id, channel_id, next_inbox_nonce)) { let response = match msg.payload { - VersionedPayload::V0(Payload::Protocol(msg)) => match msg { - RequestResponse::Request(req) => { - Self::process_incoming_protocol_message_req(dst_domain_id, channel_id, req) - } - RequestResponse::Response(_) => Err(Error::::InvalidMessagePayload.into()), + // process incoming protocol message. + VersionedPayload::V0(Payload::Protocol(RequestResponse::Request(req))) => { + Payload::Protocol(RequestResponse::Response( + Self::process_incoming_protocol_message_req(dst_domain_id, channel_id, req), + )) + } + + // process incoming endpoint message. + VersionedPayload::V0(Payload::Endpoint(RequestResponse::Request(req))) => { + let response = if let Some(endpoint_handler) = + T::get_endpoint_response_handler(&req.dst_endpoint) + { + endpoint_handler.message(dst_domain_id, req) + } else { + Err(Error::::NoMessageHandler.into()) + }; + + Payload::Endpoint(RequestResponse::Response(response)) + } + + // return error for all the remaining branches + VersionedPayload::V0(payload) => match payload { + Payload::Protocol(_) => Payload::Protocol(RequestResponse::Response(Err( + Error::::InvalidMessagePayload.into(), + ))), + Payload::Endpoint(_) => Payload::Endpoint(RequestResponse::Response(Err( + Error::::InvalidMessagePayload.into(), + ))), }, }; @@ -169,9 +195,7 @@ impl Pallet { dst_domain_id, channel_id, nonce: next_inbox_nonce, - payload: VersionedPayload::V0(Payload::Protocol(RequestResponse::Response( - ProtocolMessageResponse(response), - ))), + payload: VersionedPayload::V0(response), // this nonce is not considered in response context. last_delivered_message_response_nonce: None, }, @@ -228,7 +252,7 @@ impl Pallet { req: ProtocolMessageRequest, resp: ProtocolMessageResponse, ) -> DispatchResult { - match (req, resp.0) { + match (req, resp) { // channel open request is accepted by dst_domain. // open channel on our end. (ProtocolMessageRequest::ChannelOpen(_), Ok(_)) => { @@ -258,35 +282,58 @@ impl Pallet { // TODO(ved): maybe a bound of number of message responses to process in a single call? let mut messages_processed = 0; - while let Some(msg) = + while let Some(resp_msg) = OutboxResponses::::take((dst_domain_id, channel_id, next_message_response_nonce)) { - match msg.payload { - VersionedPayload::V0(Payload::Protocol(msg)) => match msg { - RequestResponse::Response(resp) => { - if let VersionedPayload::V0(Payload::Protocol(RequestResponse::Request( - req, - ))) = Outbox::::take(( - dst_domain_id, - channel_id, - next_message_response_nonce, - )) - .ok_or(Error::::MissingMessage)? - .payload - { - Self::process_incoming_protocol_message_response( - dst_domain_id, - channel_id, - req, - resp, - ) - } else { - Err(Error::::InvalidMessagePayload.into()) - } + // fetch original request + let req_msg = + Outbox::::take((dst_domain_id, channel_id, next_message_response_nonce)) + .ok_or(Error::::MissingMessage)?; + + let resp = match (req_msg.payload, resp_msg.payload) { + // process incoming protocol outbox message response. + ( + VersionedPayload::V0(Payload::Protocol(RequestResponse::Request(req))), + VersionedPayload::V0(Payload::Protocol(RequestResponse::Response(resp))), + ) => Self::process_incoming_protocol_message_response( + dst_domain_id, + channel_id, + req, + resp, + ), + + // process incoming endpoint outbox message response. + ( + VersionedPayload::V0(Payload::Endpoint(RequestResponse::Request(req))), + VersionedPayload::V0(Payload::Endpoint(RequestResponse::Response(resp))), + ) => { + if let Some(endpoint_handler) = + T::get_endpoint_response_handler(&req.dst_endpoint) + { + endpoint_handler.message_response(dst_domain_id, req, resp) + } else { + Err(Error::::NoMessageHandler.into()) } - RequestResponse::Request(_) => Err(Error::::InvalidMessagePayload.into()), - }, - }?; + } + + (_, _) => Err(Error::::InvalidMessagePayload.into()), + }; + + // deposit event notifying the message status. + match resp { + Ok(_) => Self::deposit_event(Event::OutboxMessageStatus { + domain_id: dst_domain_id, + channel_id, + nonce: next_message_response_nonce, + status: OutboxMessageStatus::Ok, + }), + Err(err) => Self::deposit_event(Event::OutboxMessageStatus { + domain_id: dst_domain_id, + channel_id, + nonce: next_message_response_nonce, + status: OutboxMessageStatus::Err(err), + }), + } last_message_response_nonce = Some(next_message_response_nonce); next_message_response_nonce = next_message_response_nonce diff --git a/crates/pallet-messenger/src/mock.rs b/crates/pallet-messenger/src/mock.rs index 9a269a0c9a..44093e7daa 100644 --- a/crates/pallet-messenger/src/mock.rs +++ b/crates/pallet-messenger/src/mock.rs @@ -1,7 +1,9 @@ use crate::{ChannelId, Channels, Config, InboxResponses, Nonce, Outbox, StateRootOf}; use frame_support::storage::generator::StorageDoubleMap; use sp_core::storage::StorageKey; +use sp_messenger::endpoint::{EndpointHandler, EndpointRequest, EndpointResponse}; use sp_runtime::traits::BlakeTwo256; +use sp_runtime::DispatchResult; use sp_state_machine::backend::Backend; use sp_state_machine::{prove_read, InMemoryBackend}; use sp_trie::StorageProof; @@ -12,12 +14,13 @@ pub type TestExternalities = sp_state_machine::TestExternalities; macro_rules! impl_runtime { ($runtime:ty, $domain_id:literal) => { - use crate::mock::{mock_system_domain_tracker, DomainId,TestExternalities}; + use crate::mock::{mock_system_domain_tracker, DomainId, TestExternalities, MockEndpoint}; use frame_support::parameter_types; use sp_core::H256; use sp_runtime::testing::Header; use sp_runtime::traits::{BlakeTwo256, ConstU16, ConstU32, ConstU64, IdentityLookup}; use sp_std::vec::Vec; + use sp_messenger::endpoint::{EndpointHandler, Endpoint}; type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; type Block = frame_system::mocking::MockBlock; @@ -77,6 +80,12 @@ macro_rules! impl_runtime { type DomainId = DomainId; type SelfDomainId = SelfDomainId; type SystemDomainTracker = SystemDomainTracker; + /// function to fetch endpoint response handler by Endpoint. + fn get_endpoint_response_handler( + _endpoint: &Endpoint, + ) -> Option>>{ + Some(Box::new(MockEndpoint{})) + } } pub fn new_test_ext() -> TestExternalities { @@ -91,6 +100,26 @@ macro_rules! impl_runtime { }; } +pub struct MockEndpoint {} +impl EndpointHandler for MockEndpoint { + fn message(&self, _src_domain_id: DomainId, req: EndpointRequest) -> EndpointResponse { + let req = req.payload; + assert_eq!(req, vec![1, 2, 3, 4]); + Ok(vec![5, 6, 7, 8]) + } + + fn message_response( + &self, + _dst_domain_id: DomainId, + _req: EndpointRequest, + resp: EndpointResponse, + ) -> DispatchResult { + let resp = resp.unwrap(); + assert_eq!(resp, vec![5, 6, 7, 8]); + Ok(()) + } +} + pub(crate) mod domain_a { impl_runtime!(Runtime, 1); } diff --git a/crates/pallet-messenger/src/tests.rs b/crates/pallet-messenger/src/tests.rs index 9e004e58c8..98828bfb58 100644 --- a/crates/pallet-messenger/src/tests.rs +++ b/crates/pallet-messenger/src/tests.rs @@ -1,6 +1,5 @@ use crate::messages::{ - CrossDomainMessage, Message, Payload, ProtocolMessageRequest, ProtocolMessageResponse, - RequestResponse, VersionedPayload, + CrossDomainMessage, Payload, ProtocolMessageRequest, RequestResponse, VersionedPayload, }; use crate::mock::domain_a::{ new_test_ext as new_domain_a_ext, Event, Messenger, Origin, Runtime, System, @@ -12,11 +11,12 @@ use crate::mock::{ use crate::verification::{Proof, StorageProofVerifier, VerificationError}; use crate::{ Channel, ChannelId, ChannelState, Channels, Error, Inbox, InboxResponses, - InitiateChannelParams, Nonce, Outbox, OutboxResponses, U256, + InitiateChannelParams, Nonce, Outbox, OutboxMessageStatus, OutboxResponses, U256, }; use frame_support::{assert_err, assert_ok}; use sp_core::storage::StorageKey; use sp_core::Blake2Hasher; +use sp_messenger::endpoint::{Endpoint, EndpointPayload, EndpointRequest, Sender}; use sp_runtime::traits::ValidateUnsigned; use sp_runtime::transaction_validity::TransactionSource; @@ -281,6 +281,68 @@ fn open_channel_between_domains( channel_id } +fn send_message_between_domains( + domain_a_test_ext: &mut TestExternalities, + domain_b_test_ext: &mut TestExternalities, + msg: EndpointPayload, + channel_id: ChannelId, +) { + let domain_b_id = domain_b::SelfDomainId::get(); + + // send message form outbox + domain_a_test_ext.execute_with(|| { + let resp = >::send_message( + domain_b_id, + EndpointRequest { + src_endpoint: Endpoint::Id(0), + dst_endpoint: Endpoint::Id(0), + payload: msg, + }, + ); + assert_ok!(resp); + domain_a::System::assert_last_event(Event::Messenger( + crate::Event::::OutboxMessage { + domain_id: domain_b_id, + channel_id, + nonce: Nonce::one(), + }, + )); + }); + + channel_relay_request_and_response( + domain_a_test_ext, + domain_b_test_ext, + channel_id, + Nonce::one(), + ); + + // check state on domain_b + domain_b_test_ext.execute_with(|| { + // Outbox, Outbox responses, Inbox, InboxResponses must be empty + assert_eq!(Outbox::::count(), 0); + assert_eq!(OutboxResponses::::count(), 0); + assert_eq!(Inbox::::count(), 0); + + // latest inbox message response is cleared on next message + assert_eq!(InboxResponses::::count(), 1); + }); + + // check state on domain_a + domain_a_test_ext.execute_with(|| { + // Outbox, Outbox responses, Inbox, InboxResponses must be empty + assert_eq!(Outbox::::count(), 0); + assert_eq!(OutboxResponses::::count(), 0); + assert_eq!(Inbox::::count(), 0); + assert_eq!(InboxResponses::::count(), 0); + + let channel = domain_a::Messenger::channels(domain_b_id, channel_id).unwrap(); + assert_eq!( + channel.latest_response_received_message_nonce, + Some(Nonce::one()) + ); + }); +} + fn close_channel_between_domains( domain_a_test_ext: &mut TestExternalities, domain_b_test_ext: &mut TestExternalities, @@ -398,6 +460,14 @@ fn channel_relay_request_and_response( let result = domain_b::Messenger::relay_message(domain_b::Origin::none(), xdm); assert_ok!(result); + domain_b::System::assert_has_event(domain_b::Event::Messenger(crate::Event::< + domain_b::Runtime, + >::InboxMessage { + domain_id: domain_a_id, + channel_id, + nonce, + })); + domain_b::System::assert_has_event(domain_b::Event::Messenger(crate::Event::< domain_b::Runtime, >::InboxMessageResponse { @@ -408,19 +478,10 @@ fn channel_relay_request_and_response( let response = domain_b::Messenger::inbox_responses((domain_a_id, channel_id, nonce)).unwrap(); - assert_eq!( - response, - Message { - src_domain_id: domain_b_id, - dst_domain_id: domain_a_id, - channel_id, - nonce, - payload: VersionedPayload::V0(Payload::Protocol(RequestResponse::Response( - ProtocolMessageResponse(Ok(())) - ))), - last_delivered_message_response_nonce: None - } - ); + assert_eq!(response.src_domain_id, domain_b_id); + assert_eq!(response.dst_domain_id, domain_a_id); + assert_eq!(response.channel_id, channel_id); + assert_eq!(response.nonce, nonce); assert_eq!( domain_a::Messenger::inbox((domain_b_id, channel_id, nonce)), None @@ -469,6 +530,15 @@ fn channel_relay_request_and_response( domain_a::Messenger::outbox_responses((domain_b_id, channel_id, nonce)), None ); + + domain_a::System::assert_has_event(domain_a::Event::Messenger(crate::Event::< + domain_a::Runtime, + >::OutboxMessageStatus { + domain_id: domain_b_id, + channel_id, + nonce, + status: OutboxMessageStatus::Ok, + })); }) } @@ -492,3 +562,20 @@ fn test_close_channel_between_domains() { // close open channel close_channel_between_domains(&mut domain_a_test_ext, &mut domain_b_test_ext, channel_id) } + +#[test] +fn test_send_message_between_domains() { + let mut domain_a_test_ext = domain_a::new_test_ext(); + let mut domain_b_test_ext = domain_b::new_test_ext(); + // open channel between domain_a and domain_b + // domain_a initiates the channel open + let channel_id = open_channel_between_domains(&mut domain_a_test_ext, &mut domain_b_test_ext); + + // send message + send_message_between_domains( + &mut domain_a_test_ext, + &mut domain_b_test_ext, + vec![1, 2, 3, 4], + channel_id, + ) +} diff --git a/crates/sp-messenger/Cargo.toml b/crates/sp-messenger/Cargo.toml index b7de38cef8..34e827cb77 100644 --- a/crates/sp-messenger/Cargo.toml +++ b/crates/sp-messenger/Cargo.toml @@ -14,8 +14,14 @@ include = [ ] [dependencies] - +codec = { package = "parity-scale-codec", version = "3.1.5", default-features = false, features = ["derive"] } +scale-info = { version = "2.1.2", default-features = false, features = ["derive"] } +sp-runtime = { version = "6.0.0", default-features = false, git = "https://github.com/subspace/substrate", rev = "1a7c28721fa77ecce9632ad9ce473f2d3cf1a598" } [features] default = ["std"] -std = [] +std = [ + "codec/std", + "scale-info/std", + "sp-runtime/std" +] diff --git a/crates/sp-messenger/src/endpoint.rs b/crates/sp-messenger/src/endpoint.rs new file mode 100644 index 0000000000..3b58b8beef --- /dev/null +++ b/crates/sp-messenger/src/endpoint.rs @@ -0,0 +1,47 @@ +use codec::{Decode, Encode}; +use scale_info::TypeInfo; +use sp_runtime::{DispatchError, DispatchResult}; + +/// Endpoint as defined in the formal spec. +/// Endpoint is an application that can send and receive messages from other domains. +#[derive(Debug, Encode, Decode, Clone, Eq, PartialEq, TypeInfo)] +pub enum Endpoint { + /// Id of the endpoint on a specific domain. + Id(u64), +} + +/// Endpoint request or response payload. +pub type EndpointPayload = Vec; + +/// Request sent by src_endpoint to dst_endpoint. +#[derive(Debug, Encode, Decode, Clone, Eq, PartialEq, TypeInfo)] +pub struct EndpointRequest { + pub src_endpoint: Endpoint, + pub dst_endpoint: Endpoint, + pub payload: EndpointPayload, +} + +/// Response for the message request. +pub type EndpointResponse = Result; + +/// Sender provides abstraction on sending messages to other domains. +pub trait Sender { + /// sends a message to dst_domain_id. + fn send_message(dst_domain_id: DomainId, req: EndpointRequest) -> DispatchResult; +} + +/// Handler to +/// - handle message request from other domains. +/// - handle requested message responses from other domains. +pub trait EndpointHandler { + /// Triggered by pallet-messenger when a new inbox message is received and bound for this handler. + fn message(&self, src_domain_id: DomainId, req: EndpointRequest) -> EndpointResponse; + + /// Triggered by pallet-messenger when a response for a request is received from dst_domain_id. + fn message_response( + &self, + dst_domain_id: DomainId, + req: EndpointRequest, + resp: EndpointResponse, + ) -> DispatchResult; +} diff --git a/crates/sp-messenger/src/lib.rs b/crates/sp-messenger/src/lib.rs index a61d7fafce..0ec4653a01 100644 --- a/crates/sp-messenger/src/lib.rs +++ b/crates/sp-messenger/src/lib.rs @@ -17,6 +17,8 @@ #![cfg_attr(not(feature = "std"), no_std)] +pub mod endpoint; + /// A trait used by domains to track and fetch info about system domain. pub trait SystemDomainTracker { /// Get the latest state roots of the K-deep System domain blocks. From e8555b707729e9bd45bdd07622ecfe689dee9d39 Mon Sep 17 00:00:00 2001 From: vedhavyas Date: Mon, 10 Oct 2022 08:31:39 +0200 Subject: [PATCH 5/8] update comment --- crates/pallet-messenger/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/pallet-messenger/src/lib.rs b/crates/pallet-messenger/src/lib.rs index 408686d9c0..bd903aec06 100644 --- a/crates/pallet-messenger/src/lib.rs +++ b/crates/pallet-messenger/src/lib.rs @@ -219,7 +219,7 @@ mod pallet { nonce: Nonce, }, - /// Emits when a handler returns an error after handling the message response. + /// Emits handler response to the message response delivery. OutboxMessageStatus { domain_id: T::DomainId, channel_id: ChannelId, From db7a72a145181b9859da291c345acac3461def20 Mon Sep 17 00:00:00 2001 From: vedhavyas Date: Mon, 10 Oct 2022 08:52:55 +0200 Subject: [PATCH 6/8] remove explcit transactional as its default --- crates/pallet-messenger/src/lib.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/crates/pallet-messenger/src/lib.rs b/crates/pallet-messenger/src/lib.rs index bd903aec06..83880d2289 100644 --- a/crates/pallet-messenger/src/lib.rs +++ b/crates/pallet-messenger/src/lib.rs @@ -97,7 +97,6 @@ mod pallet { StateRootOf, U256, }; use frame_support::pallet_prelude::*; - use frame_support::transactional; use frame_system::pallet_prelude::*; use sp_core::storage::StorageKey; use sp_messenger::endpoint::{Endpoint, EndpointHandler, EndpointRequest, Sender}; @@ -309,7 +308,6 @@ mod pallet { /// Channel is set to initiated and do not accept or receive any messages. /// Only a root user can create the channel. #[pallet::weight((10_000, Pays::No))] - #[transactional] pub fn initiate_channel( origin: OriginFor, dst_domain_id: T::DomainId, @@ -338,7 +336,6 @@ mod pallet { /// Channel is set to Closed and do not accept or receive any messages. /// Only a root user can close an open channel. #[pallet::weight((10_000, Pays::No))] - #[transactional] pub fn close_channel( origin: OriginFor, domain_id: T::DomainId, From 232ada058a732568ebd2baae247d4d50bd25f282 Mon Sep 17 00:00:00 2001 From: vedhavyas Date: Mon, 10 Oct 2022 09:37:48 +0200 Subject: [PATCH 7/8] rename OutboxMessageStatus to OutboxMessageResult --- crates/pallet-messenger/src/lib.rs | 10 +++++----- crates/pallet-messenger/src/messages.rs | 10 +++++----- crates/pallet-messenger/src/tests.rs | 6 +++--- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/crates/pallet-messenger/src/lib.rs b/crates/pallet-messenger/src/lib.rs index 83880d2289..e6901c35cc 100644 --- a/crates/pallet-messenger/src/lib.rs +++ b/crates/pallet-messenger/src/lib.rs @@ -76,7 +76,7 @@ pub struct InitiateChannelParams { } #[derive(Debug, Encode, Decode, Clone, Eq, PartialEq, TypeInfo, Copy)] -pub enum OutboxMessageStatus { +pub enum OutboxMessageResult { /// Message response handler returned Ok. Ok, /// Message response handler failed with Err. @@ -93,7 +93,7 @@ mod pallet { }; use crate::verification::{StorageProofVerifier, VerificationError}; use crate::{ - Channel, ChannelId, ChannelState, InitiateChannelParams, Nonce, OutboxMessageStatus, + Channel, ChannelId, ChannelState, InitiateChannelParams, Nonce, OutboxMessageResult, StateRootOf, U256, }; use frame_support::pallet_prelude::*; @@ -218,12 +218,12 @@ mod pallet { nonce: Nonce, }, - /// Emits handler response to the message response delivery. - OutboxMessageStatus { + /// Emits outbox message event. + OutboxMessageResult { domain_id: T::DomainId, channel_id: ChannelId, nonce: Nonce, - status: OutboxMessageStatus, + result: OutboxMessageResult, }, /// Emits when a new inbox message is validated and added to Inbox. diff --git a/crates/pallet-messenger/src/messages.rs b/crates/pallet-messenger/src/messages.rs index 0f758c78e4..71772fe538 100644 --- a/crates/pallet-messenger/src/messages.rs +++ b/crates/pallet-messenger/src/messages.rs @@ -1,7 +1,7 @@ use crate::verification::Proof; use crate::{ ChannelId, Channels, Config, Error, Event, Inbox, InboxResponses, InitiateChannelParams, Nonce, - Outbox, OutboxMessageStatus, OutboxResponses, Pallet, + Outbox, OutboxMessageResult, OutboxResponses, Pallet, }; use codec::{Decode, Encode}; use frame_support::ensure; @@ -321,17 +321,17 @@ impl Pallet { // deposit event notifying the message status. match resp { - Ok(_) => Self::deposit_event(Event::OutboxMessageStatus { + Ok(_) => Self::deposit_event(Event::OutboxMessageResult { domain_id: dst_domain_id, channel_id, nonce: next_message_response_nonce, - status: OutboxMessageStatus::Ok, + result: OutboxMessageResult::Ok, }), - Err(err) => Self::deposit_event(Event::OutboxMessageStatus { + Err(err) => Self::deposit_event(Event::OutboxMessageResult { domain_id: dst_domain_id, channel_id, nonce: next_message_response_nonce, - status: OutboxMessageStatus::Err(err), + result: OutboxMessageResult::Err(err), }), } diff --git a/crates/pallet-messenger/src/tests.rs b/crates/pallet-messenger/src/tests.rs index 98828bfb58..3b6a999548 100644 --- a/crates/pallet-messenger/src/tests.rs +++ b/crates/pallet-messenger/src/tests.rs @@ -11,7 +11,7 @@ use crate::mock::{ use crate::verification::{Proof, StorageProofVerifier, VerificationError}; use crate::{ Channel, ChannelId, ChannelState, Channels, Error, Inbox, InboxResponses, - InitiateChannelParams, Nonce, Outbox, OutboxMessageStatus, OutboxResponses, U256, + InitiateChannelParams, Nonce, Outbox, OutboxMessageResult, OutboxResponses, U256, }; use frame_support::{assert_err, assert_ok}; use sp_core::storage::StorageKey; @@ -533,11 +533,11 @@ fn channel_relay_request_and_response( domain_a::System::assert_has_event(domain_a::Event::Messenger(crate::Event::< domain_a::Runtime, - >::OutboxMessageStatus { + >::OutboxMessageResult { domain_id: domain_b_id, channel_id, nonce, - status: OutboxMessageStatus::Ok, + result: OutboxMessageResult::Ok, })); }) } From 21feccef0b2a6e6a12d7a2b970aea0cdd693262b Mon Sep 17 00:00:00 2001 From: vedhavyas Date: Mon, 10 Oct 2022 10:55:35 +0200 Subject: [PATCH 8/8] separate the logic between validate and pre_dispatch --- crates/pallet-messenger/src/lib.rs | 76 ++++++++++++++++++++++------ crates/pallet-messenger/src/tests.rs | 12 ++--- 2 files changed, 65 insertions(+), 23 deletions(-) diff --git a/crates/pallet-messenger/src/lib.rs b/crates/pallet-messenger/src/lib.rs index e6901c35cc..cf5943efd3 100644 --- a/crates/pallet-messenger/src/lib.rs +++ b/crates/pallet-messenger/src/lib.rs @@ -218,6 +218,15 @@ mod pallet { nonce: Nonce, }, + /// Emits when a message response is available for Outbox message. + OutboxMessageResponse { + /// Destination domain ID. + domain_id: T::DomainId, + /// Channel Is + channel_id: ChannelId, + nonce: Nonce, + }, + /// Emits outbox message event. OutboxMessageResult { domain_id: T::DomainId, @@ -261,12 +270,32 @@ mod pallet { impl ValidateUnsigned for Pallet { type Call = Call; + fn pre_dispatch(call: &Self::Call) -> Result<(), TransactionValidityError> { + match call { + Call::relay_message { msg: xdm } => { + let (msg, should_init_chanel) = Self::do_validate_relay_message(xdm)?; + Self::pre_dispatch_relay_message(msg, should_init_chanel) + } + Call::relay_message_response { msg: xdm } => { + let msg = Self::do_validate_relay_message_response(xdm)?; + Self::pre_dispatch_relay_message_response(msg) + } + _ => Err(InvalidTransaction::Call.into()), + } + } + /// Validate unsigned call to this module. fn validate_unsigned(_source: TransactionSource, call: &Self::Call) -> TransactionValidity { match call { - Call::relay_message { msg: xdm } => Self::do_validate_relay_message(xdm), + Call::relay_message { msg: xdm } => { + let (msg, _should_init_chanel) = Self::do_validate_relay_message(xdm)?; + let provides_tag = (msg.dst_domain_id, msg.channel_id, msg.nonce); + unsigned_validity::("MessengerInbox", provides_tag) + } Call::relay_message_response { msg: xdm } => { - Self::do_validate_relay_message_response(xdm) + let msg = Self::do_validate_relay_message_response(xdm)?; + let provides_tag = (msg.dst_domain_id, msg.channel_id, msg.nonce); + unsigned_validity::("MessengerOutboxResponse", provides_tag) } _ => InvalidTransaction::Call.into(), } @@ -493,7 +522,7 @@ mod pallet { pub(crate) fn do_validate_relay_message( xdm: &CrossDomainMessage>, - ) -> TransactionValidity { + ) -> Result<(Message, bool), TransactionValidityError> { let mut should_init_channel = false; let next_nonce = match Channels::::get(xdm.src_domain_id, xdm.channel_id) { None => { @@ -519,9 +548,15 @@ mod pallet { xdm.nonce, ))); - // verify, decode, and store the message + // verify and decode message let msg = Self::do_verify_xdm(next_nonce, key, xdm)?; + Ok((msg, should_init_channel)) + } + pub(crate) fn pre_dispatch_relay_message( + msg: Message, + should_init_channel: bool, + ) -> Result<(), TransactionValidityError> { if should_init_channel { if let VersionedPayload::V0(Payload::Protocol(RequestResponse::Request( ProtocolMessageRequest::ChannelOpen(params), @@ -530,23 +565,22 @@ mod pallet { Self::do_init_channel(msg.src_domain_id, params) .map_err(|_| InvalidTransaction::Call)?; } else { - return InvalidTransaction::Call.into(); + return Err(InvalidTransaction::Call.into()); } } - let provides_tag = (msg.dst_domain_id, msg.channel_id, msg.nonce); - Inbox::::insert((xdm.src_domain_id, xdm.channel_id, msg.nonce), msg); Self::deposit_event(Event::InboxMessage { - domain_id: xdm.src_domain_id, - channel_id: xdm.channel_id, - nonce: xdm.nonce, + domain_id: msg.src_domain_id, + channel_id: msg.channel_id, + nonce: msg.nonce, }); - unsigned_validity::("MessengerInbox", provides_tag) + Inbox::::insert((msg.src_domain_id, msg.channel_id, msg.nonce), msg); + Ok(()) } pub(crate) fn do_validate_relay_message_response( xdm: &CrossDomainMessage>, - ) -> TransactionValidity { + ) -> Result, TransactionValidityError> { // channel should be open and message should be present in outbox let next_nonce = match Channels::::get(xdm.src_domain_id, xdm.channel_id) { // unknown channel. return @@ -575,12 +609,22 @@ mod pallet { xdm.channel_id, xdm.nonce, ))); + // verify, decode, and store the message - let msg = Self::do_verify_xdm(next_nonce, key, xdm)?; - let provides_tag = (msg.dst_domain_id, msg.channel_id, xdm.nonce); - OutboxResponses::::insert((xdm.src_domain_id, xdm.channel_id, xdm.nonce), msg); + Self::do_verify_xdm(next_nonce, key, xdm) + } - unsigned_validity::("MessengerOutboxResponse", provides_tag) + pub(crate) fn pre_dispatch_relay_message_response( + msg: Message, + ) -> Result<(), TransactionValidityError> { + Self::deposit_event(Event::OutboxMessageResponse { + domain_id: msg.src_domain_id, + channel_id: msg.channel_id, + nonce: msg.nonce, + }); + + OutboxResponses::::insert((msg.src_domain_id, msg.channel_id, msg.nonce), msg); + Ok(()) } pub(crate) fn do_verify_xdm( diff --git a/crates/pallet-messenger/src/tests.rs b/crates/pallet-messenger/src/tests.rs index 3b6a999548..0fc08a2974 100644 --- a/crates/pallet-messenger/src/tests.rs +++ b/crates/pallet-messenger/src/tests.rs @@ -18,7 +18,6 @@ use sp_core::storage::StorageKey; use sp_core::Blake2Hasher; use sp_messenger::endpoint::{Endpoint, EndpointPayload, EndpointRequest, Sender}; use sp_runtime::traits::ValidateUnsigned; -use sp_runtime::transaction_validity::TransactionSource; fn create_channel(domain_id: DomainId, channel_id: ChannelId) { let params = InitiateChannelParams { @@ -450,10 +449,10 @@ fn channel_relay_request_and_response( domain_b::SystemDomainTracker::set_state_root(xdm.proof.state_root); // validate the message - let pre_check = crate::Pallet::::validate_unsigned( - TransactionSource::Local, - &crate::Call::relay_message { msg: xdm.clone() }, - ); + let pre_check = + crate::Pallet::::pre_dispatch(&crate::Call::relay_message { + msg: xdm.clone(), + }); assert_ok!(pre_check); // process inbox message @@ -511,8 +510,7 @@ fn channel_relay_request_and_response( domain_a::SystemDomainTracker::set_state_root(xdm.proof.state_root); // validate message response - let pre_check = crate::Pallet::::validate_unsigned( - TransactionSource::Local, + let pre_check = crate::Pallet::::pre_dispatch( &crate::Call::relay_message_response { msg: xdm.clone() }, ); assert_ok!(pre_check);