From f129063b4444538d78017508cf987db97856fc21 Mon Sep 17 00:00:00 2001 From: Pavlo Khrystenko Date: Mon, 9 Sep 2024 11:02:18 +0200 Subject: [PATCH 1/7] add tests for unstable_backend --- subxt/src/backend/mod.rs | 1098 +++++++++++++++++---- subxt/src/backend/unstable/rpc_methods.rs | 51 + 2 files changed, 974 insertions(+), 175 deletions(-) diff --git a/subxt/src/backend/mod.rs b/subxt/src/backend/mod.rs index b035d34b49..4e8b9158ac 100644 --- a/subxt/src/backend/mod.rs +++ b/subxt/src/backend/mod.rs @@ -336,54 +336,96 @@ pub struct StorageResponse { #[cfg(test)] mod test { use super::*; + pub use crate::{backend::StorageResponse, error::RpcError}; + pub use futures::StreamExt; + pub use serde::Serialize; + pub use std::collections::{HashMap, VecDeque}; + pub use subxt_core::{config::DefaultExtrinsicParams, Config}; + pub use tokio::sync::{mpsc, Mutex}; + pub type RpcResult = Result; + pub type Item = RpcResult; + pub use crate::backend::rpc::RawRpcSubscription; + pub use rpc::RpcClientT; + pub use serde_json::value::RawValue; + + fn storage_response>, V: Into>>(key: K, value: V) -> StorageResponse + where + Vec: From, + { + StorageResponse { + key: key.into(), + value: value.into(), + } + } + pub mod rpc_client { - mod legacy { - use super::rpc::{RpcClient, RpcClientT}; - use crate::backend::rpc::RawRpcSubscription; - use crate::backend::BackendExt; - use crate::{ - backend::{ - legacy::rpc_methods::Bytes, legacy::rpc_methods::RuntimeVersion, - legacy::LegacyBackend, StorageResponse, - }, - error::RpcError, - }; - use futures::StreamExt; - use serde::Serialize; - use serde_json::value::RawValue; - use std::{ - collections::{HashMap, VecDeque}, - sync::Arc, - }; - use subxt_core::{config::DefaultExtrinsicParams, Config}; - use tokio::sync::{mpsc, Mutex}; + use std::time::Duration; + + use super::*; - type RpcResult = Result; - type Item = RpcResult; + pub type SubscriptionHandler = Box< + dyn for<'a> Fn( + &'a mut MockDataTable, + &'a mut Option, + Option>, + ) + -> super::rpc::RawRpcFuture<'a, super::rpc::RawRpcSubscription> + + Send, + >; + + pub type MethodHandler = Box< + dyn for<'a> Fn( + &'a mut MockDataTable, + &'a mut Option, + Option>, + ) + -> super::rpc::RawRpcFuture<'a, Box> + + Send, + >; - struct MockDataTable { - items: HashMap, VecDeque>, + pub enum Message { + Many(RpcResult>), + Single(T), } - impl MockDataTable { - fn new() -> Self { - MockDataTable { - items: HashMap::new(), + impl Message { + pub fn single(self) -> T { + match self { + Self::Single(s) => s, + _ => todo!(), + } + } + pub fn many(self) -> RpcResult> { + match self { + Self::Many(s) => s, + _ => todo!(), } } + } - fn from_iter<'a, T: Serialize, I: IntoIterator)>>( - item: I, - ) -> Self { - let mut data = Self::new(); - for (key, item) in item.into_iter() { - data.push(key.into(), item); + pub struct MockDataTable { + items: HashMap, VecDeque>>, + } + + impl MockDataTable { + pub fn new() -> Self { + MockDataTable { + items: HashMap::new(), } - data } - fn push(&mut self, key: Vec, item: RpcResult) { - let item = item.map(|x| serde_json::to_string(&x).unwrap()); + pub fn push(&mut self, key: Vec, item: Message>) { + let item = match item { + Message::Many(items) => Message::Many(items.map(|items| { + items + .into_iter() + .map(|item| item.map(|x| serde_json::to_string(&x).unwrap())) + .collect() + })), + Message::Single(item) => { + Message::Single(item.map(|x| serde_json::to_string(&x).unwrap())) + } + }; match self.items.entry(key) { std::collections::hash_map::Entry::Occupied(v) => v.into_mut().push_back(item), std::collections::hash_map::Entry::Vacant(e) => { @@ -392,121 +434,211 @@ mod test { } } - fn pop(&mut self, key: Vec) -> Item { + pub fn pop(&mut self, key: Vec) -> Message { self.items.get_mut(&key).unwrap().pop_front().unwrap() } } - struct Subscription { - sender: mpsc::Sender>>, - receiver: mpsc::Receiver>>, + pub struct Subscription { + sender: mpsc::Sender, } impl Subscription { - fn new() -> Self { + pub fn new() -> (Self, mpsc::Receiver) { let (sender, receiver) = mpsc::channel(32); - Self { sender, receiver } + (Self { sender }, receiver) } - async fn from_iter< - T: Serialize, - S: IntoIterator>>>, - >( - items: S, - ) -> Self { - let sub = Self::new(); - for i in items { - let i: RpcResult> = i.map(|items| { - items - .into_iter() - .map(|item| item.map(|i| serde_json::to_string(&i).unwrap())) - .collect() - }); - sub.write(i).await - } - sub + pub async fn write(&self, items: Message) { + match items { + Message::Many(items) => { + for i in items.unwrap() { + self.sender.send(i).await.unwrap() + } + } + Message::Single(item) => self.sender.send(item).await.unwrap(), + }; } - async fn read(&mut self) -> RpcResult> { - self.receiver.recv().await.unwrap() - } + pub async fn write_delayed(&self, items: Message) { + let sender = self.sender.clone(); + tokio::spawn(async move { + tokio::time::sleep(Duration::from_millis(500)).await; - async fn write(&self, items: RpcResult>) { - self.sender.send(items).await.unwrap() + match items { + Message::Many(items) => { + for i in items.unwrap() { + let _ = sender.send(i).await; + } + } + Message::Single(item) => sender.send(item).await.unwrap(), + }; + }); } } struct Data { - request: MockDataTable, - subscription: Subscription, + data_table: MockDataTable, + subscription_channel: Option, + subscription_handlers: HashMap, + method_handlers: HashMap, + } + + impl Data { + async fn call_meth<'a>( + &'a mut self, + meth: &str, + params: Option>, + ) -> super::rpc::RawRpcFuture<'a, Box> { + let method = self.method_handlers.get(meth).expect(&format!( + "no method named {} registered. Params: {:?}", + meth, params + )); + + (*method)(&mut self.data_table, &mut self.subscription_channel, params) + } + + async fn call_subscription<'a>( + &'a mut self, + sub: &str, + params: Option>, + ) -> super::rpc::RawRpcFuture<'a, super::rpc::RawRpcSubscription> { + let sub = self.subscription_handlers.get(sub).expect(&format!( + "no subscription named {} registered. Params: {:?}", + sub, params + )); + + (*sub)(&mut self.data_table, &mut self.subscription_channel, params) + } + } + pub struct MockRpcBuilder { + data: Data, + } + + impl MockRpcBuilder { + pub fn new() -> Self { + Self { + data: Data { + data_table: MockDataTable::new(), + subscription_channel: None, + subscription_handlers: HashMap::new(), + method_handlers: HashMap::new(), + }, + } + } + + pub fn add_method(mut self, method_name: &str, meth: MethodHandler) -> Self { + self.data.method_handlers.insert(method_name.into(), meth); + self + } + + pub fn add_subscription( + mut self, + subscription_name: &str, + subscription_handler: SubscriptionHandler, + ) -> Self { + self.data + .subscription_handlers + .insert(subscription_name.into(), subscription_handler); + self + } + + pub fn add_mock_data_from_iter< + 'a, + T: Serialize, + I: IntoIterator>)>, + >( + mut self, + item: I, + ) -> Self { + let data = &mut self.data.data_table; + for (key, item) in item.into_iter() { + data.push(key.into(), item); + } + self + } + + pub fn build(self) -> MockRpcClient { + MockRpcClient { + data: Arc::new(Mutex::new(self.data)), + } + } } - struct MockRpcClientStorage { + pub struct MockRpcClient { data: Arc>, } - impl RpcClientT for MockRpcClientStorage { + impl RpcClientT for MockRpcClient { fn request_raw<'a>( &'a self, method: &'a str, params: Option>, ) -> super::rpc::RawRpcFuture<'a, Box> { - Box::pin(async move { - match method { - "state_getStorage" => { - let mut data = self.data.lock().await; - let params = params.map(|p| p.get().to_string()); - let rpc_params = jsonrpsee::types::Params::new(params.as_deref()); - let key: sp_core::Bytes = rpc_params.sequence().next().unwrap(); - let value = data.request.pop(key.0); - value.map(|v| serde_json::value::RawValue::from_string(v).unwrap()) - } - "chain_getBlockHash" => { - let mut data = self.data.lock().await; - let value = data.request.pop("chain_getBlockHash".into()); - value.map(|v| serde_json::value::RawValue::from_string(v).unwrap()) - } - _ => todo!(), - } + Box::pin(async { + let mut data = self.data.lock().await; + data.call_meth(method, params).await.await }) } fn subscribe_raw<'a>( &'a self, - _sub: &'a str, - _params: Option>, + sub: &'a str, + params: Option>, _unsub: &'a str, ) -> super::rpc::RawRpcFuture<'a, super::rpc::RawRpcSubscription> { Box::pin(async { let mut data = self.data.lock().await; - let values: RpcResult>>> = - data.subscription.read().await.map(|v| { - v.into_iter() - .map(|v| { - v.map(|v| serde_json::value::RawValue::from_string(v).unwrap()) - }) - .collect::>>>() - }); - values.map(|v| RawRpcSubscription { - stream: futures::stream::iter(v).boxed(), - id: Some("ID".to_string()), - }) + data.call_subscription(sub, params).await.await }) } } + } - // Define dummy config - enum Conf {} - impl Config for Conf { - type Hash = crate::utils::H256; - type AccountId = crate::utils::AccountId32; - type Address = crate::utils::MultiAddress; - type Signature = crate::utils::MultiSignature; - type Hasher = crate::config::substrate::BlakeTwo256; - type Header = crate::config::substrate::SubstrateHeader; - type ExtrinsicParams = DefaultExtrinsicParams; + // Define dummy config + enum Conf {} + impl Config for Conf { + type Hash = crate::utils::H256; + type AccountId = crate::utils::AccountId32; + type Address = crate::utils::MultiAddress; + type Signature = crate::utils::MultiSignature; + type Hasher = crate::config::substrate::BlakeTwo256; + type Header = crate::config::substrate::SubstrateHeader; + type ExtrinsicParams = DefaultExtrinsicParams; + + type AssetId = u32; + } - type AssetId = u32; + mod legacy { + use super::*; + use crate::backend::{ + legacy::rpc_methods::Bytes, legacy::rpc_methods::RuntimeVersion, legacy::LegacyBackend, + }; + use rpc_client::*; + + pub fn setup_mock_rpc() -> MockRpcBuilder { + MockRpcBuilder::new() + .add_method( + "state_getStorage", + Box::new(|data, _sub, params| { + Box::pin(async move { + let params = params.map(|p| p.get().to_string()); + let rpc_params = jsonrpsee::types::Params::new(params.as_deref()); + let key: sp_core::Bytes = rpc_params.sequence().next().unwrap(); + let value = data.pop(key.0).single(); + value.map(|v| serde_json::value::RawValue::from_string(v).unwrap()) + }) + }), + ) + .add_method( + "chain_getBlockHash", + Box::new(|data, _, _| { + Box::pin(async move { + let value = data.pop("chain_getBlockHash".into()).single(); + value.map(|v| serde_json::value::RawValue::from_string(v).unwrap()) + }) + }), + ) } use crate::backend::Backend; @@ -530,54 +662,26 @@ mod test { Ok(Some(Bytes(str.into()))) } - fn storage_response>, V: Into>>(key: K, value: V) -> StorageResponse - where - Vec: From, - { - StorageResponse { - key: key.into(), - value: value.into(), - } - } - - async fn build_mock_client< - 'a, - T: Serialize, - D: IntoIterator)>, - S: IntoIterator>>>, - >( - table_data: D, - subscription_data: S, - ) -> RpcClient { - let data = Data { - request: MockDataTable::from_iter(table_data), - subscription: Subscription::from_iter(subscription_data).await, - }; - RpcClient::new(MockRpcClientStorage { - data: Arc::new(Mutex::new(data)), - }) - } - #[tokio::test] async fn storage_fetch_values() { let mock_data = vec![ - ("ID1", bytes("Data1")), + ("ID1", Message::Single(bytes("Data1"))), ( "ID2", - Err(RpcError::DisconnectedWillReconnect( + Message::Single(Err(RpcError::DisconnectedWillReconnect( "Reconnecting".to_string(), - )), + ))), ), - ("ID2", bytes("Data2")), + ("ID2", Message::Single(bytes("Data2"))), ( "ID3", - Err(RpcError::DisconnectedWillReconnect( + Message::Single(Err(RpcError::DisconnectedWillReconnect( "Reconnecting".to_string(), - )), + ))), ), - ("ID3", bytes("Data3")), + ("ID3", Message::Single(bytes("Data3"))), ]; - let rpc_client = build_mock_client(mock_data, vec![]).await; + let rpc_client = setup_mock_rpc().add_mock_data_from_iter(mock_data).build(); let backend: LegacyBackend = LegacyBackend::builder().build(rpc_client); // Test @@ -609,13 +713,13 @@ mod test { let mock_data = [ ( "ID1", - Err(RpcError::DisconnectedWillReconnect( + Message::Single(Err(RpcError::DisconnectedWillReconnect( "Reconnecting".to_string(), - )), + ))), ), - ("ID1", bytes("Data1")), + ("ID1", Message::Single(bytes("Data1"))), ]; - let rpc_client = build_mock_client(mock_data, vec![]).await; + let rpc_client = setup_mock_rpc().add_mock_data_from_iter(mock_data).build(); // Test let backend: LegacyBackend = LegacyBackend::builder().build(rpc_client); @@ -648,13 +752,13 @@ mod test { let mock_data = vec![ ( "chain_getBlockHash", - Err(RpcError::DisconnectedWillReconnect( + Message::Single(Err(RpcError::DisconnectedWillReconnect( "Reconnecting".to_string(), - )), + ))), ), - ("chain_getBlockHash", Ok(Some(hash))), + ("chain_getBlockHash", Message::Single(Ok(Some(hash)))), ]; - let rpc_client = build_mock_client(mock_data, vec![]).await; + let rpc_client = setup_mock_rpc().add_mock_data_from_iter(mock_data).build(); // Test let backend: LegacyBackend = LegacyBackend::builder().build(rpc_client); @@ -688,27 +792,60 @@ mod test { /// ``` async fn stream_simple() { let mock_subscription_data = vec![ - Ok(vec![ - Ok(runtime_version(0)), - Err(RpcError::DisconnectedWillReconnect( - "Reconnecting".to_string(), - )), - Ok(runtime_version(1)), - ]), - Ok(vec![ - Err(RpcError::DisconnectedWillReconnect( - "Reconnecting".to_string(), - )), - Ok(runtime_version(2)), - Ok(runtime_version(3)), - ]), - Ok(vec![ - Ok(runtime_version(4)), - Ok(runtime_version(5)), - Err(RpcError::RequestRejected("Reconnecting".to_string())), - ]), + ( + "state_subscribeRuntimeVersion", + Message::Many(Ok(vec![ + Ok(runtime_version(0)), + Err(RpcError::DisconnectedWillReconnect( + "Reconnecting".to_string(), + )), + Ok(runtime_version(1)), + ])), + ), + ( + "state_subscribeRuntimeVersion", + Message::Many(Ok(vec![ + Err(RpcError::DisconnectedWillReconnect( + "Reconnecting".to_string(), + )), + Ok(runtime_version(2)), + Ok(runtime_version(3)), + ])), + ), + ( + "state_subscribeRuntimeVersion", + Message::Many(Ok(vec![ + Ok(runtime_version(4)), + Ok(runtime_version(5)), + Err(RpcError::RequestRejected("Reconnecting".to_string())), + ])), + ), ]; - let rpc_client = build_mock_client(vec![], mock_subscription_data).await; + let rpc_client = setup_mock_rpc() + .add_subscription( + "state_subscribeRuntimeVersion", + Box::new(|data, _, _| { + Box::pin(async move { + let values = data.pop("state_subscribeRuntimeVersion".into()).many(); + let values: RpcResult>>> = + values.map(|v| { + v.into_iter() + .map(|v| { + v.map(|v| { + serde_json::value::RawValue::from_string(v).unwrap() + }) + }) + .collect::>>>() + }); + values.map(|v| RawRpcSubscription { + stream: futures::stream::iter(v).boxed(), + id: Some("ID".to_string()), + }) + }) + }), + ) + .add_mock_data_from_iter(mock_subscription_data) + .build(); // Test let backend: LegacyBackend = LegacyBackend::builder().build(rpc_client); @@ -734,4 +871,615 @@ mod test { assert!(results.next().await.is_none()) } } + + mod unstable_backend { + + use std::sync::atomic::AtomicBool; + + use futures::task::Poll; + use rpc_client::{Message, MockRpcBuilder, Subscription}; + use rpc_methods::{ + Bytes, Initialized, MethodResponse, MethodResponseStarted, OperationError, OperationId, + OperationStorageItems, RuntimeSpec, RuntimeVersionEvent, + }; + use sp_core::H256; + + use super::unstable::*; + use super::*; + + fn runtime_spec() -> RuntimeSpec { + let spec = serde_json::json!({ + "specName": "westend", + "implName": "parity-westend", + "specVersion": 9122, + "implVersion": 0, + "transactionVersion": 7, + "apis": { + "0xdf6acb689907609b": 3, + "0x37e397fc7c91f5e4": 1, + "0x40fe3ad401f8959a": 5, + "0xd2bc9897eed08f15": 3, + "0xf78b278be53f454c": 2, + "0xaf2c0297a23e6d3d": 1, + "0x49eaaf1b548a0cb0": 1, + "0x91d5df18b0d2cf58": 1, + "0xed99c5acb25eedf5": 3, + "0xcbca25e39f142387": 2, + "0x687ad44ad37f03c2": 1, + "0xab3c0572291feb8b": 1, + "0xbc9d89904f5b923f": 1, + "0x37c8bb1350a9a2a8": 1 + } + }); + serde_json::from_value(spec).unwrap() + } + fn init_hash() -> crate::utils::H256 { + crate::utils::H256::random() + } + + type FollowEvent = unstable::rpc_methods::FollowEvent<::Hash>; + + fn setup_mock_rpc_client(cycle_ids: bool) -> MockRpcBuilder { + let hash = init_hash(); + let mut id = 0; + rpc_client::MockRpcBuilder::new().add_subscription( + "chainHead_v1_follow", + Box::new(move |_, sub, _| { + Box::pin(async move { + if cycle_ids { + id += 1; + } + let follow_event = + FollowEvent::Initialized(Initialized::<::Hash> { + finalized_block_hashes: vec![hash.clone()], + finalized_block_runtime: Some(rpc_methods::RuntimeEvent::Valid( + RuntimeVersionEvent { + spec: runtime_spec(), + }, + )), + }); + let (subscription, mut receiver) = Subscription::new(); + subscription + .write(Message::Single(Ok( + serde_json::to_string(&follow_event).unwrap() + ))) + .await; + sub.replace(subscription); + let read_stream = + futures::stream::poll_fn(move |cx| -> Poll> { + receiver.poll_recv(cx) + }) + .map(|item| item.map(|x| RawValue::from_string(x).unwrap())); + let stream = RawRpcSubscription { + stream: read_stream.boxed(), + id: Some(format!("ID{}", id)), + }; + Ok(stream) + }) + }), + ) + } + + fn response_started(id: &str) -> MethodResponse { + MethodResponse::Started(MethodResponseStarted { + operation_id: id.to_owned(), + discarded_items: None, + }) + } + + fn operation_error(id: &str) -> FollowEvent { + FollowEvent::OperationError(OperationError { + operation_id: id.to_owned(), + error: "errro".to_owned(), + }) + } + + fn storage_done(id: &str) -> FollowEvent { + FollowEvent::OperationStorageDone(OperationId { + operation_id: id.to_owned(), + }) + } + fn storage_result(key: &str, value: &str) -> unstable::rpc_methods::StorageResult { + unstable::rpc_methods::StorageResult { + key: Bytes(key.to_owned().into()), + result: rpc_methods::StorageResultType::Value(Bytes(value.to_owned().into())), + } + } + fn storage_items(id: &str, items: &[unstable::rpc_methods::StorageResult]) -> FollowEvent { + FollowEvent::OperationStorageItems(OperationStorageItems { + operation_id: id.to_owned(), + items: VecDeque::from(items.to_owned()), + }) + } + + fn operation_continue(id: &str) -> FollowEvent { + FollowEvent::OperationWaitingForContinue(OperationId { + operation_id: id.to_owned(), + }) + } + + #[tokio::test] + async fn storage_fetch_values_returns_stream_with_single_error() { + let response_data = vec![( + "method_response", + Message::Single(Ok(response_started("Id1"))), + )]; + let mock_subscription_data = vec![( + "chainHead_v1_storage", + Message::Many(Ok(vec![Ok(operation_error("Id1")), Ok(FollowEvent::Stop)])), + )]; + let rpc_client = setup_mock_rpc_client(false) + .add_method( + "chainHead_v1_storage", + Box::new(|data, sub, _| { + Box::pin(async move { + let response = data.pop("method_response".into()).single(); + if response.is_ok() { + let item = data.pop("chainHead_v1_storage".into()); + if let Some(sub) = sub { + let item = item; + sub.write_delayed(item).await + } + } + response.map(|x| RawValue::from_string(x).unwrap()) + }) + }), + ) + .add_mock_data_from_iter(mock_subscription_data) + .add_mock_data_from_iter(response_data) + .build(); + + let (backend, mut driver): (UnstableBackend, _) = + UnstableBackend::builder().build(rpc_client); + + let _ = tokio::spawn(async move { + while let Some(val) = driver.next().await { + if let Err(e) = val { + eprintln!("Error driving unstable backend: {e}; terminating client"); + } + } + }); + + // Test + // This request should encounter an error on `request` and do a retry. + let response = backend + .storage_fetch_values( + ["ID1".into(), "ID2".into(), "ID3".into()].into(), + crate::utils::H256::random(), + ) + .await + .unwrap(); + + // operation returned FollowEvent::OperationError + let response = response + .collect::>>() + .await; + + assert!(matches!( + response.as_slice(), + [Err(Error::Other(s) )] if *s == "errro".to_owned() + )); + } + + #[tokio::test] + /// Tests that the method will retry on failed query + async fn storage_fetch_values_retry_query() { + let response_data = vec![ + ( + "method_response", + Message::Single(Err(RpcError::DisconnectedWillReconnect("Error".into()))), + ), + ( + "method_response", + Message::Single(Ok(response_started("Id1"))), + ), + ]; + let mock_data = vec![( + "chainHead_v1_storage", + Message::Many(Ok(vec![ + Ok(storage_items( + "Id1", + &[ + storage_result("ID1", "Data1"), + storage_result("ID2", "Data2"), + storage_result("ID3", "Data3"), + ], + )), + Ok(storage_done("Id1")), + ])), + )]; + let rpc_client = setup_mock_rpc_client(false) + .add_method( + "chainHead_v1_storage", + Box::new(|data, sub, _| { + Box::pin(async move { + let response = data.pop("method_response".into()).single(); + if response.is_ok() { + let item = data.pop("chainHead_v1_storage".into()); + if let Some(sub) = sub { + let item = item; + sub.write_delayed(item).await + } + } + response.map(|x| RawValue::from_string(x).unwrap()) + }) + }), + ) + .add_mock_data_from_iter(mock_data) + .add_mock_data_from_iter(response_data) + .build(); + let (backend, mut driver): (UnstableBackend, _) = + UnstableBackend::builder().build(rpc_client); + + let _ = tokio::spawn(async move { + while let Some(val) = driver.next().await { + if let Err(e) = val { + eprintln!("Error driving unstable backend: {e}; terminating client"); + } + } + }); + + // We try again and should succeed + let response = backend + .storage_fetch_values( + ["ID1".into(), "ID2".into(), "ID3".into()].into(), + crate::utils::H256::random(), + ) + .await + .unwrap(); + + let response = response + .map(|x| x.unwrap()) + .collect::>() + .await; + + assert_eq!( + vec![ + storage_response("ID1", "Data1"), + storage_response("ID2", "Data2"), + storage_response("ID3", "Data3"), + ], + response + ) + } + #[tokio::test] + /// Tests the error behaviour in streams based on chainHead_v1_continue calls + async fn storage_fetch_values_retry_continue() { + fn compare_storage_responses( + expected_response: &StorageResponse, + received_response: &StorageResponse, + ) -> bool { + expected_response == received_response + } + + let response_data = vec![ + ( + "method_response", + Message::Single(Err::( + RpcError::DisconnectedWillReconnect("Error".into()), + )), + ), + ( + "method_response", + Message::Single(Ok(response_started("Id1"))), + ), + ( + "method_response", + Message::Single(Ok(response_started("Id1"))), + ), + ]; + let continue_data = vec![ + ("continue_response", Message::Single(Ok(()))), + ("continue_response", Message::Single(Ok(()))), + ( + "continue_response", + Message::Single(Err(RpcError::DisconnectedWillReconnect("Error".into()))), + ), + ("continue_response", Message::Single(Ok(()))), + ("continue_response", Message::Single(Ok(()))), + ]; + let mock_data = vec![ + ( + "chainHead_v1_storage", + Message::Many(Ok(vec![ + Ok(storage_items("Id1", &[storage_result("ID1", "Data1")])), + Ok(operation_continue("Id1")), + ])), + ), + ( + "chainHead_v1_storage", + Message::Many(Ok(vec![ + Ok(storage_items("Id1", &[storage_result("ID2", "Data2")])), + Ok(operation_continue("Id1")), + ])), + ), + ( + "chainHead_v1_storage", + Message::Many(Ok(vec![Ok(operation_error("Id1")), Ok(FollowEvent::Stop)])), + ), + ( + "chainHead_v1_storage", + Message::Many(Ok(vec![ + Ok(storage_items("Id1", &[storage_result("ID1", "Data1")])), + Ok(operation_continue("Id1")), + ])), + ), + ( + "chainHead_v1_storage", + Message::Many(Ok(vec![ + Ok(storage_items("Id1", &[storage_result("ID2", "Data2")])), + Ok(operation_continue("Id1")), + ])), + ), + ( + "chainHead_v1_storage", + Message::Many(Ok(vec![ + Ok(storage_items("Id1", &[storage_result("ID3", "Data3")])), + Ok(storage_done("Id1")), + ])), + ), + ]; + let rpc_client = setup_mock_rpc_client(false) + .add_method( + "chainHead_v1_storage", + Box::new(|data, sub, _| { + Box::pin(async move { + let response = data.pop("method_response".into()).single(); + if response.is_ok() { + let item = data.pop("chainHead_v1_storage".into()); + if let Some(sub) = sub { + let item = item; + sub.write_delayed(item).await + } + } + response.map(|x| RawValue::from_string(x).unwrap()) + }) + }), + ) + .add_method( + "chainHead_v1_continue", + Box::new(|data, sub, _| { + Box::pin(async move { + let response = data.pop("continue_response".into()).single(); + if response.is_ok() { + let item = data.pop("chainHead_v1_storage".into()); + if let Some(sub) = sub { + let item = item; + sub.write_delayed(item).await + } + } + response.map(|x| RawValue::from_string(x).unwrap()) + }) + }), + ) + .add_mock_data_from_iter(mock_data) + .add_mock_data_from_iter(response_data) + .add_mock_data_from_iter(continue_data) + .build(); + let (backend, mut driver): (UnstableBackend, _) = + UnstableBackend::builder().build(rpc_client); + + let _ = tokio::spawn(async move { + while let Some(val) = driver.next().await { + if let Err(e) = val { + eprintln!("Error driving unstable backend: {e}; terminating client"); + } + } + }); + + // We try again and should fail mid way + let response = backend + .storage_fetch_values( + ["ID1".into(), "ID2".into(), "ID3".into()].into(), + crate::utils::H256::random(), + ) + .await + .unwrap(); + // operation returned FollowEvent::OperationError + let response = response + .collect::>>() + .await; + + assert!(matches!( + response.as_slice(), + [ + Ok(resp1 @ StorageResponse { .. }), + Ok(resp2 @ StorageResponse { .. }), + Err(Error::Other(s)) + ] if *s == "errro".to_owned() + && compare_storage_responses(&storage_response("ID1", "Data1"), resp1) + && compare_storage_responses(&storage_response("ID2", "Data2"), resp2) + )); + + // We try again and should succeed + let response = backend + .storage_fetch_values( + ["ID1".into(), "ID2".into(), "ID3".into()].into(), + crate::utils::H256::random(), + ) + .await + .unwrap(); + + let response = response + .map(|x| x.unwrap()) + .collect::>() + .await; + + assert_eq!( + vec![ + storage_response("ID1", "Data1"), + storage_response("ID2", "Data2"), + storage_response("ID3", "Data3"), + ], + response + ) + } + + #[tokio::test] + async fn simple_fetch() { + let hash = init_hash(); + + let mock_data = vec![ + ( + "chainSpec_v1_genesisHash", + Message::Single(Err::(RpcError::RequestRejected( + "Error".to_owned(), + ))), + ), + ( + "chainSpec_v1_genesisHash", + Message::Single(Err(RpcError::DisconnectedWillReconnect("Error".to_owned()))), + ), + ("chainSpec_v1_genesisHash", Message::Single(Ok(hash.into()))), + ]; + let rpc_client = setup_mock_rpc_client(false) + .add_method( + "chainSpec_v1_genesisHash", + Box::new(|data, _, _| { + Box::pin(async move { + let response = data.pop("chainSpec_v1_genesisHash".into()).single(); + response.map(|x| RawValue::from_string(x).unwrap()) + }) + }), + ) + .add_mock_data_from_iter(mock_data) + .build(); + + let (backend, mut driver): (UnstableBackend, _) = + UnstableBackend::builder().build(rpc_client); + + let _ = tokio::spawn(async move { + while let Some(val) = driver.next().await { + if let Err(e) = val { + eprintln!("Error driving unstable backend: {e}; terminating client"); + } + } + }); + + // Test + // This request should encounter an error on `request` and do a retry. + let response_hash = backend.genesis_hash().await.unwrap(); + + assert_eq!(hash, response_hash) + } + + #[tokio::test] + // Failure as we do not wait for subscription id to be updated + async fn stale_subscription_id_failure() { + let response_data = vec![ + ( + "method_response", + Message::Single(Ok(response_started("Id1"))), + ), + ( + "method_response", + Message::Single(Err(RpcError::RequestRejected("stale id".into()))), + ), + ( + "method_response", + Message::Single(Ok(response_started("Id1"))), + ), + ]; + + let mock_data = vec![ + ( + "chainHead_v1_storage", + Message::Many(Ok(vec![Ok(operation_error("Id1")), Ok(FollowEvent::Stop)])), + ), + ( + "chainHead_v1_storage", + Message::Many(Ok(vec![ + Ok(storage_items( + "Id1", + &[ + storage_result("ID1", "Data1"), + storage_result("ID2", "Data2"), + storage_result("ID3", "Data3"), + ], + )), + Ok(storage_done("Id1")), + ])), + ), + ]; + let rpc_client = setup_mock_rpc_client(true) + .add_method( + "chainHead_v1_storage", + Box::new({ + let subscription_expired = Arc::new(AtomicBool::new(false)); + move |data, sub, params| { + let subscription_expired = subscription_expired.clone(); + Box::pin(async move { + let subscription_expired = subscription_expired.clone(); + if subscription_expired.load(std::sync::atomic::Ordering::SeqCst) { + let params = params.map(|p| p.get().to_string()); + let rpc_params = + jsonrpsee::types::Params::new(params.as_deref()); + let key: String = rpc_params.sequence().next().unwrap(); + if key == "ID1".to_owned() { + return Err(RpcError::RequestRejected("stale id".into())); + } else { + subscription_expired + .swap(false, std::sync::atomic::Ordering::SeqCst); + } + } + let response = data.pop("method_response".into()).single(); + if response.is_ok() { + let item = data.pop("chainHead_v1_storage".into()); + if let Some(sub) = sub { + let item = item; + sub.write_delayed(item).await + } + } else { + subscription_expired + .swap(true, std::sync::atomic::Ordering::SeqCst); + } + response.map(|x| RawValue::from_string(x).unwrap()) + }) + } + }), + ) + .add_mock_data_from_iter(mock_data) + .add_mock_data_from_iter(response_data) + .build(); + let (backend, mut driver): (UnstableBackend, _) = + UnstableBackend::builder().build(rpc_client); + + let _ = driver.next().await.unwrap(); + let _ = driver.next().await.unwrap(); + + // not getting new subscription id and hitting request rejected > 10 times + let response = backend + .storage_fetch_values( + ["ID1".into(), "ID2".into(), "ID3".into()].into(), + crate::utils::H256::random(), + ) + .await; + + let _ = driver.next().await.unwrap(); + + let binding = response + .unwrap() + .collect::>>() + .await; + let response = binding.last().unwrap(); + + assert!(matches!( + response, + Err(Error::Other(reason)) if reason == "errro" + )); + + // not getting new subscription id and hitting request rejected > 10 times + let response = backend + .storage_fetch_values( + ["ID1".into(), "ID2".into(), "ID3".into()].into(), + crate::utils::H256::random(), + ) + .await; + + assert!(matches!( + response, + Err(Error::Rpc(RpcError::RequestRejected(reason))) if reason == "stale id" + )) + } + } } diff --git a/subxt/src/backend/unstable/rpc_methods.rs b/subxt/src/backend/unstable/rpc_methods.rs index e72f2cadef..25e3fd1068 100644 --- a/subxt/src/backend/unstable/rpc_methods.rs +++ b/subxt/src/backend/unstable/rpc_methods.rs @@ -321,6 +321,7 @@ impl UnstableRpcMethods { #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(rename_all = "camelCase")] #[serde(tag = "event")] +#[cfg_attr(test, derive(Serialize))] pub enum FollowEvent { /// The latest finalized block. /// @@ -363,6 +364,8 @@ pub enum FollowEvent { /// This is the first event generated by the `follow` subscription /// and is submitted only once. #[derive(Debug, Clone, PartialEq, Eq)] +#[cfg_attr(test, derive(Serialize))] +#[cfg_attr(test, serde(rename_all = "camelCase"))] pub struct Initialized { /// The hashes of the last finalized blocks. pub finalized_block_hashes: Vec, @@ -404,6 +407,7 @@ impl<'de, Hash: Deserialize<'de>> Deserialize<'de> for Initialized { #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(rename_all = "camelCase")] #[serde(tag = "type")] +#[cfg_attr(test, derive(Serialize))] pub enum RuntimeEvent { /// The runtime version of this block. Valid(RuntimeVersionEvent), @@ -418,6 +422,7 @@ pub enum RuntimeEvent { /// - blocks that suffered a change in runtime compared with their parents #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(rename_all = "camelCase")] +#[cfg_attr(test, derive(Serialize))] pub struct RuntimeVersionEvent { /// Details about this runtime. pub spec: RuntimeSpec, @@ -427,6 +432,7 @@ pub struct RuntimeVersionEvent { /// the "initialized" event of `chainHead_follow` if the `withRuntime` flag is set. #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(rename_all = "camelCase")] +#[cfg_attr(test, derive(Serialize))] pub struct RuntimeSpec { /// Opaque string indicating the name of the chain. pub spec_name: String, @@ -460,6 +466,7 @@ pub struct RuntimeSpec { /// The operation could not be processed due to an error. #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(rename_all = "camelCase")] +#[cfg_attr(test, derive(Serialize))] pub struct ErrorEvent { /// Reason of the error. pub error: String, @@ -468,6 +475,7 @@ pub struct ErrorEvent { /// Indicate a new non-finalized block. #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(rename_all = "camelCase")] +#[cfg_attr(test, derive(Serialize))] pub struct NewBlock { /// The hash of the new block. pub block_hash: Hash, @@ -485,6 +493,7 @@ pub struct NewBlock { /// Indicate the block hash of the new best block. #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(rename_all = "camelCase")] +#[cfg_attr(test, derive(Serialize))] pub struct BestBlockChanged { /// The block hash of the new best block. pub best_block_hash: Hash, @@ -493,6 +502,7 @@ pub struct BestBlockChanged { /// Indicate the finalized and pruned block hashes. #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(rename_all = "camelCase")] +#[cfg_attr(test, derive(Serialize))] pub struct Finalized { /// Block hashes that are finalized. pub finalized_block_hashes: Vec, @@ -503,6 +513,7 @@ pub struct Finalized { /// Indicate the operation id of the event. #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(rename_all = "camelCase")] +#[cfg_attr(test, derive(Serialize))] pub struct OperationId { /// The operation id of the event. pub operation_id: String, @@ -511,6 +522,7 @@ pub struct OperationId { /// The response of the `chainHead_body` method. #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(rename_all = "camelCase")] +#[cfg_attr(test, derive(Serialize))] pub struct OperationBodyDone { /// The operation id of the event. pub operation_id: String, @@ -521,6 +533,7 @@ pub struct OperationBodyDone { /// The response of the `chainHead_call` method. #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(rename_all = "camelCase")] +#[cfg_attr(test, derive(Serialize))] pub struct OperationCallDone { /// The operation id of the event. pub operation_id: String, @@ -531,6 +544,7 @@ pub struct OperationCallDone { /// The response of the `chainHead_call` method. #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(rename_all = "camelCase")] +#[cfg_attr(test, derive(Serialize))] pub struct OperationStorageItems { /// The operation id of the event. pub operation_id: String, @@ -541,6 +555,7 @@ pub struct OperationStorageItems { /// Indicate a problem during the operation. #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(rename_all = "camelCase")] +#[cfg_attr(test, derive(Serialize))] pub struct OperationError { /// The operation id of the event. pub operation_id: String, @@ -551,6 +566,7 @@ pub struct OperationError { /// The storage result. #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(rename_all = "camelCase")] +#[cfg_attr(test, derive(Serialize))] pub struct StorageResult { /// The hex-encoded key of the result. pub key: Bytes, @@ -562,6 +578,7 @@ pub struct StorageResult { /// The type of the storage query. #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(rename_all = "camelCase")] +#[cfg_attr(test, derive(Serialize))] pub enum StorageResultType { /// Fetch the value of the provided key. Value(Bytes), @@ -575,6 +592,7 @@ pub enum StorageResultType { #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(rename_all = "camelCase")] #[serde(tag = "result")] +#[cfg_attr(test, derive(Serialize))] pub enum MethodResponse { /// The method has started. Started(MethodResponseStarted), @@ -585,6 +603,7 @@ pub enum MethodResponse { /// The `started` result of a method. #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(rename_all = "camelCase")] +#[cfg_attr(test, derive(Serialize))] pub struct MethodResponseStarted { /// The operation id of the response. pub operation_id: String, @@ -704,6 +723,7 @@ impl Stream for TransactionSubscription { #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(rename_all = "camelCase")] #[serde(tag = "event")] +#[cfg_attr(test, derive(Serialize))] pub enum TransactionStatus { /// Transaction is part of the future queue. Validated, @@ -745,6 +765,7 @@ pub enum TransactionStatus { /// Details of a block that a transaction is seen in. #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] +#[cfg_attr(test, derive(Serialize))] pub struct TransactionBlockDetails { /// The block hash. pub hash: Hash, @@ -778,6 +799,9 @@ pub(crate) mod unsigned_number_as_string { use serde::de::{Deserializer, Visitor}; use std::fmt; + #[cfg(test)] + use serde::ser::Serializer; + /// Deserialize a number from a string or number. pub fn deserialize<'de, N: From, D>(deserializer: D) -> Result where @@ -786,6 +810,14 @@ pub(crate) mod unsigned_number_as_string { deserializer.deserialize_any(NumberVisitor(std::marker::PhantomData)) } + /// Serialize a number as string + #[cfg(test)] + pub fn serialize(item: &u64, serializer: S) -> Result + where + S: Serializer, + { + serializer.serialize_str(&item.to_string()) + } struct NumberVisitor(std::marker::PhantomData); impl<'de, N: From> Visitor<'de> for NumberVisitor { @@ -828,6 +860,9 @@ pub(crate) mod hashmap_as_tuple_list { use std::hash::{BuildHasher, Hash}; use std::marker::PhantomData; + #[cfg(test)] + use serde::ser::{Serialize, SerializeSeq, Serializer}; + /// Deserialize a [`HashMap`] from a list of tuples or object pub fn deserialize<'de, K, V, BH, D>(deserializer: D) -> Result, D::Error> where @@ -839,6 +874,22 @@ pub(crate) mod hashmap_as_tuple_list { deserializer.deserialize_any(HashMapVisitor(PhantomData)) } + /// Serialize a number as string + #[cfg(test)] + pub fn serialize( + item: &HashMap, + serializer: S, + ) -> Result + where + S: Serializer, + { + let mut seq = serializer.serialize_seq(None)?; + for i in item { + seq.serialize_element(&i)?; + } + seq.end() + } + #[allow(clippy::type_complexity)] struct HashMapVisitor(PhantomData HashMap>); From 42af8fd0f3b76e6f7b4301d0dc12c1a19a35f58c Mon Sep 17 00:00:00 2001 From: Pavlo Khrystenko Date: Mon, 9 Sep 2024 12:05:57 +0200 Subject: [PATCH 2/7] clippy --- subxt/src/backend/mod.rs | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/subxt/src/backend/mod.rs b/subxt/src/backend/mod.rs index 4e8b9158ac..471ee2f1d5 100644 --- a/subxt/src/backend/mod.rs +++ b/subxt/src/backend/mod.rs @@ -490,10 +490,9 @@ mod test { meth: &str, params: Option>, ) -> super::rpc::RawRpcFuture<'a, Box> { - let method = self.method_handlers.get(meth).expect(&format!( - "no method named {} registered. Params: {:?}", - meth, params - )); + let method = self.method_handlers.get(meth).unwrap_or_else(|| { + panic!("no method named {} registered. Params: {:?}", meth, params) + }); (*method)(&mut self.data_table, &mut self.subscription_channel, params) } @@ -503,10 +502,12 @@ mod test { sub: &str, params: Option>, ) -> super::rpc::RawRpcFuture<'a, super::rpc::RawRpcSubscription> { - let sub = self.subscription_handlers.get(sub).expect(&format!( - "no subscription named {} registered. Params: {:?}", - sub, params - )); + let sub = self.subscription_handlers.get(sub).unwrap_or_else(|| { + panic!( + "no subscription named {} registered. Params: {:?}", + sub, params + ) + }); (*sub)(&mut self.data_table, &mut self.subscription_channel, params) } @@ -931,7 +932,7 @@ mod test { } let follow_event = FollowEvent::Initialized(Initialized::<::Hash> { - finalized_block_hashes: vec![hash.clone()], + finalized_block_hashes: vec![hash], finalized_block_runtime: Some(rpc_methods::RuntimeEvent::Valid( RuntimeVersionEvent { spec: runtime_spec(), @@ -1032,7 +1033,7 @@ mod test { let (backend, mut driver): (UnstableBackend, _) = UnstableBackend::builder().build(rpc_client); - let _ = tokio::spawn(async move { + tokio::spawn(async move { while let Some(val) = driver.next().await { if let Err(e) = val { eprintln!("Error driving unstable backend: {e}; terminating client"); @@ -1057,7 +1058,7 @@ mod test { assert!(matches!( response.as_slice(), - [Err(Error::Other(s) )] if *s == "errro".to_owned() + [Err(Error::Other(s) )] if s == "errro" )); } @@ -1111,7 +1112,7 @@ mod test { let (backend, mut driver): (UnstableBackend, _) = UnstableBackend::builder().build(rpc_client); - let _ = tokio::spawn(async move { + tokio::spawn(async move { while let Some(val) = driver.next().await { if let Err(e) = val { eprintln!("Error driving unstable backend: {e}; terminating client"); @@ -1259,7 +1260,7 @@ mod test { let (backend, mut driver): (UnstableBackend, _) = UnstableBackend::builder().build(rpc_client); - let _ = tokio::spawn(async move { + tokio::spawn(async move { while let Some(val) = driver.next().await { if let Err(e) = val { eprintln!("Error driving unstable backend: {e}; terminating client"); @@ -1286,7 +1287,7 @@ mod test { Ok(resp1 @ StorageResponse { .. }), Ok(resp2 @ StorageResponse { .. }), Err(Error::Other(s)) - ] if *s == "errro".to_owned() + ] if s == "errro" && compare_storage_responses(&storage_response("ID1", "Data1"), resp1) && compare_storage_responses(&storage_response("ID2", "Data2"), resp2) )); @@ -1330,7 +1331,7 @@ mod test { "chainSpec_v1_genesisHash", Message::Single(Err(RpcError::DisconnectedWillReconnect("Error".to_owned()))), ), - ("chainSpec_v1_genesisHash", Message::Single(Ok(hash.into()))), + ("chainSpec_v1_genesisHash", Message::Single(Ok(hash))), ]; let rpc_client = setup_mock_rpc_client(false) .add_method( @@ -1348,7 +1349,7 @@ mod test { let (backend, mut driver): (UnstableBackend, _) = UnstableBackend::builder().build(rpc_client); - let _ = tokio::spawn(async move { + tokio::spawn(async move { while let Some(val) = driver.next().await { if let Err(e) = val { eprintln!("Error driving unstable backend: {e}; terminating client"); @@ -1415,7 +1416,7 @@ mod test { let rpc_params = jsonrpsee::types::Params::new(params.as_deref()); let key: String = rpc_params.sequence().next().unwrap(); - if key == "ID1".to_owned() { + if key == *"ID1" { return Err(RpcError::RequestRejected("stale id".into())); } else { subscription_expired From 4fd47def57b3d960ec54cb63d2a889921f0b2da5 Mon Sep 17 00:00:00 2001 From: Pavlo Khrystenko Date: Mon, 23 Sep 2024 08:50:55 +0200 Subject: [PATCH 3/7] review comments --- subxt/src/backend/mod.rs | 138 ++++++++++------------ subxt/src/backend/unstable/rpc_methods.rs | 66 +++++++---- 2 files changed, 103 insertions(+), 101 deletions(-) diff --git a/subxt/src/backend/mod.rs b/subxt/src/backend/mod.rs index 471ee2f1d5..c25f2e7190 100644 --- a/subxt/src/backend/mod.rs +++ b/subxt/src/backend/mod.rs @@ -477,27 +477,30 @@ mod test { } } - struct Data { + struct InnerMockedRpcClient { data_table: MockDataTable, subscription_channel: Option, subscription_handlers: HashMap, method_handlers: HashMap, } - impl Data { - async fn call_meth<'a>( + impl InnerMockedRpcClient { + async fn call<'a>( &'a mut self, - meth: &str, + method_handler: &str, params: Option>, ) -> super::rpc::RawRpcFuture<'a, Box> { - let method = self.method_handlers.get(meth).unwrap_or_else(|| { - panic!("no method named {} registered. Params: {:?}", meth, params) + let method = self.method_handlers.get(method_handler).unwrap_or_else(|| { + panic!( + "no method named {} registered. Params: {:?}", + method_handler, params + ) }); (*method)(&mut self.data_table, &mut self.subscription_channel, params) } - async fn call_subscription<'a>( + async fn subscribe<'a>( &'a mut self, sub: &str, params: Option>, @@ -513,13 +516,13 @@ mod test { } } pub struct MockRpcBuilder { - data: Data, + data: InnerMockedRpcClient, } impl MockRpcBuilder { pub fn new() -> Self { Self { - data: Data { + data: InnerMockedRpcClient { data_table: MockDataTable::new(), subscription_channel: None, subscription_handlers: HashMap::new(), @@ -528,8 +531,10 @@ mod test { } } - pub fn add_method(mut self, method_name: &str, meth: MethodHandler) -> Self { - self.data.method_handlers.insert(method_name.into(), meth); + pub fn add_method(mut self, method_name: &str, method_handler: MethodHandler) -> Self { + self.data + .method_handlers + .insert(method_name.into(), method_handler); self } @@ -544,7 +549,7 @@ mod test { self } - pub fn add_mock_data_from_iter< + pub fn add_mock_data< 'a, T: Serialize, I: IntoIterator>)>, @@ -567,7 +572,7 @@ mod test { } pub struct MockRpcClient { - data: Arc>, + data: Arc>, } impl RpcClientT for MockRpcClient { @@ -578,7 +583,7 @@ mod test { ) -> super::rpc::RawRpcFuture<'a, Box> { Box::pin(async { let mut data = self.data.lock().await; - data.call_meth(method, params).await.await + data.call(method, params).await.await }) } @@ -590,7 +595,7 @@ mod test { ) -> super::rpc::RawRpcFuture<'a, super::rpc::RawRpcSubscription> { Box::pin(async { let mut data = self.data.lock().await; - data.call_subscription(sub, params).await.await + data.subscribe(sub, params).await.await }) } } @@ -682,7 +687,7 @@ mod test { ), ("ID3", Message::Single(bytes("Data3"))), ]; - let rpc_client = setup_mock_rpc().add_mock_data_from_iter(mock_data).build(); + let rpc_client = setup_mock_rpc().add_mock_data(mock_data).build(); let backend: LegacyBackend = LegacyBackend::builder().build(rpc_client); // Test @@ -720,7 +725,7 @@ mod test { ), ("ID1", Message::Single(bytes("Data1"))), ]; - let rpc_client = setup_mock_rpc().add_mock_data_from_iter(mock_data).build(); + let rpc_client = setup_mock_rpc().add_mock_data(mock_data).build(); // Test let backend: LegacyBackend = LegacyBackend::builder().build(rpc_client); @@ -759,7 +764,7 @@ mod test { ), ("chain_getBlockHash", Message::Single(Ok(Some(hash)))), ]; - let rpc_client = setup_mock_rpc().add_mock_data_from_iter(mock_data).build(); + let rpc_client = setup_mock_rpc().add_mock_data(mock_data).build(); // Test let backend: LegacyBackend = LegacyBackend::builder().build(rpc_client); @@ -845,7 +850,7 @@ mod test { }) }), ) - .add_mock_data_from_iter(mock_subscription_data) + .add_mock_data(mock_subscription_data) .build(); // Test @@ -888,6 +893,26 @@ mod test { use super::unstable::*; use super::*; + fn run_backend( + rpc_client: impl RpcClientT, + ) -> (UnstableBackend, UnstableBackendDriver) { + let (backend, driver): (UnstableBackend, _) = + UnstableBackend::builder().build(rpc_client); + (backend, driver) + } + + fn run_backend_spawn_background(rpc_client: impl RpcClientT) -> UnstableBackend { + let (backend, mut driver) = run_backend(rpc_client); + tokio::spawn(async move { + while let Some(val) = driver.next().await { + if let Err(e) = val { + eprintln!("Error driving unstable backend: {e}; terminating client"); + } + } + }); + backend + } + fn runtime_spec() -> RuntimeSpec { let spec = serde_json::json!({ "specName": "westend", @@ -971,7 +996,7 @@ mod test { fn operation_error(id: &str) -> FollowEvent { FollowEvent::OperationError(OperationError { operation_id: id.to_owned(), - error: "errro".to_owned(), + error: "error".to_owned(), }) } @@ -1026,20 +1051,11 @@ mod test { }) }), ) - .add_mock_data_from_iter(mock_subscription_data) - .add_mock_data_from_iter(response_data) + .add_mock_data(mock_subscription_data) + .add_mock_data(response_data) .build(); - let (backend, mut driver): (UnstableBackend, _) = - UnstableBackend::builder().build(rpc_client); - - tokio::spawn(async move { - while let Some(val) = driver.next().await { - if let Err(e) = val { - eprintln!("Error driving unstable backend: {e}; terminating client"); - } - } - }); + let backend = run_backend_spawn_background(rpc_client); // Test // This request should encounter an error on `request` and do a retry. @@ -1058,7 +1074,7 @@ mod test { assert!(matches!( response.as_slice(), - [Err(Error::Other(s) )] if s == "errro" + [Err(Error::Other(s) )] if s == "error" )); } @@ -1106,19 +1122,10 @@ mod test { }) }), ) - .add_mock_data_from_iter(mock_data) - .add_mock_data_from_iter(response_data) + .add_mock_data(mock_data) + .add_mock_data(response_data) .build(); - let (backend, mut driver): (UnstableBackend, _) = - UnstableBackend::builder().build(rpc_client); - - tokio::spawn(async move { - while let Some(val) = driver.next().await { - if let Err(e) = val { - eprintln!("Error driving unstable backend: {e}; terminating client"); - } - } - }); + let backend = run_backend_spawn_background(rpc_client); // We try again and should succeed let response = backend @@ -1253,20 +1260,11 @@ mod test { }) }), ) - .add_mock_data_from_iter(mock_data) - .add_mock_data_from_iter(response_data) - .add_mock_data_from_iter(continue_data) + .add_mock_data(mock_data) + .add_mock_data(response_data) + .add_mock_data(continue_data) .build(); - let (backend, mut driver): (UnstableBackend, _) = - UnstableBackend::builder().build(rpc_client); - - tokio::spawn(async move { - while let Some(val) = driver.next().await { - if let Err(e) = val { - eprintln!("Error driving unstable backend: {e}; terminating client"); - } - } - }); + let backend = run_backend_spawn_background(rpc_client); // We try again and should fail mid way let response = backend @@ -1287,7 +1285,7 @@ mod test { Ok(resp1 @ StorageResponse { .. }), Ok(resp2 @ StorageResponse { .. }), Err(Error::Other(s)) - ] if s == "errro" + ] if s == "error" && compare_storage_responses(&storage_response("ID1", "Data1"), resp1) && compare_storage_responses(&storage_response("ID2", "Data2"), resp2) )); @@ -1343,19 +1341,10 @@ mod test { }) }), ) - .add_mock_data_from_iter(mock_data) + .add_mock_data(mock_data) .build(); - let (backend, mut driver): (UnstableBackend, _) = - UnstableBackend::builder().build(rpc_client); - - tokio::spawn(async move { - while let Some(val) = driver.next().await { - if let Err(e) = val { - eprintln!("Error driving unstable backend: {e}; terminating client"); - } - } - }); + let backend = run_backend_spawn_background(rpc_client); // Test // This request should encounter an error on `request` and do a retry. @@ -1439,11 +1428,10 @@ mod test { } }), ) - .add_mock_data_from_iter(mock_data) - .add_mock_data_from_iter(response_data) + .add_mock_data(mock_data) + .add_mock_data(response_data) .build(); - let (backend, mut driver): (UnstableBackend, _) = - UnstableBackend::builder().build(rpc_client); + let (backend, mut driver): (UnstableBackend, _) = run_backend(rpc_client); let _ = driver.next().await.unwrap(); let _ = driver.next().await.unwrap(); @@ -1466,7 +1454,7 @@ mod test { assert!(matches!( response, - Err(Error::Other(reason)) if reason == "errro" + Err(Error::Other(reason)) if reason == "error" )); // not getting new subscription id and hitting request rejected > 10 times diff --git a/subxt/src/backend/unstable/rpc_methods.rs b/subxt/src/backend/unstable/rpc_methods.rs index 25e3fd1068..a758f72797 100644 --- a/subxt/src/backend/unstable/rpc_methods.rs +++ b/subxt/src/backend/unstable/rpc_methods.rs @@ -459,7 +459,11 @@ pub struct RuntimeSpec { /// /// **Note:** In Substrate, the keys in the apis field consists of the hexadecimal-encoded 8-bytes blake2 /// hash of the name of the API. For example, the `TaggedTransactionQueue` API is 0xd2bc9897eed08f15. - #[serde(with = "hashmap_as_tuple_list")] + #[serde(deserialize_with = "hashmap_as_tuple_list::deserialize")] + #[cfg_attr( + test, + serde(serialize_with = "hashmap_as_tuple_list::for_test::serialize") + )] pub apis: HashMap, } @@ -770,7 +774,11 @@ pub struct TransactionBlockDetails { /// The block hash. pub hash: Hash, /// The index of the transaction in the block. - #[serde(with = "unsigned_number_as_string")] + #[serde(deserialize_with = "unsigned_number_as_string::deserialize")] + #[cfg_attr( + test, + serde(serialize_with = "unsigned_number_as_string::for_test::serialize") + )] pub index: u64, } @@ -799,9 +807,6 @@ pub(crate) mod unsigned_number_as_string { use serde::de::{Deserializer, Visitor}; use std::fmt; - #[cfg(test)] - use serde::ser::Serializer; - /// Deserialize a number from a string or number. pub fn deserialize<'de, N: From, D>(deserializer: D) -> Result where @@ -810,14 +815,19 @@ pub(crate) mod unsigned_number_as_string { deserializer.deserialize_any(NumberVisitor(std::marker::PhantomData)) } - /// Serialize a number as string #[cfg(test)] - pub fn serialize(item: &u64, serializer: S) -> Result - where - S: Serializer, - { - serializer.serialize_str(&item.to_string()) + pub mod for_test { + use serde::ser::Serializer; + + /// Serialize a number as string + pub fn serialize(item: &u64, serializer: S) -> Result + where + S: Serializer, + { + serializer.serialize_str(&item.to_string()) + } } + struct NumberVisitor(std::marker::PhantomData); impl<'de, N: From> Visitor<'de> for NumberVisitor { @@ -860,9 +870,6 @@ pub(crate) mod hashmap_as_tuple_list { use std::hash::{BuildHasher, Hash}; use std::marker::PhantomData; - #[cfg(test)] - use serde::ser::{Serialize, SerializeSeq, Serializer}; - /// Deserialize a [`HashMap`] from a list of tuples or object pub fn deserialize<'de, K, V, BH, D>(deserializer: D) -> Result, D::Error> where @@ -874,20 +881,27 @@ pub(crate) mod hashmap_as_tuple_list { deserializer.deserialize_any(HashMapVisitor(PhantomData)) } - /// Serialize a number as string #[cfg(test)] - pub fn serialize( - item: &HashMap, - serializer: S, - ) -> Result - where - S: Serializer, - { - let mut seq = serializer.serialize_seq(None)?; - for i in item { - seq.serialize_element(&i)?; + pub mod for_test { + use std::collections::HashMap; + use std::hash::Hash; + + use serde::ser::{Serialize, SerializeSeq, Serializer}; + + /// Serialize hashmap as list of tuples + pub fn serialize( + item: &HashMap, + serializer: S, + ) -> Result + where + S: Serializer, + { + let mut seq = serializer.serialize_seq(None)?; + for i in item { + seq.serialize_element(&i)?; + } + seq.end() } - seq.end() } #[allow(clippy::type_complexity)] From 33960020b601731a8e30e4501e8cad5a94a10640 Mon Sep 17 00:00:00 2001 From: Pavlo Khrystenko Date: Mon, 23 Sep 2024 11:21:08 +0200 Subject: [PATCH 4/7] review comments 2: the sequel --- subxt/src/backend/mod.rs | 40 ++++++------- subxt/src/backend/unstable/rpc_methods.rs | 70 +++++++++++------------ 2 files changed, 52 insertions(+), 58 deletions(-) diff --git a/subxt/src/backend/mod.rs b/subxt/src/backend/mod.rs index c25f2e7190..fbae3f22f3 100644 --- a/subxt/src/backend/mod.rs +++ b/subxt/src/backend/mod.rs @@ -358,10 +358,8 @@ mod test { } } pub mod rpc_client { - - use std::time::Duration; - use super::*; + use std::time::Duration; pub type SubscriptionHandler = Box< dyn for<'a> Fn( @@ -389,16 +387,16 @@ mod test { } impl Message { - pub fn single(self) -> T { + pub fn unwrap_single(self) -> T { match self { Self::Single(s) => s, - _ => todo!(), + _ => panic!(), } } - pub fn many(self) -> RpcResult> { + pub fn unwrap_many(self) -> RpcResult> { match self { Self::Many(s) => s, - _ => todo!(), + _ => panic!(), } } } @@ -426,12 +424,7 @@ mod test { Message::Single(item.map(|x| serde_json::to_string(&x).unwrap())) } }; - match self.items.entry(key) { - std::collections::hash_map::Entry::Occupied(v) => v.into_mut().push_back(item), - std::collections::hash_map::Entry::Vacant(e) => { - e.insert(VecDeque::from([item])); - } - } + self.items.entry(key).or_default().push_back(item); } pub fn pop(&mut self, key: Vec) -> Message { @@ -631,7 +624,7 @@ mod test { let params = params.map(|p| p.get().to_string()); let rpc_params = jsonrpsee::types::Params::new(params.as_deref()); let key: sp_core::Bytes = rpc_params.sequence().next().unwrap(); - let value = data.pop(key.0).single(); + let value = data.pop(key.0).unwrap_single(); value.map(|v| serde_json::value::RawValue::from_string(v).unwrap()) }) }), @@ -640,7 +633,7 @@ mod test { "chain_getBlockHash", Box::new(|data, _, _| { Box::pin(async move { - let value = data.pop("chain_getBlockHash".into()).single(); + let value = data.pop("chain_getBlockHash".into()).unwrap_single(); value.map(|v| serde_json::value::RawValue::from_string(v).unwrap()) }) }), @@ -832,7 +825,9 @@ mod test { "state_subscribeRuntimeVersion", Box::new(|data, _, _| { Box::pin(async move { - let values = data.pop("state_subscribeRuntimeVersion".into()).many(); + let values = data + .pop("state_subscribeRuntimeVersion".into()) + .unwrap_many(); let values: RpcResult>>> = values.map(|v| { v.into_iter() @@ -1039,7 +1034,7 @@ mod test { "chainHead_v1_storage", Box::new(|data, sub, _| { Box::pin(async move { - let response = data.pop("method_response".into()).single(); + let response = data.pop("method_response".into()).unwrap_single(); if response.is_ok() { let item = data.pop("chainHead_v1_storage".into()); if let Some(sub) = sub { @@ -1110,7 +1105,7 @@ mod test { "chainHead_v1_storage", Box::new(|data, sub, _| { Box::pin(async move { - let response = data.pop("method_response".into()).single(); + let response = data.pop("method_response".into()).unwrap_single(); if response.is_ok() { let item = data.pop("chainHead_v1_storage".into()); if let Some(sub) = sub { @@ -1232,7 +1227,7 @@ mod test { "chainHead_v1_storage", Box::new(|data, sub, _| { Box::pin(async move { - let response = data.pop("method_response".into()).single(); + let response = data.pop("method_response".into()).unwrap_single(); if response.is_ok() { let item = data.pop("chainHead_v1_storage".into()); if let Some(sub) = sub { @@ -1248,7 +1243,7 @@ mod test { "chainHead_v1_continue", Box::new(|data, sub, _| { Box::pin(async move { - let response = data.pop("continue_response".into()).single(); + let response = data.pop("continue_response".into()).unwrap_single(); if response.is_ok() { let item = data.pop("chainHead_v1_storage".into()); if let Some(sub) = sub { @@ -1336,7 +1331,8 @@ mod test { "chainSpec_v1_genesisHash", Box::new(|data, _, _| { Box::pin(async move { - let response = data.pop("chainSpec_v1_genesisHash".into()).single(); + let response = + data.pop("chainSpec_v1_genesisHash".into()).unwrap_single(); response.map(|x| RawValue::from_string(x).unwrap()) }) }), @@ -1412,7 +1408,7 @@ mod test { .swap(false, std::sync::atomic::Ordering::SeqCst); } } - let response = data.pop("method_response".into()).single(); + let response = data.pop("method_response".into()).unwrap_single(); if response.is_ok() { let item = data.pop("chainHead_v1_storage".into()); if let Some(sub) = sub { diff --git a/subxt/src/backend/unstable/rpc_methods.rs b/subxt/src/backend/unstable/rpc_methods.rs index a758f72797..0e730c4fb1 100644 --- a/subxt/src/backend/unstable/rpc_methods.rs +++ b/subxt/src/backend/unstable/rpc_methods.rs @@ -815,19 +815,6 @@ pub(crate) mod unsigned_number_as_string { deserializer.deserialize_any(NumberVisitor(std::marker::PhantomData)) } - #[cfg(test)] - pub mod for_test { - use serde::ser::Serializer; - - /// Serialize a number as string - pub fn serialize(item: &u64, serializer: S) -> Result - where - S: Serializer, - { - serializer.serialize_str(&item.to_string()) - } - } - struct NumberVisitor(std::marker::PhantomData); impl<'de, N: From> Visitor<'de> for NumberVisitor { @@ -846,6 +833,18 @@ pub(crate) mod unsigned_number_as_string { Ok(v.into()) } } + #[cfg(test)] + pub mod for_test { + use serde::ser::Serializer; + + /// Serialize a number as string + pub fn serialize(item: &u64, serializer: S) -> Result + where + S: Serializer, + { + serializer.serialize_str(&item.to_string()) + } + } } /// A temporary shim to decode "spec.apis" if it comes back as an array like: @@ -881,29 +880,6 @@ pub(crate) mod hashmap_as_tuple_list { deserializer.deserialize_any(HashMapVisitor(PhantomData)) } - #[cfg(test)] - pub mod for_test { - use std::collections::HashMap; - use std::hash::Hash; - - use serde::ser::{Serialize, SerializeSeq, Serializer}; - - /// Serialize hashmap as list of tuples - pub fn serialize( - item: &HashMap, - serializer: S, - ) -> Result - where - S: Serializer, - { - let mut seq = serializer.serialize_seq(None)?; - for i in item { - seq.serialize_element(&i)?; - } - seq.end() - } - } - #[allow(clippy::type_complexity)] struct HashMapVisitor(PhantomData HashMap>); @@ -945,6 +921,28 @@ pub(crate) mod hashmap_as_tuple_list { Ok(map) } } + #[cfg(test)] + pub mod for_test { + use std::collections::HashMap; + use std::hash::Hash; + + use serde::ser::{Serialize, SerializeSeq, Serializer}; + + /// Serialize hashmap as list of tuples + pub fn serialize( + item: &HashMap, + serializer: S, + ) -> Result + where + S: Serializer, + { + let mut seq = serializer.serialize_seq(None)?; + for i in item { + seq.serialize_element(&i)?; + } + seq.end() + } + } } #[cfg(test)] From b0126ba89991839c5d9073ddde4ad5f50daf4827 Mon Sep 17 00:00:00 2001 From: Pavlo Khrystenko Date: Tue, 24 Sep 2024 14:11:34 +0200 Subject: [PATCH 5/7] review comments --- subxt/src/backend/mod.rs | 50 +++++++------------ .../src/full_client/client/mod.rs | 2 +- 2 files changed, 19 insertions(+), 33 deletions(-) diff --git a/subxt/src/backend/mod.rs b/subxt/src/backend/mod.rs index fbae3f22f3..6cebaa7e33 100644 --- a/subxt/src/backend/mod.rs +++ b/subxt/src/backend/mod.rs @@ -336,17 +336,18 @@ pub struct StorageResponse { #[cfg(test)] mod test { use super::*; + pub use crate::backend::rpc::{RawRpcFuture, RawRpcSubscription}; pub use crate::{backend::StorageResponse, error::RpcError}; pub use futures::StreamExt; + pub use rpc::RpcClientT; pub use serde::Serialize; + pub use serde_json::value::RawValue; pub use std::collections::{HashMap, VecDeque}; pub use subxt_core::{config::DefaultExtrinsicParams, Config}; pub use tokio::sync::{mpsc, Mutex}; + pub type RpcResult = Result; pub type Item = RpcResult; - pub use crate::backend::rpc::RawRpcSubscription; - pub use rpc::RpcClientT; - pub use serde_json::value::RawValue; fn storage_response>, V: Into>>(key: K, value: V) -> StorageResponse where @@ -366,8 +367,7 @@ mod test { &'a mut MockDataTable, &'a mut Option, Option>, - ) - -> super::rpc::RawRpcFuture<'a, super::rpc::RawRpcSubscription> + ) -> RawRpcFuture<'a, RawRpcSubscription> + Send, >; @@ -376,8 +376,7 @@ mod test { &'a mut MockDataTable, &'a mut Option, Option>, - ) - -> super::rpc::RawRpcFuture<'a, Box> + ) -> RawRpcFuture<'a, Box> + Send, >; @@ -401,17 +400,12 @@ mod test { } } + #[derive(Default)] pub struct MockDataTable { items: HashMap, VecDeque>>, } impl MockDataTable { - pub fn new() -> Self { - MockDataTable { - items: HashMap::new(), - } - } - pub fn push(&mut self, key: Vec, item: Message>) { let item = match item { Message::Many(items) => Message::Many(items.map(|items| { @@ -470,6 +464,7 @@ mod test { } } + #[derive(Default)] struct InnerMockedRpcClient { data_table: MockDataTable, subscription_channel: Option, @@ -482,7 +477,7 @@ mod test { &'a mut self, method_handler: &str, params: Option>, - ) -> super::rpc::RawRpcFuture<'a, Box> { + ) -> RawRpcFuture<'a, Box> { let method = self.method_handlers.get(method_handler).unwrap_or_else(|| { panic!( "no method named {} registered. Params: {:?}", @@ -497,7 +492,7 @@ mod test { &'a mut self, sub: &str, params: Option>, - ) -> super::rpc::RawRpcFuture<'a, super::rpc::RawRpcSubscription> { + ) -> RawRpcFuture<'a, RawRpcSubscription> { let sub = self.subscription_handlers.get(sub).unwrap_or_else(|| { panic!( "no subscription named {} registered. Params: {:?}", @@ -508,22 +503,13 @@ mod test { (*sub)(&mut self.data_table, &mut self.subscription_channel, params) } } + + #[derive(Default)] pub struct MockRpcBuilder { data: InnerMockedRpcClient, } impl MockRpcBuilder { - pub fn new() -> Self { - Self { - data: InnerMockedRpcClient { - data_table: MockDataTable::new(), - subscription_channel: None, - subscription_handlers: HashMap::new(), - method_handlers: HashMap::new(), - }, - } - } - pub fn add_method(mut self, method_name: &str, method_handler: MethodHandler) -> Self { self.data .method_handlers @@ -573,7 +559,7 @@ mod test { &'a self, method: &'a str, params: Option>, - ) -> super::rpc::RawRpcFuture<'a, Box> { + ) -> RawRpcFuture<'a, Box> { Box::pin(async { let mut data = self.data.lock().await; data.call(method, params).await.await @@ -585,7 +571,7 @@ mod test { sub: &'a str, params: Option>, _unsub: &'a str, - ) -> super::rpc::RawRpcFuture<'a, super::rpc::RawRpcSubscription> { + ) -> RawRpcFuture<'a, RawRpcSubscription> { Box::pin(async { let mut data = self.data.lock().await; data.subscribe(sub, params).await.await @@ -604,7 +590,6 @@ mod test { type Hasher = crate::config::substrate::BlakeTwo256; type Header = crate::config::substrate::SubstrateHeader; type ExtrinsicParams = DefaultExtrinsicParams; - type AssetId = u32; } @@ -616,7 +601,7 @@ mod test { use rpc_client::*; pub fn setup_mock_rpc() -> MockRpcBuilder { - MockRpcBuilder::new() + MockRpcBuilder::default() .add_method( "state_getStorage", Box::new(|data, _sub, params| { @@ -943,7 +928,7 @@ mod test { fn setup_mock_rpc_client(cycle_ids: bool) -> MockRpcBuilder { let hash = init_hash(); let mut id = 0; - rpc_client::MockRpcBuilder::new().add_subscription( + rpc_client::MockRpcBuilder::default().add_subscription( "chainHead_v1_follow", Box::new(move |_, sub, _| { Box::pin(async move { @@ -1350,7 +1335,8 @@ mod test { } #[tokio::test] - // Failure as we do not wait for subscription id to be updated + // Failure as we do not wait for subscription id to be updated. + // see https://github.com/paritytech/subxt/issues/1567 async fn stale_subscription_id_failure() { let response_data = vec![ ( diff --git a/testing/integration-tests/src/full_client/client/mod.rs b/testing/integration-tests/src/full_client/client/mod.rs index 9835af28f3..b3b55f933a 100644 --- a/testing/integration-tests/src/full_client/client/mod.rs +++ b/testing/integration-tests/src/full_client/client/mod.rs @@ -361,7 +361,7 @@ pub struct InclusionFee { /// - `targeted_fee_adjustment`: This is a multiplier that can tune the final fee based on the /// congestion of the network. /// - `weight_fee`: This amount is computed based on the weight of the transaction. Weight - /// accounts for the execution time of a transaction. + /// accounts for the execution time of a transaction. /// /// adjusted_weight_fee = targeted_fee_adjustment * weight_fee pub adjusted_weight_fee: u128, From 217c3e9a28f434a73aecac5cb66f03b6e27d6aef Mon Sep 17 00:00:00 2001 From: Pavlo Khrystenko Date: Thu, 26 Sep 2024 11:58:34 +0200 Subject: [PATCH 6/7] review comments --- subxt/src/backend/mod.rs | 351 +++++++++++++++++++-------------------- 1 file changed, 169 insertions(+), 182 deletions(-) diff --git a/subxt/src/backend/mod.rs b/subxt/src/backend/mod.rs index 6cebaa7e33..0287e80910 100644 --- a/subxt/src/backend/mod.rs +++ b/subxt/src/backend/mod.rs @@ -342,6 +342,7 @@ mod test { pub use rpc::RpcClientT; pub use serde::Serialize; pub use serde_json::value::RawValue; + pub use sp_core::H256; pub use std::collections::{HashMap, VecDeque}; pub use subxt_core::{config::DefaultExtrinsicParams, Config}; pub use tokio::sync::{mpsc, Mutex}; @@ -349,6 +350,10 @@ mod test { pub type RpcResult = Result; pub type Item = RpcResult; + fn random_hash() -> H256 { + H256::random() + } + fn storage_response>, V: Into>>(key: K, value: V) -> StorageResponse where Vec: From, @@ -389,13 +394,13 @@ mod test { pub fn unwrap_single(self) -> T { match self { Self::Single(s) => s, - _ => panic!(), + _ => panic!("cannot unwrap_single on Message::Many"), } } pub fn unwrap_many(self) -> RpcResult> { match self { Self::Many(s) => s, - _ => panic!(), + _ => panic!("cannot unwrap_many on Message::Single"), } } } @@ -510,21 +515,40 @@ mod test { } impl MockRpcBuilder { - pub fn add_method(mut self, method_name: &str, method_handler: MethodHandler) -> Self { + pub fn add_method(mut self, method_name: &str, method_handler: F) -> Self + where + F: Send + + for<'a> Fn( + &'a mut MockDataTable, + &'a mut Option, + Option>, + ) + -> RawRpcFuture<'a, Box> + + 'static, + { self.data .method_handlers - .insert(method_name.into(), method_handler); + .insert(method_name.into(), Box::new(method_handler)); self } - pub fn add_subscription( + pub fn add_subscription( mut self, subscription_name: &str, - subscription_handler: SubscriptionHandler, - ) -> Self { + subscription_handler: F, + ) -> Self + where + F: Send + + for<'a> Fn( + &'a mut MockDataTable, + &'a mut Option, + Option>, + ) -> RawRpcFuture<'a, RawRpcSubscription> + + 'static, + { self.data .subscription_handlers - .insert(subscription_name.into(), subscription_handler); + .insert(subscription_name.into(), Box::new(subscription_handler)); self } @@ -583,7 +607,7 @@ mod test { // Define dummy config enum Conf {} impl Config for Conf { - type Hash = crate::utils::H256; + type Hash = H256; type AccountId = crate::utils::AccountId32; type Address = crate::utils::MultiAddress; type Signature = crate::utils::MultiSignature; @@ -602,27 +626,21 @@ mod test { pub fn setup_mock_rpc() -> MockRpcBuilder { MockRpcBuilder::default() - .add_method( - "state_getStorage", - Box::new(|data, _sub, params| { - Box::pin(async move { - let params = params.map(|p| p.get().to_string()); - let rpc_params = jsonrpsee::types::Params::new(params.as_deref()); - let key: sp_core::Bytes = rpc_params.sequence().next().unwrap(); - let value = data.pop(key.0).unwrap_single(); - value.map(|v| serde_json::value::RawValue::from_string(v).unwrap()) - }) - }), - ) - .add_method( - "chain_getBlockHash", - Box::new(|data, _, _| { - Box::pin(async move { - let value = data.pop("chain_getBlockHash".into()).unwrap_single(); - value.map(|v| serde_json::value::RawValue::from_string(v).unwrap()) - }) - }), - ) + .add_method("state_getStorage", |data, _sub, params| { + Box::pin(async move { + let params = params.map(|p| p.get().to_string()); + let rpc_params = jsonrpsee::types::Params::new(params.as_deref()); + let key: sp_core::Bytes = rpc_params.sequence().next().unwrap(); + let value = data.pop(key.0).unwrap_single(); + value.map(|v| serde_json::value::RawValue::from_string(v).unwrap()) + }) + }) + .add_method("chain_getBlockHash", |data, _, _| { + Box::pin(async move { + let value = data.pop("chain_getBlockHash".into()).unwrap_single(); + value.map(|v| serde_json::value::RawValue::from_string(v).unwrap()) + }) + }) } use crate::backend::Backend; @@ -672,7 +690,7 @@ mod test { let response = backend .storage_fetch_values( ["ID1".into(), "ID2".into(), "ID3".into()].into(), - crate::utils::H256::random(), + random_hash(), ) .await .unwrap(); @@ -708,7 +726,7 @@ mod test { // Test let backend: LegacyBackend = LegacyBackend::builder().build(rpc_client); let response = backend - .storage_fetch_value("ID1".into(), crate::utils::H256::random()) + .storage_fetch_value("ID1".into(), random_hash()) .await .unwrap(); @@ -732,7 +750,7 @@ mod test { /// } /// ``` async fn simple_fetch() { - let hash = crate::utils::H256::random(); + let hash = random_hash(); let mock_data = vec![ ( "chain_getBlockHash", @@ -806,30 +824,24 @@ mod test { ), ]; let rpc_client = setup_mock_rpc() - .add_subscription( - "state_subscribeRuntimeVersion", - Box::new(|data, _, _| { - Box::pin(async move { - let values = data - .pop("state_subscribeRuntimeVersion".into()) - .unwrap_many(); - let values: RpcResult>>> = - values.map(|v| { - v.into_iter() - .map(|v| { - v.map(|v| { - serde_json::value::RawValue::from_string(v).unwrap() - }) - }) - .collect::>>>() - }); - values.map(|v| RawRpcSubscription { - stream: futures::stream::iter(v).boxed(), - id: Some("ID".to_string()), - }) + .add_subscription("state_subscribeRuntimeVersion", |data, _, _| { + Box::pin(async move { + let values = data + .pop("state_subscribeRuntimeVersion".into()) + .unwrap_many(); + let values: RpcResult>>> = values.map(|v| { + v.into_iter() + .map(|v| { + v.map(|v| serde_json::value::RawValue::from_string(v).unwrap()) + }) + .collect::>>>() + }); + values.map(|v| RawRpcSubscription { + stream: futures::stream::iter(v).boxed(), + id: Some("ID".to_string()), }) - }), - ) + }) + }) .add_mock_data(mock_subscription_data) .build(); @@ -868,12 +880,11 @@ mod test { Bytes, Initialized, MethodResponse, MethodResponseStarted, OperationError, OperationId, OperationStorageItems, RuntimeSpec, RuntimeVersionEvent, }; - use sp_core::H256; use super::unstable::*; use super::*; - fn run_backend( + fn build_backend( rpc_client: impl RpcClientT, ) -> (UnstableBackend, UnstableBackendDriver) { let (backend, driver): (UnstableBackend, _) = @@ -881,8 +892,8 @@ mod test { (backend, driver) } - fn run_backend_spawn_background(rpc_client: impl RpcClientT) -> UnstableBackend { - let (backend, mut driver) = run_backend(rpc_client); + fn build_backend_spawn_background(rpc_client: impl RpcClientT) -> UnstableBackend { + let (backend, mut driver) = build_backend(rpc_client); tokio::spawn(async move { while let Some(val) = driver.next().await { if let Err(e) = val { @@ -919,18 +930,15 @@ mod test { }); serde_json::from_value(spec).unwrap() } - fn init_hash() -> crate::utils::H256 { - crate::utils::H256::random() - } type FollowEvent = unstable::rpc_methods::FollowEvent<::Hash>; fn setup_mock_rpc_client(cycle_ids: bool) -> MockRpcBuilder { - let hash = init_hash(); + let hash = random_hash(); let mut id = 0; rpc_client::MockRpcBuilder::default().add_subscription( "chainHead_v1_follow", - Box::new(move |_, sub, _| { + move |_, sub, _| { Box::pin(async move { if cycle_ids { id += 1; @@ -962,7 +970,7 @@ mod test { }; Ok(stream) }) - }), + }, ) } @@ -1015,34 +1023,31 @@ mod test { Message::Many(Ok(vec![Ok(operation_error("Id1")), Ok(FollowEvent::Stop)])), )]; let rpc_client = setup_mock_rpc_client(false) - .add_method( - "chainHead_v1_storage", - Box::new(|data, sub, _| { - Box::pin(async move { - let response = data.pop("method_response".into()).unwrap_single(); - if response.is_ok() { - let item = data.pop("chainHead_v1_storage".into()); - if let Some(sub) = sub { - let item = item; - sub.write_delayed(item).await - } + .add_method("chainHead_v1_storage", |data, sub, _| { + Box::pin(async move { + let response = data.pop("method_response".into()).unwrap_single(); + if response.is_ok() { + let item = data.pop("chainHead_v1_storage".into()); + if let Some(sub) = sub { + let item = item; + sub.write_delayed(item).await } - response.map(|x| RawValue::from_string(x).unwrap()) - }) - }), - ) + } + response.map(|x| RawValue::from_string(x).unwrap()) + }) + }) .add_mock_data(mock_subscription_data) .add_mock_data(response_data) .build(); - let backend = run_backend_spawn_background(rpc_client); + let backend = build_backend_spawn_background(rpc_client); // Test // This request should encounter an error on `request` and do a retry. let response = backend .storage_fetch_values( ["ID1".into(), "ID2".into(), "ID3".into()].into(), - crate::utils::H256::random(), + random_hash(), ) .await .unwrap(); @@ -1086,32 +1091,29 @@ mod test { ])), )]; let rpc_client = setup_mock_rpc_client(false) - .add_method( - "chainHead_v1_storage", - Box::new(|data, sub, _| { - Box::pin(async move { - let response = data.pop("method_response".into()).unwrap_single(); - if response.is_ok() { - let item = data.pop("chainHead_v1_storage".into()); - if let Some(sub) = sub { - let item = item; - sub.write_delayed(item).await - } + .add_method("chainHead_v1_storage", |data, sub, _| { + Box::pin(async move { + let response = data.pop("method_response".into()).unwrap_single(); + if response.is_ok() { + let item = data.pop("chainHead_v1_storage".into()); + if let Some(sub) = sub { + let item = item; + sub.write_delayed(item).await } - response.map(|x| RawValue::from_string(x).unwrap()) - }) - }), - ) + } + response.map(|x| RawValue::from_string(x).unwrap()) + }) + }) .add_mock_data(mock_data) .add_mock_data(response_data) .build(); - let backend = run_backend_spawn_background(rpc_client); + let backend = build_backend_spawn_background(rpc_client); // We try again and should succeed let response = backend .storage_fetch_values( ["ID1".into(), "ID2".into(), "ID3".into()].into(), - crate::utils::H256::random(), + random_hash(), ) .await .unwrap(); @@ -1131,8 +1133,7 @@ mod test { ) } #[tokio::test] - /// Tests the error behaviour in streams based on chainHead_v1_continue calls - async fn storage_fetch_values_retry_continue() { + async fn storage_fetch_values_retry_chainhead_continue() { fn compare_storage_responses( expected_response: &StorageResponse, received_response: &StorageResponse, @@ -1208,49 +1209,43 @@ mod test { ), ]; let rpc_client = setup_mock_rpc_client(false) - .add_method( - "chainHead_v1_storage", - Box::new(|data, sub, _| { - Box::pin(async move { - let response = data.pop("method_response".into()).unwrap_single(); - if response.is_ok() { - let item = data.pop("chainHead_v1_storage".into()); - if let Some(sub) = sub { - let item = item; - sub.write_delayed(item).await - } + .add_method("chainHead_v1_storage", |data, sub, _| { + Box::pin(async move { + let response = data.pop("method_response".into()).unwrap_single(); + if response.is_ok() { + let item = data.pop("chainHead_v1_storage".into()); + if let Some(sub) = sub { + let item = item; + sub.write_delayed(item).await } - response.map(|x| RawValue::from_string(x).unwrap()) - }) - }), - ) - .add_method( - "chainHead_v1_continue", - Box::new(|data, sub, _| { - Box::pin(async move { - let response = data.pop("continue_response".into()).unwrap_single(); - if response.is_ok() { - let item = data.pop("chainHead_v1_storage".into()); - if let Some(sub) = sub { - let item = item; - sub.write_delayed(item).await - } + } + response.map(|x| RawValue::from_string(x).unwrap()) + }) + }) + .add_method("chainHead_v1_continue", |data, sub, _| { + Box::pin(async move { + let response = data.pop("continue_response".into()).unwrap_single(); + if response.is_ok() { + let item = data.pop("chainHead_v1_storage".into()); + if let Some(sub) = sub { + let item = item; + sub.write_delayed(item).await } - response.map(|x| RawValue::from_string(x).unwrap()) - }) - }), - ) + } + response.map(|x| RawValue::from_string(x).unwrap()) + }) + }) .add_mock_data(mock_data) .add_mock_data(response_data) .add_mock_data(continue_data) .build(); - let backend = run_backend_spawn_background(rpc_client); + let backend = build_backend_spawn_background(rpc_client); // We try again and should fail mid way let response = backend .storage_fetch_values( ["ID1".into(), "ID2".into(), "ID3".into()].into(), - crate::utils::H256::random(), + random_hash(), ) .await .unwrap(); @@ -1274,7 +1269,7 @@ mod test { let response = backend .storage_fetch_values( ["ID1".into(), "ID2".into(), "ID3".into()].into(), - crate::utils::H256::random(), + random_hash(), ) .await .unwrap(); @@ -1296,7 +1291,7 @@ mod test { #[tokio::test] async fn simple_fetch() { - let hash = init_hash(); + let hash = random_hash(); let mock_data = vec![ ( @@ -1312,20 +1307,16 @@ mod test { ("chainSpec_v1_genesisHash", Message::Single(Ok(hash))), ]; let rpc_client = setup_mock_rpc_client(false) - .add_method( - "chainSpec_v1_genesisHash", - Box::new(|data, _, _| { - Box::pin(async move { - let response = - data.pop("chainSpec_v1_genesisHash".into()).unwrap_single(); - response.map(|x| RawValue::from_string(x).unwrap()) - }) - }), - ) + .add_method("chainSpec_v1_genesisHash", |data, _, _| { + Box::pin(async move { + let response = data.pop("chainSpec_v1_genesisHash".into()).unwrap_single(); + response.map(|x| RawValue::from_string(x).unwrap()) + }) + }) .add_mock_data(mock_data) .build(); - let backend = run_backend_spawn_background(rpc_client); + let backend = build_backend_spawn_background(rpc_client); // Test // This request should encounter an error on `request` and do a retry. @@ -1374,46 +1365,42 @@ mod test { ), ]; let rpc_client = setup_mock_rpc_client(true) - .add_method( - "chainHead_v1_storage", - Box::new({ - let subscription_expired = Arc::new(AtomicBool::new(false)); - move |data, sub, params| { + .add_method("chainHead_v1_storage", { + let subscription_expired = Arc::new(AtomicBool::new(false)); + move |data, sub, params| { + let subscription_expired = subscription_expired.clone(); + Box::pin(async move { let subscription_expired = subscription_expired.clone(); - Box::pin(async move { - let subscription_expired = subscription_expired.clone(); - if subscription_expired.load(std::sync::atomic::Ordering::SeqCst) { - let params = params.map(|p| p.get().to_string()); - let rpc_params = - jsonrpsee::types::Params::new(params.as_deref()); - let key: String = rpc_params.sequence().next().unwrap(); - if key == *"ID1" { - return Err(RpcError::RequestRejected("stale id".into())); - } else { - subscription_expired - .swap(false, std::sync::atomic::Ordering::SeqCst); - } - } - let response = data.pop("method_response".into()).unwrap_single(); - if response.is_ok() { - let item = data.pop("chainHead_v1_storage".into()); - if let Some(sub) = sub { - let item = item; - sub.write_delayed(item).await - } + if subscription_expired.load(std::sync::atomic::Ordering::SeqCst) { + let params = params.map(|p| p.get().to_string()); + let rpc_params = jsonrpsee::types::Params::new(params.as_deref()); + let key: String = rpc_params.sequence().next().unwrap(); + if key == *"ID1" { + return Err(RpcError::RequestRejected("stale id".into())); } else { subscription_expired - .swap(true, std::sync::atomic::Ordering::SeqCst); + .swap(false, std::sync::atomic::Ordering::SeqCst); } - response.map(|x| RawValue::from_string(x).unwrap()) - }) - } - }), - ) + } + let response = data.pop("method_response".into()).unwrap_single(); + if response.is_ok() { + let item = data.pop("chainHead_v1_storage".into()); + if let Some(sub) = sub { + let item = item; + sub.write_delayed(item).await + } + } else { + subscription_expired + .swap(true, std::sync::atomic::Ordering::SeqCst); + } + response.map(|x| RawValue::from_string(x).unwrap()) + }) + } + }) .add_mock_data(mock_data) .add_mock_data(response_data) .build(); - let (backend, mut driver): (UnstableBackend, _) = run_backend(rpc_client); + let (backend, mut driver): (UnstableBackend, _) = build_backend(rpc_client); let _ = driver.next().await.unwrap(); let _ = driver.next().await.unwrap(); @@ -1422,7 +1409,7 @@ mod test { let response = backend .storage_fetch_values( ["ID1".into(), "ID2".into(), "ID3".into()].into(), - crate::utils::H256::random(), + random_hash(), ) .await; @@ -1443,7 +1430,7 @@ mod test { let response = backend .storage_fetch_values( ["ID1".into(), "ID2".into(), "ID3".into()].into(), - crate::utils::H256::random(), + random_hash(), ) .await; From 8444df47da40f7551454da04d604ed672a68e1aa Mon Sep 17 00:00:00 2001 From: Pavlo Khrystenko Date: Thu, 26 Sep 2024 12:03:35 +0200 Subject: [PATCH 7/7] redundant await --- subxt/src/backend/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/subxt/src/backend/mod.rs b/subxt/src/backend/mod.rs index 0287e80910..40779e593d 100644 --- a/subxt/src/backend/mod.rs +++ b/subxt/src/backend/mod.rs @@ -478,7 +478,7 @@ mod test { } impl InnerMockedRpcClient { - async fn call<'a>( + fn call<'a>( &'a mut self, method_handler: &str, params: Option>, @@ -493,7 +493,7 @@ mod test { (*method)(&mut self.data_table, &mut self.subscription_channel, params) } - async fn subscribe<'a>( + fn subscribe<'a>( &'a mut self, sub: &str, params: Option>, @@ -586,7 +586,7 @@ mod test { ) -> RawRpcFuture<'a, Box> { Box::pin(async { let mut data = self.data.lock().await; - data.call(method, params).await.await + data.call(method, params).await }) } @@ -598,7 +598,7 @@ mod test { ) -> RawRpcFuture<'a, RawRpcSubscription> { Box::pin(async { let mut data = self.data.lock().await; - data.subscribe(sub, params).await.await + data.subscribe(sub, params).await }) } }