From 0402f92af84bb239401768d6b49d0593f04879f9 Mon Sep 17 00:00:00 2001 From: Tadeo hepperle Date: Thu, 21 Sep 2023 09:58:36 +0200 Subject: [PATCH 1/6] adjust tests --- substrate/client/rpc-spec-v2/src/chain_head/event.rs | 5 +++-- substrate/client/rpc-spec-v2/src/chain_head/tests.rs | 12 ++++++------ .../client/rpc-spec-v2/src/transaction/event.rs | 5 ++--- substrate/client/rpc/src/state/tests.rs | 12 ++++++------ 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/substrate/client/rpc-spec-v2/src/chain_head/event.rs b/substrate/client/rpc-spec-v2/src/chain_head/event.rs index 65bc8b247c880..b703d290e2c58 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/event.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/event.rs @@ -392,7 +392,7 @@ mod tests { let exp = concat!( r#"{"event":"initialized","finalizedBlockHash":"0x1","#, r#""finalizedBlockRuntime":{"type":"valid","spec":{"specName":"ABC","implName":"Impl","authoringVersion":0,"#, - r#""specVersion":1,"implVersion":0,"apis":[],"transactionVersion":0,"stateVersion":0}}}"#, + r#""specVersion":1,"implVersion":0,"apis":{},"transactionVersion":0,"stateVersion":0}}}"#, ); assert_eq!(ser, exp); @@ -429,6 +429,7 @@ mod tests { spec_name: "ABC".into(), impl_name: "Impl".into(), spec_version: 1, + apis: vec![([0, 0, 0, 0, 0, 0, 0, 0], 2), ([1, 0, 0, 0, 0, 0, 0, 0], 3)].into(), ..Default::default() }; @@ -446,7 +447,7 @@ mod tests { let exp = concat!( r#"{"event":"newBlock","blockHash":"0x1","parentBlockHash":"0x2","#, r#""newRuntime":{"type":"valid","spec":{"specName":"ABC","implName":"Impl","authoringVersion":0,"#, - r#""specVersion":1,"implVersion":0,"apis":[],"transactionVersion":0,"stateVersion":0}}}"#, + r#""specVersion":1,"implVersion":0,"apis":{"0x0000000000000000":2,"0x0100000000000000":3},"transactionVersion":0,"stateVersion":0}}}"#, ); assert_eq!(ser, exp); diff --git a/substrate/client/rpc-spec-v2/src/chain_head/tests.rs b/substrate/client/rpc-spec-v2/src/chain_head/tests.rs index 3ab47991c4e55..a7580e14cb6ea 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/tests.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/tests.rs @@ -234,12 +234,12 @@ async fn follow_with_runtime() { let event: FollowEvent = get_next_event(&mut sub).await; // it is basically json-encoded substrate_test_runtime_client::runtime::VERSION - let runtime_str = "{\"specName\":\"test\",\"implName\":\"parity-test\",\"authoringVersion\":1,\ - \"specVersion\":2,\"implVersion\":2,\"apis\":[[\"0xdf6acb689907609b\",4],\ - [\"0x37e397fc7c91f5e4\",2],[\"0xd2bc9897eed08f15\",3],[\"0x40fe3ad401f8959a\",6],\ - [\"0xbc9d89904f5b923f\",1],[\"0xc6e9a76309f39b09\",2],[\"0xdd718d5cc53262d4\",1],\ - [\"0xcbca25e39f142387\",2],[\"0xf78b278be53f454c\",2],[\"0xab3c0572291feb8b\",1],\ - [\"0xed99c5acb25eedf5\",3],[\"0xfbc577b9d747efd6\",1]],\"transactionVersion\":1,\"stateVersion\":1}"; + let runtime_str = "{\"specName\":\"test\",\"implName\":\"parity-test\",\"authoringVersion\":1, + \"specVersion\":2,\"implVersion\":2,\"apis\":{\"0xdf6acb689907609b\":4, + \"0x37e397fc7c91f5e4\":2,\"0xd2bc9897eed08f15\":3,\"0x40fe3ad401f8959a\":6, + \"0xbc9d89904f5b923f\":1,\"0xc6e9a76309f39b09\":2,\"0xdd718d5cc53262d4\":1, + \"0xcbca25e39f142387\":2,\"0xf78b278be53f454c\":2,\"0xab3c0572291feb8b\":1, + \"0xed99c5acb25eedf5\":3,\"0xfbc577b9d747efd6\":1},\"transactionVersion\":1,\"stateVersion\":1}"; let runtime: RuntimeVersion = serde_json::from_str(runtime_str).unwrap(); diff --git a/substrate/client/rpc-spec-v2/src/transaction/event.rs b/substrate/client/rpc-spec-v2/src/transaction/event.rs index bdc126366fb92..2a9b4831a9ef5 100644 --- a/substrate/client/rpc-spec-v2/src/transaction/event.rs +++ b/substrate/client/rpc-spec-v2/src/transaction/event.rs @@ -45,7 +45,6 @@ pub struct TransactionBlock { /// The hash of the block the transaction was included into. pub hash: Hash, /// The index (zero-based) of the transaction within the body of the block. - #[serde(with = "as_string")] pub index: usize, } @@ -288,7 +287,7 @@ mod tests { })); let ser = serde_json::to_string(&event).unwrap(); - let exp = r#"{"event":"bestChainBlockIncluded","block":{"hash":"0x0000000000000000000000000000000000000000000000000000000000000001","index":"2"}}"#; + let exp = r#"{"event":"bestChainBlockIncluded","block":{"hash":"0x0000000000000000000000000000000000000000000000000000000000000001","index":2}}"#; assert_eq!(ser, exp); let event_dec: TransactionEvent = serde_json::from_str(exp).unwrap(); @@ -303,7 +302,7 @@ mod tests { }); let ser = serde_json::to_string(&event).unwrap(); - let exp = r#"{"event":"finalized","block":{"hash":"0x0000000000000000000000000000000000000000000000000000000000000001","index":"10"}}"#; + let exp = r#"{"event":"finalized","block":{"hash":"0x0000000000000000000000000000000000000000000000000000000000000001","index":10}}"#; assert_eq!(ser, exp); let event_dec: TransactionEvent = serde_json::from_str(exp).unwrap(); diff --git a/substrate/client/rpc/src/state/tests.rs b/substrate/client/rpc/src/state/tests.rs index 35352f6d890ed..3541b95b5ef7d 100644 --- a/substrate/client/rpc/src/state/tests.rs +++ b/substrate/client/rpc/src/state/tests.rs @@ -513,12 +513,12 @@ async fn should_return_runtime_version() { let (api, _child) = new_full(client.clone(), test_executor(), DenyUnsafe::No); // it is basically json-encoded substrate_test_runtime_client::runtime::VERSION - let result = "{\"specName\":\"test\",\"implName\":\"parity-test\",\"authoringVersion\":1,\ - \"specVersion\":2,\"implVersion\":2,\"apis\":[[\"0xdf6acb689907609b\",4],\ - [\"0x37e397fc7c91f5e4\",2],[\"0xd2bc9897eed08f15\",3],[\"0x40fe3ad401f8959a\",6],\ - [\"0xbc9d89904f5b923f\",1],[\"0xc6e9a76309f39b09\",2],[\"0xdd718d5cc53262d4\",1],\ - [\"0xcbca25e39f142387\",2],[\"0xf78b278be53f454c\",2],[\"0xab3c0572291feb8b\",1],\ - [\"0xed99c5acb25eedf5\",3],[\"0xfbc577b9d747efd6\",1]],\"transactionVersion\":1,\"stateVersion\":1}"; + let result = "{\"specName\":\"test\",\"implName\":\"parity-test\",\"authoringVersion\":1, + \"specVersion\":2,\"implVersion\":2,\"apis\":{\"0xdf6acb689907609b\":4, + \"0x37e397fc7c91f5e4\":2,\"0xd2bc9897eed08f15\":3,\"0x40fe3ad401f8959a\":6, + \"0xbc9d89904f5b923f\":1,\"0xc6e9a76309f39b09\":2,\"0xdd718d5cc53262d4\":1, + \"0xcbca25e39f142387\":2,\"0xf78b278be53f454c\":2,\"0xab3c0572291feb8b\":1, + \"0xed99c5acb25eedf5\":3,\"0xfbc577b9d747efd6\":1},\"transactionVersion\":1,\"stateVersion\":1}"; let runtime_version = api.runtime_version(None.into()).unwrap(); let serialized = serde_json::to_string(&runtime_version).unwrap(); From da35ef9791dffe1772cfa4d4ed23cde766890d45 Mon Sep 17 00:00:00 2001 From: Tadeo hepperle Date: Thu, 21 Sep 2023 17:10:06 +0200 Subject: [PATCH 2/6] custom serialization --- Cargo.lock | 1 + substrate/client/rpc-spec-v2/Cargo.toml | 1 + .../rpc-spec-v2/src/chain_head/error.rs | 1 - .../rpc-spec-v2/src/chain_head/event.rs | 154 +++++++++++++++++- .../rpc-spec-v2/src/chain_head/tests.rs | 12 +- substrate/client/rpc/src/state/tests.rs | 4 +- 6 files changed, 160 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9dfff599300d6..ff88f3a3bb9a0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -15511,6 +15511,7 @@ dependencies = [ "futures", "futures-util", "hex", + "impl-serde", "jsonrpsee", "log", "parity-scale-codec", diff --git a/substrate/client/rpc-spec-v2/Cargo.toml b/substrate/client/rpc-spec-v2/Cargo.toml index c93006753afbb..889920e9db13e 100644 --- a/substrate/client/rpc-spec-v2/Cargo.toml +++ b/substrate/client/rpc-spec-v2/Cargo.toml @@ -36,6 +36,7 @@ tokio = { version = "1.22.0", features = ["sync"] } array-bytes = "6.1" log = "0.4.17" futures-util = { version = "0.3.19", default-features = false } +impl-serde = { version = "0.4.0", default-features = false } [dev-dependencies] serde_json = "1.0" diff --git a/substrate/client/rpc-spec-v2/src/chain_head/error.rs b/substrate/client/rpc-spec-v2/src/chain_head/error.rs index 3b2edb2b00c8c..811666428c5a5 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/error.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/error.rs @@ -69,7 +69,6 @@ impl From for ErrorObject<'static> { Error::InvalidSubscriptionID => ErrorObject::owned(INVALID_SUB_ID, msg, None::<()>), Error::InvalidContinue => ErrorObject::owned(INVALID_CONTINUE, msg, None::<()>), } - .into() } } diff --git a/substrate/client/rpc-spec-v2/src/chain_head/event.rs b/substrate/client/rpc-spec-v2/src/chain_head/event.rs index b703d290e2c58..40b2a8ddc809b 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/event.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/event.rs @@ -39,6 +39,10 @@ pub struct ErrorEvent { #[serde(rename_all = "camelCase")] pub struct RuntimeVersionEvent { /// The runtime version. + #[serde( + serialize_with = "custom_serialize::serialize_runtime_version", + deserialize_with = "custom_serialize::deserialize_runtime_version" + )] pub spec: RuntimeVersion, } @@ -349,6 +353,148 @@ pub struct MethodResponseStarted { pub discarded_items: Option, } +mod custom_serialize { + use serde::{de, ser::SerializeMap, Deserialize, Serialize, Serializer}; + + pub fn serialize_runtime_version( + runtime_version: &sp_version::RuntimeVersion, + ser: S, + ) -> Result + where + S: Serializer, + { + let representation = RuntimeVersionV2::from(runtime_version); + representation.serialize(ser) + } + + pub fn deserialize_runtime_version<'de, D>( + deserializer: D, + ) -> Result + where + D: de::Deserializer<'de>, + { + let representation = RuntimeVersionV2Owned::deserialize(deserializer)?; + Ok(representation.into()) + } + + #[derive(Serialize)] + #[serde(rename_all = "camelCase")] + struct RuntimeVersionV2<'a> { + spec_name: &'a str, + impl_name: &'a str, + spec_version: u32, + impl_version: u32, + #[serde(serialize_with = "serialize_apis_vec_as_map")] + apis: &'a sp_version::ApisVec, + transaction_version: u32, + } + + impl<'a> From<&'a sp_version::RuntimeVersion> for RuntimeVersionV2<'a> { + fn from(value: &'a sp_version::RuntimeVersion) -> Self { + RuntimeVersionV2 { + spec_name: &value.spec_name, + impl_name: &value.impl_name, + spec_version: value.spec_version, + impl_version: value.impl_version, + apis: &value.apis, + transaction_version: value.transaction_version, + } + } + } + + #[derive(Deserialize)] + #[serde(rename_all = "camelCase")] + struct RuntimeVersionV2Owned { + spec_name: sp_runtime::RuntimeString, + impl_name: sp_runtime::RuntimeString, + spec_version: u32, + impl_version: u32, + #[serde(deserialize_with = "deserialize_apis_vec_as_map")] + apis: sp_version::ApisVec, + transaction_version: u32, + } + + impl From for sp_version::RuntimeVersion { + fn from(value: RuntimeVersionV2Owned) -> Self { + sp_version::RuntimeVersion { + spec_name: value.spec_name, + impl_name: value.impl_name, + spec_version: value.spec_version, + impl_version: value.impl_version, + apis: value.apis, + transaction_version: value.transaction_version, + ..Default::default() + } + } + } + + pub fn serialize_apis_vec_as_map(apis: &[([u8; 8], u32)], ser: S) -> Result + where + S: Serializer, + { + let len = apis.len(); + let mut map = ser.serialize_map(Some(len))?; + for (api, ver) in apis.iter() { + let api = ApiId(api); + map.serialize_entry(&api, ver)?; + } + map.end() + } + + pub fn deserialize_apis_vec_as_map<'de, D>( + deserializer: D, + ) -> Result + where + D: de::Deserializer<'de>, + { + struct Visitor; + impl<'de> de::Visitor<'de> for Visitor { + type Value = sp_version::ApisVec; + + fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { + formatter.write_str("a map from api id to version") + } + + fn visit_map(self, mut visitor: V) -> Result + where + V: de::MapAccess<'de>, + { + let mut apis = Vec::new(); + while let Some((key, value)) = visitor.next_entry::()? { + apis.push((key.0, value)); + } + Ok(apis.into()) + } + } + deserializer.deserialize_map(Visitor) + } + + #[derive(Serialize)] + struct ApiId<'a>(#[serde(serialize_with = "serialize_api_id")] &'a sp_version::ApiId); + + #[derive(Deserialize)] + struct ApiIdOwned(#[serde(deserialize_with = "deserialize_api_id")] sp_version::ApiId); + + pub fn serialize_api_id(&api_id: &&sp_version::ApiId, ser: S) -> Result + where + S: Serializer, + { + impl_serde::serialize::serialize(api_id, ser) + } + + pub fn deserialize_api_id<'de, D>(d: D) -> Result + where + D: de::Deserializer<'de>, + { + let mut arr = [0; 8]; + impl_serde::serialize::deserialize_check_len( + d, + impl_serde::serialize::ExpectedLen::Exact(&mut arr[..]), + )?; + Ok(arr) + } +} + #[cfg(test)] mod tests { use super::*; @@ -391,8 +537,8 @@ mod tests { let ser = serde_json::to_string(&event).unwrap(); let exp = concat!( r#"{"event":"initialized","finalizedBlockHash":"0x1","#, - r#""finalizedBlockRuntime":{"type":"valid","spec":{"specName":"ABC","implName":"Impl","authoringVersion":0,"#, - r#""specVersion":1,"implVersion":0,"apis":{},"transactionVersion":0,"stateVersion":0}}}"#, + r#""finalizedBlockRuntime":{"type":"valid","spec":{"specName":"ABC","implName":"Impl","#, + r#""specVersion":1,"implVersion":0,"apis":{},"transactionVersion":0}}}"#, ); assert_eq!(ser, exp); @@ -446,8 +592,8 @@ mod tests { let ser = serde_json::to_string(&event).unwrap(); let exp = concat!( r#"{"event":"newBlock","blockHash":"0x1","parentBlockHash":"0x2","#, - r#""newRuntime":{"type":"valid","spec":{"specName":"ABC","implName":"Impl","authoringVersion":0,"#, - r#""specVersion":1,"implVersion":0,"apis":{"0x0000000000000000":2,"0x0100000000000000":3},"transactionVersion":0,"stateVersion":0}}}"#, + r#""newRuntime":{"type":"valid","spec":{"specName":"ABC","implName":"Impl","#, + r#""specVersion":1,"implVersion":0,"apis":{"0x0000000000000000":2,"0x0100000000000000":3},"transactionVersion":0}}}"#, ); assert_eq!(ser, exp); diff --git a/substrate/client/rpc-spec-v2/src/chain_head/tests.rs b/substrate/client/rpc-spec-v2/src/chain_head/tests.rs index a7580e14cb6ea..e0551ff7aecea 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/tests.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/tests.rs @@ -234,12 +234,12 @@ async fn follow_with_runtime() { let event: FollowEvent = get_next_event(&mut sub).await; // it is basically json-encoded substrate_test_runtime_client::runtime::VERSION - let runtime_str = "{\"specName\":\"test\",\"implName\":\"parity-test\",\"authoringVersion\":1, - \"specVersion\":2,\"implVersion\":2,\"apis\":{\"0xdf6acb689907609b\":4, - \"0x37e397fc7c91f5e4\":2,\"0xd2bc9897eed08f15\":3,\"0x40fe3ad401f8959a\":6, - \"0xbc9d89904f5b923f\":1,\"0xc6e9a76309f39b09\":2,\"0xdd718d5cc53262d4\":1, - \"0xcbca25e39f142387\":2,\"0xf78b278be53f454c\":2,\"0xab3c0572291feb8b\":1, - \"0xed99c5acb25eedf5\":3,\"0xfbc577b9d747efd6\":1},\"transactionVersion\":1,\"stateVersion\":1}"; + let runtime_str = "{\"specName\":\"test\",\"implName\":\"parity-test\",\"authoringVersion\":0,\ + \"specVersion\":2,\"implVersion\":2,\"apis\":[[\"0xdf6acb689907609b\",4],\ + [\"0x37e397fc7c91f5e4\",2],[\"0xd2bc9897eed08f15\",3],[\"0x40fe3ad401f8959a\",6],\ + [\"0xbc9d89904f5b923f\",1],[\"0xc6e9a76309f39b09\",2],[\"0xdd718d5cc53262d4\",1],\ + [\"0xcbca25e39f142387\",2],[\"0xf78b278be53f454c\",2],[\"0xab3c0572291feb8b\",1],\ + [\"0xed99c5acb25eedf5\",3],[\"0xfbc577b9d747efd6\",1]],\"transactionVersion\":1,\"stateVersion\":0}"; let runtime: RuntimeVersion = serde_json::from_str(runtime_str).unwrap(); diff --git a/substrate/client/rpc/src/state/tests.rs b/substrate/client/rpc/src/state/tests.rs index 3541b95b5ef7d..05b1a49787e0d 100644 --- a/substrate/client/rpc/src/state/tests.rs +++ b/substrate/client/rpc/src/state/tests.rs @@ -513,12 +513,12 @@ async fn should_return_runtime_version() { let (api, _child) = new_full(client.clone(), test_executor(), DenyUnsafe::No); // it is basically json-encoded substrate_test_runtime_client::runtime::VERSION - let result = "{\"specName\":\"test\",\"implName\":\"parity-test\",\"authoringVersion\":1, + let result = "{\"specName\":\"test\",\"implName\":\"parity-test\", \"specVersion\":2,\"implVersion\":2,\"apis\":{\"0xdf6acb689907609b\":4, \"0x37e397fc7c91f5e4\":2,\"0xd2bc9897eed08f15\":3,\"0x40fe3ad401f8959a\":6, \"0xbc9d89904f5b923f\":1,\"0xc6e9a76309f39b09\":2,\"0xdd718d5cc53262d4\":1, \"0xcbca25e39f142387\":2,\"0xf78b278be53f454c\":2,\"0xab3c0572291feb8b\":1, - \"0xed99c5acb25eedf5\":3,\"0xfbc577b9d747efd6\":1},\"transactionVersion\":1,\"stateVersion\":1}"; + \"0xed99c5acb25eedf5\":3,\"0xfbc577b9d747efd6\":1},\"transactionVersion\":1}"; let runtime_version = api.runtime_version(None.into()).unwrap(); let serialized = serde_json::to_string(&runtime_version).unwrap(); From e846fc2e3146bbb53f96c1267e88c8febda0f63d Mon Sep 17 00:00:00 2001 From: Tadeo hepperle Date: Mon, 25 Sep 2023 10:48:56 +0200 Subject: [PATCH 3/6] from into serialization --- Cargo.lock | 1 - substrate/client/rpc-spec-v2/Cargo.toml | 2 - .../rpc-spec-v2/src/chain_head/event.rs | 245 +++++++----------- substrate/client/rpc/src/state/tests.rs | 12 +- 4 files changed, 104 insertions(+), 156 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1f5815bed8389..ddb19c24e1f6a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -15529,7 +15529,6 @@ dependencies = [ "futures", "futures-util", "hex", - "impl-serde", "jsonrpsee", "log", "parity-scale-codec", diff --git a/substrate/client/rpc-spec-v2/Cargo.toml b/substrate/client/rpc-spec-v2/Cargo.toml index 889920e9db13e..00b4c7a5e884d 100644 --- a/substrate/client/rpc-spec-v2/Cargo.toml +++ b/substrate/client/rpc-spec-v2/Cargo.toml @@ -36,8 +36,6 @@ tokio = { version = "1.22.0", features = ["sync"] } array-bytes = "6.1" log = "0.4.17" futures-util = { version = "0.3.19", default-features = false } -impl-serde = { version = "0.4.0", default-features = false } - [dev-dependencies] serde_json = "1.0" tokio = { version = "1.22.0", features = ["macros"] } diff --git a/substrate/client/rpc-spec-v2/src/chain_head/event.rs b/substrate/client/rpc-spec-v2/src/chain_head/event.rs index 40b2a8ddc809b..8d380cc509c8f 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/event.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/event.rs @@ -21,6 +21,8 @@ use serde::{ser::SerializeStruct, Deserialize, Serialize, Serializer}; use sp_api::ApiError; use sp_version::RuntimeVersion; +use std::collections::BTreeMap; +use thiserror::Error; /// The operation could not be processed due to an error. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] @@ -35,17 +37,107 @@ pub struct ErrorEvent { /// This event is generated for: /// - the first announced block by the follow subscription /// - blocks that suffered a change in runtime compared with their parents -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] -#[serde(rename_all = "camelCase")] +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde( + rename_all = "camelCase", + try_from = "RuntimeVersionEventWrapper", + into = "RuntimeVersionEventWrapper" +)] pub struct RuntimeVersionEvent { /// The runtime version. - #[serde( - serialize_with = "custom_serialize::serialize_runtime_version", - deserialize_with = "custom_serialize::deserialize_runtime_version" - )] pub spec: RuntimeVersion, } + +// Note: PartialEq mainly used in tests, manual implementation necessary, because BTreeMap in RuntimeVersionWrapper does not preserve order of apis vec. +impl PartialEq for RuntimeVersionEvent { + fn eq(&self, other: &Self) -> bool { + RuntimeVersionWrapper::from(self.spec.clone()) == RuntimeVersionWrapper::from(other.spec.clone()) + } +} + +#[derive(Clone, PartialEq, Debug, Serialize, Deserialize)] +struct RuntimeVersionEventWrapper { + spec: RuntimeVersionWrapper, +} + +impl TryFrom for RuntimeVersionEvent { + type Error = ApiFromHexError; + + fn try_from(val: RuntimeVersionEventWrapper) -> Result { + let spec = val.spec.try_into()?; + Ok(Self { spec }) + } +} + +impl From for RuntimeVersionEventWrapper { + fn from(val: RuntimeVersionEvent) -> Self { + Self { spec: val.spec.into() } + } +} + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +struct RuntimeVersionWrapper { + pub spec_name: String, + pub impl_name: String, + pub spec_version: u32, + pub impl_version: u32, + pub apis: BTreeMap, + pub transaction_version: u32, +} + +#[derive(Error, Debug)] +enum ApiFromHexError { + #[error("invalid hex string provided")] + FromHexError(#[from] sp_core::bytes::FromHexError), + #[error("buffer must be 8 bytes long")] + InvalidLength, +} + +impl TryFrom for RuntimeVersion { + type Error = ApiFromHexError; + + fn try_from(val: RuntimeVersionWrapper) -> Result { + let apis = val + .apis + .into_iter() + .map(|(api, version)| -> Result<([u8; 8], u32), ApiFromHexError> { + let bytes_vec = sp_core::bytes::from_hex(&api)?; + let api: [u8; 8] = + bytes_vec.try_into().map_err(|_| ApiFromHexError::InvalidLength)?; + Ok((api, version)) + }) + .collect::>()?; + Ok(Self { + spec_name: sp_runtime::RuntimeString::Owned(val.spec_name), + impl_name: sp_runtime::RuntimeString::Owned(val.impl_name), + spec_version: val.spec_version, + impl_version: val.impl_version, + apis, + transaction_version: val.transaction_version, + ..Default::default() + }) + } +} + +impl From for RuntimeVersionWrapper { + fn from(val: RuntimeVersion) -> Self { + Self { + spec_name: val.spec_name.into(), + impl_name: val.impl_name.into(), + spec_version: val.spec_version, + impl_version: val.impl_version, + apis: val + .apis + .into_iter() + .map(|(api, version)| (sp_core::bytes::to_hex(api, false), *version)) + .collect(), + transaction_version: val.transaction_version, + } + } +} + /// The runtime event generated if the `follow` subscription /// has set the `with_runtime` flag. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] @@ -353,147 +445,6 @@ pub struct MethodResponseStarted { pub discarded_items: Option, } -mod custom_serialize { - use serde::{de, ser::SerializeMap, Deserialize, Serialize, Serializer}; - - pub fn serialize_runtime_version( - runtime_version: &sp_version::RuntimeVersion, - ser: S, - ) -> Result - where - S: Serializer, - { - let representation = RuntimeVersionV2::from(runtime_version); - representation.serialize(ser) - } - - pub fn deserialize_runtime_version<'de, D>( - deserializer: D, - ) -> Result - where - D: de::Deserializer<'de>, - { - let representation = RuntimeVersionV2Owned::deserialize(deserializer)?; - Ok(representation.into()) - } - - #[derive(Serialize)] - #[serde(rename_all = "camelCase")] - struct RuntimeVersionV2<'a> { - spec_name: &'a str, - impl_name: &'a str, - spec_version: u32, - impl_version: u32, - #[serde(serialize_with = "serialize_apis_vec_as_map")] - apis: &'a sp_version::ApisVec, - transaction_version: u32, - } - - impl<'a> From<&'a sp_version::RuntimeVersion> for RuntimeVersionV2<'a> { - fn from(value: &'a sp_version::RuntimeVersion) -> Self { - RuntimeVersionV2 { - spec_name: &value.spec_name, - impl_name: &value.impl_name, - spec_version: value.spec_version, - impl_version: value.impl_version, - apis: &value.apis, - transaction_version: value.transaction_version, - } - } - } - - #[derive(Deserialize)] - #[serde(rename_all = "camelCase")] - struct RuntimeVersionV2Owned { - spec_name: sp_runtime::RuntimeString, - impl_name: sp_runtime::RuntimeString, - spec_version: u32, - impl_version: u32, - #[serde(deserialize_with = "deserialize_apis_vec_as_map")] - apis: sp_version::ApisVec, - transaction_version: u32, - } - - impl From for sp_version::RuntimeVersion { - fn from(value: RuntimeVersionV2Owned) -> Self { - sp_version::RuntimeVersion { - spec_name: value.spec_name, - impl_name: value.impl_name, - spec_version: value.spec_version, - impl_version: value.impl_version, - apis: value.apis, - transaction_version: value.transaction_version, - ..Default::default() - } - } - } - - pub fn serialize_apis_vec_as_map(apis: &[([u8; 8], u32)], ser: S) -> Result - where - S: Serializer, - { - let len = apis.len(); - let mut map = ser.serialize_map(Some(len))?; - for (api, ver) in apis.iter() { - let api = ApiId(api); - map.serialize_entry(&api, ver)?; - } - map.end() - } - - pub fn deserialize_apis_vec_as_map<'de, D>( - deserializer: D, - ) -> Result - where - D: de::Deserializer<'de>, - { - struct Visitor; - impl<'de> de::Visitor<'de> for Visitor { - type Value = sp_version::ApisVec; - - fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { - formatter.write_str("a map from api id to version") - } - - fn visit_map(self, mut visitor: V) -> Result - where - V: de::MapAccess<'de>, - { - let mut apis = Vec::new(); - while let Some((key, value)) = visitor.next_entry::()? { - apis.push((key.0, value)); - } - Ok(apis.into()) - } - } - deserializer.deserialize_map(Visitor) - } - - #[derive(Serialize)] - struct ApiId<'a>(#[serde(serialize_with = "serialize_api_id")] &'a sp_version::ApiId); - - #[derive(Deserialize)] - struct ApiIdOwned(#[serde(deserialize_with = "deserialize_api_id")] sp_version::ApiId); - - pub fn serialize_api_id(&api_id: &&sp_version::ApiId, ser: S) -> Result - where - S: Serializer, - { - impl_serde::serialize::serialize(api_id, ser) - } - - pub fn deserialize_api_id<'de, D>(d: D) -> Result - where - D: de::Deserializer<'de>, - { - let mut arr = [0; 8]; - impl_serde::serialize::deserialize_check_len( - d, - impl_serde::serialize::ExpectedLen::Exact(&mut arr[..]), - )?; - Ok(arr) - } -} #[cfg(test)] mod tests { diff --git a/substrate/client/rpc/src/state/tests.rs b/substrate/client/rpc/src/state/tests.rs index 05b1a49787e0d..35352f6d890ed 100644 --- a/substrate/client/rpc/src/state/tests.rs +++ b/substrate/client/rpc/src/state/tests.rs @@ -513,12 +513,12 @@ async fn should_return_runtime_version() { let (api, _child) = new_full(client.clone(), test_executor(), DenyUnsafe::No); // it is basically json-encoded substrate_test_runtime_client::runtime::VERSION - let result = "{\"specName\":\"test\",\"implName\":\"parity-test\", - \"specVersion\":2,\"implVersion\":2,\"apis\":{\"0xdf6acb689907609b\":4, - \"0x37e397fc7c91f5e4\":2,\"0xd2bc9897eed08f15\":3,\"0x40fe3ad401f8959a\":6, - \"0xbc9d89904f5b923f\":1,\"0xc6e9a76309f39b09\":2,\"0xdd718d5cc53262d4\":1, - \"0xcbca25e39f142387\":2,\"0xf78b278be53f454c\":2,\"0xab3c0572291feb8b\":1, - \"0xed99c5acb25eedf5\":3,\"0xfbc577b9d747efd6\":1},\"transactionVersion\":1}"; + let result = "{\"specName\":\"test\",\"implName\":\"parity-test\",\"authoringVersion\":1,\ + \"specVersion\":2,\"implVersion\":2,\"apis\":[[\"0xdf6acb689907609b\",4],\ + [\"0x37e397fc7c91f5e4\",2],[\"0xd2bc9897eed08f15\",3],[\"0x40fe3ad401f8959a\",6],\ + [\"0xbc9d89904f5b923f\",1],[\"0xc6e9a76309f39b09\",2],[\"0xdd718d5cc53262d4\",1],\ + [\"0xcbca25e39f142387\",2],[\"0xf78b278be53f454c\",2],[\"0xab3c0572291feb8b\",1],\ + [\"0xed99c5acb25eedf5\",3],[\"0xfbc577b9d747efd6\",1]],\"transactionVersion\":1,\"stateVersion\":1}"; let runtime_version = api.runtime_version(None.into()).unwrap(); let serialized = serde_json::to_string(&runtime_version).unwrap(); From ef610473cb45a502bb8f9bcf5f999d7cee41e296 Mon Sep 17 00:00:00 2001 From: Tadeo hepperle Date: Mon, 25 Sep 2023 10:50:02 +0200 Subject: [PATCH 4/6] formatting --- substrate/client/rpc-spec-v2/src/chain_head/event.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/substrate/client/rpc-spec-v2/src/chain_head/event.rs b/substrate/client/rpc-spec-v2/src/chain_head/event.rs index 8d380cc509c8f..bbfa3cacbda44 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/event.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/event.rs @@ -48,12 +48,13 @@ pub struct RuntimeVersionEvent { pub spec: RuntimeVersion, } - -// Note: PartialEq mainly used in tests, manual implementation necessary, because BTreeMap in RuntimeVersionWrapper does not preserve order of apis vec. +// Note: PartialEq mainly used in tests, manual implementation necessary, because BTreeMap in +// RuntimeVersionWrapper does not preserve order of apis vec. impl PartialEq for RuntimeVersionEvent { - fn eq(&self, other: &Self) -> bool { - RuntimeVersionWrapper::from(self.spec.clone()) == RuntimeVersionWrapper::from(other.spec.clone()) - } + fn eq(&self, other: &Self) -> bool { + RuntimeVersionWrapper::from(self.spec.clone()) == + RuntimeVersionWrapper::from(other.spec.clone()) + } } #[derive(Clone, PartialEq, Debug, Serialize, Deserialize)] @@ -445,7 +446,6 @@ pub struct MethodResponseStarted { pub discarded_items: Option, } - #[cfg(test)] mod tests { use super::*; From 2629d865c5ffb6618beb8c22c560141401890518 Mon Sep 17 00:00:00 2001 From: Tadeo hepperle Date: Tue, 26 Sep 2023 10:22:34 +0200 Subject: [PATCH 5/6] add ChainHeadRuntimeVersion --- .../src/chain_head/chain_head_follow.rs | 4 +- .../rpc-spec-v2/src/chain_head/event.rs | 72 +++++++------------ .../rpc-spec-v2/src/chain_head/tests.rs | 5 +- .../rpc-spec-v2/src/transaction/event.rs | 19 +---- 4 files changed, 31 insertions(+), 69 deletions(-) diff --git a/substrate/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs b/substrate/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs index 0fa995ce73a09..b981e69f2e473 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/chain_head_follow.rs @@ -158,7 +158,7 @@ where let parent = match parent { Some(parent) => parent, // Nothing to compare against, always report. - None => return Some(RuntimeEvent::Valid(RuntimeVersionEvent { spec: block_rt })), + None => return Some(RuntimeEvent::Valid(RuntimeVersionEvent { spec: block_rt.into() })), }; let parent_rt = match self.client.runtime_version_at(parent) { @@ -168,7 +168,7 @@ where // Report the runtime version change. if block_rt != parent_rt { - Some(RuntimeEvent::Valid(RuntimeVersionEvent { spec: block_rt })) + Some(RuntimeEvent::Valid(RuntimeVersionEvent { spec: block_rt.into() })) } else { None } diff --git a/substrate/client/rpc-spec-v2/src/chain_head/event.rs b/substrate/client/rpc-spec-v2/src/chain_head/event.rs index bbfa3cacbda44..acad396e0fa4e 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/event.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/event.rs @@ -37,79 +37,57 @@ pub struct ErrorEvent { /// This event is generated for: /// - the first announced block by the follow subscription /// - blocks that suffered a change in runtime compared with their parents -#[derive(Debug, Clone, Serialize, Deserialize)] -#[serde( - rename_all = "camelCase", - try_from = "RuntimeVersionEventWrapper", - into = "RuntimeVersionEventWrapper" -)] +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[serde(rename_all = "camelCase")] pub struct RuntimeVersionEvent { /// The runtime version. - pub spec: RuntimeVersion, -} - -// Note: PartialEq mainly used in tests, manual implementation necessary, because BTreeMap in -// RuntimeVersionWrapper does not preserve order of apis vec. -impl PartialEq for RuntimeVersionEvent { - fn eq(&self, other: &Self) -> bool { - RuntimeVersionWrapper::from(self.spec.clone()) == - RuntimeVersionWrapper::from(other.spec.clone()) - } -} - -#[derive(Clone, PartialEq, Debug, Serialize, Deserialize)] -struct RuntimeVersionEventWrapper { - spec: RuntimeVersionWrapper, -} - -impl TryFrom for RuntimeVersionEvent { - type Error = ApiFromHexError; - - fn try_from(val: RuntimeVersionEventWrapper) -> Result { - let spec = val.spec.try_into()?; - Ok(Self { spec }) - } -} - -impl From for RuntimeVersionEventWrapper { - fn from(val: RuntimeVersionEvent) -> Self { - Self { spec: val.spec.into() } - } + pub spec: ChainHeadRuntimeVersion, } +/// Simplified type clone of `sp_version::RuntimeVersion`. Used instead of +/// `sp_version::RuntimeVersion` to conform to RPC spec V2. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] -struct RuntimeVersionWrapper { +pub struct ChainHeadRuntimeVersion { + /// Identifies the different Substrate runtimes. pub spec_name: String, + /// Name of the implementation of the spec. pub impl_name: String, + /// Version of the runtime specification. pub spec_version: u32, + /// Version of the implementation of the specification. pub impl_version: u32, + /// Map of all supported API "features" and their versions. pub apis: BTreeMap, + /// Transaction version. pub transaction_version: u32, } +/// Error when trying to convert a `ChainHeadRuntimeVersion` into a `RuntimeVersion` #[derive(Error, Debug)] -enum ApiFromHexError { +pub enum IntoRuntimeVersionError { + /// Invalid characters in hex string. #[error("invalid hex string provided")] FromHexError(#[from] sp_core::bytes::FromHexError), + /// Hex string did not contain exactly 8 bytes. #[error("buffer must be 8 bytes long")] InvalidLength, } -impl TryFrom for RuntimeVersion { - type Error = ApiFromHexError; +impl TryFrom for RuntimeVersion { + type Error = IntoRuntimeVersionError; - fn try_from(val: RuntimeVersionWrapper) -> Result { + fn try_from(val: ChainHeadRuntimeVersion) -> Result { let apis = val .apis .into_iter() - .map(|(api, version)| -> Result<([u8; 8], u32), ApiFromHexError> { + .map(|(api, version)| -> Result<([u8; 8], u32), IntoRuntimeVersionError> { let bytes_vec = sp_core::bytes::from_hex(&api)?; let api: [u8; 8] = - bytes_vec.try_into().map_err(|_| ApiFromHexError::InvalidLength)?; + bytes_vec.try_into().map_err(|_| IntoRuntimeVersionError::InvalidLength)?; Ok((api, version)) }) - .collect::>()?; + .collect::>()?; Ok(Self { spec_name: sp_runtime::RuntimeString::Owned(val.spec_name), impl_name: sp_runtime::RuntimeString::Owned(val.impl_name), @@ -122,7 +100,7 @@ impl TryFrom for RuntimeVersion { } } -impl From for RuntimeVersionWrapper { +impl From for ChainHeadRuntimeVersion { fn from(val: RuntimeVersion) -> Self { Self { spec_name: val.spec_name.into(), @@ -477,7 +455,7 @@ mod tests { ..Default::default() }; - let runtime_event = RuntimeEvent::Valid(RuntimeVersionEvent { spec: runtime }); + let runtime_event = RuntimeEvent::Valid(RuntimeVersionEvent { spec: runtime.into() }); let mut initialized = Initialized { finalized_block_hash: "0x1".into(), finalized_block_runtime: Some(runtime_event), @@ -530,7 +508,7 @@ mod tests { ..Default::default() }; - let runtime_event = RuntimeEvent::Valid(RuntimeVersionEvent { spec: runtime }); + let runtime_event = RuntimeEvent::Valid(RuntimeVersionEvent { spec: runtime.into() }); let mut new_block = NewBlock { block_hash: "0x1".into(), parent_block_hash: "0x2".into(), diff --git a/substrate/client/rpc-spec-v2/src/chain_head/tests.rs b/substrate/client/rpc-spec-v2/src/chain_head/tests.rs index e0551ff7aecea..1d96fff03272a 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/tests.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/tests.rs @@ -244,7 +244,7 @@ async fn follow_with_runtime() { let runtime: RuntimeVersion = serde_json::from_str(runtime_str).unwrap(); let finalized_block_runtime = - Some(RuntimeEvent::Valid(RuntimeVersionEvent { spec: runtime.clone() })); + Some(RuntimeEvent::Valid(RuntimeVersionEvent { spec: runtime.clone().into() })); // Runtime must always be reported with the first event. let expected = FollowEvent::Initialized(Initialized { finalized_block_hash: format!("{:?}", finalized_hash), @@ -308,7 +308,8 @@ async fn follow_with_runtime() { let best_hash = block.header.hash(); client.import(BlockOrigin::Own, block.clone()).await.unwrap(); - let new_runtime = Some(RuntimeEvent::Valid(RuntimeVersionEvent { spec: runtime.clone() })); + let new_runtime = + Some(RuntimeEvent::Valid(RuntimeVersionEvent { spec: runtime.clone().into() })); let event: FollowEvent = get_next_event(&mut sub).await; let expected = FollowEvent::NewBlock(NewBlock { block_hash: format!("{:?}", best_hash), diff --git a/substrate/client/rpc-spec-v2/src/transaction/event.rs b/substrate/client/rpc-spec-v2/src/transaction/event.rs index 2a9b4831a9ef5..8b80fcda17beb 100644 --- a/substrate/client/rpc-spec-v2/src/transaction/event.rs +++ b/substrate/client/rpc-spec-v2/src/transaction/event.rs @@ -34,7 +34,6 @@ use serde::{Deserialize, Serialize}; #[serde(rename_all = "camelCase")] pub struct TransactionBroadcasted { /// The number of peers the transaction was broadcasted to. - #[serde(with = "as_string")] pub num_peers: usize, } @@ -223,22 +222,6 @@ impl From> for TransactionEvent { } } -/// Serialize and deserialize helper as string. -mod as_string { - use super::*; - use serde::{Deserializer, Serializer}; - - pub fn serialize(data: &usize, serializer: S) -> Result { - data.to_string().serialize(serializer) - } - - pub fn deserialize<'de, D: Deserializer<'de>>(deserializer: D) -> Result { - String::deserialize(deserializer)? - .parse() - .map_err(|e| serde::de::Error::custom(format!("Parsing failed: {}", e))) - } -} - #[cfg(test)] mod tests { use super::*; @@ -262,7 +245,7 @@ mod tests { TransactionEvent::Broadcasted(TransactionBroadcasted { num_peers: 2 }); let ser = serde_json::to_string(&event).unwrap(); - let exp = r#"{"event":"broadcasted","numPeers":"2"}"#; + let exp = r#"{"event":"broadcasted","numPeers":2}"#; assert_eq!(ser, exp); let event_dec: TransactionEvent<()> = serde_json::from_str(exp).unwrap(); From 82f6ae69c1f9cabc558a66b941df81d783c5145e Mon Sep 17 00:00:00 2001 From: Tadeo hepperle Date: Thu, 28 Sep 2023 11:37:48 +0200 Subject: [PATCH 6/6] remove conversion from ChainHeadRuntimeVersion to RuntimeVersion --- .../rpc-spec-v2/src/chain_head/event.rs | 38 ------------------- 1 file changed, 38 deletions(-) diff --git a/substrate/client/rpc-spec-v2/src/chain_head/event.rs b/substrate/client/rpc-spec-v2/src/chain_head/event.rs index acad396e0fa4e..b5f9d6cc2fff4 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/event.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/event.rs @@ -22,7 +22,6 @@ use serde::{ser::SerializeStruct, Deserialize, Serialize, Serializer}; use sp_api::ApiError; use sp_version::RuntimeVersion; use std::collections::BTreeMap; -use thiserror::Error; /// The operation could not be processed due to an error. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] @@ -63,43 +62,6 @@ pub struct ChainHeadRuntimeVersion { pub transaction_version: u32, } -/// Error when trying to convert a `ChainHeadRuntimeVersion` into a `RuntimeVersion` -#[derive(Error, Debug)] -pub enum IntoRuntimeVersionError { - /// Invalid characters in hex string. - #[error("invalid hex string provided")] - FromHexError(#[from] sp_core::bytes::FromHexError), - /// Hex string did not contain exactly 8 bytes. - #[error("buffer must be 8 bytes long")] - InvalidLength, -} - -impl TryFrom for RuntimeVersion { - type Error = IntoRuntimeVersionError; - - fn try_from(val: ChainHeadRuntimeVersion) -> Result { - let apis = val - .apis - .into_iter() - .map(|(api, version)| -> Result<([u8; 8], u32), IntoRuntimeVersionError> { - let bytes_vec = sp_core::bytes::from_hex(&api)?; - let api: [u8; 8] = - bytes_vec.try_into().map_err(|_| IntoRuntimeVersionError::InvalidLength)?; - Ok((api, version)) - }) - .collect::>()?; - Ok(Self { - spec_name: sp_runtime::RuntimeString::Owned(val.spec_name), - impl_name: sp_runtime::RuntimeString::Owned(val.impl_name), - spec_version: val.spec_version, - impl_version: val.impl_version, - apis, - transaction_version: val.transaction_version, - ..Default::default() - }) - } -} - impl From for ChainHeadRuntimeVersion { fn from(val: RuntimeVersion) -> Self { Self {