Skip to content

Commit

Permalink
Fix on demand parachains relay when no parachain head at target (pari…
Browse files Browse the repository at this point in the history 
…tytech#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
  • Loading branch information
@svyatonik
svyatonik authored Jan 31, 2023
1 parent d1d684d commit 8196b56
Show file tree
Hide file tree
Showing 6 changed files with 150 additions and 98 deletions.
3 changes: 2 additions & 1 deletion bridges/relays/lib-substrate-relay/src/finality/target.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
22 changes: 11 additions & 11 deletions bridges/relays/lib-substrate-relay/src/messages_source.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -456,8 +457,7 @@ where
(),
Some(at_self_hash),
)
.await?
.ok_or(SubstrateError::BridgePalletIsNotInitialized)
.await
}

fn validate_out_msgs_details<C: Chain>(
Expand Down
94 changes: 58 additions & 36 deletions bridges/relays/lib-substrate-relay/src/on_demand/parachains.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)),
};

Expand All @@ -502,51 +504,70 @@ 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,
})
}

/// 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.
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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),
Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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)),
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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),
Expand All @@ -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,
Expand All @@ -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,
Expand Down
Loading

0 comments on commit 8196b56

Please sign in to comment.