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

CFE RPC Client optional genesis hash check for tests #4041

Merged
merged 2 commits into from
Sep 25, 2023
Merged

CFE RPC Client optional genesis hash check for tests #4041

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
956611f
refactor: optional genesis hash for tests
j4m1ef0rd Sep 22, 2023
5e8d453
chore: renamed new_ext -> new_inner
j4m1ef0rd Sep 25, 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
27 changes: 15 additions & 12 deletions engine/src/dot/http_rpc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use subxt::{
};

use anyhow::Result;
use tracing::{error, warn};
use utilities::{make_periodic_tick, redact_endpoint_secret::SecretUrl};

use crate::constants::RPC_RETRY_CONNECTION_INTERVAL;
Expand Down Expand Up @@ -82,7 +83,7 @@ pub struct DotHttpRpcClient {
impl DotHttpRpcClient {
pub fn new(
url: SecretUrl,
expected_genesis_hash: PolkadotHash,
expected_genesis_hash: Option<PolkadotHash>,
) -> Result<impl Future<Output = Self>> {
let polkadot_http_client = Arc::new(PolkadotHttpClient::new(&url)?);

Expand All @@ -98,17 +99,22 @@ impl DotHttpRpcClient {
.await
{
Ok(online_client) => {
let genesis_hash = online_client.genesis_hash();
if genesis_hash == expected_genesis_hash {
break online_client
if let Some(expected_genesis_hash) = expected_genesis_hash {
let genesis_hash = online_client.genesis_hash();
if genesis_hash == expected_genesis_hash {
break online_client
} else {
error!(
"Connected to Polkadot node at {url} but the genesis hash {genesis_hash} does not match the expected genesis hash {expected_genesis_hash}. Please check your CFE configuration file."
)
}
} else {
tracing::error!(
"Connected to Polkadot node at {url} but the genesis hash {genesis_hash} does not match the expected genesis hash {expected_genesis_hash}. Please check your CFE configuration file."
)
warn!("Skipping Polkadot genesis hash check");
break online_client
}
},
Err(e) => {
tracing::error!(
error!(
"Failed to connect to Polkadot node at {url} with error: {e}. Please check your CFE
configuration file. Retrying in {:?}...",
poll_interval.period()
Expand Down Expand Up @@ -213,11 +219,8 @@ mod tests {
#[ignore = "requires local node"]
#[tokio::test]
async fn test_http_rpc() {
// This will no longer work because we need to know the genesis hash
let dot_http_rpc =
DotHttpRpcClient::new("http://localhost:9945".into(), PolkadotHash::default())
.unwrap()
.await;
DotHttpRpcClient::new("http://localhost:9945".into(), None).unwrap().await;
let block_hash = dot_http_rpc.block_hash(1).await.unwrap();
println!("block_hash: {:?}", block_hash);
}
Expand Down
16 changes: 12 additions & 4 deletions engine/src/dot/retry_rpc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,15 @@ impl DotRetryRpcClient {
scope: &Scope<'_, anyhow::Error>,
nodes: NodeContainer<WsHttpEndpoints>,
expected_genesis_hash: PolkadotHash,
) -> Result<Self> {
Self::new_inner(scope, nodes, Some(expected_genesis_hash))
}

fn new_inner(
scope: &Scope<'_, anyhow::Error>,
nodes: NodeContainer<WsHttpEndpoints>,
// The genesis hash is optional to facilitate testing
expected_genesis_hash: Option<PolkadotHash>,
) -> Result<Self> {
let f_create_clients = |endpoints: WsHttpEndpoints| {
Result::<_, anyhow::Error>::Ok((
Expand All @@ -58,7 +67,7 @@ impl DotRetryRpcClient {
let (backup_rpc_client, backup_sub_client) =
option_inner(nodes.backup.map(f_create_clients).transpose()?);

Ok(Self {
Ok(DotRetryRpcClient {
rpc_retry_client: RetrierClient::new(
scope,
"dot_rpc",
Expand Down Expand Up @@ -307,8 +316,7 @@ mod tests {
async fn my_test() {
task_scope(|scope| {
async move {
// This will no longer work because we need to know the genesis hash
let dot_retry_rpc_client = DotRetryRpcClient::new(
let dot_retry_rpc_client = DotRetryRpcClient::new_inner(
scope,
NodeContainer {
primary: WsHttpEndpoints {
Expand All @@ -317,7 +325,7 @@ mod tests {
},
backup: None,
},
PolkadotHash::default(),
None,
)
.unwrap();

Expand Down
39 changes: 25 additions & 14 deletions engine/src/dot/rpc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use subxt::{
Config, OnlineClient, PolkadotConfig,
};
use tokio::sync::RwLock;
use tracing::warn;
use utilities::redact_endpoint_secret::SecretUrl;

use anyhow::{anyhow, bail, Result};
Expand Down Expand Up @@ -136,11 +137,11 @@ impl DotRpcApi for DotRpcClient {
#[derive(Clone)]
pub struct DotSubClient {
pub ws_endpoint: SecretUrl,
expected_genesis_hash: PolkadotHash,
expected_genesis_hash: Option<PolkadotHash>,
}

impl DotSubClient {
pub fn new(ws_endpoint: SecretUrl, expected_genesis_hash: PolkadotHash) -> Self {
pub fn new(ws_endpoint: SecretUrl, expected_genesis_hash: Option<PolkadotHash>) -> Self {
Self { ws_endpoint, expected_genesis_hash }
}
}
Expand All @@ -150,12 +151,7 @@ impl DotSubscribeApi for DotSubClient {
async fn subscribe_best_heads(
&self,
) -> Result<Pin<Box<dyn Stream<Item = Result<PolkadotHeader>> + Send>>> {
let client = OnlineClient::<PolkadotConfig>::from_url(&self.ws_endpoint).await?;

let genesis_hash = client.genesis_hash();
if genesis_hash != self.expected_genesis_hash {
bail!("Expected genesis hash {} but got {genesis_hash}", self.expected_genesis_hash);
}
let client = create_online_client(&self.ws_endpoint, self.expected_genesis_hash).await?;

Ok(Box::pin(
client
Expand All @@ -170,12 +166,7 @@ impl DotSubscribeApi for DotSubClient {
async fn subscribe_finalized_heads(
&self,
) -> Result<Pin<Box<dyn Stream<Item = Result<PolkadotHeader>> + Send>>> {
let client = OnlineClient::<PolkadotConfig>::from_url(&self.ws_endpoint).await?;

let genesis_hash = client.genesis_hash();
if genesis_hash != self.expected_genesis_hash {
bail!("Expected genesis hash {} but got {genesis_hash}", self.expected_genesis_hash);
}
let client = create_online_client(&self.ws_endpoint, self.expected_genesis_hash).await?;

Ok(Box::pin(
client
Expand All @@ -188,6 +179,26 @@ impl DotSubscribeApi for DotSubClient {
}
}

/// Creates an OnlineClient from the given websocket endpoint and checks the genesis hash if
/// provided.
async fn create_online_client(
ws_endpoint: &SecretUrl,
expected_genesis_hash: Option<PolkadotHash>,
) -> Result<OnlineClient<PolkadotConfig>> {
let client = OnlineClient::<PolkadotConfig>::from_url(ws_endpoint).await?;

if let Some(expected_genesis_hash) = expected_genesis_hash {
let genesis_hash = client.genesis_hash();
if genesis_hash != expected_genesis_hash {
bail!("Expected Polkadot genesis hash {expected_genesis_hash} but got {genesis_hash}");
}
} else {
warn!("Skipping Polkadot genesis hash check");
}

Ok(client)
}

#[async_trait]
impl DotSubscribeApi for DotRpcClient {
async fn subscribe_best_heads(
Expand Down