diff --git a/bridges/relays/bin-substrate/src/cli/relay_headers_and_messages.rs b/bridges/relays/bin-substrate/src/cli/relay_headers_and_messages.rs index 11220af3e7e8f..4a4d24964fc5e 100644 --- a/bridges/relays/bin-substrate/src/cli/relay_headers_and_messages.rs +++ b/bridges/relays/bin-substrate/src/cli/relay_headers_and_messages.rs @@ -133,10 +133,6 @@ macro_rules! select_bridge { type LeftAccountIdConverter = bp_millau::AccountIdConverter; type RightAccountIdConverter = bp_rialto::AccountIdConverter; - const MAX_MISSING_LEFT_HEADERS_AT_RIGHT: bp_millau::BlockNumber = - bp_millau::SESSION_LENGTH; - const MAX_MISSING_RIGHT_HEADERS_AT_LEFT: bp_rialto::BlockNumber = - bp_rialto::SESSION_LENGTH; const LEFT_RUNTIME_VERSION: Option = Some(millau_runtime::VERSION); const RIGHT_RUNTIME_VERSION: Option = @@ -185,11 +181,6 @@ macro_rules! select_bridge { type LeftAccountIdConverter = bp_rococo::AccountIdConverter; type RightAccountIdConverter = bp_wococo::AccountIdConverter; - const MAX_MISSING_LEFT_HEADERS_AT_RIGHT: bp_rococo::BlockNumber = - bp_rococo::SESSION_LENGTH; - const MAX_MISSING_RIGHT_HEADERS_AT_LEFT: bp_wococo::BlockNumber = - bp_wococo::SESSION_LENGTH; - const LEFT_RUNTIME_VERSION: Option = Some(bp_rococo::VERSION); const RIGHT_RUNTIME_VERSION: Option = @@ -268,11 +259,6 @@ macro_rules! select_bridge { type LeftAccountIdConverter = bp_kusama::AccountIdConverter; type RightAccountIdConverter = bp_polkadot::AccountIdConverter; - const MAX_MISSING_LEFT_HEADERS_AT_RIGHT: bp_kusama::BlockNumber = - bp_kusama::SESSION_LENGTH; - const MAX_MISSING_RIGHT_HEADERS_AT_LEFT: bp_polkadot::BlockNumber = - bp_polkadot::SESSION_LENGTH; - const LEFT_RUNTIME_VERSION: Option = Some(bp_kusama::VERSION); const RIGHT_RUNTIME_VERSION: Option = @@ -543,14 +529,12 @@ impl RelayHeadersAndMessages { left_client.clone(), right_client.clone(), left_to_right_transaction_params, - MAX_MISSING_LEFT_HEADERS_AT_RIGHT, params.shared.only_mandatory_headers, ); let right_to_left_on_demand_headers = OnDemandHeadersRelay::new::( right_client.clone(), left_client.clone(), right_to_left_transaction_params, - MAX_MISSING_RIGHT_HEADERS_AT_LEFT, params.shared.only_mandatory_headers, ); diff --git a/bridges/relays/lib-substrate-relay/src/on_demand_headers.rs b/bridges/relays/lib-substrate-relay/src/on_demand_headers.rs index 2e8a6db7983c6..9a6a062d7d8e0 100644 --- a/bridges/relays/lib-substrate-relay/src/on_demand_headers.rs +++ b/bridges/relays/lib-substrate-relay/src/on_demand_headers.rs @@ -18,7 +18,7 @@ use async_std::sync::{Arc, Mutex}; use futures::{select, FutureExt}; -use num_traits::{CheckedSub, One, Zero}; +use num_traits::{One, Zero}; use finality_relay::{FinalitySyncParams, SourceHeader, TargetClient as FinalityTargetClient}; use relay_substrate_client::{ @@ -55,7 +55,6 @@ impl OnDemandHeadersRelay { source_client: Client, target_client: Client, target_transaction_params: TransactionParams>, - maximal_headers_difference: BlockNumberOf, only_mandatory_headers: bool, ) -> Self where @@ -73,7 +72,6 @@ impl OnDemandHeadersRelay { source_client, target_client, target_transaction_params, - maximal_headers_difference, only_mandatory_headers, required_header_number, ) @@ -105,7 +103,6 @@ async fn background_task( source_client: Client, target_client: Client, target_transaction_params: TransactionParams>, - maximal_headers_difference: BlockNumberOf, only_mandatory_headers: bool, required_header_number: RequiredHeaderNumberRef, ) where @@ -165,15 +162,28 @@ async fn background_task( } // submit mandatory header if some headers are missing + let best_finalized_source_header_at_source_fmt = + format!("{:?}", best_finalized_source_header_at_source); let best_finalized_source_header_at_target_fmt = format!("{:?}", best_finalized_source_header_at_target); + let required_header_number_value = *required_header_number.lock().await; let mandatory_scan_range = mandatory_headers_scan_range::( best_finalized_source_header_at_source.ok(), best_finalized_source_header_at_target.ok(), - maximal_headers_difference, - &required_header_number, + required_header_number_value, ) .await; + + log::trace!( + target: "bridge", + "Mandatory headers scan range in {}: ({:?}, {:?}, {:?}) -> {:?}", + relay_task_name, + required_header_number_value, + best_finalized_source_header_at_source_fmt, + best_finalized_source_header_at_target_fmt, + mandatory_scan_range, + ); + if let Some(mandatory_scan_range) = mandatory_scan_range { let relay_mandatory_header_result = relay_mandatory_header_from_range( &finality_source, @@ -227,6 +237,25 @@ async fn background_task( // start/restart relay if restart_relay { + let stall_timeout = relay_substrate_client::transaction_stall_timeout( + target_transactions_mortality, + P::TargetChain::AVERAGE_BLOCK_INTERVAL, + STALL_TIMEOUT, + ); + + log::info!( + target: "bridge", + "Starting {} relay\n\t\ + Only mandatory headers: {}\n\t\ + Tx mortality: {:?} (~{}m)\n\t\ + Stall timeout: {:?}", + relay_task_name, + only_mandatory_headers, + target_transactions_mortality, + stall_timeout.as_secs_f64() / 60.0f64, + stall_timeout, + ); + finality_relay_task.set( finality_relay::run( finality_source.clone(), @@ -237,11 +266,7 @@ async fn background_task( P::TargetChain::AVERAGE_BLOCK_INTERVAL, ), recent_finality_proofs_limit: RECENT_FINALITY_PROOFS_LIMIT, - stall_timeout: relay_substrate_client::transaction_stall_timeout( - target_transactions_mortality, - P::TargetChain::AVERAGE_BLOCK_INTERVAL, - STALL_TIMEOUT, - ), + stall_timeout, only_mandatory_headers, }, MetricsParams::disabled(), @@ -260,11 +285,8 @@ async fn background_task( async fn mandatory_headers_scan_range( best_finalized_source_header_at_source: Option, best_finalized_source_header_at_target: Option, - maximal_headers_difference: C::BlockNumber, - required_header_number: &RequiredHeaderNumberRef, + required_header_number: BlockNumberOf, ) -> Option<(C::BlockNumber, C::BlockNumber)> { - let required_header_number = *required_header_number.lock().await; - // if we have been unable to read header number from the target, then let's assume // that it is the same as required header number. Otherwise we risk submitting // unneeded transactions @@ -276,23 +298,8 @@ async fn mandatory_headers_scan_range( let best_finalized_source_header_at_source = best_finalized_source_header_at_source.unwrap_or(best_finalized_source_header_at_target); - // if there are too many source headers missing from the target node, sync mandatory - // headers to target - // - // why do we need that? When complex headers+messages relay is used, it'll normally only relay - // headers when there are undelivered messages/confirmations. But security model of the - // `pallet-bridge-grandpa` module relies on the fact that headers are synced in real-time and - // that it'll see authorities-change header before unbonding period will end for previous - // authorities set. - let current_headers_difference = best_finalized_source_header_at_source - .checked_sub(&best_finalized_source_header_at_target) - .unwrap_or_else(Zero::zero); - if current_headers_difference <= maximal_headers_difference { - return None - } - - // if relay is already asked to sync headers, don't do anything yet - if required_header_number > best_finalized_source_header_at_target { + // if relay is already asked to sync more headers than we have at source, don't do anything yet + if required_header_number >= best_finalized_source_header_at_source { return None } @@ -423,29 +430,18 @@ mod tests { const AT_TARGET: Option = Some(1); #[async_std::test] - async fn mandatory_headers_scan_range_selects_range_if_too_many_headers_are_missing() { + async fn mandatory_headers_scan_range_selects_range_if_some_headers_are_missing() { assert_eq!( - mandatory_headers_scan_range::( - AT_SOURCE, - AT_TARGET, - 5, - &Arc::new(Mutex::new(0)) - ) - .await, + mandatory_headers_scan_range::(AT_SOURCE, AT_TARGET, 0,).await, Some((AT_TARGET.unwrap() + 1, AT_SOURCE.unwrap())), ); } #[async_std::test] - async fn mandatory_headers_scan_range_selects_nothing_if_enough_headers_are_relayed() { + async fn mandatory_headers_scan_range_selects_nothing_if_already_queued() { assert_eq!( - mandatory_headers_scan_range::( - AT_SOURCE, - AT_TARGET, - 10, - &Arc::new(Mutex::new(0)) - ) - .await, + mandatory_headers_scan_range::(AT_SOURCE, AT_TARGET, AT_SOURCE.unwrap(),) + .await, None, ); }