From f34878149f185efbb14efd8d6ab94bbee3191803 Mon Sep 17 00:00:00 2001 From: Pavlo Khrystenko Date: Wed, 14 Aug 2024 15:53:43 +0200 Subject: [PATCH 1/7] Add unstable-reconnecting-rpc to the integration tests --- .github/workflows/rust.yml | 6 +++++ testing/integration-tests/Cargo.toml | 8 +++++-- testing/integration-tests/src/lib.rs | 5 ++++ .../integration-tests/src/utils/node_proc.rs | 23 +++++++++++++++++++ 4 files changed, 40 insertions(+), 2 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index f049cf6d91..d36fb427a7 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -362,6 +362,12 @@ jobs: command: test args: --release --package integration-tests --features unstable-light-client + - name: Run tests + uses: actions-rs/cargo@v1.0.3 + with: + command: test + args: --release --package integration-tests --no-default-features --features unstable-reconnecting-rpc-client + - if: "failure()" uses: "andymckay/cancel-action@a955d435292c0d409d104b57d8e78435a93a6ef1" # v0.5 diff --git a/testing/integration-tests/Cargo.toml b/testing/integration-tests/Cargo.toml index 451a99a500..0caf54cec7 100644 --- a/testing/integration-tests/Cargo.toml +++ b/testing/integration-tests/Cargo.toml @@ -13,7 +13,7 @@ homepage.workspace = true description = "Subxt integration tests that rely on the Substrate binary" [features] -default = [] +default = [ "subxt/jsonrpsee" ] # Enable to run the tests with Light Client support. unstable-light-client = ["subxt/unstable-light-client"] @@ -25,6 +25,10 @@ unstable-light-client-long-running = ["subxt/unstable-light-client"] # the default one which relies on the "old" RPC methods. unstable-backend-client = [] +# Enable the unstable reconnecting rpc-client +unstable-reconnecting-rpc-client = ["subxt/unstable-reconnecting-rpc-client"] + + [dev-dependencies] assert_matches = { workspace = true } codec = { package = "parity-scale-codec", workspace = true, features = ["derive", "bit-vec"] } @@ -36,7 +40,7 @@ serde = { workspace = true } scale-info = { workspace = true, features = ["bit-vec"] } sp-core = { workspace = true } syn = { workspace = true } -subxt = { workspace = true, features = ["unstable-metadata", "native", "jsonrpsee", "substrate-compat"] } +subxt = { workspace = true, features = ["unstable-metadata", "native", "substrate-compat"] } subxt-signer = { workspace = true, features = ["default"] } subxt-codegen = { workspace = true } subxt-metadata = { workspace = true } diff --git a/testing/integration-tests/src/lib.rs b/testing/integration-tests/src/lib.rs index a57790d452..e9c7226c05 100644 --- a/testing/integration-tests/src/lib.rs +++ b/testing/integration-tests/src/lib.rs @@ -7,6 +7,11 @@ compile_error!( "The features 'unstable-light-client' and 'unstable-backend-client' cannot be used together" ); +#[cfg(all(feature = "unstable-reconnecting-rpc-client", feature = "default"))] +compile_error!( + "The features 'unstable-reconnecting-rpc-client' and 'default' cannot be used together" +); + #[cfg(test)] pub mod utils; diff --git a/testing/integration-tests/src/utils/node_proc.rs b/testing/integration-tests/src/utils/node_proc.rs index b2c0197d5f..a0f14128ee 100644 --- a/testing/integration-tests/src/utils/node_proc.rs +++ b/testing/integration-tests/src/utils/node_proc.rs @@ -70,6 +70,7 @@ where unstable::UnstableRpcMethods::new(rpc_client) } + #[cfg(feature = "default")] /// Hand back an RPC client connected to the test node. pub async fn rpc_client(&self) -> rpc::RpcClient { let url = get_url(self.proc.as_ref().map(|p| p.ws_port())); @@ -78,6 +79,17 @@ where .expect("Unable to connect RPC client to test node") } + #[cfg(feature = "unstable-reconnecting-rpc-client")] + /// Hand back an RPC client connected to the test node. + pub async fn rpc_client(&self) -> rpc::RpcClient { + let url = get_url(self.proc.as_ref().map(|p| p.ws_port())); + let client = subxt::backend::rpc::reconnecting_rpc_client::Client::builder() + .build(url) + .await + .expect("unable to connect RPC client to test node"); + rpc::RpcClient::new(client) + } + /// Always return a client using the unstable backend. /// Only use for comparing backends; use [`TestNodeProcess::client()`] normally, /// which enables us to run each test against both backends. @@ -198,6 +210,7 @@ impl TestNodeProcessBuilder { } } +#[cfg(feature = "default")] async fn build_rpc_client(ws_url: &str) -> Result { let rpc_client = rpc::RpcClient::from_insecure_url(ws_url) .await @@ -206,6 +219,16 @@ async fn build_rpc_client(ws_url: &str) -> Result { Ok(rpc_client) } +#[cfg(feature = "unstable-reconnecting-rpc-client")] +async fn build_rpc_client(ws_url: &str) -> Result { + let client = subxt::backend::rpc::reconnecting_rpc_client::Client::builder() + .build(ws_url.to_string()) + .await + .map_err(|e| format!("Cannot construct RPC client: {e}"))?; + + Ok(rpc::RpcClient::new(client)) +} + async fn build_legacy_client( rpc_client: rpc::RpcClient, ) -> Result, String> { From f7d7e1546ee710e75a666fa1da44800bca1e599b Mon Sep 17 00:00:00 2001 From: Pavlo Khrystenko Date: Thu, 15 Aug 2024 12:08:06 +0200 Subject: [PATCH 2/7] add node restart --- .../src/full_client/client/mod.rs | 3 + .../integration-tests/src/utils/node_proc.rs | 17 ++++- testing/substrate-runner/src/lib.rs | 69 ++++++++++++++++++- 3 files changed, 86 insertions(+), 3 deletions(-) diff --git a/testing/integration-tests/src/full_client/client/mod.rs b/testing/integration-tests/src/full_client/client/mod.rs index 2685551335..ac24fd8762 100644 --- a/testing/integration-tests/src/full_client/client/mod.rs +++ b/testing/integration-tests/src/full_client/client/mod.rs @@ -121,6 +121,9 @@ async fn transaction_validation() { wait_for_blocks(&api).await; + #[cfg(feature = "unstable-reconnecting-rpc-client")] + let _ctx = ctx.restart().await; + let tx = node_runtime::tx() .balances() .transfer_allow_death(bob.public_key().into(), 10_000); diff --git a/testing/integration-tests/src/utils/node_proc.rs b/testing/integration-tests/src/utils/node_proc.rs index a0f14128ee..fd2204142b 100644 --- a/testing/integration-tests/src/utils/node_proc.rs +++ b/testing/integration-tests/src/utils/node_proc.rs @@ -58,6 +58,17 @@ where TestNodeProcessBuilder::new(paths) } + pub async fn restart(mut self) -> Self { + tokio::task::spawn_blocking(move || { + if let Some(ref mut proc) = &mut self.proc { + let _ = proc.restart().unwrap(); + } + self + }) + .await + .expect("to succeed") + } + /// Hand back an RPC client connected to the test node which exposes the legacy RPC methods. pub async fn legacy_rpc_methods(&self) -> legacy::LegacyRpcMethods { let rpc_client = self.rpc_client().await; @@ -73,7 +84,8 @@ where #[cfg(feature = "default")] /// Hand back an RPC client connected to the test node. pub async fn rpc_client(&self) -> rpc::RpcClient { - let url = get_url(self.proc.as_ref().map(|p| p.ws_port())); + let url = self.proc.as_ref().map(|p| p.ws_port()); + let url = get_url(url); rpc::RpcClient::from_url(url) .await .expect("Unable to connect RPC client to test node") @@ -82,7 +94,8 @@ where #[cfg(feature = "unstable-reconnecting-rpc-client")] /// Hand back an RPC client connected to the test node. pub async fn rpc_client(&self) -> rpc::RpcClient { - let url = get_url(self.proc.as_ref().map(|p| p.ws_port())); + let url = self.proc.as_ref().map(|p| p.ws_port()); + let url = get_url(url); let client = subxt::backend::rpc::reconnecting_rpc_client::Client::builder() .build(url) .await diff --git a/testing/substrate-runner/src/lib.rs b/testing/substrate-runner/src/lib.rs index eb2e6064b6..8716137aab 100644 --- a/testing/substrate-runner/src/lib.rs +++ b/testing/substrate-runner/src/lib.rs @@ -70,16 +70,27 @@ impl SubstrateNodeBuilder { } /// Spawn the node, handing back an object which, when dropped, will stop it. - pub fn spawn(self) -> Result { + pub fn spawn(mut self) -> Result { // Try to spawn the binary at each path, returning the // first "ok" or last error that we encountered. let mut res = Err(io::Error::new( io::ErrorKind::Other, "No binary path provided", )); + + let path = Command::new("mktemp") + .arg("-d") + .output() + .expect("failed to create base dir"); + let path = String::from_utf8(path.stdout).expect("bad path"); + let mut bin_path = OsString::new(); for binary_path in &self.binary_paths { + self.custom_flags + .insert("base-path".into(), Some(path.clone().into())); + res = SubstrateNodeBuilder::try_spawn(binary_path, &self.custom_flags); if res.is_ok() { + bin_path = binary_path.clone(); break; } } @@ -98,10 +109,13 @@ impl SubstrateNodeBuilder { let p2p_port = p2p_port.ok_or(Error::CouldNotExtractP2pPort)?; Ok(SubstrateNode { + binary_path: bin_path, + custom_flags: self.custom_flags, proc, ws_port, p2p_address, p2p_port, + base_path: path, }) } @@ -131,10 +145,13 @@ impl SubstrateNodeBuilder { } pub struct SubstrateNode { + binary_path: OsString, + custom_flags: HashMap>, proc: process::Child, ws_port: u16, p2p_address: String, p2p_port: u32, + base_path: String, } impl SubstrateNode { @@ -167,11 +184,61 @@ impl SubstrateNode { pub fn kill(&mut self) -> std::io::Result<()> { self.proc.kill() } + + /// restart the node, handing back an object which, when dropped, will stop it. + pub fn restart(&mut self) -> Result<(), std::io::Error> { + let res = self.kill(); + + match res { + Ok(_) => (), + Err(e) => { + self.cleanup(); + return Err(e); + } + } + + let proc = self.try_spawn()?; + + self.proc = proc; + // Wait for RPC port to be logged (it's logged to stderr). + + Ok(()) + } + + // Attempt to spawn a binary with the path/flags given. + fn try_spawn(&mut self) -> Result { + let mut cmd = Command::new(&self.binary_path); + + cmd.env("RUST_LOG", "info,libp2p_tcp=debug") + .stdout(process::Stdio::piped()) + .stderr(process::Stdio::piped()) + .arg("--dev"); + + for (key, val) in &self.custom_flags { + let arg = match val { + Some(val) => format!("--{key}={val}"), + None => format!("--{key}"), + }; + cmd.arg(arg); + } + + cmd.arg(format!("--rpc-port={}", self.ws_port)); + cmd.arg(format!("--port={}", self.p2p_port)); + cmd.spawn() + } + + fn cleanup(&self) { + let _ = Command::new("rm") + .args(["-rf", &self.base_path]) + .output() + .expect("success"); + } } impl Drop for SubstrateNode { fn drop(&mut self) { let _ = self.kill(); + self.cleanup() } } From eea204734c4d41427d2bbd39df3ef66a473be31b Mon Sep 17 00:00:00 2001 From: Pavlo Khrystenko Date: Wed, 21 Aug 2024 21:15:33 +0200 Subject: [PATCH 3/7] review comments --- .github/workflows/rust.yml | 6 -- Cargo.toml | 9 ++- testing/integration-tests/Cargo.toml | 6 +- .../src/full_client/client/mod.rs | 42 +++++++++++-- testing/integration-tests/src/lib.rs | 5 -- .../integration-tests/src/utils/context.rs | 11 +++- .../integration-tests/src/utils/node_proc.rs | 59 +++++++++---------- testing/substrate-runner/src/lib.rs | 4 +- testing/test-runtime/Cargo.toml | 5 +- 9 files changed, 91 insertions(+), 56 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index d36fb427a7..f049cf6d91 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -362,12 +362,6 @@ jobs: command: test args: --release --package integration-tests --features unstable-light-client - - name: Run tests - uses: actions-rs/cargo@v1.0.3 - with: - command: test - args: --release --package integration-tests --no-default-features --features unstable-reconnecting-rpc-client - - if: "failure()" uses: "andymckay/cancel-action@a955d435292c0d409d104b57d8e78435a93a6ef1" # v0.5 diff --git a/Cargo.toml b/Cargo.toml index ecc3491907..456e237927 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,7 +26,7 @@ exclude = [ "testing/wasm-lightclient-tests", "signer/wasm-tests", "examples/wasm-example", - "examples/parachain-example" + "examples/parachain-example", ] resolver = "2" @@ -125,7 +125,12 @@ rand = "0.8.5" pin-project = "1.1.5" # Light client wasm: -web-sys = { version = "0.3.69", features = ["BinaryType", "CloseEvent", "MessageEvent", "WebSocket"] } +web-sys = { version = "0.3.69", features = [ + "BinaryType", + "CloseEvent", + "MessageEvent", + "WebSocket", +] } wasm-bindgen = "0.2.92" send_wrapper = "0.6.0" js-sys = "0.3.69" diff --git a/testing/integration-tests/Cargo.toml b/testing/integration-tests/Cargo.toml index 0caf54cec7..dfd5ec6410 100644 --- a/testing/integration-tests/Cargo.toml +++ b/testing/integration-tests/Cargo.toml @@ -13,7 +13,7 @@ homepage.workspace = true description = "Subxt integration tests that rely on the Substrate binary" [features] -default = [ "subxt/jsonrpsee" ] +default = [] # Enable to run the tests with Light Client support. unstable-light-client = ["subxt/unstable-light-client"] @@ -26,7 +26,7 @@ unstable-light-client-long-running = ["subxt/unstable-light-client"] unstable-backend-client = [] # Enable the unstable reconnecting rpc-client -unstable-reconnecting-rpc-client = ["subxt/unstable-reconnecting-rpc-client"] +unstable-reconnecting-rpc-client = [] [dev-dependencies] @@ -40,7 +40,7 @@ serde = { workspace = true } scale-info = { workspace = true, features = ["bit-vec"] } sp-core = { workspace = true } syn = { workspace = true } -subxt = { workspace = true, features = ["unstable-metadata", "native", "substrate-compat"] } +subxt = { workspace = true, features = ["unstable-metadata", "native", "jsonrpsee", "substrate-compat", "unstable-reconnecting-rpc-client"] } subxt-signer = { workspace = true, features = ["default"] } subxt-codegen = { workspace = true } subxt-metadata = { workspace = true } diff --git a/testing/integration-tests/src/full_client/client/mod.rs b/testing/integration-tests/src/full_client/client/mod.rs index ac24fd8762..439d933f53 100644 --- a/testing/integration-tests/src/full_client/client/mod.rs +++ b/testing/integration-tests/src/full_client/client/mod.rs @@ -3,7 +3,7 @@ // see LICENSE for license details. use crate::{ - subxt_test, test_context, + subxt_test, test_context, test_context_unstable_rpc_client, utils::{node_runtime, wait_for_blocks}, }; use codec::{Decode, Encode}; @@ -121,9 +121,6 @@ async fn transaction_validation() { wait_for_blocks(&api).await; - #[cfg(feature = "unstable-reconnecting-rpc-client")] - let _ctx = ctx.restart().await; - let tx = node_runtime::tx() .balances() .transfer_allow_death(bob.public_key().into(), 10_000); @@ -412,3 +409,40 @@ async fn partial_fee_estimate_correct() { // Both methods should yield the same fee assert_eq!(partial_fee_1, partial_fee_2); } + +#[cfg(feature = "unstable-reconnecting-rpc-client")] +#[subxt_test] +async fn transaction_validation_with_reconnection() { + let ctx = test_context_unstable_rpc_client().await; + let api = ctx.client(); + + let alice = dev::alice(); + let bob = dev::bob(); + + wait_for_blocks(&api).await; + + let _ctx = ctx.restart().await; + + let tx = node_runtime::tx() + .balances() + .transfer_allow_death(bob.public_key().into(), 10_000); + + let signed_extrinsic = api + .tx() + .create_signed(&tx, &alice, Default::default()) + .await + .unwrap(); + + signed_extrinsic + .validate() + .await + .expect("validation failed"); + + signed_extrinsic + .submit_and_watch() + .await + .unwrap() + .wait_for_finalized_success() + .await + .unwrap(); +} diff --git a/testing/integration-tests/src/lib.rs b/testing/integration-tests/src/lib.rs index e9c7226c05..a57790d452 100644 --- a/testing/integration-tests/src/lib.rs +++ b/testing/integration-tests/src/lib.rs @@ -7,11 +7,6 @@ compile_error!( "The features 'unstable-light-client' and 'unstable-backend-client' cannot be used together" ); -#[cfg(all(feature = "unstable-reconnecting-rpc-client", feature = "default"))] -compile_error!( - "The features 'unstable-reconnecting-rpc-client' and 'default' cannot be used together" -); - #[cfg(test)] pub mod utils; diff --git a/testing/integration-tests/src/utils/context.rs b/testing/integration-tests/src/utils/context.rs index e987763eb3..1528ea46fd 100644 --- a/testing/integration-tests/src/utils/context.rs +++ b/testing/integration-tests/src/utils/context.rs @@ -7,17 +7,20 @@ pub(crate) use crate::{node_runtime, utils::TestNodeProcess}; use subxt::client::OnlineClient; use subxt::SubstrateConfig; +use super::node_proc::RpcClientKind; + /// `substrate-node` should be installed on the $PATH. We fall back /// to also checking for an older `substrate` binary. const SUBSTRATE_NODE_PATHS: &str = "substrate-node,substrate"; -pub async fn test_context_with(authority: String) -> TestContext { +pub async fn test_context_with(authority: String, rpc_client_kind: RpcClientKind) -> TestContext { let paths = std::env::var("SUBSTRATE_NODE_PATH").unwrap_or_else(|_| SUBSTRATE_NODE_PATHS.to_string()); let paths: Vec<_> = paths.split(',').map(|p| p.trim()).collect(); let mut proc = TestContext::build(&paths); proc.with_authority(authority); + proc.with_rpc_client_kind(rpc_client_kind); proc.spawn::().await.unwrap() } @@ -28,5 +31,9 @@ pub type TestContext = TestNodeProcess; pub type TestClient = OnlineClient; pub async fn test_context() -> TestContext { - test_context_with("alice".to_string()).await + test_context_with("alice".to_string(), RpcClientKind::Legacy).await +} + +pub async fn test_context_unstable_rpc_client() -> TestContext { + test_context_with("alice".to_string(), RpcClientKind::UnstableReconnecting).await } diff --git a/testing/integration-tests/src/utils/node_proc.rs b/testing/integration-tests/src/utils/node_proc.rs index fd2204142b..9a1311e33d 100644 --- a/testing/integration-tests/src/utils/node_proc.rs +++ b/testing/integration-tests/src/utils/node_proc.rs @@ -5,7 +5,11 @@ use std::cell::RefCell; use std::ffi::{OsStr, OsString}; use std::sync::Arc; +use std::time::Duration; use substrate_runner::SubstrateNode; +use subxt::backend::rpc::reconnecting_rpc_client::{ + Client as UnstableReconnectingRpcClient, ExponentialBackoff, +}; use subxt::{ backend::{legacy, rpc, unstable}, Config, OnlineClient, @@ -71,38 +75,16 @@ where /// Hand back an RPC client connected to the test node which exposes the legacy RPC methods. pub async fn legacy_rpc_methods(&self) -> legacy::LegacyRpcMethods { - let rpc_client = self.rpc_client().await; + let rpc_client = self.rpc_client.clone(); legacy::LegacyRpcMethods::new(rpc_client) } /// Hand back an RPC client connected to the test node which exposes the unstable RPC methods. pub async fn unstable_rpc_methods(&self) -> unstable::UnstableRpcMethods { - let rpc_client = self.rpc_client().await; + let rpc_client = self.rpc_client.clone(); unstable::UnstableRpcMethods::new(rpc_client) } - #[cfg(feature = "default")] - /// Hand back an RPC client connected to the test node. - pub async fn rpc_client(&self) -> rpc::RpcClient { - let url = self.proc.as_ref().map(|p| p.ws_port()); - let url = get_url(url); - rpc::RpcClient::from_url(url) - .await - .expect("Unable to connect RPC client to test node") - } - - #[cfg(feature = "unstable-reconnecting-rpc-client")] - /// Hand back an RPC client connected to the test node. - pub async fn rpc_client(&self) -> rpc::RpcClient { - let url = self.proc.as_ref().map(|p| p.ws_port()); - let url = get_url(url); - let client = subxt::backend::rpc::reconnecting_rpc_client::Client::builder() - .build(url) - .await - .expect("unable to connect RPC client to test node"); - rpc::RpcClient::new(client) - } - /// Always return a client using the unstable backend. /// Only use for comparing backends; use [`TestNodeProcess::client()`] normally, /// which enables us to run each test against both backends. @@ -136,10 +118,17 @@ where } } +/// Kind of rpc client to use in tests +pub enum RpcClientKind { + Legacy, + UnstableReconnecting, +} + /// Construct a test node process. pub struct TestNodeProcessBuilder { node_paths: Vec, authority: Option, + rpc_client: RpcClientKind, } impl TestNodeProcessBuilder { @@ -157,9 +146,16 @@ impl TestNodeProcessBuilder { Self { node_paths: paths, authority: None, + rpc_client: RpcClientKind::Legacy, } } + /// Set the testRunner to use a preferred RpcClient impl, ie Legacy or Unstable + pub fn with_rpc_client_kind(&mut self, rpc_client_kind: RpcClientKind) -> &mut Self { + self.rpc_client = rpc_client_kind; + self + } + /// Set the authority dev account for a node in validator mode e.g. --alice. pub fn with_authority(&mut self, account: String) -> &mut Self { self.authority = Some(account); @@ -186,9 +182,11 @@ impl TestNodeProcessBuilder { }; let ws_url = get_url(proc.as_ref().map(|p| p.ws_port())); - let rpc_client = build_rpc_client(&ws_url) - .await - .map_err(|e| format!("Failed to connect to node at {ws_url}: {e}"))?; + let rpc_client = match self.rpc_client { + RpcClientKind::Legacy => build_rpc_client(&ws_url).await, + RpcClientKind::UnstableReconnecting => build_unstable_rpc_client(&ws_url).await, + } + .map_err(|e| format!("Failed to connect to node at {ws_url}: {e}"))?; // Cache whatever client we build, and None for the other. #[allow(unused_assignments, unused_mut)] @@ -223,7 +221,6 @@ impl TestNodeProcessBuilder { } } -#[cfg(feature = "default")] async fn build_rpc_client(ws_url: &str) -> Result { let rpc_client = rpc::RpcClient::from_insecure_url(ws_url) .await @@ -232,9 +229,9 @@ async fn build_rpc_client(ws_url: &str) -> Result { Ok(rpc_client) } -#[cfg(feature = "unstable-reconnecting-rpc-client")] -async fn build_rpc_client(ws_url: &str) -> Result { - let client = subxt::backend::rpc::reconnecting_rpc_client::Client::builder() +async fn build_unstable_rpc_client(ws_url: &str) -> Result { + let client = UnstableReconnectingRpcClient::builder() + .retry_policy(ExponentialBackoff::from_millis(100).max_delay(Duration::from_secs(10))) .build(ws_url.to_string()) .await .map_err(|e| format!("Cannot construct RPC client: {e}"))?; diff --git a/testing/substrate-runner/src/lib.rs b/testing/substrate-runner/src/lib.rs index 8716137aab..5798806d13 100644 --- a/testing/substrate-runner/src/lib.rs +++ b/testing/substrate-runner/src/lib.rs @@ -90,7 +90,7 @@ impl SubstrateNodeBuilder { res = SubstrateNodeBuilder::try_spawn(binary_path, &self.custom_flags); if res.is_ok() { - bin_path = binary_path.clone(); + bin_path.clone_from(binary_path); break; } } @@ -187,7 +187,7 @@ impl SubstrateNode { /// restart the node, handing back an object which, when dropped, will stop it. pub fn restart(&mut self) -> Result<(), std::io::Error> { - let res = self.kill(); + let res: Result<(), io::Error> = self.kill(); match res { Ok(_) => (), diff --git a/testing/test-runtime/Cargo.toml b/testing/test-runtime/Cargo.toml index b7b69f22fb..108f784eda 100644 --- a/testing/test-runtime/Cargo.toml +++ b/testing/test-runtime/Cargo.toml @@ -14,7 +14,10 @@ serde = { workspace = true } tokio = { workspace = true, features = ["rt-multi-thread"] } tokio-util = { workspace = true, features = ["compat"] } which = { workspace = true } -jsonrpsee = { workspace = true, features = ["async-client", "client-ws-transport-tls"] } +jsonrpsee = { workspace = true, features = [ + "async-client", + "client-ws-transport-tls", +] } hex = { workspace = true } codec = { workspace = true } From b7ff94c3a1e7789f7d35be3377f9960e0ba11f4e Mon Sep 17 00:00:00 2001 From: Pavlo Khrystenko <45178695+pkhry@users.noreply.github.com> Date: Sun, 25 Aug 2024 17:04:50 +0200 Subject: [PATCH 4/7] Update testing/integration-tests/Cargo.toml Co-authored-by: Niklas Adolfsson --- testing/integration-tests/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/testing/integration-tests/Cargo.toml b/testing/integration-tests/Cargo.toml index dfd5ec6410..78ea97d8d0 100644 --- a/testing/integration-tests/Cargo.toml +++ b/testing/integration-tests/Cargo.toml @@ -28,7 +28,6 @@ unstable-backend-client = [] # Enable the unstable reconnecting rpc-client unstable-reconnecting-rpc-client = [] - [dev-dependencies] assert_matches = { workspace = true } codec = { package = "parity-scale-codec", workspace = true, features = ["derive", "bit-vec"] } From 02a4982b8775f2c225ebfea9dd5f9cebc688b686 Mon Sep 17 00:00:00 2001 From: Pavlo Khrystenko Date: Wed, 28 Aug 2024 14:56:41 +0200 Subject: [PATCH 5/7] change test --- testing/integration-tests/Cargo.toml | 3 - .../src/full_client/client/mod.rs | 100 +++++++++++++----- .../integration-tests/src/utils/context.rs | 2 +- .../integration-tests/src/utils/node_proc.rs | 11 +- 4 files changed, 80 insertions(+), 36 deletions(-) diff --git a/testing/integration-tests/Cargo.toml b/testing/integration-tests/Cargo.toml index 78ea97d8d0..ecf485d57b 100644 --- a/testing/integration-tests/Cargo.toml +++ b/testing/integration-tests/Cargo.toml @@ -25,9 +25,6 @@ unstable-light-client-long-running = ["subxt/unstable-light-client"] # the default one which relies on the "old" RPC methods. unstable-backend-client = [] -# Enable the unstable reconnecting rpc-client -unstable-reconnecting-rpc-client = [] - [dev-dependencies] assert_matches = { workspace = true } codec = { package = "parity-scale-codec", workspace = true, features = ["derive", "bit-vec"] } diff --git a/testing/integration-tests/src/full_client/client/mod.rs b/testing/integration-tests/src/full_client/client/mod.rs index 439d933f53..630afc253c 100644 --- a/testing/integration-tests/src/full_client/client/mod.rs +++ b/testing/integration-tests/src/full_client/client/mod.rs @@ -2,8 +2,10 @@ // This file is dual-licensed as Apache-2.0 or GPL-3.0. // see LICENSE for license details. +use std::collections::HashSet; + use crate::{ - subxt_test, test_context, test_context_unstable_rpc_client, + subxt_test, test_context, test_context_reconnecting_rpc_client, utils::{node_runtime, wait_for_blocks}, }; use codec::{Decode, Encode}; @@ -17,6 +19,7 @@ use subxt::{ tx::{TransactionInvalid, ValidationResult}, }; use subxt_signer::sr25519::dev; +use tokio::task::JoinError; #[cfg(fullclient)] mod legacy_rpcs; @@ -410,39 +413,80 @@ async fn partial_fee_estimate_correct() { assert_eq!(partial_fee_1, partial_fee_2); } -#[cfg(feature = "unstable-reconnecting-rpc-client")] #[subxt_test] -async fn transaction_validation_with_reconnection() { - let ctx = test_context_unstable_rpc_client().await; - let api = ctx.client(); +async fn legacy_and_unstable_block_subscription_reconnect() { + let ctx = test_context_reconnecting_rpc_client().await; + + let api = ctx.unstable_client().await; + + let unstable_client_blocks = move |num: usize| { + let api = api.clone(); + tokio::spawn(async move { + api.blocks() + .subscribe_finalized() + .await + .unwrap() + .take(num) + .map(|x| x.unwrap().hash().to_string()) + .collect::>() + .await + }) + }; - let alice = dev::alice(); - let bob = dev::bob(); + let legacy_api = ctx.client(); + + let legacy_client_blocks = move |num: usize| { + let legacy_api = legacy_api.clone(); + + tokio::spawn(async move { + legacy_api + .blocks() + .subscribe_finalized() + .await + .unwrap() + .take(num) + .map(|x| x.unwrap().hash().to_string()) + .collect::>() + .await + }) + }; + tokio::pin! { + let blocks1 = unstable_client_blocks(3); + let blocks2 = legacy_client_blocks(3); + }; + let (blocks, legacy_blocks) = tokio::join!(blocks1, blocks2); + let blocks: HashSet = HashSet::from_iter(blocks.unwrap().into_iter()); + let legacy_blocks: HashSet = HashSet::from_iter(legacy_blocks.unwrap().into_iter()); + let set = blocks + .intersection(&legacy_blocks) + .collect::>(); - wait_for_blocks(&api).await; + // Union of block hashes, we use 2/3 because sometimes one of the clients might be 1 block late. + assert!(set.len() >= 2); - let _ctx = ctx.restart().await; + let ctx = ctx.restart().await; - let tx = node_runtime::tx() - .balances() - .transfer_allow_death(bob.public_key().into(), 10_000); + // Make both clients aware that connection was dropped and force them to reconnect + let _ = ctx.client().backend().genesis_hash().await; + let _ = ctx.unstable_client().await.backend().genesis_hash().await; - let signed_extrinsic = api - .tx() - .create_signed(&tx, &alice, Default::default()) - .await - .unwrap(); + tokio::pin! { + let blocks1 = unstable_client_blocks(6); + let blocks2 = legacy_client_blocks(6); + }; - signed_extrinsic - .validate() - .await - .expect("validation failed"); + let (unstable_client_blocks, legacy_client_blocks): ( + Result, JoinError>, + Result, JoinError>, + ) = tokio::join!(blocks1, blocks2); - signed_extrinsic - .submit_and_watch() - .await - .unwrap() - .wait_for_finalized_success() - .await - .unwrap(); + let unstable_blocks: HashSet = + HashSet::from_iter(unstable_client_blocks.unwrap().into_iter()); + let legacy_blocks: HashSet = + HashSet::from_iter(legacy_client_blocks.unwrap().into_iter()); + let intersection = unstable_blocks.intersection(&legacy_blocks).count(); + + // legacy client will return completely new blocks + // intersection should be of size 2 or 3 blocks depending whether one of the subscription started later due to scheduling + assert!(intersection >= 2 && intersection < 4); } diff --git a/testing/integration-tests/src/utils/context.rs b/testing/integration-tests/src/utils/context.rs index 1528ea46fd..7542a0dac4 100644 --- a/testing/integration-tests/src/utils/context.rs +++ b/testing/integration-tests/src/utils/context.rs @@ -34,6 +34,6 @@ pub async fn test_context() -> TestContext { test_context_with("alice".to_string(), RpcClientKind::Legacy).await } -pub async fn test_context_unstable_rpc_client() -> TestContext { +pub async fn test_context_reconnecting_rpc_client() -> TestContext { test_context_with("alice".to_string(), RpcClientKind::UnstableReconnecting).await } diff --git a/testing/integration-tests/src/utils/node_proc.rs b/testing/integration-tests/src/utils/node_proc.rs index 9a1311e33d..84124129d3 100644 --- a/testing/integration-tests/src/utils/node_proc.rs +++ b/testing/integration-tests/src/utils/node_proc.rs @@ -7,9 +7,7 @@ use std::ffi::{OsStr, OsString}; use std::sync::Arc; use std::time::Duration; use substrate_runner::SubstrateNode; -use subxt::backend::rpc::reconnecting_rpc_client::{ - Client as UnstableReconnectingRpcClient, ExponentialBackoff, -}; +use subxt::backend::rpc::reconnecting_rpc_client::{ExponentialBackoff, RpcClientBuilder}; use subxt::{ backend::{legacy, rpc, unstable}, Config, OnlineClient, @@ -116,6 +114,11 @@ where pub fn client(&self) -> OnlineClient { self.client.clone() } + + /// Returns the rpc client connected to the node + pub fn rpc_client(&self) -> rpc::RpcClient { + self.rpc_client.clone() + } } /// Kind of rpc client to use in tests @@ -230,7 +233,7 @@ async fn build_rpc_client(ws_url: &str) -> Result { } async fn build_unstable_rpc_client(ws_url: &str) -> Result { - let client = UnstableReconnectingRpcClient::builder() + let client = RpcClientBuilder::new() .retry_policy(ExponentialBackoff::from_millis(100).max_delay(Duration::from_secs(10))) .build(ws_url.to_string()) .await From 51710c9dba974bf0059b57068d5793e64ad32686 Mon Sep 17 00:00:00 2001 From: Pavlo Khrystenko Date: Wed, 28 Aug 2024 16:06:11 +0200 Subject: [PATCH 6/7] clippy --- testing/integration-tests/src/full_client/client/mod.rs | 2 +- testing/integration-tests/src/utils/node_proc.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/testing/integration-tests/src/full_client/client/mod.rs b/testing/integration-tests/src/full_client/client/mod.rs index 630afc253c..069c9aeafe 100644 --- a/testing/integration-tests/src/full_client/client/mod.rs +++ b/testing/integration-tests/src/full_client/client/mod.rs @@ -488,5 +488,5 @@ async fn legacy_and_unstable_block_subscription_reconnect() { // legacy client will return completely new blocks // intersection should be of size 2 or 3 blocks depending whether one of the subscription started later due to scheduling - assert!(intersection >= 2 && intersection < 4); + assert!((2..4).contains(&intersection)); } diff --git a/testing/integration-tests/src/utils/node_proc.rs b/testing/integration-tests/src/utils/node_proc.rs index 84124129d3..96e2cbd22a 100644 --- a/testing/integration-tests/src/utils/node_proc.rs +++ b/testing/integration-tests/src/utils/node_proc.rs @@ -63,7 +63,7 @@ where pub async fn restart(mut self) -> Self { tokio::task::spawn_blocking(move || { if let Some(ref mut proc) = &mut self.proc { - let _ = proc.restart().unwrap(); + proc.restart().unwrap(); } self }) From 5f9bb1e0132a846479ccf92396a84c4a0c2d855c Mon Sep 17 00:00:00 2001 From: Pavlo Khrystenko Date: Wed, 28 Aug 2024 16:54:45 +0200 Subject: [PATCH 7/7] change the test to be less flaky --- .../src/full_client/client/mod.rs | 61 +++---------------- 1 file changed, 10 insertions(+), 51 deletions(-) diff --git a/testing/integration-tests/src/full_client/client/mod.rs b/testing/integration-tests/src/full_client/client/mod.rs index 069c9aeafe..3e08a8055a 100644 --- a/testing/integration-tests/src/full_client/client/mod.rs +++ b/testing/integration-tests/src/full_client/client/mod.rs @@ -19,7 +19,6 @@ use subxt::{ tx::{TransactionInvalid, ValidationResult}, }; use subxt_signer::sr25519::dev; -use tokio::task::JoinError; #[cfg(fullclient)] mod legacy_rpcs; @@ -421,7 +420,7 @@ async fn legacy_and_unstable_block_subscription_reconnect() { let unstable_client_blocks = move |num: usize| { let api = api.clone(); - tokio::spawn(async move { + async move { api.blocks() .subscribe_finalized() .await @@ -430,63 +429,23 @@ async fn legacy_and_unstable_block_subscription_reconnect() { .map(|x| x.unwrap().hash().to_string()) .collect::>() .await - }) + } }; - let legacy_api = ctx.client(); + let blocks = unstable_client_blocks(3).await; + let blocks: HashSet = HashSet::from_iter(blocks.into_iter()); - let legacy_client_blocks = move |num: usize| { - let legacy_api = legacy_api.clone(); - - tokio::spawn(async move { - legacy_api - .blocks() - .subscribe_finalized() - .await - .unwrap() - .take(num) - .map(|x| x.unwrap().hash().to_string()) - .collect::>() - .await - }) - }; - tokio::pin! { - let blocks1 = unstable_client_blocks(3); - let blocks2 = legacy_client_blocks(3); - }; - let (blocks, legacy_blocks) = tokio::join!(blocks1, blocks2); - let blocks: HashSet = HashSet::from_iter(blocks.unwrap().into_iter()); - let legacy_blocks: HashSet = HashSet::from_iter(legacy_blocks.unwrap().into_iter()); - let set = blocks - .intersection(&legacy_blocks) - .collect::>(); - - // Union of block hashes, we use 2/3 because sometimes one of the clients might be 1 block late. - assert!(set.len() >= 2); + assert!(blocks.len() == 3); let ctx = ctx.restart().await; - // Make both clients aware that connection was dropped and force them to reconnect - let _ = ctx.client().backend().genesis_hash().await; + // Make client aware that connection was dropped and force them to reconnect let _ = ctx.unstable_client().await.backend().genesis_hash().await; - tokio::pin! { - let blocks1 = unstable_client_blocks(6); - let blocks2 = legacy_client_blocks(6); - }; - - let (unstable_client_blocks, legacy_client_blocks): ( - Result, JoinError>, - Result, JoinError>, - ) = tokio::join!(blocks1, blocks2); + let unstable_blocks = unstable_client_blocks(6).await; - let unstable_blocks: HashSet = - HashSet::from_iter(unstable_client_blocks.unwrap().into_iter()); - let legacy_blocks: HashSet = - HashSet::from_iter(legacy_client_blocks.unwrap().into_iter()); - let intersection = unstable_blocks.intersection(&legacy_blocks).count(); + let unstable_blocks: HashSet = HashSet::from_iter(unstable_blocks.into_iter()); + let intersection = unstable_blocks.intersection(&blocks).count(); - // legacy client will return completely new blocks - // intersection should be of size 2 or 3 blocks depending whether one of the subscription started later due to scheduling - assert!((2..4).contains(&intersection)); + assert!(intersection == 3); }