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

Use specific error for case when para head is missing from the bridge pallet #1829

Merged
merged 2 commits into from
Jan 30, 2023
Merged

Use specific error for case when para head is missing from the bridge pallet #1829

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6e42457
use specific error for case when para head is missing from the bridge…
svyatonik Jan 30, 2023
879ee12
fix match to support both error types
svyatonik Jan 30, 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: 3 additions & 0 deletions relays/client-substrate/src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ pub enum Error {
/// The bridge pallet is not yet initialized and all transactions will be rejected.
#[error("Bridge pallet is not initialized.")]
BridgePalletIsNotInitialized,
/// There's no best head of the parachain at the `pallet-bridge-parachains` at the target side.
#[error("No head of the ParaId({0}) at the bridge parachains pallet at {1}.")]
NoParachainHeadAtTarget(u32, String),
/// An error has happened when we have tried to parse storage proof.
#[error("Error when parsing storage proof: {0:?}.")]
StorageProofError(bp_runtime::StorageProofError),
Expand Down
7 changes: 4 additions & 3 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 @@ -473,12 +473,13 @@ where
P::SourceParachain,
>(target.client(), best_target_block_hash)
.await;
// if there are no parachain heads at the target (`BridgePalletIsNotInitialized`), we'll need
// to submit at least one. Otherwise the pallet will be treated as uninitialized and messages
// if there are no parachain heads at the target (`NoParachainHeadAtTarget`), we'll need to
// 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) => None,
Err(SubstrateError::BridgePalletIsNotInitialized) |
Err(SubstrateError::NoParachainHeadAtTarget(_, _)) => None,
Copy link
Contributor

Choose a reason for hiding this comment

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

@svyatonik
para_header_at_target is returned from best_finalized_peer_header_at_self
so what about .ok_or(SubstrateError::BridgePalletIsNotInitialized) here?

/// Reads best `PeerChain` header known to the `SelfChain` using provided runtime API method.
///
/// Method is supposed to be the `<PeerChain>FinalityApi::best_finalized()` method.
pub async fn best_finalized_peer_header_at_self<SelfChain, PeerChain>(
	self_client: &Client<SelfChain>,
	at_self_hash: HashOf<SelfChain>,
) -> Result<HeaderIdOf<PeerChain>, SubstrateError>
where
	SelfChain: Chain,
	PeerChain: Chain,
{
	// now let's read id of best finalized peer header at our best finalized block
	self_client
		.typed_state_call::<_, Option<_>>(
			PeerChain::BEST_FINALIZED_HEADER_ID_METHOD.into(),
			(),
			Some(at_self_hash),
		)
		.await?
		.ok_or(SubstrateError::BridgePalletIsNotInitialized)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This code works with both standalone and parachains, so we can't use parachain-specific error there

Copy link
Contributor Author

Choose a reason for hiding this comment

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

NVM, I see the issue now, thanks

Err(e) => return Err(map_target_err(e)),
};

Expand Down
7 changes: 5 additions & 2 deletions relays/lib-substrate-relay/src/parachains/target.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use parachains_relay::{
};
use relay_substrate_client::{
AccountIdOf, AccountKeyPairOf, BlockNumberOf, Chain, Client, Error as SubstrateError, HashOf,
HeaderIdOf, RelayChain, TransactionEra, TransactionTracker, UnsignedTransaction,
HeaderIdOf, ParachainBase, RelayChain, TransactionEra, TransactionTracker, UnsignedTransaction,
};
use relay_utils::{relay_loop::Client as RelayClient, HeaderId};
use sp_core::{Bytes, Pair};
Expand Down Expand Up @@ -110,7 +110,10 @@ where
)
.map_err(SubstrateError::ResponseParseFailed)?
.map(Ok)
.unwrap_or(Err(SubstrateError::BridgePalletIsNotInitialized))
.unwrap_or(Err(SubstrateError::NoParachainHeadAtTarget(
P::SourceParachain::PARACHAIN_ID,
P::TargetChain::NAME.into(),
)))
}

async fn parachain_head(
Expand Down