Clone + Debug on Payloads/Addresses, and compare child storage results (

#1203)

* Clone + Debug on Payloads/Addresses, and compare child storage results

* , to .

* clippy and fixes

* fix

* use derivative instead to remove boilerplate impls

* add missing import

* tidy up and extend wasm test timeout

* try 5 mins instead of 2

.github/workflows/rust.yml

@@ -240,6 +240,9 @@ jobs:
     wasm_tests:
         name: Test (WASM)
         runs-on: ubuntu-latest
+        env:
+            # Set timeout for wasm tests to be much bigger than the default 20 secs.
+            WASM_BINDGEN_TEST_TIMEOUT: 300
 
         steps:
             - uses: actions/checkout@v4

subxt/src/backend/legacy/rpc_methods.rs

@@ -8,35 +8,20 @@ use crate::backend::rpc::{rpc_params, RpcClient, RpcSubscription};
 use crate::metadata::Metadata;
 use crate::{Config, Error};
 use codec::Decode;
+use derivative::Derivative;
 use primitive_types::U256;
 use serde::{Deserialize, Serialize};
 
 /// An interface to call the legacy RPC methods. This interface is instantiated with
 /// some `T: Config` trait which determines some of the types that the RPC methods will
 /// take or hand back.
+#[derive(Derivative)]
+#[derivative(Clone(bound = ""), Debug(bound = ""))]
 pub struct LegacyRpcMethods<T> {
     client: RpcClient,
     _marker: std::marker::PhantomData<T>,
 }
 
-impl<T> Clone for LegacyRpcMethods<T> {
-    fn clone(&self) -> Self {
-        Self {
-            client: self.client.clone(),
-            _marker: self._marker,
-        }
-    }
-}
-
-impl<T> std::fmt::Debug for LegacyRpcMethods<T> {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.debug_struct("LegacyRpcMethods")
-            .field("client", &self.client)
-            .field("_marker", &self._marker)
-            .finish()
-    }
-}
-
 impl<T: Config> LegacyRpcMethods<T> {
     /// Instantiate the legacy RPC method interface.
     pub fn new(client: RpcClient) -> Self {

subxt/src/backend/unstable/rpc_methods.rs

@@ -9,6 +9,7 @@
 use crate::backend::rpc::{rpc_params, RpcClient, RpcSubscription};
 use crate::config::BlockHash;
 use crate::{Config, Error};
+use derivative::Derivative;
 use futures::{Stream, StreamExt};
 use serde::{Deserialize, Serialize};
 use std::collections::{HashMap, VecDeque};
@@ -17,29 +18,13 @@ use std::task::Poll;
 /// An interface to call the unstable RPC methods. This interface is instantiated with
 /// some `T: Config` trait which determines some of the types that the RPC methods will
 /// take or hand back.
+#[derive(Derivative)]
+#[derivative(Clone(bound = ""), Debug(bound = ""))]
 pub struct UnstableRpcMethods<T> {
     client: RpcClient,
     _marker: std::marker::PhantomData<T>,
 }
 
-impl<T> Clone for UnstableRpcMethods<T> {
-    fn clone(&self) -> Self {
-        Self {
-            client: self.client.clone(),
-            _marker: self._marker,
-        }
-    }
-}
-
-impl<T> std::fmt::Debug for UnstableRpcMethods<T> {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.debug_struct("UnstableRpcMethods")
-            .field("client", &self.client)
-            .field("_marker", &self._marker)
-            .finish()
-    }
-}
-
 impl<T: Config> UnstableRpcMethods<T> {
     /// Instantiate the legacy RPC method interface.
     pub fn new(client: RpcClient) -> Self {

subxt/src/client/light_client/builder.rs

@@ -3,7 +3,7 @@
 // see LICENSE for license details.
 
 use super::{rpc::LightClientRpc, LightClient, LightClientError};
-use crate::backend::{rpc::RpcClient, Backend};
+use crate::backend::rpc::RpcClient;
 use crate::{config::Config, error::Error, OnlineClient};
 use std::num::NonZeroU32;
 use subxt_lightclient::{AddChainConfig, AddChainConfigJsonRpc, ChainId};

subxt/src/constants/constant_address.rs

@@ -3,6 +3,7 @@
 // see LICENSE for license details.
 
 use crate::{dynamic::DecodedValueThunk, metadata::DecodeWithMetadata};
+use derivative::Derivative;
 use std::borrow::Cow;
 
 /// This represents a constant address. Anything implementing this trait
@@ -26,6 +27,8 @@ pub trait ConstantAddress {
 }
 
 /// This represents the address of a constant.
+#[derive(Derivative)]
+#[derivative(Clone(bound = ""), Debug(bound = ""))]
 pub struct Address<ReturnTy> {
     pallet_name: Cow<'static, str>,
     constant_name: Cow<'static, str>,

subxt/src/custom_values/custom_value_address.rs

@@ -1,3 +1,4 @@
+use derivative::Derivative;
 use std::marker::PhantomData;
 
 use crate::dynamic::DecodedValueThunk;
@@ -36,6 +37,8 @@ impl CustomValueAddress for str {
 pub struct Yes;
 
 /// A static address to a custom value.
+#[derive(Derivative)]
+#[derivative(Clone(bound = ""), Debug(bound = ""))]
 pub struct StaticAddress<ReturnTy, IsDecodable> {
     name: &'static str,
     hash: Option<[u8; 32]>,

subxt/src/runtime_api/runtime_payload.rs

@@ -3,6 +3,7 @@
 // see LICENSE for license details.
 
 use core::marker::PhantomData;
+use derivative::Derivative;
 use scale_encode::EncodeAsFields;
 use scale_value::Composite;
 use std::borrow::Cow;
@@ -65,7 +66,11 @@ pub trait RuntimeApiPayload {
 ///
 /// This can be created from static values (ie those generated
 /// via the `subxt` macro) or dynamic values via [`dynamic`].
-#[derive(Clone, Debug)]
+#[derive(Derivative)]
+#[derivative(
+    Clone(bound = "ArgsData: Clone"),
+    Debug(bound = "ArgsData: std::fmt::Debug")
+)]
 pub struct Payload<ArgsData, ReturnTy> {
     trait_name: Cow<'static, str>,
     method_name: Cow<'static, str>,

subxt/src/storage/storage_address.rs

@@ -8,6 +8,7 @@ use crate::{
     metadata::{DecodeWithMetadata, EncodeWithMetadata, Metadata},
     utils::{Encoded, Static},
 };
+use derivative::Derivative;
 use scale_info::TypeDef;
 use std::borrow::Cow;
 use subxt_metadata::{StorageEntryType, StorageHasher};
@@ -51,6 +52,11 @@ pub struct Yes;
 
 /// A concrete storage address. This can be created from static values (ie those generated
 /// via the `subxt` macro) or dynamic values via [`dynamic`].
+#[derive(Derivative)]
+#[derivative(
+    Clone(bound = "StorageKey: Clone"),
+    Debug(bound = "StorageKey: std::fmt::Debug")
+)]
 pub struct Address<StorageKey, ReturnTy, Fetchable, Defaultable, Iterable> {
     pallet_name: Cow<'static, str>,
     entry_name: Cow<'static, str>,

testing/integration-tests/src/full_client/client/mod.rs

@@ -60,6 +60,46 @@ async fn storage_iter() {
     assert_eq!(len, 13);
 }
 
+#[tokio::test]
+async fn storage_child_values_same_across_backends() {
+    let ctx = test_context().await;
+
+    let unstable_client = ctx.unstable_client().await;
+    let legacy_client = ctx.legacy_client().await;
+
+    let addr = node_runtime::storage().system().account_iter();
+    let block_ref = legacy_client
+        .blocks()
+        .at_latest()
+        .await
+        .unwrap()
+        .reference();
+
+    let a: Vec<_> = unstable_client
+        .storage()
+        .at(block_ref.clone())
+        .iter(addr.clone())
+        .await
+        .unwrap()
+        .collect()
+        .await;
+    let b: Vec<_> = legacy_client
+        .storage()
+        .at(block_ref.clone())
+        .iter(addr)
+        .await
+        .unwrap()
+        .collect()
+        .await;
+
+    for (a, b) in a.into_iter().zip(b.into_iter()) {
+        let a = a.unwrap();
+        let b = b.unwrap();
+
+        assert_eq!(a, b);
+    }
+}
+
 #[tokio::test]
 async fn transaction_validation() {
     let ctx = test_context().await;

testing/integration-tests/src/utils/node_proc.rs

@@ -2,6 +2,7 @@
 // This file is dual-licensed as Apache-2.0 or GPL-3.0.
 // see LICENSE for license details.
 
+use std::cell::RefCell;
 use std::ffi::{OsStr, OsString};
 use std::sync::Arc;
 use substrate_runner::SubstrateNode;
@@ -18,6 +19,12 @@ pub struct TestNodeProcess<R: Config> {
     // Keep a handle to the node; once it's dropped the node is killed.
     proc: SubstrateNode,
 
+    // Lazily construct these when asked for.
+    unstable_client: RefCell<Option<OnlineClient<R>>>,
+    legacy_client: RefCell<Option<OnlineClient<R>>>,
+
+    rpc_client: rpc::RpcClient,
+
     #[cfg(not(feature = "unstable-light-client"))]
     client: OnlineClient<R>,
 
@@ -49,14 +56,42 @@ where
         unstable::UnstableRpcMethods::new(rpc_client)
     }
 
-    async fn rpc_client(&self) -> rpc::RpcClient {
+    /// Hand back an RPC client connected to the test node.
+    pub async fn rpc_client(&self) -> rpc::RpcClient {
         let url = format!("ws://127.0.0.1:{}", self.proc.ws_port());
         rpc::RpcClient::from_url(url)
             .await
             .expect("Unable to connect RPC client to test node")
     }
 
-    /// Returns the subxt client connected to the running node.
+    /// 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.
+    pub async fn unstable_client(&self) -> OnlineClient<R> {
+        if self.unstable_client.borrow().is_none() {
+            let c = build_unstable_client(self.rpc_client.clone())
+                .await
+                .unwrap();
+            self.unstable_client.replace(Some(c));
+        }
+        self.unstable_client.borrow().as_ref().unwrap().clone()
+    }
+
+    /// Always return a client using the legacy backend.
+    /// Only use for comparing backends; use [`TestNodeProcess::client()`] normally,
+    /// which enables us to run each test against both backends.
+    pub async fn legacy_client(&self) -> OnlineClient<R> {
+        if self.legacy_client.borrow().is_none() {
+            let c = build_legacy_client(self.rpc_client.clone()).await.unwrap();
+            self.legacy_client.replace(Some(c));
+        }
+        self.legacy_client.borrow().as_ref().unwrap().clone()
+    }
+
+    /// Returns the subxt client connected to the running node. This client
+    /// will use the legacy backend by default or the unstable backend if the
+    /// "unstable-backend-client" feature is enabled, so that we can run each
+    /// test against both.
     #[cfg(not(feature = "unstable-light-client"))]
     pub fn client(&self) -> OnlineClient<R> {
         self.client.clone()
@@ -115,36 +150,57 @@ impl TestNodeProcessBuilder {
         // Spawn the node and retrieve a URL to it:
         let proc = node_builder.spawn().map_err(|e| e.to_string())?;
         let ws_url = format!("ws://127.0.0.1:{}", proc.ws_port());
+        let rpc_client = build_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)]
+        let mut unstable_client = None;
+        #[allow(unused_assignments, unused_mut)]
+        let mut legacy_client = None;
 
         #[cfg(feature = "unstable-light-client")]
-        let client = build_light_client(&proc).await;
+        let client = build_light_client(&proc).await?;
 
         #[cfg(feature = "unstable-backend-client")]
-        let client = build_unstable_client(&proc).await;
+        let client = {
+            let client = build_unstable_client(rpc_client.clone()).await?;
+            unstable_client = Some(client.clone());
+            client
+        };
 
         #[cfg(all(
             not(feature = "unstable-light-client"),
             not(feature = "unstable-backend-client")
         ))]
-        let client = build_legacy_client(&proc).await;
-
-        match client {
-            Ok(client) => Ok(TestNodeProcess { proc, client }),
-            Err(err) => Err(format!("Failed to connect to node rpc at {ws_url}: {err}")),
-        }
+        let client = {
+            let client = build_legacy_client(rpc_client.clone()).await?;
+            legacy_client = Some(client.clone());
+            client
+        };
+
+        Ok(TestNodeProcess {
+            proc,
+            client,
+            legacy_client: RefCell::new(legacy_client),
+            unstable_client: RefCell::new(unstable_client),
+            rpc_client,
+        })
     }
 }
 
-#[cfg(all(
-    not(feature = "unstable-light-client"),
-    not(feature = "unstable-backend-client")
-))]
-async fn build_legacy_client<T: Config>(proc: &SubstrateNode) -> Result<OnlineClient<T>, String> {
-    let ws_url = format!("ws://127.0.0.1:{}", proc.ws_port());
-
+async fn build_rpc_client(ws_url: &str) -> Result<rpc::RpcClient, String> {
     let rpc_client = rpc::RpcClient::from_url(ws_url)
         .await
         .map_err(|e| format!("Cannot construct RPC client: {e}"))?;
+
+    Ok(rpc_client)
+}
+
+async fn build_legacy_client<T: Config>(
+    rpc_client: rpc::RpcClient,
+) -> Result<OnlineClient<T>, String> {
     let backend = legacy::LegacyBackend::new(rpc_client);
     let client = OnlineClient::from_backend(Arc::new(backend))
         .await
@@ -153,14 +209,9 @@ async fn build_legacy_client<T: Config>(proc: &SubstrateNode) -> Result<OnlineCl
     Ok(client)
 }
 
-#[cfg(feature = "unstable-backend-client")]
-async fn build_unstable_client<T: Config>(proc: &SubstrateNode) -> Result<OnlineClient<T>, String> {
-    let ws_url = format!("ws://127.0.0.1:{}", proc.ws_port());
-
-    let rpc_client = rpc::RpcClient::from_url(ws_url)
-        .await
-        .map_err(|e| format!("Cannot construct RPC client: {e}"))?;
-
+async fn build_unstable_client<T: Config>(
+    rpc_client: rpc::RpcClient,
+) -> Result<OnlineClient<T>, String> {
     let (backend, mut driver) = unstable::UnstableBackend::builder().build(rpc_client);
 
     // The unstable backend needs driving: