Fix on demand parachains relay when no parachain head at target (#1834)

* `best_finalized_peer_at_best_self` in messages relay is now Option<> - before it was an error, which effectively blocked the lane

* unnecessary mut

* clone on return
This commit is contained in:
Svyatoslav Nikolsky
2023-01-31 18:25:06 +03:00
committed by Bastian Köcher
parent 947a0a695b
commit f0c4073dc6
6 changed files with 150 additions and 98 deletions
@@ -102,7 +102,8 @@ where
None,
)
.await?
.best_finalized_peer_at_best_self)
.best_finalized_peer_at_best_self
.ok_or(Error::BridgePalletIsNotInitialized)?)
}
async fn submit_finality_proof(
@@ -421,14 +421,15 @@ where
.await?;
// read actual header, matching the `peer_on_self_best_finalized_id` from the peer chain
let actual_peer_on_self_best_finalized_id = match peer_client {
Some(peer_client) => {
let actual_peer_on_self_best_finalized =
peer_client.header_by_number(peer_on_self_best_finalized_id.number()).await?;
actual_peer_on_self_best_finalized.id()
},
None => peer_on_self_best_finalized_id,
};
let actual_peer_on_self_best_finalized_id =
match (peer_client, peer_on_self_best_finalized_id.as_ref()) {
(Some(peer_client), Some(peer_on_self_best_finalized_id)) => {
let actual_peer_on_self_best_finalized =
peer_client.header_by_number(peer_on_self_best_finalized_id.number()).await?;
Some(actual_peer_on_self_best_finalized.id())
},
_ => peer_on_self_best_finalized_id,
};
Ok(ClientState {
best_self: self_best_id,
@@ -444,7 +445,7 @@ where
pub async fn best_finalized_peer_header_at_self<SelfChain, PeerChain>(
self_client: &Client<SelfChain>,
at_self_hash: HashOf<SelfChain>,
) -> Result<HeaderIdOf<PeerChain>, SubstrateError>
) -> Result<Option<HeaderIdOf<PeerChain>>, SubstrateError>
where
SelfChain: Chain,
PeerChain: Chain,
@@ -456,8 +457,7 @@ where
(),
Some(at_self_hash),
)
.await?
.ok_or(SubstrateError::BridgePalletIsNotInitialized)
.await
}
fn validate_out_msgs_details<C: Chain>(
@@ -422,11 +422,14 @@ struct RelayData<ParaHash, ParaNumber, RelayNumber> {
pub para_header_at_source: Option<HeaderId<ParaHash, ParaNumber>>,
/// Parachain header, that is available at the source relay chain at `relay_header_at_target`
/// block.
///
/// May be `None` if there's no `relay_header_at_target` yet, or if the
/// `relay_header_at_target` is too old and we think its state has been pruned.
pub para_header_at_relay_header_at_target: Option<HeaderId<ParaHash, ParaNumber>>,
/// Relay header number at the source chain.
pub relay_header_at_source: RelayNumber,
/// Relay header number at the target chain.
pub relay_header_at_target: RelayNumber,
pub relay_header_at_target: Option<RelayNumber>,
}
/// Read required data from source and target clients.
@@ -477,9 +480,8 @@ where
// submit at least one. Otherwise the pallet will be treated as uninitialized and messages
// sync will stall.
let para_header_at_target = match para_header_at_target {
Ok(para_header_at_target) => Some(para_header_at_target.0),
Err(SubstrateError::BridgePalletIsNotInitialized) |
Err(SubstrateError::NoParachainHeadAtTarget(_, _)) => None,
Ok(Some(para_header_at_target)) => Some(para_header_at_target.0),
Ok(None) => None,
Err(e) => return Err(map_target_err(e)),
};
@@ -502,25 +504,34 @@ where
.await
.map_err(map_target_err)?;
// if relay header at target is too old, then its state may already be discarded at the source
// if relay header at target is too old then its state may already be discarded at the source
// => just use `None` in this case
let is_relay_header_at_target_ancient =
is_ancient_block(relay_header_at_target.number(), relay_header_at_source);
let para_header_at_relay_header_at_target = if is_relay_header_at_target_ancient {
None
} else {
source
.on_chain_para_head_id(relay_header_at_target, P::SourceParachain::PARACHAIN_ID.into())
.await
.map_err(map_source_err)?
};
//
// the same is for case when there's no relay header at target at all
let available_relay_header_at_target =
relay_header_at_target.filter(|relay_header_at_target| {
!is_ancient_block(relay_header_at_target.number(), relay_header_at_source)
});
let para_header_at_relay_header_at_target =
if let Some(available_relay_header_at_target) = available_relay_header_at_target {
source
.on_chain_para_head_id(
available_relay_header_at_target,
P::SourceParachain::PARACHAIN_ID.into(),
)
.await
.map_err(map_source_err)?
} else {
None
};
Ok(RelayData {
required_para_header: required_header_number,
para_header_at_target,
para_header_at_source,
relay_header_at_source,
relay_header_at_target: relay_header_at_target.0,
relay_header_at_target: relay_header_at_target
.map(|relay_header_at_target| relay_header_at_target.0),
para_header_at_relay_header_at_target,
})
}
@@ -528,25 +539,35 @@ where
/// Select relay and parachain headers that need to be relayed.
fn select_headers_to_relay<ParaHash, ParaNumber, RelayNumber>(
data: &RelayData<ParaHash, ParaNumber, RelayNumber>,
mut state: RelayState<ParaHash, ParaNumber, RelayNumber>,
state: RelayState<ParaHash, ParaNumber, RelayNumber>,
) -> RelayState<ParaHash, ParaNumber, RelayNumber>
where
ParaHash: Clone,
ParaNumber: Copy + PartialOrd + Zero,
RelayNumber: Copy + Debug + Ord,
{
// we can't do anything until **relay chain** bridge GRANDPA pallet is not initialized at the
// target chain
let relay_header_at_target = match data.relay_header_at_target {
Some(relay_header_at_target) => relay_header_at_target,
None => return RelayState::Idle,
};
// Process the `RelayingRelayHeader` state.
if let &RelayState::RelayingRelayHeader(relay_header_number) = &state {
if data.relay_header_at_target < relay_header_number {
if relay_header_at_target < relay_header_number {
// The required relay header hasn't yet been relayed. Ask / wait for it.
return state
}
// We may switch to `RelayingParaHeader` if parachain head is available.
state = data
.para_header_at_relay_header_at_target
.clone()
.map_or(RelayState::Idle, RelayState::RelayingParaHeader);
if let Some(para_header_at_relay_header_at_target) =
data.para_header_at_relay_header_at_target.as_ref()
{
return RelayState::RelayingParaHeader(para_header_at_relay_header_at_target.clone())
}
// else use the regular process - e.g. we may require to deliver new relay header first
}
// Process the `RelayingParaHeader` state.
@@ -585,7 +606,7 @@ where
// its ancestor
// we need relay chain header first
if data.relay_header_at_target < data.relay_header_at_source {
if relay_header_at_target < data.relay_header_at_source {
return RelayState::RelayingRelayHeader(data.relay_header_at_source)
}
@@ -640,7 +661,8 @@ impl<'a, P: SubstrateParachainsPipeline>
None,
)
.await?
.best_finalized_peer_at_best_self)
.best_finalized_peer_at_best_self
.ok_or(SubstrateError::BridgePalletIsNotInitialized)?)
}
async fn best_finalized_para_block_at_source(
@@ -726,7 +748,7 @@ mod tests {
para_header_at_target: Some(50),
para_header_at_source: Some(HeaderId(110, 110)),
relay_header_at_source: 800,
relay_header_at_target: 700,
relay_header_at_target: Some(700),
para_header_at_relay_header_at_target: Some(HeaderId(100, 100)),
},
RelayState::RelayingRelayHeader(750),
@@ -744,7 +766,7 @@ mod tests {
para_header_at_target: Some(50),
para_header_at_source: Some(HeaderId(110, 110)),
relay_header_at_source: 800,
relay_header_at_target: 750,
relay_header_at_target: Some(750),
para_header_at_relay_header_at_target: Some(HeaderId(100, 100)),
},
RelayState::RelayingRelayHeader(750),
@@ -762,7 +784,7 @@ mod tests {
para_header_at_target: Some(50),
para_header_at_source: Some(HeaderId(110, 110)),
relay_header_at_source: 800,
relay_header_at_target: 780,
relay_header_at_target: Some(780),
para_header_at_relay_header_at_target: Some(HeaderId(105, 105)),
},
RelayState::RelayingRelayHeader(750),
@@ -779,7 +801,7 @@ mod tests {
para_header_at_target: Some(50),
para_header_at_source: Some(HeaderId(110, 110)),
relay_header_at_source: 800,
relay_header_at_target: 780,
relay_header_at_target: Some(780),
para_header_at_relay_header_at_target: Some(HeaderId(105, 105)),
},
RelayState::RelayingParaHeader(HeaderId(105, 105)),
@@ -797,7 +819,7 @@ mod tests {
para_header_at_target: Some(105),
para_header_at_source: Some(HeaderId(110, 110)),
relay_header_at_source: 800,
relay_header_at_target: 780,
relay_header_at_target: Some(780),
para_header_at_relay_header_at_target: Some(HeaderId(105, 105)),
},
RelayState::Idle,
@@ -815,7 +837,7 @@ mod tests {
para_header_at_target: Some(105),
para_header_at_source: None,
relay_header_at_source: 800,
relay_header_at_target: 780,
relay_header_at_target: Some(780),
para_header_at_relay_header_at_target: Some(HeaderId(105, 105)),
},
RelayState::Idle,
@@ -833,7 +855,7 @@ mod tests {
para_header_at_target: Some(105),
para_header_at_source: Some(HeaderId(110, 110)),
relay_header_at_source: 800,
relay_header_at_target: 780,
relay_header_at_target: Some(780),
para_header_at_relay_header_at_target: Some(HeaderId(105, 105)),
},
RelayState::Idle,
@@ -851,7 +873,7 @@ mod tests {
para_header_at_target: Some(105),
para_header_at_source: Some(HeaderId(125, 125)),
relay_header_at_source: 800,
relay_header_at_target: 780,
relay_header_at_target: Some(780),
para_header_at_relay_header_at_target: Some(HeaderId(105, 105)),
},
RelayState::Idle,
@@ -869,7 +891,7 @@ mod tests {
para_header_at_target: Some(105),
para_header_at_source: Some(HeaderId(125, 125)),
relay_header_at_source: 800,
relay_header_at_target: 800,
relay_header_at_target: Some(800),
para_header_at_relay_header_at_target: Some(HeaderId(125, 125)),
},
RelayState::Idle,
@@ -887,7 +909,7 @@ mod tests {
para_header_at_target: Some(105),
para_header_at_source: None,
relay_header_at_source: 800,
relay_header_at_target: 800,
relay_header_at_target: Some(800),
para_header_at_relay_header_at_target: None,
},
RelayState::RelayingRelayHeader(800),
@@ -905,7 +927,7 @@ mod tests {
para_header_at_target: None,
para_header_at_source: Some(HeaderId(125, 125)),
relay_header_at_source: 800,
relay_header_at_target: 800,
relay_header_at_target: Some(800),
para_header_at_relay_header_at_target: Some(HeaderId(125, 125)),
},
RelayState::Idle,
@@ -923,7 +945,7 @@ mod tests {
para_header_at_target: None,
para_header_at_source: Some(HeaderId(125, 125)),
relay_header_at_source: 800,
relay_header_at_target: 700,
relay_header_at_target: Some(700),
para_header_at_relay_header_at_target: Some(HeaderId(125, 125)),
},
RelayState::Idle,
@@ -249,10 +249,13 @@ pub struct ClientState<SelfHeaderId, PeerHeaderId> {
pub best_finalized_self: SelfHeaderId,
/// Best finalized header id of the peer chain read at the best block of this chain (at
/// `best_finalized_self`).
pub best_finalized_peer_at_best_self: PeerHeaderId,
///
/// It may be `None` e,g. if peer is a parachain and we haven't yet relayed any parachain
/// heads.
pub best_finalized_peer_at_best_self: Option<PeerHeaderId>,
/// Header id of the peer chain with the number, matching the
/// `best_finalized_peer_at_best_self`.
pub actual_best_finalized_peer_at_best_self: PeerHeaderId,
pub actual_best_finalized_peer_at_best_self: Option<PeerHeaderId>,
}
/// State of source client in one-way message lane.
@@ -973,15 +976,15 @@ pub(crate) mod tests {
source_state: ClientState {
best_self: HeaderId(0, 0),
best_finalized_self: HeaderId(0, 0),
best_finalized_peer_at_best_self: HeaderId(0, 0),
actual_best_finalized_peer_at_best_self: HeaderId(0, 0),
best_finalized_peer_at_best_self: Some(HeaderId(0, 0)),
actual_best_finalized_peer_at_best_self: Some(HeaderId(0, 0)),
},
source_latest_generated_nonce: 1,
target_state: ClientState {
best_self: HeaderId(0, 0),
best_finalized_self: HeaderId(0, 0),
best_finalized_peer_at_best_self: HeaderId(0, 0),
actual_best_finalized_peer_at_best_self: HeaderId(0, 0),
best_finalized_peer_at_best_self: Some(HeaderId(0, 0)),
actual_best_finalized_peer_at_best_self: Some(HeaderId(0, 0)),
},
target_latest_received_nonce: 0,
..Default::default()
@@ -997,11 +1000,11 @@ pub(crate) mod tests {
if data.is_target_reconnected {
data.is_target_fails = false;
}
if data.target_state.best_finalized_peer_at_best_self.0 < 10 {
data.target_state.best_finalized_peer_at_best_self = HeaderId(
data.target_state.best_finalized_peer_at_best_self.0 + 1,
data.target_state.best_finalized_peer_at_best_self.0 + 1,
);
if data.target_state.best_finalized_peer_at_best_self.unwrap().0 < 10 {
data.target_state.best_finalized_peer_at_best_self = Some(HeaderId(
data.target_state.best_finalized_peer_at_best_self.unwrap().0 + 1,
data.target_state.best_finalized_peer_at_best_self.unwrap().0 + 1,
));
}
if !data.submitted_messages_proofs.is_empty() {
exit_sender.unbounded_send(()).unwrap();
@@ -1025,16 +1028,16 @@ pub(crate) mod tests {
source_state: ClientState {
best_self: HeaderId(0, 0),
best_finalized_self: HeaderId(0, 0),
best_finalized_peer_at_best_self: HeaderId(0, 0),
actual_best_finalized_peer_at_best_self: HeaderId(0, 0),
best_finalized_peer_at_best_self: Some(HeaderId(0, 0)),
actual_best_finalized_peer_at_best_self: Some(HeaderId(0, 0)),
},
source_latest_generated_nonce: 1,
source_tracked_transaction_status: TrackedTransactionStatus::Lost,
target_state: ClientState {
best_self: HeaderId(0, 0),
best_finalized_self: HeaderId(0, 0),
best_finalized_peer_at_best_self: HeaderId(0, 0),
actual_best_finalized_peer_at_best_self: HeaderId(0, 0),
best_finalized_peer_at_best_self: Some(HeaderId(0, 0)),
actual_best_finalized_peer_at_best_self: Some(HeaderId(0, 0)),
},
target_latest_received_nonce: 0,
target_tracked_transaction_status: TrackedTransactionStatus::Lost,
@@ -1076,15 +1079,15 @@ pub(crate) mod tests {
source_state: ClientState {
best_self: HeaderId(0, 0),
best_finalized_self: HeaderId(0, 0),
best_finalized_peer_at_best_self: HeaderId(0, 0),
actual_best_finalized_peer_at_best_self: HeaderId(0, 0),
best_finalized_peer_at_best_self: Some(HeaderId(0, 0)),
actual_best_finalized_peer_at_best_self: Some(HeaderId(0, 0)),
},
source_latest_generated_nonce: 1,
target_state: ClientState {
best_self: HeaderId(0, 0),
best_finalized_self: HeaderId(0, 0),
best_finalized_peer_at_best_self: HeaderId(0, 0),
actual_best_finalized_peer_at_best_self: HeaderId(0, 0),
best_finalized_peer_at_best_self: Some(HeaderId(0, 0)),
actual_best_finalized_peer_at_best_self: Some(HeaderId(0, 0)),
},
target_latest_received_nonce: 0,
..Default::default()
@@ -1096,8 +1099,11 @@ pub(crate) mod tests {
data.source_state.best_finalized_self = data.source_state.best_self;
// syncing target headers -> source chain
if let Some(last_requirement) = data.target_to_source_header_requirements.last() {
if *last_requirement != data.source_state.best_finalized_peer_at_best_self {
data.source_state.best_finalized_peer_at_best_self = *last_requirement;
if *last_requirement !=
data.source_state.best_finalized_peer_at_best_self.unwrap()
{
data.source_state.best_finalized_peer_at_best_self =
Some(*last_requirement);
}
}
}),
@@ -1116,8 +1122,11 @@ pub(crate) mod tests {
data.target_state.best_finalized_self = data.target_state.best_self;
// syncing source headers -> target chain
if let Some(last_requirement) = data.source_to_target_header_requirements.last() {
if *last_requirement != data.target_state.best_finalized_peer_at_best_self {
data.target_state.best_finalized_peer_at_best_self = *last_requirement;
if *last_requirement !=
data.target_state.best_finalized_peer_at_best_self.unwrap()
{
data.target_state.best_finalized_peer_at_best_self =
Some(*last_requirement);
}
}
// if source has received all messages receiving confirmations => stop
@@ -1150,15 +1159,15 @@ pub(crate) mod tests {
source_state: ClientState {
best_self: HeaderId(10, 10),
best_finalized_self: HeaderId(10, 10),
best_finalized_peer_at_best_self: HeaderId(0, 0),
actual_best_finalized_peer_at_best_self: HeaderId(0, 0),
best_finalized_peer_at_best_self: Some(HeaderId(0, 0)),
actual_best_finalized_peer_at_best_self: Some(HeaderId(0, 0)),
},
source_latest_generated_nonce: 10,
target_state: ClientState {
best_self: HeaderId(0, 0),
best_finalized_self: HeaderId(0, 0),
best_finalized_peer_at_best_self: HeaderId(0, 0),
actual_best_finalized_peer_at_best_self: HeaderId(0, 0),
best_finalized_peer_at_best_self: Some(HeaderId(0, 0)),
actual_best_finalized_peer_at_best_self: Some(HeaderId(0, 0)),
},
target_latest_received_nonce: 0,
..Default::default()
@@ -1171,15 +1180,18 @@ pub(crate) mod tests {
// headers relay must only be started when we need new target headers at source node
if data.target_to_source_header_required.is_some() {
assert!(
data.source_state.best_finalized_peer_at_best_self.0 <
data.source_state.best_finalized_peer_at_best_self.unwrap().0 <
data.target_state.best_self.0
);
data.target_to_source_header_required = None;
}
// syncing target headers -> source chain
if let Some(last_requirement) = data.target_to_source_header_requirements.last() {
if *last_requirement != data.source_state.best_finalized_peer_at_best_self {
data.source_state.best_finalized_peer_at_best_self = *last_requirement;
if *last_requirement !=
data.source_state.best_finalized_peer_at_best_self.unwrap()
{
data.source_state.best_finalized_peer_at_best_self =
Some(*last_requirement);
}
}
}),
@@ -1192,15 +1204,18 @@ pub(crate) mod tests {
// headers relay must only be started when we need new source headers at target node
if data.source_to_target_header_required.is_some() {
assert!(
data.target_state.best_finalized_peer_at_best_self.0 <
data.target_state.best_finalized_peer_at_best_self.unwrap().0 <
data.source_state.best_self.0
);
data.source_to_target_header_required = None;
}
// syncing source headers -> target chain
if let Some(last_requirement) = data.source_to_target_header_requirements.last() {
if *last_requirement != data.target_state.best_finalized_peer_at_best_self {
data.target_state.best_finalized_peer_at_best_self = *last_requirement;
if *last_requirement !=
data.target_state.best_finalized_peer_at_best_self.unwrap()
{
data.target_state.best_finalized_peer_at_best_self =
Some(*last_requirement);
}
}
// if source has received all messages receiving confirmations => stop
@@ -1233,15 +1248,15 @@ pub(crate) mod tests {
source_state: ClientState {
best_self: HeaderId(10, 10),
best_finalized_self: HeaderId(10, 10),
best_finalized_peer_at_best_self: HeaderId(0, 0),
actual_best_finalized_peer_at_best_self: HeaderId(0, 0),
best_finalized_peer_at_best_self: Some(HeaderId(0, 0)),
actual_best_finalized_peer_at_best_self: Some(HeaderId(0, 0)),
},
source_latest_generated_nonce: 10,
target_state: ClientState {
best_self: HeaderId(0, 0),
best_finalized_self: HeaderId(0, 0),
best_finalized_peer_at_best_self: HeaderId(0, 0),
actual_best_finalized_peer_at_best_self: HeaderId(0, 0),
best_finalized_peer_at_best_self: Some(HeaderId(0, 0)),
actual_best_finalized_peer_at_best_self: Some(HeaderId(0, 0)),
},
target_latest_received_nonce: 0,
..Default::default()
@@ -308,7 +308,7 @@ pub async fn run<P: MessageRace, SC: SourceClient<P>, TC: TargetClient<P>>(
target_best_nonces_required = true;
race_state.best_target_header_id = Some(target_state.best_self);
race_state.best_finalized_source_header_id_at_best_target
= Some(target_state.best_finalized_peer_at_best_self);
= target_state.best_finalized_peer_at_best_self;
}
let is_target_finalized_state_updated = race_state.best_finalized_target_header_id.as_ref()
+26 -12
View File
@@ -66,24 +66,38 @@ impl MessageLaneLoopMetrics {
pub fn update_source_state<P: MessageLane>(&self, source_client_state: SourceClientState<P>) {
self.source_to_target_finality_metrics
.update_best_block_at_source(source_client_state.best_self.0);
self.target_to_source_finality_metrics
.update_best_block_at_target(source_client_state.best_finalized_peer_at_best_self.0);
self.target_to_source_finality_metrics.update_using_same_fork(
source_client_state.best_finalized_peer_at_best_self.1 ==
source_client_state.actual_best_finalized_peer_at_best_self.1,
);
if let Some(best_finalized_peer_at_best_self) =
source_client_state.best_finalized_peer_at_best_self
{
self.target_to_source_finality_metrics
.update_best_block_at_target(best_finalized_peer_at_best_self.0);
if let Some(actual_best_finalized_peer_at_best_self) =
source_client_state.actual_best_finalized_peer_at_best_self
{
self.target_to_source_finality_metrics.update_using_same_fork(
best_finalized_peer_at_best_self.1 == actual_best_finalized_peer_at_best_self.1,
);
}
}
}
/// Update target client state metrics.
pub fn update_target_state<P: MessageLane>(&self, target_client_state: TargetClientState<P>) {
self.target_to_source_finality_metrics
.update_best_block_at_source(target_client_state.best_self.0);
self.source_to_target_finality_metrics
.update_best_block_at_target(target_client_state.best_finalized_peer_at_best_self.0);
self.source_to_target_finality_metrics.update_using_same_fork(
target_client_state.best_finalized_peer_at_best_self.1 ==
target_client_state.actual_best_finalized_peer_at_best_self.1,
);
if let Some(best_finalized_peer_at_best_self) =
target_client_state.best_finalized_peer_at_best_self
{
self.source_to_target_finality_metrics
.update_best_block_at_target(best_finalized_peer_at_best_self.0);
if let Some(actual_best_finalized_peer_at_best_self) =
target_client_state.actual_best_finalized_peer_at_best_self
{
self.source_to_target_finality_metrics.update_using_same_fork(
best_finalized_peer_at_best_self.1 == actual_best_finalized_peer_at_best_self.1,
);
}
}
}
/// Update latest generated nonce at source.