Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

introduce a method for fetching relay chain header to RelayChainInterface #2794

Merged
merged 3 commits into from
Jul 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 5 additions & 1 deletion client/consensus/common/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::*;
use async_trait::async_trait;
use codec::Encode;
use cumulus_client_pov_recovery::RecoveryKind;
use cumulus_primitives_core::{InboundDownwardMessage, InboundHrmpMessage};
use cumulus_primitives_core::{relay_chain::BlockId, InboundDownwardMessage, InboundHrmpMessage};
use cumulus_relay_chain_interface::{
CommittedCandidateReceipt, OccupiedCoreAssumption, OverseerHandle, PHeader, ParaId,
RelayChainInterface, RelayChainResult, SessionIndex, StorageValue, ValidatorId,
Expand Down Expand Up @@ -207,6 +207,10 @@ impl RelayChainInterface for Relaychain {
})
.boxed())
}

async fn header(&self, _block_id: BlockId) -> RelayChainResult<Option<PHeader>> {
unimplemented!("Not needed for test")
}
}

fn build_block<B: InitBlockBuilder>(
Expand Down
13 changes: 13 additions & 0 deletions client/network/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

use super::*;
use async_trait::async_trait;
use cumulus_primitives_core::relay_chain::BlockId;
use cumulus_relay_chain_inprocess_interface::{check_block_in_chain, BlockCheckStatus};
use cumulus_relay_chain_interface::{
OverseerHandle, PHeader, ParaId, RelayChainError, RelayChainResult,
Expand Down Expand Up @@ -237,6 +238,18 @@ impl RelayChainInterface for DummyRelayChainInterface {
});
Ok(Box::pin(notifications_stream))
}

async fn header(&self, block_id: BlockId) -> RelayChainResult<Option<PHeader>> {
let hash = match block_id {
BlockId::Hash(hash) => hash,
BlockId::Number(num) => self.relay_client.hash(num)?.ok_or_else(|| {
RelayChainError::GenericError(format!("block with number {num} not found"))
})?,
};
let header = self.relay_client.header(hash)?;

Ok(header)
}
}

fn make_validator_and_api(
Expand Down
19 changes: 17 additions & 2 deletions client/relay-chain-inprocess-interface/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ use std::{pin::Pin, sync::Arc, time::Duration};
use async_trait::async_trait;
use cumulus_primitives_core::{
relay_chain::{
runtime_api::ParachainHost, Block as PBlock, CommittedCandidateReceipt, Hash as PHash,
Header as PHeader, InboundHrmpMessage, OccupiedCoreAssumption, SessionIndex, ValidatorId,
runtime_api::ParachainHost, Block as PBlock, BlockId, CommittedCandidateReceipt,
Hash as PHash, Header as PHeader, InboundHrmpMessage, OccupiedCoreAssumption, SessionIndex,
ValidatorId,
},
InboundDownwardMessage, ParaId, PersistedValidationData,
};
Expand Down Expand Up @@ -79,6 +80,7 @@ impl<T> Clone for RelayChainInProcessInterface<T> {
impl<Client> RelayChainInterface for RelayChainInProcessInterface<Client>
where
Client: ProvideRuntimeApi<PBlock>
+ HeaderBackend<PBlock>
+ BlockchainEvents<PBlock>
+ AuxStore
+ UsageProvider<PBlock>
Expand Down Expand Up @@ -110,6 +112,18 @@ where
)?)
}

async fn header(&self, block_id: BlockId) -> RelayChainResult<Option<PHeader>> {
let hash = match block_id {
BlockId::Hash(hash) => hash,
BlockId::Number(num) => self.full_client.hash(num)?.ok_or_else(|| {
RelayChainError::GenericError(format!("block with number {num} not found"))
})?,
};
let header = self.full_client.header(hash)?;

Ok(header)
}

