Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 4 commits into from
Jan 31, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
afbf6f0
`best_finalized_peer_at_best_self` in messages relay is now Option<> …
svyatonik Jan 31, 2023
54cb88e
unnecessary mut
svyatonik Jan 31, 2023
a9d7062
clone on return
svyatonik Jan 31, 2023
7018a87
fmt
svyatonik Jan 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion relays/lib-substrate-relay/src/finality/target.rs
View file
Open in desktop
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 relays/lib-substrate-relay/src/messages_source.rs
View file
Open in desktop
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 relays/lib-substrate-relay/src/on_demand/parachains.rs
View file
Open in desktop
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) =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very nit, what about do clone on return?
I dont know, if this is even worth optimization :)

        if let Some(ref para_header_at_relay_header_at_target) = data.para_header_at_relay_header_at_target
		{
			return RelayState::RelayingParaHeader(para_header_at_relay_header_at_target.clone())
		}

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