From d40b1b87a90712d175c6981b433dda48d98e61f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Duarte?= Date: Mon, 25 Nov 2024 19:37:40 +0000 Subject: [PATCH] fix(storagext): use subxt retry mechanism instead of hand written (#611) --- Cargo.lock | 11 +++++ Justfile | 2 +- cli/polka-storage-provider/client/Cargo.toml | 2 +- cli/polka-storage-provider/common/Cargo.toml | 2 +- cli/polka-storage-provider/server/Cargo.toml | 2 +- cli/polka-storage/storagext-cli/Cargo.toml | 2 - cli/polka-storage/storagext/Cargo.toml | 3 +- .../storagext/src/runtime/client.rs | 47 ++++++++----------- .../dockerfiles/polka-storage-node.Dockerfile | 2 +- docker/dockerfiles/storagext-cli.Dockerfile | 2 +- docs/src/getting-started/building/source.md | 1 - maat/Cargo.toml | 2 +- 12 files changed, 38 insertions(+), 40 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 12b2ab088..2e66792b0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4715,6 +4715,16 @@ dependencies = [ "scale-info", ] +[[package]] +name = "finito" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2384245d85162258a14b43567a9ee3598f5ae746a1581fb5d3d2cb780f0dbf95" +dependencies = [ + "futures-timer", + "pin-project", +] + [[package]] name = "fixed-hash" version = "0.8.0" @@ -21235,6 +21245,7 @@ dependencies = [ "async-trait", "derive-where", "either", + "finito", "frame-metadata 17.0.0", "futures", "hex", diff --git a/Justfile b/Justfile index 6690c12cc..f0d3bac9a 100644 --- a/Justfile +++ b/Justfile @@ -56,7 +56,7 @@ build-polka-storage-provider-server: # Build the storagext CLI binary build-storagext-cli: - cargo build --release --features storagext/insecure_url -p storagext-cli + cargo build --release -p storagext-cli # Build the mater CLI binary build-mater-cli: diff --git a/cli/polka-storage-provider/client/Cargo.toml b/cli/polka-storage-provider/client/Cargo.toml index 4fad6816f..0a5657353 100644 --- a/cli/polka-storage-provider/client/Cargo.toml +++ b/cli/polka-storage-provider/client/Cargo.toml @@ -14,7 +14,7 @@ polka-storage-proofs = { workspace = true, features = ["std", "substrate"] } polka-storage-provider-common = { workspace = true } primitives-commitment = { workspace = true } primitives-proofs = { workspace = true, features = ["clap", "std"] } -storagext = { workspace = true, features = ["clap", "insecure_url"] } +storagext = { workspace = true, features = ["clap"] } async-trait = { workspace = true } bls12_381 = { workspace = true } diff --git a/cli/polka-storage-provider/common/Cargo.toml b/cli/polka-storage-provider/common/Cargo.toml index 58da3dd12..5e840041b 100644 --- a/cli/polka-storage-provider/common/Cargo.toml +++ b/cli/polka-storage-provider/common/Cargo.toml @@ -11,7 +11,7 @@ version = "0.1.0" # "Homegrown" crates primitives-commitment = { workspace = true } primitives-proofs = { workspace = true } -storagext = { workspace = true, features = ["clap", "insecure_url"] } +storagext = { workspace = true, features = ["clap"] } chrono = { workspace = true, features = ["serde"] } cid = { workspace = true, features = ["serde", "std"] } diff --git a/cli/polka-storage-provider/server/Cargo.toml b/cli/polka-storage-provider/server/Cargo.toml index b83fecaa2..c2f8acb16 100644 --- a/cli/polka-storage-provider/server/Cargo.toml +++ b/cli/polka-storage-provider/server/Cargo.toml @@ -18,7 +18,7 @@ polka-storage-proofs = { workspace = true, features = ["std", "substrate"] } polka-storage-provider-common = { workspace = true } primitives-commitment = { workspace = true, features = ["serde", "std"] } primitives-proofs = { workspace = true, features = ["clap"] } -storagext = { workspace = true, features = ["clap", "insecure_url"] } +storagext = { workspace = true, features = ["clap"] } async-trait = { workspace = true } axum = { workspace = true, features = ["macros", "multipart"] } diff --git a/cli/polka-storage/storagext-cli/Cargo.toml b/cli/polka-storage/storagext-cli/Cargo.toml index 15929b2c2..33b67ac78 100644 --- a/cli/polka-storage/storagext-cli/Cargo.toml +++ b/cli/polka-storage/storagext-cli/Cargo.toml @@ -7,8 +7,6 @@ name = "storagext-cli" repository.workspace = true version = "0.1.0" -[features] -insecure_url = ["storagext/insecure_url"] [dependencies] storagext = { workspace = true, features = ["clap"] } diff --git a/cli/polka-storage/storagext/Cargo.toml b/cli/polka-storage/storagext/Cargo.toml index 0b6b1a554..19569cd63 100644 --- a/cli/polka-storage/storagext/Cargo.toml +++ b/cli/polka-storage/storagext/Cargo.toml @@ -10,7 +10,6 @@ version = "0.1.0" [features] clap = ["dep:clap"] default = [] -insecure_url = [] [dependencies] anyhow.workspace = true @@ -25,7 +24,7 @@ primitives-proofs = { workspace = true, features = ["serde"] } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } sha2 = { workspace = true } -subxt = { workspace = true, features = ["jsonrpsee", "substrate-compat"] } +subxt = { workspace = true, features = ["jsonrpsee", "reconnecting-rpc-client", "substrate-compat"] } subxt-signer = { workspace = true, features = ["subxt"] } thiserror = { workspace = true, default-features = true } tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } diff --git a/cli/polka-storage/storagext/src/runtime/client.rs b/cli/polka-storage/storagext/src/runtime/client.rs index b9319666d..a5b266671 100644 --- a/cli/polka-storage/storagext/src/runtime/client.rs +++ b/cli/polka-storage/storagext/src/runtime/client.rs @@ -2,7 +2,13 @@ use std::time::Duration; use codec::Encode; use hex::ToHex; -use subxt::{blocks::Block, events::Events, utils::H256, OnlineClient}; +use subxt::{ + backend::rpc::reconnecting_rpc_client::{FixedInterval, RpcClient}, + blocks::Block, + events::Events, + utils::H256, + OnlineClient, +}; use crate::PolkaStorageConfig; @@ -30,9 +36,6 @@ pub struct Client { impl Client { /// Create a new [`RuntimeClient`] from a target `rpc_address`. - /// - /// By default, this function does not support insecure URLs, - /// to enable support for them, use the `insecure_url` feature. #[tracing::instrument(skip_all, fields(rpc_address = rpc_address.as_ref()))] pub async fn new( rpc_address: impl AsRef, @@ -41,30 +44,18 @@ impl Client { ) -> Result { let rpc_address = rpc_address.as_ref(); - let mut current_retries = 0; - loop { - let client = if cfg!(feature = "insecure_url") { - OnlineClient::<_>::from_insecure_url(rpc_address).await - } else { - OnlineClient::<_>::from_url(rpc_address).await - }; - - match client { - Ok(client) => return Ok(Self { client }), - Err(err) => { - tracing::error!( - attempt = current_retries, - "failed to connect to node, error: {}", - err - ); - current_retries += 1; - if current_retries >= n_retries { - return Err(err); - } - tokio::time::sleep(retry_interval).await; - } - } - } + let rpc_client = RpcClient::builder() + // the cast should never pose an issue since storagext is target at 64bit systems + .retry_policy(FixedInterval::new(retry_interval).take(n_retries as usize)) + .build(rpc_address) + // subxt-style conversion + // https://github.com/paritytech/subxt/blob/v0.38.0/subxt/src/backend/rpc/rpc_client.rs#L38 + .await + .map_err(|e| subxt::error::RpcError::ClientError(Box::new(e)))?; + + Ok(Self { + client: OnlineClient::<_>::from_rpc_client(rpc_client).await?, + }) } pub(crate) async fn unsigned( diff --git a/docker/dockerfiles/polka-storage-node.Dockerfile b/docker/dockerfiles/polka-storage-node.Dockerfile index 1d7f2f834..38dbfefe2 100644 --- a/docker/dockerfiles/polka-storage-node.Dockerfile +++ b/docker/dockerfiles/polka-storage-node.Dockerfile @@ -25,7 +25,7 @@ RUN cargo chef cook --release --recipe-path recipe.json # Build application COPY . . RUN cargo build --release --features polka-storage-runtime/testnet -p polka-storage-node -RUN cargo build --release --features storagext/insecure_url -p storagext-cli +RUN cargo build --release -p storagext-cli FROM debian:bookworm-slim AS runtime diff --git a/docker/dockerfiles/storagext-cli.Dockerfile b/docker/dockerfiles/storagext-cli.Dockerfile index 1f5337cfa..033c26293 100644 --- a/docker/dockerfiles/storagext-cli.Dockerfile +++ b/docker/dockerfiles/storagext-cli.Dockerfile @@ -21,7 +21,7 @@ COPY --from=planner /app/rust-toolchain.toml rust-toolchain.toml RUN cargo chef cook --bin storagext-cli --release --recipe-path recipe.json # Build application COPY . . -RUN cargo build --release --features storagext/insecure_url --bin storagext-cli +RUN cargo build --release --bin storagext-cli ################ ##### Runtime diff --git a/docs/src/getting-started/building/source.md b/docs/src/getting-started/building/source.md index b53eb5642..420244e51 100644 --- a/docs/src/getting-started/building/source.md +++ b/docs/src/getting-started/building/source.md @@ -77,7 +77,6 @@ After all this setup, it is time to start building the binaries, which you can d When building `polka-storage-node` you should add `--features polka-storage-runtime/testnet` which enables the testnet configuration; all the code in the repo is currently targeting this feature, not including it may lead to unexpected behavior. -When building `storagext-cli` you may want to add `--features storagext/insecure_url` which enables using non-TLS HTTP and WebSockets. ```bash diff --git a/maat/Cargo.toml b/maat/Cargo.toml index 9a5764008..7f62c68b6 100644 --- a/maat/Cargo.toml +++ b/maat/Cargo.toml @@ -32,7 +32,7 @@ tracing = { workspace = true } tracing-subscriber = { workspace = true, features = ["env-filter"] } primitives-proofs = { workspace = true, features = ["serde"] } -storagext = { workspace = true, features = ["insecure_url"] } +storagext = { workspace = true } zombienet-configuration.workspace = true zombienet-sdk.workspace = true