async fn persisted_validation_data(
&self,
hash: PHash,
Expand Down Expand Up @@ -299,6 +313,7 @@ impl ExecuteWithClient for RelayChainInProcessInterfaceBuilder {
fn execute_with_client<Client, Api, Backend>(self, client: Arc<Client>) -> Self::Output
where
Client: ProvideRuntimeApi<PBlock>
+ HeaderBackend<PBlock>
+ BlockchainEvents<PBlock>
+ AuxStore
+ UsageProvider<PBlock>
Expand Down
8 changes: 8 additions & 0 deletions client/relay-chain-interface/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use jsonrpsee_core::Error as JsonRpcError;
use parity_scale_codec::Error as CodecError;
use sp_api::ApiError;

use cumulus_primitives_core::relay_chain::BlockId;
pub use cumulus_primitives_core::{
relay_chain::{
CommittedCandidateReceipt, Hash as PHash, Header as PHeader, InboundHrmpMessage,
Expand Down Expand Up @@ -110,6 +111,9 @@ pub trait RelayChainInterface: Send + Sync {
/// Get the hash of the current best block.
async fn best_block_hash(&self) -> RelayChainResult<PHash>;

/// Fetch the block header of a given height
async fn header(&self, block_id: BlockId) -> RelayChainResult<Option<PHeader>>;
Copy link
Member

Choose a reason for hiding this comment

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

Please use PHash. No new BlockId in public interfaces anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But i specifically need to query using BlockNumber, any reason for this?

No new BlockId in public interfaces anymore

Copy link
Member

Choose a reason for hiding this comment

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

Block numbers should only be used in places where there is no other way around them. Block numbers are not ambiguous. Some interfaces (like RPC/CLI) still require block numbers, but most "internal code" can be rewritten in a way to use hashes. E.g. get the header of block X and then go back using parent_hash().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But i specifically need to query using BlockNumber, any reason for this?

The use case here is on-chain code which wishes to read the relay chain, they signal a request given a specific relay chain height (parachains don't have access to the relay chain blockhashes). So hence the BlockId here.

Doesn't seem like there's any alternatives here

Copy link
Member

Choose a reason for hiding this comment

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

And how does the parachain validates the relay chain headers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't need the header, it has the state_root from PersistedValidationData

Copy link
Member

Choose a reason for hiding this comment

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

The use case here is on-chain code which wishes to read the relay chain, they signal a request given a specific relay chain height (parachains don't have access to the relay chain blockhashes).

You say it signals which relay chain header it wants to read. So you are just wanting to get the header to get the storage root?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

Copy link
Member

Choose a reason for hiding this comment

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

Okay, when we got: https://github.com/paritytech/cumulus/issues/303

This can be changed.


/// Get the hash of the finalized block.
async fn finalized_block_hash(&self) -> RelayChainResult<PHash>;

Expand Down Expand Up @@ -293,4 +297,8 @@ where
) -> RelayChainResult<Pin<Box<dyn Stream<Item = PHeader> + Send>>> {
(**self).new_best_notification_stream().await
}

async fn header(&self, block_id: BlockId) -> RelayChainResult<Option<PHeader>> {
(**self).header(block_id).await
}
}
18 changes: 17 additions & 1 deletion client/relay-chain-rpc-interface/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ use cumulus_primitives_core::{
},
InboundDownwardMessage, ParaId, PersistedValidationData,
};
use cumulus_relay_chain_interface::{RelayChainError, RelayChainInterface, RelayChainResult};
use cumulus_relay_chain_interface::{
PHeader, RelayChainError, RelayChainInterface, RelayChainResult,
};
use futures::{FutureExt, Stream, StreamExt};
use polkadot_overseer::Handle;

Expand All @@ -33,6 +35,7 @@ use sp_state_machine::StorageValue;
use sp_storage::StorageKey;
use std::pin::Pin;

use cumulus_primitives_core::relay_chain::BlockId;
pub use url::Url;

mod reconnecting_ws_client;
Expand Down Expand Up @@ -75,6 +78,19 @@ impl RelayChainInterface for RelayChainRpcInterface {
.await
}

async fn header(&self, block_id: BlockId) -> RelayChainResult<Option<PHeader>> {
let hash = match block_id {
BlockId::Hash(hash) => hash,
BlockId::Number(num) =>
self.rpc_client.chain_get_block_hash(Some(num)).await?.ok_or_else(|| {
RelayChainError::GenericError(format!("block with number {num} not found"))
})?,
};
let header = self.rpc_client.chain_get_header(Some(hash)).await?;

Ok(header)
}

async fn persisted_validation_data(
&self,
hash: RelayHash,
Expand Down