Skip to content

Commit

Permalink
Merge pull request #3380 from autonomys/xdm_audit_issue
Browse files Browse the repository at this point in the history 
XDM limit max number of future nonces allowed and relayer header backend fix
  • Loading branch information
@vedhavyas
vedhavyas authored Feb 11, 2025
2 parents c5ac4a0 + e9bce5d commit 2b2828d
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 78 deletions.
18 changes: 9 additions & 9 deletions domains/client/cross-domain-message-gossip/src/aux_schema.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<u8> {
(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<Backend: AuxStore, T: Decode>(
Expand Down Expand Up @@ -57,35 +57,35 @@ pub struct ChannelDetail {
pub latest_response_received_message_nonce: Option<Nonce>,
}

/// 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: &Backend,
dst_chain_id: ChainId,
src_chain_id: ChainId,
self_chain_id: ChainId,
channel_id: ChannelId,
) -> ClientResult<Option<ChannelDetail>>
where
Backend: AuxStore,
{
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: &Backend,
dst_chain_id: ChainId,
src_chain_id: ChainId,
self_chain_id: ChainId,
channel_detail: ChannelDetail,
) -> ClientResult<()>
where
Backend: AuxStore,
{
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![],
Expand All @@ -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,
Expand Down
54 changes: 30 additions & 24 deletions domains/client/cross-domain-message-gossip/src/message_listener.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ where
pub fn can_allow_xdm_submission<Client, Block>(
client: &Arc<Client>,
xdm_id: XdmId,
submitted_block_id: BlockId<Block>,
maybe_submitted_block_id: Option<BlockId<Block>>,
current_block_id: BlockId<Block>,
maybe_channel_nonce: Option<ChannelNonce>,
) -> bool
Expand Down Expand Up @@ -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<Block> = 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<Block> = XDM_ACCEPT_BLOCK_LIMIT.saturated_into();
submitted_block_id.number < latest_block_number.saturating_sub(block_limit)
}

async fn handle_xdm_message<TxPool, Client, CBlock>(
Expand Down Expand Up @@ -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<TxPool>>(&**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<TxPool>>(
&**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);
Expand Down
85 changes: 43 additions & 42 deletions domains/client/relayer/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,24 +231,26 @@ where
.map_err(Error::UnableToSubmitCrossDomainMessage)
}

fn check_and_update_recent_xdm_submission<CClient, CBlock>(
consensus_client: &Arc<CClient>,
fn check_and_update_recent_xdm_submission<Backend, Client, Block>(
backend: &Backend,
client: &Arc<Client>,
xdm_id: XdmId,
msg: &BlockMessageWithStorageKey,
) -> bool
where
CBlock: BlockT,
CClient: AuxStore + HeaderBackend<CBlock>,
Backend: AuxStore,
Block: BlockT,
Client: HeaderBackend<Block>,
{
let prefix = (RELAYER_PREFIX, msg.src_chain_id).encode();
let current_block_id: BlockId<CBlock> = 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<Block> = 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,
) {
Expand All @@ -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 {:?}: {:?}",
Expand All @@ -277,17 +277,18 @@ where
true
}

fn should_relay_outbox_message<Client, Block, CClient, CBlock>(
consensus_client: &Arc<CClient>,
fn should_relay_outbox_message<Backend, Client, Block, CBlock>(
backend: &Backend,
client: &Arc<Client>,
api: &ApiRef<'_, Client::Api>,
best_hash: Block::Hash,
msg: &BlockMessageWithStorageKey,
) -> bool
where
Backend: AuxStore,
Block: BlockT,
CBlock: BlockT,
Client: ProvideRuntimeApi<Block>,
CClient: AuxStore + HeaderBackend<CBlock>,
Client: ProvideRuntimeApi<Block> + HeaderBackend<Block>,
Client::Api: RelayerApi<Block, NumberFor<Block>, NumberFor<CBlock>, CBlock::Hash>,
{
let id = msg.id();
Expand All @@ -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,
Expand All @@ -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<Client, Block, CClient, CBlock>(
consensus_client: &Arc<CClient>,
fn should_relay_inbox_responses_message<Backend, Client, Block, CBlock>(
backend: &Backend,
client: &Arc<Client>,
api: &ApiRef<'_, Client::Api>,
best_hash: Block::Hash,
msg: &BlockMessageWithStorageKey,
) -> bool
where
Backend: AuxStore,
Block: BlockT,
CBlock: BlockT,
Client: ProvideRuntimeApi<Block>,
CClient: AuxStore + HeaderBackend<CBlock>,
Client: ProvideRuntimeApi<Block> + HeaderBackend<Block>,
Client::Api: RelayerApi<Block, NumberFor<Block>, NumberFor<CBlock>, CBlock::Hash>,
{
let id = msg.id();
Expand All @@ -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
{
Expand All @@ -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
Expand All @@ -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::<Client, _, _, CBlock>(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::<Client, _, _, CBlock>(
consensus_client,
should_relay_inbox_responses_message::<_, _, _, CBlock>(
&**consensus_client,
client,
&api,
best_hash,
msg,
Expand Down
Loading

0 comments on commit 2b2828d

Please sign in to comment.