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

Some relayer improvments #2902

Merged
merged 6 commits into from
Apr 1, 2024
Merged

Some relayer improvments #2902

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a70bdd8
added CLI arguments: full WS URI + separate for WS path URI component…
svyatonik Mar 28, 2024
8c00c87
Merge branch 'polkadot-staging' into sv-some-relayer-improvements
svyatonik Mar 28, 2024
06a78bb
URI -> URL?
svyatonik Mar 28, 2024
5b03942
Merge branch 'sv-some-relayer-improvements' of https://github.com/par…
svyatonik Mar 28, 2024
4903da3
added TODO
svyatonik Apr 1, 2024
62a5bd3
fmt
svyatonik Apr 1, 2024
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
22 changes: 16 additions & 6 deletions relays/client-substrate/src/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,12 +264,22 @@ impl<C: Chain> Client<C> {
params: &ConnectionParams,
) -> Result<(Arc<tokio::runtime::Runtime>, Arc<RpcClient>)> {
let tokio = tokio::runtime::Runtime::new()?;
let uri = format!(
"{}://{}:{}",
if params.secure { "wss" } else { "ws" },
params.host,
params.port,
);

let uri = match params.uri {
Some(ref uri) => uri.clone(),
None => {
format!(
"{}://{}:{}{}",
if params.secure { "wss" } else { "ws" },
params.host,
params.port,
match params.path {
Some(ref path) => format!("/{}", path),
None => String::new(),
},
)
},
};
log::info!(target: "bridge", "Connecting to {} node at {}", C::NAME, uri);

let client = tokio
Expand Down
7 changes: 7 additions & 0 deletions relays/client-substrate/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,15 @@ pub use bp_runtime::{
/// Substrate-over-websocket connection params.
#[derive(Debug, Clone)]
pub struct ConnectionParams {
/// Websocket endpoint URL. Overrides all other URL components (`host`, `port`, `path` and
/// `secure`).
pub uri: Option<String>,
/// Websocket server host name.
pub host: String,
/// Websocket server TCP port.
pub port: u16,
/// Websocket endpoint path at server.
pub path: Option<String>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we mark host, port, path and secure as deprecated in order to not forget about them ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMO there's no much point in deprecating this in code - it'll only propagate to multiple #[allow(deprecated)] attributes across the code and end-users (those who run relays) won't see it anyway. I've just tried to find something deprecation-related in the clap/structopt to deprecate arguments instead, but there's nothing for that. And I don't want to remove arguments right now, because we have several relayers running && it'll take some time to propagate this change there. I could add a suffix to associated CLI arguments: "Deprecated: use uri instead" as a compromise. WDYT?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, makes sense.

I could add a suffix to associated CLI arguments: "Deprecated: use uri instead" as a compromise. WDYT?

I don't know if it's necessary. We can leave it as it is. At most we can add a TODO comment, just to make sure we don't forget about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've filed #2909 to track it

/// Use secure websocket connection.
pub secure: bool,
/// Defined chain runtime version
Expand All @@ -70,8 +75,10 @@ pub struct ConnectionParams {
impl Default for ConnectionParams {
fn default() -> Self {
ConnectionParams {
uri: None,
host: "localhost".into(),
port: 9944,
path: None,
secure: false,
chain_runtime_version: ChainRuntimeVersion::Auto,
}
Expand Down
15 changes: 13 additions & 2 deletions relays/lib-substrate-relay/src/cli/chain_schema.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,24 @@ macro_rules! declare_chain_runtime_version_params_cli_schema {
macro_rules! declare_chain_connection_params_cli_schema {
($chain:ident, $chain_prefix:ident) => {
bp_runtime::paste::item! {
// TODO: https://github.com/paritytech/parity-bridges-common/issues/2909
// remove all obsolete arguments (separate URI components)

#[doc = $chain " connection params."]
#[derive(StructOpt, Debug, PartialEq, Eq, Clone)]
pub struct [<$chain ConnectionParams>] {
#[doc = "Connect to " $chain " node at given host."]
#[doc = "WS endpoint of " $chain ": full URI. Overrides all other connection string components (host, port, path, secure)."]
#[structopt(long)]
pub [<$chain_prefix _uri>]: Option<String>,
#[doc = "WS endpoint of " $chain ": host component."]
#[structopt(long, default_value = "127.0.0.1")]
pub [<$chain_prefix _host>]: String,
#[doc = "Connect to " $chain " node websocket server at given port."]
#[doc = "WS endpoint of " $chain ": port component."]
#[structopt(long, default_value = "9944")]
pub [<$chain_prefix _port>]: u16,
#[doc = "WS endpoint of " $chain ": path component."]
#[structopt(long)]
pub [<$chain_prefix _path>]: Option<String>,
#[doc = "Use secure websocket connection."]
#[structopt(long)]
pub [<$chain_prefix _secure>]: bool,
Expand All @@ -119,8 +128,10 @@ macro_rules! declare_chain_connection_params_cli_schema {
.[<$chain_prefix _runtime_version>]
.into_runtime_version(Chain::RUNTIME_VERSION)?;
Ok(relay_substrate_client::Client::new(relay_substrate_client::ConnectionParams {
uri: self.[<$chain_prefix _uri>],
host: self.[<$chain_prefix _host>],
port: self.[<$chain_prefix _port>],
path: self.[<$chain_prefix _path>],
secure: self.[<$chain_prefix _secure>],
chain_runtime_version,
})
Expand Down
8 changes: 8 additions & 0 deletions relays/lib-substrate-relay/src/cli/relay_headers_and_messages/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,8 +425,10 @@ mod tests {
},
},
left: BridgeHubKusamaConnectionParams {
bridge_hub_kusama_uri: None,
bridge_hub_kusama_host: "bridge-hub-kusama-node-collator1".into(),
bridge_hub_kusama_port: 9944,
bridge_hub_kusama_path: None,
bridge_hub_kusama_secure: false,
bridge_hub_kusama_runtime_version: BridgeHubKusamaRuntimeVersionParams {
bridge_hub_kusama_version_mode: RuntimeVersionType::Bundle,
Expand All @@ -442,8 +444,10 @@ mod tests {
bridge_hub_kusama_transactions_mortality: Some(64),
},
left_relay: KusamaConnectionParams {
kusama_uri: None,
kusama_host: "kusama-alice".into(),
kusama_port: 9944,
kusama_path: None,
kusama_secure: false,
kusama_runtime_version: KusamaRuntimeVersionParams {
kusama_version_mode: RuntimeVersionType::Bundle,
Expand All @@ -452,8 +456,10 @@ mod tests {
},
},
right: BridgeHubPolkadotConnectionParams {
bridge_hub_polkadot_uri: None,
bridge_hub_polkadot_host: "bridge-hub-polkadot-collator1".into(),
bridge_hub_polkadot_port: 9944,
bridge_hub_polkadot_path: None,
bridge_hub_polkadot_secure: false,
bridge_hub_polkadot_runtime_version: BridgeHubPolkadotRuntimeVersionParams {
bridge_hub_polkadot_version_mode: RuntimeVersionType::Bundle,
Expand All @@ -469,8 +475,10 @@ mod tests {
bridge_hub_polkadot_transactions_mortality: Some(64),
},
right_relay: PolkadotConnectionParams {
polkadot_uri: None,
polkadot_host: "polkadot-alice".into(),
polkadot_port: 9944,
polkadot_path: None,
polkadot_secure: false,
polkadot_runtime_version: PolkadotRuntimeVersionParams {
polkadot_version_mode: RuntimeVersionType::Bundle,
Expand Down
7 changes: 7 additions & 0 deletions relays/lib-substrate-relay/src/parachains/source.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,13 @@ where
// parachain head - we simply return `Unavailable`
let best_block_number = self.client.best_finalized_header_number().await?;
if is_ancient_block(at_block.number(), best_block_number) {
log::trace!(
target: "bridge",
"{} block {:?} is ancient. Cannot prove the {} header there",
P::SourceRelayChain::NAME,
at_block,
P::SourceParachain::NAME,
);
return Ok(AvailableHeader::Unavailable)
}

Expand Down
8 changes: 8 additions & 0 deletions substrate-relay/src/cli/relay_headers_and_messages.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,8 +316,10 @@ mod tests {
},
},
left_relay: KusamaConnectionParams {
kusama_uri: None,
kusama_host: "kusama-alice".into(),
kusama_port: 9944,
kusama_path: None,
kusama_secure: false,
kusama_runtime_version: KusamaRuntimeVersionParams {
kusama_version_mode: RuntimeVersionType::Bundle,
Expand All @@ -326,8 +328,10 @@ mod tests {
},
},
left: BridgeHubKusamaConnectionParams {
bridge_hub_kusama_uri: None,
bridge_hub_kusama_host: "bridge-hub-kusama-node-collator1".into(),
bridge_hub_kusama_port: 9944,
bridge_hub_kusama_path: None,
bridge_hub_kusama_secure: false,
bridge_hub_kusama_runtime_version: BridgeHubKusamaRuntimeVersionParams {
bridge_hub_kusama_version_mode: RuntimeVersionType::Bundle,
Expand All @@ -343,8 +347,10 @@ mod tests {
bridge_hub_kusama_transactions_mortality: Some(64),
},
right: BridgeHubPolkadotConnectionParams {
bridge_hub_polkadot_uri: None,
bridge_hub_polkadot_host: "bridge-hub-polkadot-collator1".into(),
bridge_hub_polkadot_port: 9944,
bridge_hub_polkadot_path: None,
bridge_hub_polkadot_secure: false,
bridge_hub_polkadot_runtime_version:
BridgeHubPolkadotRuntimeVersionParams {
Expand All @@ -361,8 +367,10 @@ mod tests {
bridge_hub_polkadot_transactions_mortality: Some(64),
},
right_relay: PolkadotConnectionParams {
polkadot_uri: None,
polkadot_host: "polkadot-alice".into(),
polkadot_port: 9944,
polkadot_path: None,
polkadot_secure: false,
polkadot_runtime_version: PolkadotRuntimeVersionParams {
polkadot_version_mode: RuntimeVersionType::Bundle,
Expand Down