diff --git a/domains/client/cross-domain-message-gossip/src/aux_schema.rs b/domains/client/cross-domain-message-gossip/src/aux_schema.rs index 60d9d72001..24b5cd2a91 100644 --- a/domains/client/cross-domain-message-gossip/src/aux_schema.rs +++ b/domains/client/cross-domain-message-gossip/src/aux_schema.rs @@ -16,10 +16,10 @@ const LOG_TARGET: &str = "gossip_aux_schema"; fn channel_detail_key( src_chain_id: ChainId, - dst_chain_id: ChainId, + self_chain_id: ChainId, channel_id: ChannelId, ) -> Vec { - (CHANNEL_DETAIL, src_chain_id, dst_chain_id, channel_id).encode() + (CHANNEL_DETAIL, src_chain_id, self_chain_id, channel_id).encode() } fn load_decode( @@ -57,11 +57,11 @@ pub struct ChannelDetail { pub latest_response_received_message_nonce: Option, } -/// Load the channel state of self_chain_id on chain_id. +/// Load the channel state of self_chain_id on src_chain_id. pub fn get_channel_state( backend: &Backend, - dst_chain_id: ChainId, src_chain_id: ChainId, + self_chain_id: ChainId, channel_id: ChannelId, ) -> ClientResult> where @@ -69,15 +69,15 @@ where { load_decode( backend, - channel_detail_key(src_chain_id, dst_chain_id, channel_id).as_slice(), + channel_detail_key(src_chain_id, self_chain_id, channel_id).as_slice(), ) } -/// Set the channel state of self_chain_id on chain_id. +/// Set the channel state of self_chain_id on src_chain_id. pub fn set_channel_state( backend: &Backend, - dst_chain_id: ChainId, src_chain_id: ChainId, + self_chain_id: ChainId, channel_detail: ChannelDetail, ) -> ClientResult<()> where @@ -85,7 +85,7 @@ where { backend.insert_aux( &[( - channel_detail_key(src_chain_id, dst_chain_id, channel_detail.channel_id).as_slice(), + channel_detail_key(src_chain_id, self_chain_id, channel_detail.channel_id).as_slice(), channel_detail.encode().as_slice(), )], vec![], @@ -95,7 +95,7 @@ where relay_msg_nonce: Some(channel_detail.next_inbox_nonce), relay_response_msg_nonce: channel_detail.latest_response_received_message_nonce, }; - let prefix = (RELAYER_PREFIX, src_chain_id).encode(); + let prefix = (RELAYER_PREFIX, src_chain_id, self_chain_id).encode(); cleanup_chain_channel_storages( backend, &prefix, diff --git a/domains/client/cross-domain-message-gossip/src/message_listener.rs b/domains/client/cross-domain-message-gossip/src/message_listener.rs index c54c62c1e7..a5dcf81eb4 100644 --- a/domains/client/cross-domain-message-gossip/src/message_listener.rs +++ b/domains/client/cross-domain-message-gossip/src/message_listener.rs @@ -481,7 +481,7 @@ where pub fn can_allow_xdm_submission( client: &Arc, xdm_id: XdmId, - submitted_block_id: BlockId, + maybe_submitted_block_id: Option>, current_block_id: BlockId, maybe_channel_nonce: Option, ) -> bool @@ -517,20 +517,25 @@ where } } - match client.hash(submitted_block_id.number).ok().flatten() { - // there is no block at this number, allow xdm submission - None => return true, - Some(hash) => { - if hash != submitted_block_id.hash { - // client re-org'ed, allow xdm submission - return true; + match maybe_submitted_block_id { + None => true, + Some(submitted_block_id) => { + match client.hash(submitted_block_id.number).ok().flatten() { + // there is no block at this number, allow xdm submission + None => return true, + Some(hash) => { + if hash != submitted_block_id.hash { + // client re-org'ed, allow xdm submission + return true; + } + } } + + let latest_block_number = current_block_id.number; + let block_limit: NumberFor = XDM_ACCEPT_BLOCK_LIMIT.saturated_into(); + submitted_block_id.number < latest_block_number.saturating_sub(block_limit) } } - - let latest_block_number = current_block_id.number; - let block_limit: NumberFor = XDM_ACCEPT_BLOCK_LIMIT.saturated_into(); - submitted_block_id.number < latest_block_number.saturating_sub(block_limit) } async fn handle_xdm_message( @@ -565,21 +570,22 @@ where let maybe_channel_nonce = runtime_api.channel_nonce(block_id.hash, src_chain_id, channel_id)?; - if let Some(submitted_block_id) = - get_xdm_processed_block_number::<_, BlockOf>(&**client, TX_POOL_PREFIX, xdm_id)? - && !can_allow_xdm_submission( - client, - xdm_id, - submitted_block_id.clone(), - block_id.clone(), - maybe_channel_nonce, - ) - { + let maybe_submitted_xdm_block = get_xdm_processed_block_number::<_, BlockOf>( + &**client, + TX_POOL_PREFIX, + xdm_id, + )?; + if !can_allow_xdm_submission( + client, + xdm_id, + maybe_submitted_xdm_block, + block_id.clone(), + maybe_channel_nonce, + ) { tracing::debug!( target: LOG_TARGET, - "Skipping XDM[{:?}] submission. At: {:?} and Now: {:?}", + "Skipping XDM[{:?}] submission. At: {:?}", xdm_id, - submitted_block_id, block_id ); return Ok(true); diff --git a/domains/client/relayer/src/lib.rs b/domains/client/relayer/src/lib.rs index 664bf113d6..ce7580ec01 100644 --- a/domains/client/relayer/src/lib.rs +++ b/domains/client/relayer/src/lib.rs @@ -231,24 +231,26 @@ where .map_err(Error::UnableToSubmitCrossDomainMessage) } -fn check_and_update_recent_xdm_submission( - consensus_client: &Arc, +fn check_and_update_recent_xdm_submission( + backend: &Backend, + client: &Arc, xdm_id: XdmId, msg: &BlockMessageWithStorageKey, ) -> bool where - CBlock: BlockT, - CClient: AuxStore + HeaderBackend, + Backend: AuxStore, + Block: BlockT, + Client: HeaderBackend, { - let prefix = (RELAYER_PREFIX, msg.src_chain_id).encode(); - let current_block_id: BlockId = consensus_client.info().into(); - if let Ok(Some(submitted_block_id)) = - get_xdm_processed_block_number::<_, CBlock>(&**consensus_client, &prefix, xdm_id) + let prefix = (RELAYER_PREFIX, msg.dst_chain_id, msg.src_chain_id).encode(); + let current_block_id: BlockId = client.info().into(); + if let Ok(maybe_submitted_block_id) = + get_xdm_processed_block_number::<_, Block>(backend, &prefix, xdm_id) { if !can_allow_xdm_submission( - consensus_client, + client, xdm_id, - submitted_block_id, + maybe_submitted_block_id, current_block_id.clone(), None, ) { @@ -262,9 +264,7 @@ where } } - if let Err(err) = - set_xdm_message_processed_at(&**consensus_client, &prefix, xdm_id, current_block_id) - { + if let Err(err) = set_xdm_message_processed_at(backend, &prefix, xdm_id, current_block_id) { log::error!( target: LOG_TARGET, "Failed to store submitted message from {:?} to {:?}: {:?}", @@ -277,17 +277,18 @@ where true } -fn should_relay_outbox_message( - consensus_client: &Arc, +fn should_relay_outbox_message( + backend: &Backend, + client: &Arc, api: &ApiRef<'_, Client::Api>, best_hash: Block::Hash, msg: &BlockMessageWithStorageKey, ) -> bool where + Backend: AuxStore, Block: BlockT, CBlock: BlockT, - Client: ProvideRuntimeApi, - CClient: AuxStore + HeaderBackend, + Client: ProvideRuntimeApi + HeaderBackend, Client::Api: RelayerApi, NumberFor, CBlock::Hash>, { let id = msg.id(); @@ -305,14 +306,10 @@ where } }; - if let Some(dst_channel_state) = get_channel_state( - &**consensus_client, - msg.dst_chain_id, - msg.src_chain_id, - msg.channel_id, - ) - .ok() - .flatten() + if let Some(dst_channel_state) = + get_channel_state(backend, msg.dst_chain_id, msg.src_chain_id, msg.channel_id) + .ok() + .flatten() { // if this message should relay, // check if the dst_chain inbox nonce is more than message nonce, @@ -330,20 +327,21 @@ where } let xdm_id = XdmId::RelayMessage((msg.dst_chain_id, msg.channel_id, msg.nonce)); - check_and_update_recent_xdm_submission(consensus_client, xdm_id, msg) + check_and_update_recent_xdm_submission(backend, client, xdm_id, msg) } -fn should_relay_inbox_responses_message( - consensus_client: &Arc, +fn should_relay_inbox_responses_message( + backend: &Backend, + client: &Arc, api: &ApiRef<'_, Client::Api>, best_hash: Block::Hash, msg: &BlockMessageWithStorageKey, ) -> bool where + Backend: AuxStore, Block: BlockT, CBlock: BlockT, - Client: ProvideRuntimeApi, - CClient: AuxStore + HeaderBackend, + Client: ProvideRuntimeApi + HeaderBackend, Client::Api: RelayerApi, NumberFor, CBlock::Hash>, { let id = msg.id(); @@ -361,14 +359,10 @@ where } }; - if let Some(dst_channel_state) = get_channel_state( - &**consensus_client, - msg.dst_chain_id, - msg.src_chain_id, - msg.channel_id, - ) - .ok() - .flatten() + if let Some(dst_channel_state) = + get_channel_state(backend, msg.dst_chain_id, msg.src_chain_id, msg.channel_id) + .ok() + .flatten() && let Some(dst_chain_outbox_response_nonce) = dst_channel_state.latest_response_received_message_nonce { @@ -386,7 +380,7 @@ where } let xdm_id = XdmId::RelayResponseMessage((msg.dst_chain_id, msg.channel_id, msg.nonce)); - check_and_update_recent_xdm_submission(consensus_client, xdm_id, msg) + check_and_update_recent_xdm_submission(backend, client, xdm_id, msg) } // Fetch the XDM at the a given block and filter any already relayed XDM according to the best block @@ -410,12 +404,19 @@ where let api = client.runtime_api(); let best_hash = client.info().best_hash; msgs.outbox.retain(|msg| { - should_relay_outbox_message::(consensus_client, &api, best_hash, msg) + should_relay_outbox_message::<_, _, _, CBlock>( + &**consensus_client, + client, + &api, + best_hash, + msg, + ) }); msgs.inbox_responses.retain(|msg| { - should_relay_inbox_responses_message::( - consensus_client, + should_relay_inbox_responses_message::<_, _, _, CBlock>( + &**consensus_client, + client, &api, best_hash, msg, diff --git a/domains/pallets/messenger/src/lib.rs b/domains/pallets/messenger/src/lib.rs index 4d6d41cf30..5fbe64eb5b 100644 --- a/domains/pallets/messenger/src/lib.rs +++ b/domains/pallets/messenger/src/lib.rs @@ -48,6 +48,20 @@ use sp_messenger::messages::{ use sp_runtime::traits::{Extrinsic, Hash}; use sp_runtime::DispatchError; +/// Maximum number of XDMs per domain/channel with future nonces that are allowed to be validated. +/// Any XDM comes with a nonce above Maximum future nonce will be rejected. +// TODO: We need to benchmark how many XDMs can fit in to a +// - Single consensus block +// - Single domain block(includes all bundles filled with XDMs) +// Once we have that info, we can make a better judgement on how many XDMs +// we want to include per block while allowing other extrinsics to be included as well. +// Note: Currently XDM takes priority over other extrinsics unless they come with priority fee +const MAX_FUTURE_ALLOWED_NONCES: u32 = 256; + +/// Transaction validity for a given validated XDM extrinsic. +/// If the extrinsic is not included in the bundle, extrinsic is removed from the TxPool. +const XDM_TRANSACTION_LONGEVITY: u64 = 10; + pub(crate) mod verification_errors { // When updating these error codes, check for clashes between: // @@ -56,6 +70,7 @@ pub(crate) mod verification_errors { pub(crate) const NONCE_OVERFLOW: u8 = 202; // This error code was previously 200, but that clashed with ERR_BALANCE_OVERFLOW. pub(crate) const INVALID_CHANNEL: u8 = 203; + pub(crate) const IN_FUTURE_NONCE: u8 = 204; } #[derive(Debug, Encode, Decode, Clone, Eq, PartialEq, TypeInfo, Copy)] @@ -118,7 +133,7 @@ mod pallet { use crate::{ BalanceOf, ChainAllowlistUpdate, Channel, ChannelId, ChannelState, CloseChannelBy, FeeModel, HoldIdentifier, Nonce, OutboxMessageResult, StateRootOf, ValidatedRelayMessage, - STORAGE_VERSION, U256, + MAX_FUTURE_ALLOWED_NONCES, STORAGE_VERSION, U256, XDM_TRANSACTION_LONGEVITY, }; #[cfg(not(feature = "std"))] use alloc::boxed::Box; @@ -428,6 +443,15 @@ mod pallet { let mut valid_tx_builder = ValidTransaction::with_tag_prefix("MessengerInbox"); // Only add the requires tag if the msg nonce is in future if msg_nonce > next_nonce { + let max_future_nonce = + next_nonce.saturating_add(MAX_FUTURE_ALLOWED_NONCES.into()); + if msg_nonce > max_future_nonce { + return Err(InvalidTransaction::Custom( + crate::verification_errors::IN_FUTURE_NONCE, + ) + .into()); + } + valid_tx_builder = valid_tx_builder.and_requires(( dst_chain_id, channel_id, @@ -438,7 +462,7 @@ mod pallet { // XDM have a bit higher priority than normal extrinsic but must less than // fraud proof .priority(1) - .longevity(TransactionLongevity::MAX) + .longevity(XDM_TRANSACTION_LONGEVITY) .and_provides((dst_chain_id, channel_id, msg_nonce)) .propagate(true) .build() @@ -461,6 +485,15 @@ mod pallet { ValidTransaction::with_tag_prefix("MessengerOutboxResponse"); // Only add the requires tag if the msg nonce is in future if msg_nonce > next_nonce { + let max_future_nonce = + next_nonce.saturating_add(MAX_FUTURE_ALLOWED_NONCES.into()); + if msg_nonce > max_future_nonce { + return Err(InvalidTransaction::Custom( + crate::verification_errors::IN_FUTURE_NONCE, + ) + .into()); + } + valid_tx_builder = valid_tx_builder.and_requires(( dst_chain_id, channel_id, @@ -471,7 +504,7 @@ mod pallet { // XDM have a bit higher priority than normal extrinsic but must less than // fraud proof .priority(1) - .longevity(TransactionLongevity::MAX) + .longevity(XDM_TRANSACTION_LONGEVITY) .and_provides((dst_chain_id, channel_id, msg_nonce)) .propagate(true) .build()