From 2985373223b30ea80126ef1ec80ab8dd2c776a7a Mon Sep 17 00:00:00 2001 From: Tadeo Hepperle <62739623+tadeohepperle@users.noreply.github.com> Date: Fri, 29 Sep 2023 09:08:19 +0200 Subject: [PATCH] [RPC-Spec-V2] chainHead: use integer for block index and adjust RuntimeVersion JSON format (#1666) This PR adjusts the serialized format of the the returned RuntimeVersion in the rpc-spec-v2 methods. This is done to match the format defined here: https://paritytech.github.io/json-rpc-interface-spec/api/chainHead_unstable_follow.html#about-the-runtime - ##### `apis` field as object `apis` field of `RuntimeVersion` is now returned as an object, e.g. ``` "apis": { "0xdf6acb689907609b": 3, "0x37e397fc7c91f5e4": 1, } ``` instead of ``` "apis": [ ["0xdf6acb689907609b", 3], ["0x37e397fc7c91f5e4", 1], ] ``` - ##### removed `stateVersion` and `authoringVersion` `stateVersion` and `authoringVersion` are no longer returned in the `RuntimeVersion` JSON Object. - ##### block index in chain head events as integer ### Related Issues Closes: #1507 Closes: #1146 ### Testing Done Adjusted existing tests to make sure data is returned in the correct format. --- substrate/client/rpc-spec-v2/Cargo.toml | 1 - .../src/chain_head/chain_head_follow.rs | 4 +- .../rpc-spec-v2/src/chain_head/error.rs | 1 - .../rpc-spec-v2/src/chain_head/event.rs | 54 ++++++++++++++++--- .../rpc-spec-v2/src/chain_head/tests.rs | 9 ++-- .../rpc-spec-v2/src/transaction/event.rs | 24 ++------- 6 files changed, 56 insertions(+), 37 deletions(-) diff --git a/substrate/client/rpc-spec-v2/Cargo.toml b/substrate/client/rpc-spec-v2/Cargo.toml index f4068d1bc5940..1eaed65706e04 100644 --- a/substrate/client/rpc-spec-v2/Cargo.toml +++ b/substrate/client/rpc-spec-v2/Cargo.toml @@ -36,7 +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 } - [dev-dependencies] serde_json = "1.0" tokio = { version = "1.22.0", features = ["macros"] } 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/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 65bc8b247c880..b5f9d6cc2fff4 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,7 @@ use serde::{ser::SerializeStruct, Deserialize, Serialize, Serializer}; use sp_api::ApiError; use sp_version::RuntimeVersion; +use std::collections::BTreeMap; /// The operation could not be processed due to an error. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] @@ -35,11 +36,47 @@ 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)] +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] #[serde(rename_all = "camelCase")] pub struct RuntimeVersionEvent { /// The runtime version. - pub spec: RuntimeVersion, + 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")] +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, +} + +impl From for ChainHeadRuntimeVersion { + 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 @@ -380,7 +417,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), @@ -391,8 +428,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); @@ -429,10 +466,11 @@ 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() }; - 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(), @@ -445,8 +483,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":[],"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 1d5b45260a22d..8aaeb413cdff9 100644 --- a/substrate/client/rpc-spec-v2/src/chain_head/tests.rs +++ b/substrate/client/rpc-spec-v2/src/chain_head/tests.rs @@ -234,17 +234,17 @@ 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,\ + 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\":1}"; + [\"0xed99c5acb25eedf5\",3],[\"0xfbc577b9d747efd6\",1]],\"transactionVersion\":1,\"stateVersion\":0}"; 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 bdc126366fb92..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, } @@ -45,7 +44,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, } @@ -224,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::*; @@ -263,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(); @@ -288,7 +270,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 +285,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();