From b635538485c8eaec5bda5ddf736104704799c14d Mon Sep 17 00:00:00 2001 From: Tadeo hepperle Date: Mon, 12 Jun 2023 19:28:17 +0200 Subject: [PATCH 01/17] diffing pallets and runtime apis --- cli/src/commands/diff.rs | 175 +++++++++++++++++++++++++++++++ cli/src/commands/mod.rs | 1 + cli/src/main.rs | 2 + cli/src/utils.rs | 2 +- metadata/src/lib.rs | 25 +++-- metadata/src/utils/validation.rs | 10 +- 6 files changed, 201 insertions(+), 14 deletions(-) create mode 100644 cli/src/commands/diff.rs diff --git a/cli/src/commands/diff.rs b/cli/src/commands/diff.rs new file mode 100644 index 0000000000..9bf2ac09b0 --- /dev/null +++ b/cli/src/commands/diff.rs @@ -0,0 +1,175 @@ +use std::collections::{HashMap, HashSet}; +use std::path::PathBuf; +use std::str::FromStr; +use clap::{ArgMatches, Args, Command, Error, FromArgMatches, Parser as ClapParser}; +use codec::Decode; +use color_eyre::eyre::eyre; +use frame_metadata::{RuntimeMetadata, RuntimeMetadataPrefixed}; +use frame_metadata::v15::RuntimeMetadataV15; +use jsonrpsee::client_transport::ws::Uri; +use subxt_codegen::utils::{MetadataVersion}; +use subxt_metadata::{ConstantMetadata, Metadata, PalletMetadata, RuntimeApiMetadata}; +use crate::utils::FileOrUrl; + +/// todo: add docs +#[derive(Debug, ClapParser)] +pub struct Opts { + node1: String, + node2: String, + #[clap(long, short)] + version: Option, + #[clap(long, short)] + pallet: Option, +} + +// cargo run -- diff ../artifacts/polkadot_metadata_small.scale ../artifacts/polkadot_metadata_tiny.scale +pub async fn run(opts: Opts) -> color_eyre::Result<()> { + println!("{} {}", opts.node1, opts.node2); + + let node_1_file_or_url: FileOrUrl = (opts.node1.as_str(), opts.version).try_into().map_err(|_| eyre!("{} is not a valid url nor path for node 1.", opts.node1))?; + let node_2_file_or_url: FileOrUrl = (opts.node2.as_str(), opts.version).try_into().map_err(|_| eyre!("{} is not a valid url nor path for node 2.", opts.node2))?; + + let bytes = node_1_file_or_url.fetch().await?; + let node_1_metadata: Metadata = RuntimeMetadataPrefixed::decode(&mut &bytes[..])?.try_into()?; + + let bytes = node_2_file_or_url.fetch().await?; + let node_2_metadata: Metadata = RuntimeMetadataPrefixed::decode(&mut &bytes[..])?.try_into()?; + + let pallet_differences = pallet_differences(&node_1_metadata, &node_2_metadata); + let pallet_differences_names: Vec> = pallet_differences.into_iter().map(|diff| diff.map(|p| p.name().to_string())).collect(); + dbg!(pallet_differences_names); + + let runtime_api_differences = runtime_api_differences(&node_1_metadata, &node_2_metadata); + let runtime_api_differences_names: Vec> = runtime_api_differences.into_iter().map(|diff| diff.map(|p| p.name().to_string())).collect(); + dbg!(runtime_api_differences_names); + + + Ok(()) +} + +impl TryFrom<(&str, Option)> for FileOrUrl { + type Error = (); + + fn try_from(value: (&str, Option)) -> Result { + match PathBuf::from_str(value.0) { + Ok(path_buf) => Ok(FileOrUrl { + url: None, + file: Some(path_buf), + version: value.1, + }), + Err(_) => { + Uri::from_str(value.0).map_err(|_| ()).map(|uri| FileOrUrl { + url: Some(uri), + file: None, + version: value.1, + }) + } + } + } +} + +fn constants_differences<'a>(pallet_metadata_1: &'a PalletMetadata<'a>, pallet_metadata_2: &'a PalletMetadata<'a>) -> Vec>> { + let mut constants: HashMap<&str, (Option<&'a ConstantMetadata>, Option<&'a ConstantMetadata>)> = HashMap::new(); + for constant in pallet_metadata_1.constants() { + let (e1, _) = constants.entry(constant.name()).or_default(); + *e1 = Some(constant); + } + + for constant in pallet_metadata_2.constants() { + let (e1, e2) = constants.entry(constant.name()).or_default(); + // skip all entries with the same hash: + if let Some(e1_inner) = e1 { + let e1_hash = get_type_hash(pallet_metadata_1, e1_inner.ty(), &mut HashSet::new()); + if constan == runtime_api.hash() { + *e1 = None; + continue; + } + } + *e2 = Some(constant); + } + todo!() +} + + +fn runtime_api_differences<'a>(metadata_1: &'a Metadata, metadata_2: &'a Metadata) -> Vec>> { + let mut runtime_apis: HashMap<&str, (Option>, Option>)> = HashMap::new(); + + for runtime_api in metadata_1.runtime_api_traits() { + let (e1, _) = runtime_apis.entry(runtime_api.name()).or_default(); + *e1 = Some(runtime_api); + } + + for runtime_api in metadata_2.runtime_api_traits() { + let (e1, e2) = runtime_apis.entry(runtime_api.name()).or_default(); + // skip all entries with the same hash: + if let Some(e1_inner) = e1 { + if e1_inner.hash() == runtime_api.hash() { + *e1 = None; + continue; + } + } + *e2 = Some(runtime_api); + } + runtime_apis.into_iter().map(|(_, tuple)| + Diff::try_from(tuple).unwrap() + ).collect() +} + +fn pallet_differences<'a>(metadata_1: &'a Metadata, metadata_2: &'a Metadata) -> Vec>> { + let mut pallets: HashMap<&str, (Option>, Option>)> = HashMap::new(); + + for pallet_metadata in metadata_1.pallets() { + let (e1, _) = pallets.entry(pallet_metadata.name()).or_default(); + *e1 = Some(pallet_metadata); + } + + for pallet_metadata in metadata_2.pallets() { + let (e1, e2) = pallets.entry(pallet_metadata.name()).or_default(); + // skip all entries with the same hash: + if let Some(e1_inner) = e1 { + if e1_inner.hash() == pallet_metadata.hash() { + *e1 = None; + continue; + } + } + *e2 = Some(pallet_metadata); + } + + pallets.into_iter().map(|(_, tuple)| + Diff::try_from(tuple).unwrap() + ).collect() +} + +#[derive(Debug, Clone)] +enum Diff { + Added(T), + Changed { from: T, to: T }, + Removed(T), +} + +impl Diff { + pub fn map(self, mut f: F) -> Diff where F: FnMut(T) -> R { + match self { + Diff::Added(new) => Diff::Added(f(new)), + Diff::Changed { from, to } => Diff::Changed { from: f(from), to: f(to) }, + Diff::Removed(old) => Diff::Removed(f(old)), + } + } +} + +impl TryFrom<(Option, Option)> for Diff { + type Error = (); + + fn try_from(value: (Option, Option)) -> Result { + match value { + (None, None) => Err(()), + (Some(old), None) => { Ok(Diff::Removed(old)) } + (None, Some(new)) => { Ok(Diff::Added(new)) } + (Some(old), Some(new)) => { + Ok(Diff::Changed { from: old, to: new }) + } + } + } +} + + diff --git a/cli/src/commands/mod.rs b/cli/src/commands/mod.rs index 69542ad40d..10a52a5a2f 100644 --- a/cli/src/commands/mod.rs +++ b/cli/src/commands/mod.rs @@ -7,3 +7,4 @@ pub mod compatibility; pub mod explore; pub mod metadata; pub mod version; +pub mod diff; \ No newline at end of file diff --git a/cli/src/main.rs b/cli/src/main.rs index 956fd5e0b7..d31e39cc6c 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -14,6 +14,7 @@ enum Command { Metadata(commands::metadata::Opts), Codegen(commands::codegen::Opts), Compatibility(commands::compatibility::Opts), + Diff(commands::diff::Opts), Version(commands::version::Opts), Explore(commands::explore::Opts), } @@ -27,6 +28,7 @@ async fn main() -> color_eyre::Result<()> { Command::Metadata(opts) => commands::metadata::run(opts).await, Command::Codegen(opts) => commands::codegen::run(opts).await, Command::Compatibility(opts) => commands::compatibility::run(opts).await, + Command::Diff(opts) => commands::diff::run(opts).await, Command::Version(opts) => commands::version::run(opts), Command::Explore(opts) => commands::explore::run(opts).await, } diff --git a/cli/src/utils.rs b/cli/src/utils.rs index ec2e224a08..c2cb043181 100644 --- a/cli/src/utils.rs +++ b/cli/src/utils.rs @@ -32,7 +32,7 @@ pub struct FileOrUrl { /// /// Defaults to 14. #[clap(long)] - version: Option, + pub(crate) version: Option, } impl FileOrUrl { diff --git a/metadata/src/lib.rs b/metadata/src/lib.rs index b629cbe47e..f59c55c2e7 100644 --- a/metadata/src/lib.rs +++ b/metadata/src/lib.rs @@ -78,7 +78,7 @@ impl Metadata { } /// An iterator over all of the available pallets. - pub fn pallets(&self) -> impl ExactSizeIterator> { + pub fn pallets(&self) -> impl ExactSizeIterator> { self.pallets.values().iter().map(|inner| PalletMetadata { inner, types: self.types(), @@ -109,7 +109,7 @@ impl Metadata { } /// An iterator over all of the runtime APIs. - pub fn runtime_api_traits(&self) -> impl ExactSizeIterator> { + pub fn runtime_api_traits(&self) -> impl ExactSizeIterator> { self.apis.values().iter().map(|inner| RuntimeApiMetadata { inner, types: self.types(), @@ -132,9 +132,9 @@ impl Metadata { /// Filter out any pallets that we don't want to keep, retaining only those that we do. pub fn retain(&mut self, pallet_filter: F, api_filter: G) - where - F: FnMut(&str) -> bool, - G: FnMut(&str) -> bool, + where + F: FnMut(&str) -> bool, + G: FnMut(&str) -> bool, { utils::retain::retain_metadata(self, pallet_filter, api_filter); } @@ -236,7 +236,7 @@ impl<'a> PalletMetadata<'a> { } /// An iterator over the constants in this pallet. - pub fn constants(&self) -> impl ExactSizeIterator { + pub fn constants(&self) -> impl ExactSizeIterator { self.inner.constants.values().iter() } @@ -303,7 +303,7 @@ impl StorageMetadata { } /// An iterator over the storage entries. - pub fn entries(&self) -> impl ExactSizeIterator { + pub fn entries(&self) -> impl ExactSizeIterator { self.entries.values().iter() } @@ -490,7 +490,7 @@ pub struct RuntimeApiMetadata<'a> { impl<'a> RuntimeApiMetadata<'a> { /// Trait name. - pub fn name(&self) -> &str { + pub fn name(&self) -> &'a str { &self.inner.name } /// Trait documentation. @@ -498,7 +498,7 @@ impl<'a> RuntimeApiMetadata<'a> { &self.inner.docs } /// An iterator over the trait methods. - pub fn methods(&self) -> impl ExactSizeIterator { + pub fn methods(&self) -> impl ExactSizeIterator { self.inner.methods.values().iter() } /// Get a specific trait method given its name. @@ -509,6 +509,11 @@ impl<'a> RuntimeApiMetadata<'a> { pub fn method_hash(&self, method_name: &str) -> Option<[u8; 32]> { crate::utils::validation::get_runtime_api_hash(self, method_name) } + + /// Return a hash for the entire pallet. + pub fn hash(&self) -> [u8; 32] { + crate::utils::validation::get_runtime_trait_hash(*self) + } } #[derive(Debug, Clone)] @@ -544,7 +549,7 @@ impl RuntimeApiMethodMetadata { &self.docs } /// Method inputs. - pub fn inputs(&self) -> impl ExactSizeIterator { + pub fn inputs(&self) -> impl ExactSizeIterator { self.inputs.iter() } /// Method return type. diff --git a/metadata/src/utils/validation.rs b/metadata/src/utils/validation.rs index 6c58f396a3..5ef62632bd 100644 --- a/metadata/src/utils/validation.rs +++ b/metadata/src/utils/validation.rs @@ -283,7 +283,7 @@ fn get_runtime_method_hash( } /// Obtain the hash of all of a runtime API trait, including all of its methods. -fn get_runtime_trait_hash(trait_metadata: RuntimeApiMetadata) -> [u8; HASH_LEN] { +pub fn get_runtime_trait_hash(trait_metadata: RuntimeApiMetadata) -> [u8; HASH_LEN] { let mut visited_ids = HashSet::new(); let trait_name = &*trait_metadata.inner.name; let method_bytes = trait_metadata @@ -496,6 +496,7 @@ mod tests { struct A { pub b: Box, } + #[allow(dead_code)] #[derive(scale_info::TypeInfo)] struct B { @@ -507,6 +508,7 @@ mod tests { #[derive(scale_info::TypeInfo)] // TypeDef::Composite with TypeDef::Array with Typedef::Primitive. struct AccountId32([u8; HASH_LEN]); + #[allow(dead_code)] #[derive(scale_info::TypeInfo)] // TypeDef::Variant. @@ -525,6 +527,7 @@ mod tests { // TypeDef::BitSequence. BitSeq(BitVec), } + #[allow(dead_code)] #[derive(scale_info::TypeInfo)] // Ensure recursive types and TypeDef variants are captured. @@ -533,6 +536,7 @@ mod tests { composite: AccountId32, type_def: DigestItem, } + #[allow(dead_code)] #[derive(scale_info::TypeInfo)] // Simulate a PalletCallMetadata. @@ -591,8 +595,8 @@ mod tests { meta_type::<()>(), vec![], ) - .try_into() - .expect("can build valid metadata") + .try_into() + .expect("can build valid metadata") } #[test] From e67e16e04c102d44c78765ecf2983a0e729c9443 Mon Sep 17 00:00:00 2001 From: Tadeo hepperle Date: Tue, 13 Jun 2023 16:50:41 +0200 Subject: [PATCH 02/17] print diff --- cli/src/commands/diff.rs | 230 +++++++++++++++++++++++++++++++++------ cli/src/utils.rs | 8 +- 2 files changed, 203 insertions(+), 35 deletions(-) diff --git a/cli/src/commands/diff.rs b/cli/src/commands/diff.rs index 9bf2ac09b0..251c1bd0ce 100644 --- a/cli/src/commands/diff.rs +++ b/cli/src/commands/diff.rs @@ -7,9 +7,13 @@ use color_eyre::eyre::eyre; use frame_metadata::{RuntimeMetadata, RuntimeMetadataPrefixed}; use frame_metadata::v15::RuntimeMetadataV15; use jsonrpsee::client_transport::ws::Uri; +use scale_info::form::PortableForm; +use scale_info::Variant; use subxt_codegen::utils::{MetadataVersion}; -use subxt_metadata::{ConstantMetadata, Metadata, PalletMetadata, RuntimeApiMetadata}; -use crate::utils::FileOrUrl; +use subxt_metadata::{ConstantMetadata, Metadata, PalletMetadata, RuntimeApiMetadata, StorageEntryMetadata}; +use crate::utils::{FileOrUrl}; +use std::io::Write; +use color_eyre::owo_colors::OwoColorize; /// todo: add docs #[derive(Debug, ClapParser)] @@ -24,8 +28,117 @@ pub struct Opts { // cargo run -- diff ../artifacts/polkadot_metadata_small.scale ../artifacts/polkadot_metadata_tiny.scale pub async fn run(opts: Opts) -> color_eyre::Result<()> { - println!("{} {}", opts.node1, opts.node2); + let (node_1_metadata, node_2_metadata) = get_metadata(&opts).await?; + let node_diff = MetadataDiff::construct(&node_1_metadata, &node_2_metadata); + let mut output = std::io::stdout(); + + + if node_diff.is_empty() { + writeln!(output, "No difference in Metadata found.")?; + return Ok(()); + } + if !node_diff.pallets.is_empty() { + writeln!(output, "Pallets:")?; + for diff in node_diff.pallets { + match diff { + Diff::Added(new) => writeln!(output, "{}", format!(" + {} (Added)", new.name()).green())?, + Diff::Removed(old) => writeln!(output, "{}", format!(" - {} (Removed)", old.name()).red())?, + Diff::Changed { from, to } => { + writeln!(output, "{}", format!(" ~ {} (Changed)", from.name()).yellow())?; + + let pallet_diff = PalletDiff::construct(&from, &to); + if !pallet_diff.calls.is_empty() { + writeln!(output, " Calls:")?; + for diff in pallet_diff.calls { + match diff { + Diff::Added(new) => writeln!(output, "{}", format!(" + {} (Added)", &new.name).green())?, + Diff::Removed(old) => writeln!(output, "{}", format!(" - {} (Removed)", &old.name).red())?, + Diff::Changed { from, to } => writeln!(output, "{}", format!(" ~ {} (Changed)", &from.name).yellow())?, + } + } + } + + if !pallet_diff.constants.is_empty() { + writeln!(output, " Constants:")?; + for diff in pallet_diff.constants { + match diff { + Diff::Added(new) => writeln!(output, "{}", format!(" + {} (Added)", new.name()).green())?, + Diff::Removed(old) => writeln!(output, "{}", format!(" - {} (Removed)", old.name()).red())?, + Diff::Changed { from, to } => writeln!(output, "{}", format!(" ~ {} (Changed)", from.name()).yellow())?, + } + } + } + + if !pallet_diff.storage_entries.is_empty() { + writeln!(output, " Storage Entries:")?; + for diff in pallet_diff.storage_entries { + match diff { + Diff::Added(new) => writeln!(output, "{}", format!(" + {} (Added)", new.name()).green())?, + Diff::Removed(old) => writeln!(output, "{}", format!(" - {} (Removed)", old.name()).red())?, + Diff::Changed { from, to } => writeln!(output, "{}", format!(" ~ {} (Changed)", from.name()).yellow())?, + } + } + } + } + } + } + } + + if !node_diff.runtime_apis.is_empty() { + writeln!(output, "Runtime APIs:")?; + for diff in node_diff.runtime_apis { + match diff { + Diff::Added(new) => writeln!(output, "{}", format!(" + {} (Added)", new.name()).green())?, + Diff::Removed(old) => writeln!(output, "{}", format!(" - {} (Removed)", old.name()).red())?, + Diff::Changed { from, to } => writeln!(output, "{}", format!(" ~ {} (Changed)", from.name()).yellow())?, + } + } + } + Ok(()) +} + + +struct MetadataDiff<'a> { + pallets: Vec>>, + runtime_apis: Vec>>, +} + +impl<'a> MetadataDiff<'a> { + fn construct(metadata_1: &'a Metadata, metadata_2: &'a Metadata) -> MetadataDiff<'a> { + let pallets = pallet_differences(metadata_1, metadata_2); + let runtime_apis = runtime_api_differences(metadata_1, metadata_2); + MetadataDiff { pallets, runtime_apis } + } + + fn is_empty(&self) -> bool { + self.pallets.is_empty() && self.runtime_apis.is_empty() + } +} + + +#[derive(Default)] +struct PalletDiff<'a> { + calls: Vec>>, + constants: Vec>, + storage_entries: Vec>, +} + +impl<'a> PalletDiff<'a> { + fn construct(pallet_metadata_1: &'a PalletMetadata<'a>, pallet_metadata_2: &'a PalletMetadata<'a>) -> PalletDiff<'a> { + let calls = calls_differences(&pallet_metadata_1, &pallet_metadata_2); + let constants = constants_differences(&pallet_metadata_1, &pallet_metadata_2); + let storage_entries = storage_differences(&pallet_metadata_1, &pallet_metadata_2); + PalletDiff { calls, constants, storage_entries } + } + + fn is_empty(&self) -> bool { + self.calls.is_empty() && self.constants.is_empty() && self.storage_entries.is_empty() + } +} + + +async fn get_metadata(opts: &Opts) -> color_eyre::Result<(Metadata, Metadata)> { let node_1_file_or_url: FileOrUrl = (opts.node1.as_str(), opts.version).try_into().map_err(|_| eyre!("{} is not a valid url nor path for node 1.", opts.node1))?; let node_2_file_or_url: FileOrUrl = (opts.node2.as_str(), opts.version).try_into().map_err(|_| eyre!("{} is not a valid url nor path for node 2.", opts.node2))?; @@ -35,59 +148,111 @@ pub async fn run(opts: Opts) -> color_eyre::Result<()> { let bytes = node_2_file_or_url.fetch().await?; let node_2_metadata: Metadata = RuntimeMetadataPrefixed::decode(&mut &bytes[..])?.try_into()?; - let pallet_differences = pallet_differences(&node_1_metadata, &node_2_metadata); - let pallet_differences_names: Vec> = pallet_differences.into_iter().map(|diff| diff.map(|p| p.name().to_string())).collect(); - dbg!(pallet_differences_names); - - let runtime_api_differences = runtime_api_differences(&node_1_metadata, &node_2_metadata); - let runtime_api_differences_names: Vec> = runtime_api_differences.into_iter().map(|diff| diff.map(|p| p.name().to_string())).collect(); - dbg!(runtime_api_differences_names); - - - Ok(()) + Ok((node_1_metadata, node_2_metadata)) } impl TryFrom<(&str, Option)> for FileOrUrl { type Error = (); fn try_from(value: (&str, Option)) -> Result { - match PathBuf::from_str(value.0) { - Ok(path_buf) => Ok(FileOrUrl { + println!("value {}", &value.0); + let path = std::path::Path::new(&value.0); + if path.exists() { + Ok(FileOrUrl { url: None, - file: Some(path_buf), + file: Some(PathBuf::from(value.0)), version: value.1, - }), - Err(_) => { - Uri::from_str(value.0).map_err(|_| ()).map(|uri| FileOrUrl { - url: Some(uri), - file: None, - version: value.1, - }) + }) + } else { + Uri::from_str(value.0).map_err(|_| ()).map(|uri| FileOrUrl { + url: Some(uri), + file: None, + version: value.1, + }) + } + } +} + +fn storage_differences<'a>(pallet_metadata_1: &'a PalletMetadata<'a>, pallet_metadata_2: &'a PalletMetadata<'a>) -> Vec> { + let mut storage_entries: HashMap<&str, (Option<&'a StorageEntryMetadata>, Option<&'a StorageEntryMetadata>)> = HashMap::new(); + if let Some(storage_metadata) = pallet_metadata_1.storage() { + for storage_entry in storage_metadata.entries() { + let (e1, _) = storage_entries.entry(storage_entry.name()).or_default(); + *e1 = Some(storage_entry); + } + } + if let Some(storage_metadata) = pallet_metadata_2.storage() { + for storage_entry in storage_metadata.entries() { + let (e1, e2) = storage_entries.entry(storage_entry.name()).or_default(); + // skip all entries with the same hash: + if let Some(e1_inner) = e1 { + let e1_hash = pallet_metadata_1.storage_hash(e1_inner.name()).expect("storage entry should be present"); + let e2_hash = pallet_metadata_2.storage_hash(storage_entry.name()).expect("storage entry should be present"); + if e1_hash == e2_hash { + storage_entries.remove(storage_entry.name()); + continue; + } + } + *e2 = Some(storage_entry); + } + } + storage_entries.into_iter().map(|(_, tuple)| + Diff::try_from(tuple).unwrap() + ).collect() +} + + +fn calls_differences<'a>(pallet_metadata_1: &'a PalletMetadata<'a>, pallet_metadata_2: &'a PalletMetadata<'a>) -> Vec>> { + let mut calls: HashMap<&str, (Option<&'a Variant>, Option<&'a Variant>)> = HashMap::new(); + if let Some(call_variants) = pallet_metadata_1.call_variants() { + for call_variant in call_variants { + let (e1, _) = calls.entry(&call_variant.name).or_default(); + *e1 = Some(call_variant); + } + } + if let Some(call_variants) = pallet_metadata_2.call_variants() { + for call_variant in call_variants { + let (e1, e2) = calls.entry(&call_variant.name).or_default(); + // skip all entries with the same hash: + if let Some(e1_inner) = e1 { + let e1_hash = pallet_metadata_1.call_hash(&e1_inner.name).expect("call should be present"); + let e2_hash = pallet_metadata_2.call_hash(&call_variant.name).expect("call should be present"); + if e1_hash == e2_hash { + calls.remove(call_variant.name.as_str()); + continue; + } } + *e2 = Some(call_variant); } } + calls.into_iter().map(|(_, tuple)| + Diff::try_from(tuple).unwrap() + ).collect() } -fn constants_differences<'a>(pallet_metadata_1: &'a PalletMetadata<'a>, pallet_metadata_2: &'a PalletMetadata<'a>) -> Vec>> { + +fn constants_differences<'a>(pallet_metadata_1: &'a PalletMetadata<'a>, pallet_metadata_2: &'a PalletMetadata<'a>) -> Vec> { let mut constants: HashMap<&str, (Option<&'a ConstantMetadata>, Option<&'a ConstantMetadata>)> = HashMap::new(); for constant in pallet_metadata_1.constants() { let (e1, _) = constants.entry(constant.name()).or_default(); *e1 = Some(constant); } - for constant in pallet_metadata_2.constants() { let (e1, e2) = constants.entry(constant.name()).or_default(); // skip all entries with the same hash: if let Some(e1_inner) = e1 { - let e1_hash = get_type_hash(pallet_metadata_1, e1_inner.ty(), &mut HashSet::new()); - if constan == runtime_api.hash() { - *e1 = None; + let e1_hash = pallet_metadata_1.constant_hash(e1_inner.name()).expect("constant should be present"); + let e2_hash = pallet_metadata_2.constant_hash(constant.name()).expect("constant should be present"); + if e1_hash == e2_hash { + constants.remove(constant.name()); continue; } } *e2 = Some(constant); } - todo!() + constants.into_iter().map(|(_, tuple)| + Diff::try_from(tuple).unwrap() + ).collect() } @@ -104,7 +269,7 @@ fn runtime_api_differences<'a>(metadata_1: &'a Metadata, metadata_2: &'a Metadat // skip all entries with the same hash: if let Some(e1_inner) = e1 { if e1_inner.hash() == runtime_api.hash() { - *e1 = None; + runtime_apis.remove(runtime_api.name()); continue; } } @@ -128,7 +293,7 @@ fn pallet_differences<'a>(metadata_1: &'a Metadata, metadata_2: &'a Metadata) -> // skip all entries with the same hash: if let Some(e1_inner) = e1 { if e1_inner.hash() == pallet_metadata.hash() { - *e1 = None; + pallets.remove(pallet_metadata.name()); continue; } } @@ -148,7 +313,7 @@ enum Diff { } impl Diff { - pub fn map(self, mut f: F) -> Diff where F: FnMut(T) -> R { + fn map(self, mut f: F) -> Diff where F: FnMut(T) -> R { match self { Diff::Added(new) => Diff::Added(f(new)), Diff::Changed { from, to } => Diff::Changed { from: f(from), to: f(to) }, @@ -172,4 +337,3 @@ impl TryFrom<(Option, Option)> for Diff { } } - diff --git a/cli/src/utils.rs b/cli/src/utils.rs index c2cb043181..4e3a7e3092 100644 --- a/cli/src/utils.rs +++ b/cli/src/utils.rs @@ -6,7 +6,11 @@ use clap::Args; use color_eyre::eyre; use std::{fs, io::Read, path::PathBuf}; +use color_eyre::eyre::eyre; +use scale_info::{PortableRegistry, Type, TypeDef, TypeDefVariant}; +use scale_info::form::PortableForm; use subxt_codegen::utils::{MetadataVersion, Uri}; +use subxt_metadata::PalletMetadata; pub mod type_description; pub mod type_example; @@ -63,7 +67,7 @@ impl FileOrUrl { uri, version.unwrap_or_default(), ) - .await?), + .await?), // Default if neither is provided; fetch from local url (None, None, version) => { let uri = Uri::from_static("ws://localhost:9944"); @@ -93,4 +97,4 @@ pub(crate) fn with_indent(s: String, indent: usize) -> String { .map(|line| format!("{indent_str}{line}")) .collect::>() .join("\n") -} +} \ No newline at end of file From 3adcf3d1aa2d841482193b3e18fe04f1a43ec0b9 Mon Sep 17 00:00:00 2001 From: Tadeo hepperle Date: Tue, 13 Jun 2023 16:51:21 +0200 Subject: [PATCH 03/17] clippy fix and format --- cli/src/commands/diff.rs | 282 ++++++++++++++++++++++++++++----------- cli/src/commands/mod.rs | 2 +- cli/src/utils.rs | 9 +- 3 files changed, 207 insertions(+), 86 deletions(-) diff --git a/cli/src/commands/diff.rs b/cli/src/commands/diff.rs index 251c1bd0ce..0ab6636f88 100644 --- a/cli/src/commands/diff.rs +++ b/cli/src/commands/diff.rs @@ -1,19 +1,21 @@ -use std::collections::{HashMap, HashSet}; -use std::path::PathBuf; -use std::str::FromStr; -use clap::{ArgMatches, Args, Command, Error, FromArgMatches, Parser as ClapParser}; +use clap::{FromArgMatches, Parser as ClapParser}; use codec::Decode; use color_eyre::eyre::eyre; -use frame_metadata::{RuntimeMetadata, RuntimeMetadataPrefixed}; -use frame_metadata::v15::RuntimeMetadataV15; +use frame_metadata::RuntimeMetadataPrefixed; +use std::collections::HashMap; +use std::path::PathBuf; +use std::str::FromStr; + +use crate::utils::FileOrUrl; +use color_eyre::owo_colors::OwoColorize; use jsonrpsee::client_transport::ws::Uri; use scale_info::form::PortableForm; use scale_info::Variant; -use subxt_codegen::utils::{MetadataVersion}; -use subxt_metadata::{ConstantMetadata, Metadata, PalletMetadata, RuntimeApiMetadata, StorageEntryMetadata}; -use crate::utils::{FileOrUrl}; use std::io::Write; -use color_eyre::owo_colors::OwoColorize; +use subxt_codegen::utils::MetadataVersion; +use subxt_metadata::{ + ConstantMetadata, Metadata, PalletMetadata, RuntimeApiMetadata, StorageEntryMetadata, +}; /// todo: add docs #[derive(Debug, ClapParser)] @@ -33,7 +35,6 @@ pub async fn run(opts: Opts) -> color_eyre::Result<()> { let node_diff = MetadataDiff::construct(&node_1_metadata, &node_2_metadata); let mut output = std::io::stdout(); - if node_diff.is_empty() { writeln!(output, "No difference in Metadata found.")?; return Ok(()); @@ -42,19 +43,43 @@ pub async fn run(opts: Opts) -> color_eyre::Result<()> { writeln!(output, "Pallets:")?; for diff in node_diff.pallets { match diff { - Diff::Added(new) => writeln!(output, "{}", format!(" + {} (Added)", new.name()).green())?, - Diff::Removed(old) => writeln!(output, "{}", format!(" - {} (Removed)", old.name()).red())?, + Diff::Added(new) => writeln!( + output, + "{}", + format!(" + {} (Added)", new.name()).green() + )?, + Diff::Removed(old) => writeln!( + output, + "{}", + format!(" - {} (Removed)", old.name()).red() + )?, Diff::Changed { from, to } => { - writeln!(output, "{}", format!(" ~ {} (Changed)", from.name()).yellow())?; + writeln!( + output, + "{}", + format!(" ~ {} (Changed)", from.name()).yellow() + )?; let pallet_diff = PalletDiff::construct(&from, &to); if !pallet_diff.calls.is_empty() { writeln!(output, " Calls:")?; for diff in pallet_diff.calls { match diff { - Diff::Added(new) => writeln!(output, "{}", format!(" + {} (Added)", &new.name).green())?, - Diff::Removed(old) => writeln!(output, "{}", format!(" - {} (Removed)", &old.name).red())?, - Diff::Changed { from, to } => writeln!(output, "{}", format!(" ~ {} (Changed)", &from.name).yellow())?, + Diff::Added(new) => writeln!( + output, + "{}", + format!(" + {} (Added)", &new.name).green() + )?, + Diff::Removed(old) => writeln!( + output, + "{}", + format!(" - {} (Removed)", &old.name).red() + )?, + Diff::Changed { from, to: _ } => writeln!( + output, + "{}", + format!(" ~ {} (Changed)", &from.name).yellow() + )?, } } } @@ -63,9 +88,21 @@ pub async fn run(opts: Opts) -> color_eyre::Result<()> { writeln!(output, " Constants:")?; for diff in pallet_diff.constants { match diff { - Diff::Added(new) => writeln!(output, "{}", format!(" + {} (Added)", new.name()).green())?, - Diff::Removed(old) => writeln!(output, "{}", format!(" - {} (Removed)", old.name()).red())?, - Diff::Changed { from, to } => writeln!(output, "{}", format!(" ~ {} (Changed)", from.name()).yellow())?, + Diff::Added(new) => writeln!( + output, + "{}", + format!(" + {} (Added)", new.name()).green() + )?, + Diff::Removed(old) => writeln!( + output, + "{}", + format!(" - {} (Removed)", old.name()).red() + )?, + Diff::Changed { from, to: _ } => writeln!( + output, + "{}", + format!(" ~ {} (Changed)", from.name()).yellow() + )?, } } } @@ -74,9 +111,21 @@ pub async fn run(opts: Opts) -> color_eyre::Result<()> { writeln!(output, " Storage Entries:")?; for diff in pallet_diff.storage_entries { match diff { - Diff::Added(new) => writeln!(output, "{}", format!(" + {} (Added)", new.name()).green())?, - Diff::Removed(old) => writeln!(output, "{}", format!(" - {} (Removed)", old.name()).red())?, - Diff::Changed { from, to } => writeln!(output, "{}", format!(" ~ {} (Changed)", from.name()).yellow())?, + Diff::Added(new) => writeln!( + output, + "{}", + format!(" + {} (Added)", new.name()).green() + )?, + Diff::Removed(old) => writeln!( + output, + "{}", + format!(" - {} (Removed)", old.name()).red() + )?, + Diff::Changed { from, to: _ } => writeln!( + output, + "{}", + format!(" ~ {} (Changed)", from.name()).yellow() + )?, } } } @@ -89,16 +138,27 @@ pub async fn run(opts: Opts) -> color_eyre::Result<()> { writeln!(output, "Runtime APIs:")?; for diff in node_diff.runtime_apis { match diff { - Diff::Added(new) => writeln!(output, "{}", format!(" + {} (Added)", new.name()).green())?, - Diff::Removed(old) => writeln!(output, "{}", format!(" - {} (Removed)", old.name()).red())?, - Diff::Changed { from, to } => writeln!(output, "{}", format!(" ~ {} (Changed)", from.name()).yellow())?, + Diff::Added(new) => writeln!( + output, + "{}", + format!(" + {} (Added)", new.name()).green() + )?, + Diff::Removed(old) => writeln!( + output, + "{}", + format!(" - {} (Removed)", old.name()).red() + )?, + Diff::Changed { from, to: _ } => writeln!( + output, + "{}", + format!(" ~ {} (Changed)", from.name()).yellow() + )?, } } } Ok(()) } - struct MetadataDiff<'a> { pallets: Vec>>, runtime_apis: Vec>>, @@ -108,7 +168,10 @@ impl<'a> MetadataDiff<'a> { fn construct(metadata_1: &'a Metadata, metadata_2: &'a Metadata) -> MetadataDiff<'a> { let pallets = pallet_differences(metadata_1, metadata_2); let runtime_apis = runtime_api_differences(metadata_1, metadata_2); - MetadataDiff { pallets, runtime_apis } + MetadataDiff { + pallets, + runtime_apis, + } } fn is_empty(&self) -> bool { @@ -116,7 +179,6 @@ impl<'a> MetadataDiff<'a> { } } - #[derive(Default)] struct PalletDiff<'a> { calls: Vec>>, @@ -125,11 +187,18 @@ struct PalletDiff<'a> { } impl<'a> PalletDiff<'a> { - fn construct(pallet_metadata_1: &'a PalletMetadata<'a>, pallet_metadata_2: &'a PalletMetadata<'a>) -> PalletDiff<'a> { - let calls = calls_differences(&pallet_metadata_1, &pallet_metadata_2); - let constants = constants_differences(&pallet_metadata_1, &pallet_metadata_2); - let storage_entries = storage_differences(&pallet_metadata_1, &pallet_metadata_2); - PalletDiff { calls, constants, storage_entries } + fn construct( + pallet_metadata_1: &'a PalletMetadata<'a>, + pallet_metadata_2: &'a PalletMetadata<'a>, + ) -> PalletDiff<'a> { + let calls = calls_differences(pallet_metadata_1, pallet_metadata_2); + let constants = constants_differences(pallet_metadata_1, pallet_metadata_2); + let storage_entries = storage_differences(pallet_metadata_1, pallet_metadata_2); + PalletDiff { + calls, + constants, + storage_entries, + } } fn is_empty(&self) -> bool { @@ -137,10 +206,13 @@ impl<'a> PalletDiff<'a> { } } - async fn get_metadata(opts: &Opts) -> color_eyre::Result<(Metadata, Metadata)> { - let node_1_file_or_url: FileOrUrl = (opts.node1.as_str(), opts.version).try_into().map_err(|_| eyre!("{} is not a valid url nor path for node 1.", opts.node1))?; - let node_2_file_or_url: FileOrUrl = (opts.node2.as_str(), opts.version).try_into().map_err(|_| eyre!("{} is not a valid url nor path for node 2.", opts.node2))?; + let node_1_file_or_url: FileOrUrl = (opts.node1.as_str(), opts.version) + .try_into() + .map_err(|_| eyre!("{} is not a valid url nor path for node 1.", opts.node1))?; + let node_2_file_or_url: FileOrUrl = (opts.node2.as_str(), opts.version) + .try_into() + .map_err(|_| eyre!("{} is not a valid url nor path for node 2.", opts.node2))?; let bytes = node_1_file_or_url.fetch().await?; let node_1_metadata: Metadata = RuntimeMetadataPrefixed::decode(&mut &bytes[..])?.try_into()?; @@ -173,8 +245,17 @@ impl TryFrom<(&str, Option)> for FileOrUrl { } } -fn storage_differences<'a>(pallet_metadata_1: &'a PalletMetadata<'a>, pallet_metadata_2: &'a PalletMetadata<'a>) -> Vec> { - let mut storage_entries: HashMap<&str, (Option<&'a StorageEntryMetadata>, Option<&'a StorageEntryMetadata>)> = HashMap::new(); +fn storage_differences<'a>( + pallet_metadata_1: &'a PalletMetadata<'a>, + pallet_metadata_2: &'a PalletMetadata<'a>, +) -> Vec> { + let mut storage_entries: HashMap< + &str, + ( + Option<&'a StorageEntryMetadata>, + Option<&'a StorageEntryMetadata>, + ), + > = HashMap::new(); if let Some(storage_metadata) = pallet_metadata_1.storage() { for storage_entry in storage_metadata.entries() { let (e1, _) = storage_entries.entry(storage_entry.name()).or_default(); @@ -186,8 +267,12 @@ fn storage_differences<'a>(pallet_metadata_1: &'a PalletMetadata<'a>, pallet_met let (e1, e2) = storage_entries.entry(storage_entry.name()).or_default(); // skip all entries with the same hash: if let Some(e1_inner) = e1 { - let e1_hash = pallet_metadata_1.storage_hash(e1_inner.name()).expect("storage entry should be present"); - let e2_hash = pallet_metadata_2.storage_hash(storage_entry.name()).expect("storage entry should be present"); + let e1_hash = pallet_metadata_1 + .storage_hash(e1_inner.name()) + .expect("storage entry should be present"); + let e2_hash = pallet_metadata_2 + .storage_hash(storage_entry.name()) + .expect("storage entry should be present"); if e1_hash == e2_hash { storage_entries.remove(storage_entry.name()); continue; @@ -196,14 +281,23 @@ fn storage_differences<'a>(pallet_metadata_1: &'a PalletMetadata<'a>, pallet_met *e2 = Some(storage_entry); } } - storage_entries.into_iter().map(|(_, tuple)| - Diff::try_from(tuple).unwrap() - ).collect() + storage_entries + .into_values() + .map(|tuple| Diff::try_from(tuple).unwrap()) + .collect() } - -fn calls_differences<'a>(pallet_metadata_1: &'a PalletMetadata<'a>, pallet_metadata_2: &'a PalletMetadata<'a>) -> Vec>> { - let mut calls: HashMap<&str, (Option<&'a Variant>, Option<&'a Variant>)> = HashMap::new(); +fn calls_differences<'a>( + pallet_metadata_1: &'a PalletMetadata<'a>, + pallet_metadata_2: &'a PalletMetadata<'a>, +) -> Vec>> { + let mut calls: HashMap< + &str, + ( + Option<&'a Variant>, + Option<&'a Variant>, + ), + > = HashMap::new(); if let Some(call_variants) = pallet_metadata_1.call_variants() { for call_variant in call_variants { let (e1, _) = calls.entry(&call_variant.name).or_default(); @@ -215,8 +309,12 @@ fn calls_differences<'a>(pallet_metadata_1: &'a PalletMetadata<'a>, pallet_metad let (e1, e2) = calls.entry(&call_variant.name).or_default(); // skip all entries with the same hash: if let Some(e1_inner) = e1 { - let e1_hash = pallet_metadata_1.call_hash(&e1_inner.name).expect("call should be present"); - let e2_hash = pallet_metadata_2.call_hash(&call_variant.name).expect("call should be present"); + let e1_hash = pallet_metadata_1 + .call_hash(&e1_inner.name) + .expect("call should be present"); + let e2_hash = pallet_metadata_2 + .call_hash(&call_variant.name) + .expect("call should be present"); if e1_hash == e2_hash { calls.remove(call_variant.name.as_str()); continue; @@ -225,14 +323,18 @@ fn calls_differences<'a>(pallet_metadata_1: &'a PalletMetadata<'a>, pallet_metad *e2 = Some(call_variant); } } - calls.into_iter().map(|(_, tuple)| - Diff::try_from(tuple).unwrap() - ).collect() + calls + .into_values() + .map(|tuple| Diff::try_from(tuple).unwrap()) + .collect() } - -fn constants_differences<'a>(pallet_metadata_1: &'a PalletMetadata<'a>, pallet_metadata_2: &'a PalletMetadata<'a>) -> Vec> { - let mut constants: HashMap<&str, (Option<&'a ConstantMetadata>, Option<&'a ConstantMetadata>)> = HashMap::new(); +fn constants_differences<'a>( + pallet_metadata_1: &'a PalletMetadata<'a>, + pallet_metadata_2: &'a PalletMetadata<'a>, +) -> Vec> { + let mut constants: HashMap<&str, (Option<&'a ConstantMetadata>, Option<&'a ConstantMetadata>)> = + HashMap::new(); for constant in pallet_metadata_1.constants() { let (e1, _) = constants.entry(constant.name()).or_default(); *e1 = Some(constant); @@ -241,8 +343,12 @@ fn constants_differences<'a>(pallet_metadata_1: &'a PalletMetadata<'a>, pallet_m let (e1, e2) = constants.entry(constant.name()).or_default(); // skip all entries with the same hash: if let Some(e1_inner) = e1 { - let e1_hash = pallet_metadata_1.constant_hash(e1_inner.name()).expect("constant should be present"); - let e2_hash = pallet_metadata_2.constant_hash(constant.name()).expect("constant should be present"); + let e1_hash = pallet_metadata_1 + .constant_hash(e1_inner.name()) + .expect("constant should be present"); + let e2_hash = pallet_metadata_2 + .constant_hash(constant.name()) + .expect("constant should be present"); if e1_hash == e2_hash { constants.remove(constant.name()); continue; @@ -250,14 +356,23 @@ fn constants_differences<'a>(pallet_metadata_1: &'a PalletMetadata<'a>, pallet_m } *e2 = Some(constant); } - constants.into_iter().map(|(_, tuple)| - Diff::try_from(tuple).unwrap() - ).collect() + constants + .into_values() + .map(|tuple| Diff::try_from(tuple).unwrap()) + .collect() } - -fn runtime_api_differences<'a>(metadata_1: &'a Metadata, metadata_2: &'a Metadata) -> Vec>> { - let mut runtime_apis: HashMap<&str, (Option>, Option>)> = HashMap::new(); +fn runtime_api_differences<'a>( + metadata_1: &'a Metadata, + metadata_2: &'a Metadata, +) -> Vec>> { + let mut runtime_apis: HashMap< + &str, + ( + Option>, + Option>, + ), + > = HashMap::new(); for runtime_api in metadata_1.runtime_api_traits() { let (e1, _) = runtime_apis.entry(runtime_api.name()).or_default(); @@ -275,13 +390,18 @@ fn runtime_api_differences<'a>(metadata_1: &'a Metadata, metadata_2: &'a Metadat } *e2 = Some(runtime_api); } - runtime_apis.into_iter().map(|(_, tuple)| - Diff::try_from(tuple).unwrap() - ).collect() + runtime_apis + .into_values() + .map(|tuple| Diff::try_from(tuple).unwrap()) + .collect() } -fn pallet_differences<'a>(metadata_1: &'a Metadata, metadata_2: &'a Metadata) -> Vec>> { - let mut pallets: HashMap<&str, (Option>, Option>)> = HashMap::new(); +fn pallet_differences<'a>( + metadata_1: &'a Metadata, + metadata_2: &'a Metadata, +) -> Vec>> { + let mut pallets: HashMap<&str, (Option>, Option>)> = + HashMap::new(); for pallet_metadata in metadata_1.pallets() { let (e1, _) = pallets.entry(pallet_metadata.name()).or_default(); @@ -300,9 +420,10 @@ fn pallet_differences<'a>(metadata_1: &'a Metadata, metadata_2: &'a Metadata) -> *e2 = Some(pallet_metadata); } - pallets.into_iter().map(|(_, tuple)| - Diff::try_from(tuple).unwrap() - ).collect() + pallets + .into_values() + .map(|tuple| Diff::try_from(tuple).unwrap()) + .collect() } #[derive(Debug, Clone)] @@ -313,10 +434,16 @@ enum Diff { } impl Diff { - fn map(self, mut f: F) -> Diff where F: FnMut(T) -> R { + fn map(self, mut f: F) -> Diff + where + F: FnMut(T) -> R, + { match self { Diff::Added(new) => Diff::Added(f(new)), - Diff::Changed { from, to } => Diff::Changed { from: f(from), to: f(to) }, + Diff::Changed { from, to } => Diff::Changed { + from: f(from), + to: f(to), + }, Diff::Removed(old) => Diff::Removed(f(old)), } } @@ -328,12 +455,9 @@ impl TryFrom<(Option, Option)> for Diff { fn try_from(value: (Option, Option)) -> Result { match value { (None, None) => Err(()), - (Some(old), None) => { Ok(Diff::Removed(old)) } - (None, Some(new)) => { Ok(Diff::Added(new)) } - (Some(old), Some(new)) => { - Ok(Diff::Changed { from: old, to: new }) - } + (Some(old), None) => Ok(Diff::Removed(old)), + (None, Some(new)) => Ok(Diff::Added(new)), + (Some(old), Some(new)) => Ok(Diff::Changed { from: old, to: new }), } } } - diff --git a/cli/src/commands/mod.rs b/cli/src/commands/mod.rs index 10a52a5a2f..569c93bcb0 100644 --- a/cli/src/commands/mod.rs +++ b/cli/src/commands/mod.rs @@ -4,7 +4,7 @@ pub mod codegen; pub mod compatibility; +pub mod diff; pub mod explore; pub mod metadata; pub mod version; -pub mod diff; \ No newline at end of file diff --git a/cli/src/utils.rs b/cli/src/utils.rs index 4e3a7e3092..a54d81c72c 100644 --- a/cli/src/utils.rs +++ b/cli/src/utils.rs @@ -6,11 +6,8 @@ use clap::Args; use color_eyre::eyre; use std::{fs, io::Read, path::PathBuf}; -use color_eyre::eyre::eyre; -use scale_info::{PortableRegistry, Type, TypeDef, TypeDefVariant}; -use scale_info::form::PortableForm; + use subxt_codegen::utils::{MetadataVersion, Uri}; -use subxt_metadata::PalletMetadata; pub mod type_description; pub mod type_example; @@ -67,7 +64,7 @@ impl FileOrUrl { uri, version.unwrap_or_default(), ) - .await?), + .await?), // Default if neither is provided; fetch from local url (None, None, version) => { let uri = Uri::from_static("ws://localhost:9944"); @@ -97,4 +94,4 @@ pub(crate) fn with_indent(s: String, indent: usize) -> String { .map(|line| format!("{indent_str}{line}")) .collect::>() .join("\n") -} \ No newline at end of file +} From c21e41d38392ad8bbc0355a71a8d1c2156540206 Mon Sep 17 00:00:00 2001 From: Tadeo hepperle Date: Tue, 13 Jun 2023 17:12:26 +0200 Subject: [PATCH 04/17] change formatting --- cli/src/commands/diff.rs | 78 ++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 47 deletions(-) diff --git a/cli/src/commands/diff.rs b/cli/src/commands/diff.rs index 0ab6636f88..e8cce2af06 100644 --- a/cli/src/commands/diff.rs +++ b/cli/src/commands/diff.rs @@ -43,22 +43,14 @@ pub async fn run(opts: Opts) -> color_eyre::Result<()> { writeln!(output, "Pallets:")?; for diff in node_diff.pallets { match diff { - Diff::Added(new) => writeln!( - output, - "{}", - format!(" + {} (Added)", new.name()).green() - )?, - Diff::Removed(old) => writeln!( - output, - "{}", - format!(" - {} (Removed)", old.name()).red() - )?, + Diff::Added(new) => { + writeln!(output, "{}", format!(" + {}", new.name()).green())? + } + Diff::Removed(old) => { + writeln!(output, "{}", format!(" - {}", old.name()).red())? + } Diff::Changed { from, to } => { - writeln!( - output, - "{}", - format!(" ~ {} (Changed)", from.name()).yellow() - )?; + writeln!(output, "{}", format!(" ~ {}", from.name()).yellow())?; let pallet_diff = PalletDiff::construct(&from, &to); if !pallet_diff.calls.is_empty() { @@ -68,18 +60,20 @@ pub async fn run(opts: Opts) -> color_eyre::Result<()> { Diff::Added(new) => writeln!( output, "{}", - format!(" + {} (Added)", &new.name).green() + format!(" + {}", &new.name).green() )?, Diff::Removed(old) => writeln!( output, "{}", - format!(" - {} (Removed)", &old.name).red() - )?, - Diff::Changed { from, to: _ } => writeln!( - output, - "{}", - format!(" ~ {} (Changed)", &from.name).yellow() + format!(" - {}", &old.name).red() )?, + Diff::Changed { from, to: _ } => { + writeln!( + output, + "{}", + format!(" ~ {}", &from.name).yellow() + )?; + } } } } @@ -91,17 +85,17 @@ pub async fn run(opts: Opts) -> color_eyre::Result<()> { Diff::Added(new) => writeln!( output, "{}", - format!(" + {} (Added)", new.name()).green() + format!(" + {}", new.name()).green() )?, Diff::Removed(old) => writeln!( output, "{}", - format!(" - {} (Removed)", old.name()).red() + format!(" - {}", old.name()).red() )?, Diff::Changed { from, to: _ } => writeln!( output, "{}", - format!(" ~ {} (Changed)", from.name()).yellow() + format!(" ~ {}", from.name()).yellow() )?, } } @@ -114,17 +108,17 @@ pub async fn run(opts: Opts) -> color_eyre::Result<()> { Diff::Added(new) => writeln!( output, "{}", - format!(" + {} (Added)", new.name()).green() + format!(" + {}", new.name()).green() )?, Diff::Removed(old) => writeln!( output, "{}", - format!(" - {} (Removed)", old.name()).red() + format!(" - {}", old.name()).red() )?, Diff::Changed { from, to: _ } => writeln!( output, "{}", - format!(" ~ {} (Changed)", from.name()).yellow() + format!(" ~ {}", from.name()).yellow() )?, } } @@ -138,21 +132,15 @@ pub async fn run(opts: Opts) -> color_eyre::Result<()> { writeln!(output, "Runtime APIs:")?; for diff in node_diff.runtime_apis { match diff { - Diff::Added(new) => writeln!( - output, - "{}", - format!(" + {} (Added)", new.name()).green() - )?, - Diff::Removed(old) => writeln!( - output, - "{}", - format!(" - {} (Removed)", old.name()).red() - )?, - Diff::Changed { from, to: _ } => writeln!( - output, - "{}", - format!(" ~ {} (Changed)", from.name()).yellow() - )?, + Diff::Added(new) => { + writeln!(output, "{}", format!(" + {}", new.name()).green())? + } + Diff::Removed(old) => { + writeln!(output, "{}", format!(" - {}", old.name()).red())? + } + Diff::Changed { from, to: _ } => { + writeln!(output, "{}", format!(" ~ {}", from.name()).yellow())? + } } } } @@ -200,10 +188,6 @@ impl<'a> PalletDiff<'a> { storage_entries, } } - - fn is_empty(&self) -> bool { - self.calls.is_empty() && self.constants.is_empty() && self.storage_entries.is_empty() - } } async fn get_metadata(opts: &Opts) -> color_eyre::Result<(Metadata, Metadata)> { From de2ed82966df6673d3122d7e07ea8b5616012561 Mon Sep 17 00:00:00 2001 From: Tadeo hepperle Date: Tue, 13 Jun 2023 17:15:15 +0200 Subject: [PATCH 05/17] fmt --- cli/src/commands/diff.rs | 34 ++++++++++------------------------ 1 file changed, 10 insertions(+), 24 deletions(-) diff --git a/cli/src/commands/diff.rs b/cli/src/commands/diff.rs index e8cce2af06..bc894614d5 100644 --- a/cli/src/commands/diff.rs +++ b/cli/src/commands/diff.rs @@ -1,4 +1,4 @@ -use clap::{FromArgMatches, Parser as ClapParser}; +use clap::Parser as ClapParser; use codec::Decode; use color_eyre::eyre::eyre; use frame_metadata::RuntimeMetadataPrefixed; @@ -271,17 +271,19 @@ fn storage_differences<'a>( .collect() } +type CallsHashmap<'a> = HashMap< + &'a str, + ( + Option<&'a Variant>, + Option<&'a Variant>, + ), +>; + fn calls_differences<'a>( pallet_metadata_1: &'a PalletMetadata<'a>, pallet_metadata_2: &'a PalletMetadata<'a>, ) -> Vec>> { - let mut calls: HashMap< - &str, - ( - Option<&'a Variant>, - Option<&'a Variant>, - ), - > = HashMap::new(); + let mut calls: CallsHashmap = HashMap::new(); if let Some(call_variants) = pallet_metadata_1.call_variants() { for call_variant in call_variants { let (e1, _) = calls.entry(&call_variant.name).or_default(); @@ -417,22 +419,6 @@ enum Diff { Removed(T), } -impl Diff { - fn map(self, mut f: F) -> Diff - where - F: FnMut(T) -> R, - { - match self { - Diff::Added(new) => Diff::Added(f(new)), - Diff::Changed { from, to } => Diff::Changed { - from: f(from), - to: f(to), - }, - Diff::Removed(old) => Diff::Removed(f(old)), - } - } -} - impl TryFrom<(Option, Option)> for Diff { type Error = (); From d192808da92024e91984b7af5d022f725c910f26 Mon Sep 17 00:00:00 2001 From: Tadeo hepperle Date: Wed, 14 Jun 2023 13:25:36 +0200 Subject: [PATCH 06/17] diff working with storage details --- cli/src/commands/diff.rs | 110 +++++++++++++++++++++++++++++-- cli/src/commands/explore/mod.rs | 1 - metadata/src/lib.rs | 20 ++++-- metadata/src/utils/validation.rs | 6 +- 4 files changed, 121 insertions(+), 16 deletions(-) diff --git a/cli/src/commands/diff.rs b/cli/src/commands/diff.rs index bc894614d5..040740004e 100644 --- a/cli/src/commands/diff.rs +++ b/cli/src/commands/diff.rs @@ -15,9 +15,16 @@ use std::io::Write; use subxt_codegen::utils::MetadataVersion; use subxt_metadata::{ ConstantMetadata, Metadata, PalletMetadata, RuntimeApiMetadata, StorageEntryMetadata, + StorageEntryType, }; -/// todo: add docs +/// Explore the differences between two nodes +/// +/// # Example +/// ``` +/// subxt diff ./artifacts/polkadot_metadata_small.scale ./artifacts/polkadot_metadata_tiny.scale +/// subxt diff ./artifacts/polkadot_metadata_small.scale wss://rpc.polkadot.io:443 +/// ``` #[derive(Debug, ClapParser)] pub struct Opts { node1: String, @@ -28,7 +35,6 @@ pub struct Opts { pallet: Option, } -// cargo run -- diff ../artifacts/polkadot_metadata_small.scale ../artifacts/polkadot_metadata_tiny.scale pub async fn run(opts: Opts) -> color_eyre::Result<()> { let (node_1_metadata, node_2_metadata) = get_metadata(&opts).await?; @@ -115,11 +121,25 @@ pub async fn run(opts: Opts) -> color_eyre::Result<()> { "{}", format!(" - {}", old.name()).red() )?, - Diff::Changed { from, to: _ } => writeln!( - output, - "{}", - format!(" ~ {}", from.name()).yellow() - )?, + Diff::Changed { from, to } => { + let storage_diff = StorageEntryDiff::construct( + from, + to, + &node_1_metadata, + &node_2_metadata, + ); + + writeln!( + output, + "{}", + format!( + " ~ {} (Changed: {})", + from.name(), + storage_diff.to_strings().join(", ") + ) + .yellow() + )?; + } } } } @@ -190,6 +210,82 @@ impl<'a> PalletDiff<'a> { } } +struct StorageEntryDiff { + key_different: bool, + value_different: bool, + default_different: bool, + modifier_different: bool, +} + +impl StorageEntryDiff { + fn construct( + storage_entry_1: &StorageEntryMetadata, + storage_entry_2: &StorageEntryMetadata, + metadata_1: &Metadata, + metadata_2: &Metadata, + ) -> Self { + let value_1_ty_id = match storage_entry_1.entry_type() { + StorageEntryType::Plain(value_ty) | StorageEntryType::Map { value_ty, .. } => value_ty, + }; + let value_1_hash = metadata_1 + .type_hash(*value_1_ty_id) + .expect("type should be present"); + let value_2_ty_id = match storage_entry_2.entry_type() { + StorageEntryType::Plain(value_ty) | StorageEntryType::Map { value_ty, .. } => value_ty, + }; + let value_2_hash = metadata_1 + .type_hash(*value_2_ty_id) + .expect("type should be present"); + let value_different = value_1_hash != value_2_hash; + + let key_1_hash = match storage_entry_1.entry_type() { + StorageEntryType::Plain(_) => None, + StorageEntryType::Map { key_ty, .. } => Some(*key_ty), + } + .map(|key_ty| { + metadata_1 + .type_hash(key_ty) + .expect("type should be present") + }) + .unwrap_or_default(); + let key_2_hash = match storage_entry_2.entry_type() { + StorageEntryType::Plain(_) => None, + StorageEntryType::Map { key_ty, .. } => Some(*key_ty), + } + .map(|key_ty| { + metadata_2 + .type_hash(key_ty) + .expect("type should be present") + }) + .unwrap_or_default(); + let key_different = key_1_hash != key_2_hash; + + StorageEntryDiff { + key_different, + value_different, + default_different: storage_entry_1.default_bytes() != storage_entry_2.default_bytes(), + modifier_different: storage_entry_1.modifier() != storage_entry_2.modifier(), + } + } + + fn to_strings(&self) -> Vec<&str> { + let mut strings = Vec::<&str>::new(); + if self.key_different { + strings.push("key"); + } + if self.value_different { + strings.push("value"); + } + if self.modifier_different { + strings.push("modifier"); + } + if self.default_different { + strings.push("default value"); + } + strings + } +} + async fn get_metadata(opts: &Opts) -> color_eyre::Result<(Metadata, Metadata)> { let node_1_file_or_url: FileOrUrl = (opts.node1.as_str(), opts.version) .try_into() diff --git a/cli/src/commands/explore/mod.rs b/cli/src/commands/explore/mod.rs index 269c719531..cbb3d435b1 100644 --- a/cli/src/commands/explore/mod.rs +++ b/cli/src/commands/explore/mod.rs @@ -81,7 +81,6 @@ pub enum PalletSubcommand { Storage(StorageSubcommand), } -/// cargo run -- explore --file=../artifacts/polkadot_metadata.scale pub async fn run(opts: Opts) -> color_eyre::Result<()> { // get the metadata let bytes = opts.file_or_url.fetch().await?; diff --git a/metadata/src/lib.rs b/metadata/src/lib.rs index f59c55c2e7..2050553beb 100644 --- a/metadata/src/lib.rs +++ b/metadata/src/lib.rs @@ -20,7 +20,7 @@ mod from_into; mod utils; use scale_info::{form::PortableForm, PortableRegistry, Variant}; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::sync::Arc; use utils::ordered_map::OrderedMap; use utils::variant_index::VariantIndex; @@ -132,12 +132,22 @@ impl Metadata { /// Filter out any pallets that we don't want to keep, retaining only those that we do. pub fn retain(&mut self, pallet_filter: F, api_filter: G) - where - F: FnMut(&str) -> bool, - G: FnMut(&str) -> bool, + where + F: FnMut(&str) -> bool, + G: FnMut(&str) -> bool, { utils::retain::retain_metadata(self, pallet_filter, api_filter); } + + /// Get type hash for a type in the registry + pub fn type_hash(&self, id: u32) -> Option<[u8; 32]> { + self.types.resolve(id)?; + Some(crate::utils::validation::get_type_hash( + &self.types, + id, + &mut HashSet::::new(), + )) + } } /// Metadata for a specific pallet. @@ -387,7 +397,7 @@ pub enum StorageHasher { } /// Is the storage entry optional, or does it have a default value. -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, Eq, PartialEq)] pub enum StorageEntryModifier { /// The storage entry returns an `Option`, with `None` if the key is not present. Optional, diff --git a/metadata/src/utils/validation.rs b/metadata/src/utils/validation.rs index 5ef62632bd..003a312395 100644 --- a/metadata/src/utils/validation.rs +++ b/metadata/src/utils/validation.rs @@ -175,7 +175,7 @@ fn get_type_def_hash( } /// Obtain the hash representation of a `scale_info::Type` identified by id. -fn get_type_hash( +pub fn get_type_hash( registry: &PortableRegistry, id: u32, visited_ids: &mut HashSet, @@ -595,8 +595,8 @@ mod tests { meta_type::<()>(), vec![], ) - .try_into() - .expect("can build valid metadata") + .try_into() + .expect("can build valid metadata") } #[test] From 1e0019ff21dcbbaaefebd118ea0e5404fb2818de Mon Sep 17 00:00:00 2001 From: Tadeo hepperle Date: Wed, 14 Jun 2023 13:31:37 +0200 Subject: [PATCH 07/17] fix diff --- cli/src/commands/diff.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cli/src/commands/diff.rs b/cli/src/commands/diff.rs index 040740004e..b37cf5b6d8 100644 --- a/cli/src/commands/diff.rs +++ b/cli/src/commands/diff.rs @@ -11,7 +11,6 @@ use color_eyre::owo_colors::OwoColorize; use jsonrpsee::client_transport::ws::Uri; use scale_info::form::PortableForm; use scale_info::Variant; -use std::io::Write; use subxt_codegen::utils::MetadataVersion; use subxt_metadata::{ ConstantMetadata, Metadata, PalletMetadata, RuntimeApiMetadata, StorageEntryMetadata, @@ -35,11 +34,10 @@ pub struct Opts { pallet: Option, } -pub async fn run(opts: Opts) -> color_eyre::Result<()> { +pub async fn run(opts: Opts, output: &mut impl std::io::Write) -> color_eyre::Result<()> { let (node_1_metadata, node_2_metadata) = get_metadata(&opts).await?; let node_diff = MetadataDiff::construct(&node_1_metadata, &node_2_metadata); - let mut output = std::io::stdout(); if node_diff.is_empty() { writeln!(output, "No difference in Metadata found.")?; From 3134d86a445ab4d5a49c3ac60cb97a3a4b5f173d Mon Sep 17 00:00:00 2001 From: Tadeo hepperle Date: Wed, 14 Jun 2023 14:06:14 +0200 Subject: [PATCH 08/17] cargo fmt --- metadata/src/lib.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/metadata/src/lib.rs b/metadata/src/lib.rs index 2050553beb..6019079b94 100644 --- a/metadata/src/lib.rs +++ b/metadata/src/lib.rs @@ -78,7 +78,7 @@ impl Metadata { } /// An iterator over all of the available pallets. - pub fn pallets(&self) -> impl ExactSizeIterator> { + pub fn pallets(&self) -> impl ExactSizeIterator> { self.pallets.values().iter().map(|inner| PalletMetadata { inner, types: self.types(), @@ -109,7 +109,7 @@ impl Metadata { } /// An iterator over all of the runtime APIs. - pub fn runtime_api_traits(&self) -> impl ExactSizeIterator> { + pub fn runtime_api_traits(&self) -> impl ExactSizeIterator> { self.apis.values().iter().map(|inner| RuntimeApiMetadata { inner, types: self.types(), @@ -246,7 +246,7 @@ impl<'a> PalletMetadata<'a> { } /// An iterator over the constants in this pallet. - pub fn constants(&self) -> impl ExactSizeIterator { + pub fn constants(&self) -> impl ExactSizeIterator { self.inner.constants.values().iter() } @@ -313,7 +313,7 @@ impl StorageMetadata { } /// An iterator over the storage entries. - pub fn entries(&self) -> impl ExactSizeIterator { + pub fn entries(&self) -> impl ExactSizeIterator { self.entries.values().iter() } @@ -508,7 +508,7 @@ impl<'a> RuntimeApiMetadata<'a> { &self.inner.docs } /// An iterator over the trait methods. - pub fn methods(&self) -> impl ExactSizeIterator { + pub fn methods(&self) -> impl ExactSizeIterator { self.inner.methods.values().iter() } /// Get a specific trait method given its name. @@ -559,7 +559,7 @@ impl RuntimeApiMethodMetadata { &self.docs } /// Method inputs. - pub fn inputs(&self) -> impl ExactSizeIterator { + pub fn inputs(&self) -> impl ExactSizeIterator { self.inputs.iter() } /// Method return type. From ed8595d0b2359ecee6c0d80a7eaa3f3ff9e059f1 Mon Sep 17 00:00:00 2001 From: Tadeo hepperle Date: Wed, 14 Jun 2023 14:12:28 +0200 Subject: [PATCH 09/17] remove printing of node --- cli/src/commands/diff.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/cli/src/commands/diff.rs b/cli/src/commands/diff.rs index b37cf5b6d8..3f8a78cea1 100644 --- a/cli/src/commands/diff.rs +++ b/cli/src/commands/diff.rs @@ -305,7 +305,6 @@ impl TryFrom<(&str, Option)> for FileOrUrl { type Error = (); fn try_from(value: (&str, Option)) -> Result { - println!("value {}", &value.0); let path = std::path::Path::new(&value.0); if path.exists() { Ok(FileOrUrl { From 37b4ec50ea82e31108f266db9d35eef6118cc880 Mon Sep 17 00:00:00 2001 From: Tadeo hepperle Date: Wed, 14 Jun 2023 14:24:11 +0200 Subject: [PATCH 10/17] change strings --- cli/src/commands/diff.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/src/commands/diff.rs b/cli/src/commands/diff.rs index 3f8a78cea1..636cfbaeea 100644 --- a/cli/src/commands/diff.rs +++ b/cli/src/commands/diff.rs @@ -269,10 +269,10 @@ impl StorageEntryDiff { fn to_strings(&self) -> Vec<&str> { let mut strings = Vec::<&str>::new(); if self.key_different { - strings.push("key"); + strings.push("key type"); } if self.value_different { - strings.push("value"); + strings.push("value type"); } if self.modifier_different { strings.push("modifier"); From 42648f1536f3710017406d00fdb434ac5bd71d2f Mon Sep 17 00:00:00 2001 From: Tadeo hepperle Date: Thu, 15 Jun 2023 14:17:06 +0200 Subject: [PATCH 11/17] handle parsing differently --- cli/src/commands/diff.rs | 72 ++++++++++++---------------------------- cli/src/utils.rs | 26 ++++++++++++++- 2 files changed, 47 insertions(+), 51 deletions(-) diff --git a/cli/src/commands/diff.rs b/cli/src/commands/diff.rs index 636cfbaeea..ea69ab2353 100644 --- a/cli/src/commands/diff.rs +++ b/cli/src/commands/diff.rs @@ -26,26 +26,22 @@ use subxt_metadata::{ /// ``` #[derive(Debug, ClapParser)] pub struct Opts { - node1: String, - node2: String, - #[clap(long, short)] - version: Option, - #[clap(long, short)] - pallet: Option, + entry1: FileOrUrl, + entry2: FileOrUrl, } pub async fn run(opts: Opts, output: &mut impl std::io::Write) -> color_eyre::Result<()> { - let (node_1_metadata, node_2_metadata) = get_metadata(&opts).await?; + let (entry_1_metadata, entry_2_metadata) = get_metadata(&opts).await?; - let node_diff = MetadataDiff::construct(&node_1_metadata, &node_2_metadata); + let metadata_diff = MetadataDiff::construct(&entry_1_metadata, &entry_2_metadata); - if node_diff.is_empty() { - writeln!(output, "No difference in Metadata found.")?; + if metadata_diff.is_empty() { + writeln!(output, "No difference in metadata found.")?; return Ok(()); } - if !node_diff.pallets.is_empty() { + if !metadata_diff.pallets.is_empty() { writeln!(output, "Pallets:")?; - for diff in node_diff.pallets { + for diff in metadata_diff.pallets { match diff { Diff::Added(new) => { writeln!(output, "{}", format!(" + {}", new.name()).green())? @@ -123,8 +119,8 @@ pub async fn run(opts: Opts, output: &mut impl std::io::Write) -> color_eyre::Re let storage_diff = StorageEntryDiff::construct( from, to, - &node_1_metadata, - &node_2_metadata, + &entry_1_metadata, + &entry_2_metadata, ); writeln!( @@ -146,9 +142,9 @@ pub async fn run(opts: Opts, output: &mut impl std::io::Write) -> color_eyre::Re } } - if !node_diff.runtime_apis.is_empty() { + if !metadata_diff.runtime_apis.is_empty() { writeln!(output, "Runtime APIs:")?; - for diff in node_diff.runtime_apis { + for diff in metadata_diff.runtime_apis { match diff { Diff::Added(new) => { writeln!(output, "{}", format!(" + {}", new.name()).green())? @@ -285,41 +281,15 @@ impl StorageEntryDiff { } async fn get_metadata(opts: &Opts) -> color_eyre::Result<(Metadata, Metadata)> { - let node_1_file_or_url: FileOrUrl = (opts.node1.as_str(), opts.version) - .try_into() - .map_err(|_| eyre!("{} is not a valid url nor path for node 1.", opts.node1))?; - let node_2_file_or_url: FileOrUrl = (opts.node2.as_str(), opts.version) - .try_into() - .map_err(|_| eyre!("{} is not a valid url nor path for node 2.", opts.node2))?; + let bytes = opts.entry1.fetch().await?; + let entry_1_metadata: Metadata = + RuntimeMetadataPrefixed::decode(&mut &bytes[..])?.try_into()?; - let bytes = node_1_file_or_url.fetch().await?; - let node_1_metadata: Metadata = RuntimeMetadataPrefixed::decode(&mut &bytes[..])?.try_into()?; + let bytes = opts.entry2.fetch().await?; + let entry_2_metadata: Metadata = + RuntimeMetadataPrefixed::decode(&mut &bytes[..])?.try_into()?; - let bytes = node_2_file_or_url.fetch().await?; - let node_2_metadata: Metadata = RuntimeMetadataPrefixed::decode(&mut &bytes[..])?.try_into()?; - - Ok((node_1_metadata, node_2_metadata)) -} - -impl TryFrom<(&str, Option)> for FileOrUrl { - type Error = (); - - fn try_from(value: (&str, Option)) -> Result { - let path = std::path::Path::new(&value.0); - if path.exists() { - Ok(FileOrUrl { - url: None, - file: Some(PathBuf::from(value.0)), - version: value.1, - }) - } else { - Uri::from_str(value.0).map_err(|_| ()).map(|uri| FileOrUrl { - url: Some(uri), - file: None, - version: value.1, - }) - } - } + Ok((entry_1_metadata, entry_2_metadata)) } fn storage_differences<'a>( @@ -501,7 +471,9 @@ fn pallet_differences<'a>( pallets .into_values() - .map(|tuple| Diff::try_from(tuple).unwrap()) + .map(|tuple| { + Diff::try_from(tuple).expect("unreachable, fails only if two None values are present") + }) .collect() } diff --git a/cli/src/utils.rs b/cli/src/utils.rs index 4ccdc927b1..2a549b0a56 100644 --- a/cli/src/utils.rs +++ b/cli/src/utils.rs @@ -5,6 +5,7 @@ use clap::Args; use color_eyre::eyre; +use std::str::FromStr; use std::{fs, io::Read, path::PathBuf}; use subxt_codegen::utils::{MetadataVersion, Uri}; @@ -13,7 +14,7 @@ pub mod type_description; pub mod type_example; /// The source of the metadata. -#[derive(Debug, Args)] +#[derive(Debug, Args, Clone)] pub struct FileOrUrl { /// The url of the substrate node to query for metadata for codegen. #[clap(long, value_parser)] @@ -95,3 +96,26 @@ pub fn with_indent(s: String, indent: usize) -> String { .collect::>() .join("\n") } + +impl FromStr for FileOrUrl { + type Err = &'static str; + + fn from_str(s: &str) -> Result { + let path = std::path::Path::new(s); + if path.exists() { + Ok(FileOrUrl { + url: None, + file: Some(PathBuf::from(s)), + version: None, + }) + } else { + Uri::from_str(s) + .map_err(|_| "no path or uri could be crated") + .map(|uri| FileOrUrl { + url: Some(uri), + file: None, + version: None, + }) + } + } +} From 624a9cb39b5512110d96fefbe5ee845f5459568d Mon Sep 17 00:00:00 2001 From: Tadeo hepperle Date: Thu, 15 Jun 2023 14:17:30 +0200 Subject: [PATCH 12/17] clippy fix --- cli/src/commands/diff.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cli/src/commands/diff.rs b/cli/src/commands/diff.rs index ea69ab2353..b4d085914c 100644 --- a/cli/src/commands/diff.rs +++ b/cli/src/commands/diff.rs @@ -1,17 +1,17 @@ use clap::Parser as ClapParser; use codec::Decode; -use color_eyre::eyre::eyre; + use frame_metadata::RuntimeMetadataPrefixed; use std::collections::HashMap; -use std::path::PathBuf; -use std::str::FromStr; + + use crate::utils::FileOrUrl; use color_eyre::owo_colors::OwoColorize; -use jsonrpsee::client_transport::ws::Uri; + use scale_info::form::PortableForm; use scale_info::Variant; -use subxt_codegen::utils::MetadataVersion; + use subxt_metadata::{ ConstantMetadata, Metadata, PalletMetadata, RuntimeApiMetadata, StorageEntryMetadata, StorageEntryType, From f1a6c7084b7a35562320f4d159ac0cd80d19cf14 Mon Sep 17 00:00:00 2001 From: Tadeo hepperle Date: Fri, 16 Jun 2023 10:25:22 +0200 Subject: [PATCH 13/17] cargo fmt --- cli/src/commands/diff.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/cli/src/commands/diff.rs b/cli/src/commands/diff.rs index b4d085914c..7bade5aa4d 100644 --- a/cli/src/commands/diff.rs +++ b/cli/src/commands/diff.rs @@ -4,8 +4,6 @@ use codec::Decode; use frame_metadata::RuntimeMetadataPrefixed; use std::collections::HashMap; - - use crate::utils::FileOrUrl; use color_eyre::owo_colors::OwoColorize; From cbd24371b97636923ee930861c876e7bf403ea36 Mon Sep 17 00:00:00 2001 From: Tadeo hepperle Date: Mon, 19 Jun 2023 12:50:26 +0200 Subject: [PATCH 14/17] more abstraction --- cli/src/commands/diff.rs | 308 +++++++++++----------------- cli/src/commands/explore/storage.rs | 8 +- codegen/src/api/storage.rs | 8 +- metadata/src/lib.rs | 22 +- metadata/src/utils/validation.rs | 6 +- 5 files changed, 149 insertions(+), 203 deletions(-) diff --git a/cli/src/commands/diff.rs b/cli/src/commands/diff.rs index 7bade5aa4d..731193b2bd 100644 --- a/cli/src/commands/diff.rs +++ b/cli/src/commands/diff.rs @@ -1,8 +1,9 @@ -use clap::Parser as ClapParser; +use clap::{Args, Parser as ClapParser}; use codec::Decode; use frame_metadata::RuntimeMetadataPrefixed; use std::collections::HashMap; +use std::hash::Hash; use crate::utils::FileOrUrl; use color_eyre::owo_colors::OwoColorize; @@ -12,7 +13,7 @@ use scale_info::Variant; use subxt_metadata::{ ConstantMetadata, Metadata, PalletMetadata, RuntimeApiMetadata, StorageEntryMetadata, - StorageEntryType, + StorageEntryType, StorageMetadata, }; /// Explore the differences between two nodes @@ -22,10 +23,13 @@ use subxt_metadata::{ /// subxt diff ./artifacts/polkadot_metadata_small.scale ./artifacts/polkadot_metadata_tiny.scale /// subxt diff ./artifacts/polkadot_metadata_small.scale wss://rpc.polkadot.io:443 /// ``` -#[derive(Debug, ClapParser)] +#[derive(Debug, Args)] +#[command(author, version, about, long_about = None)] pub struct Opts { - entry1: FileOrUrl, - entry2: FileOrUrl, + /// metadata file or node URL + metadata_or_url_1: FileOrUrl, + /// metadata file or node URL + metadata_or_url_2: FileOrUrl, } pub async fn run(opts: Opts, output: &mut impl std::io::Write) -> color_eyre::Result<()> { @@ -129,7 +133,7 @@ pub async fn run(opts: Opts, output: &mut impl std::io::Write) -> color_eyre::Re from.name(), storage_diff.to_strings().join(", ") ) - .yellow() + .yellow() )?; } } @@ -234,22 +238,22 @@ impl StorageEntryDiff { StorageEntryType::Plain(_) => None, StorageEntryType::Map { key_ty, .. } => Some(*key_ty), } - .map(|key_ty| { - metadata_1 - .type_hash(key_ty) - .expect("type should be present") - }) - .unwrap_or_default(); + .map(|key_ty| { + metadata_1 + .type_hash(key_ty) + .expect("type should be present") + }) + .unwrap_or_default(); let key_2_hash = match storage_entry_2.entry_type() { StorageEntryType::Plain(_) => None, StorageEntryType::Map { key_ty, .. } => Some(*key_ty), } - .map(|key_ty| { - metadata_2 - .type_hash(key_ty) - .expect("type should be present") - }) - .unwrap_or_default(); + .map(|key_ty| { + metadata_2 + .type_hash(key_ty) + .expect("type should be present") + }) + .unwrap_or_default(); let key_different = key_1_hash != key_2_hash; StorageEntryDiff { @@ -279,11 +283,11 @@ impl StorageEntryDiff { } async fn get_metadata(opts: &Opts) -> color_eyre::Result<(Metadata, Metadata)> { - let bytes = opts.entry1.fetch().await?; + let bytes = opts.metadata_or_url_1.fetch().await?; let entry_1_metadata: Metadata = RuntimeMetadataPrefixed::decode(&mut &bytes[..])?.try_into()?; - let bytes = opts.entry2.fetch().await?; + let bytes = opts.metadata_or_url_2.fetch().await?; let entry_2_metadata: Metadata = RuntimeMetadataPrefixed::decode(&mut &bytes[..])?.try_into()?; @@ -294,185 +298,96 @@ fn storage_differences<'a>( pallet_metadata_1: &'a PalletMetadata<'a>, pallet_metadata_2: &'a PalletMetadata<'a>, ) -> Vec> { - let mut storage_entries: HashMap< - &str, - ( - Option<&'a StorageEntryMetadata>, - Option<&'a StorageEntryMetadata>, - ), - > = HashMap::new(); - if let Some(storage_metadata) = pallet_metadata_1.storage() { - for storage_entry in storage_metadata.entries() { - let (e1, _) = storage_entries.entry(storage_entry.name()).or_default(); - *e1 = Some(storage_entry); - } - } - if let Some(storage_metadata) = pallet_metadata_2.storage() { - for storage_entry in storage_metadata.entries() { - let (e1, e2) = storage_entries.entry(storage_entry.name()).or_default(); - // skip all entries with the same hash: - if let Some(e1_inner) = e1 { - let e1_hash = pallet_metadata_1 - .storage_hash(e1_inner.name()) - .expect("storage entry should be present"); - let e2_hash = pallet_metadata_2 - .storage_hash(storage_entry.name()) - .expect("storage entry should be present"); - if e1_hash == e2_hash { - storage_entries.remove(storage_entry.name()); - continue; - } - } - *e2 = Some(storage_entry); - } - } - storage_entries - .into_values() - .map(|tuple| Diff::try_from(tuple).unwrap()) - .collect() + diff( + pallet_metadata_1 + .storage() + .map(|s| s.entries()) + .unwrap_or_default(), + pallet_metadata_2 + .storage() + .map(|s| s.entries()) + .unwrap_or_default(), + |e| { + pallet_metadata_1 + .storage_hash(e.name()) + .expect("storage entry should be present") + }, + |e| { + pallet_metadata_2 + .storage_hash(e.name()) + .expect("storage entry should be present") + }, + |e| e.name(), + ) } -type CallsHashmap<'a> = HashMap< - &'a str, - ( - Option<&'a Variant>, - Option<&'a Variant>, - ), ->; fn calls_differences<'a>( pallet_metadata_1: &'a PalletMetadata<'a>, pallet_metadata_2: &'a PalletMetadata<'a>, ) -> Vec>> { - let mut calls: CallsHashmap = HashMap::new(); - if let Some(call_variants) = pallet_metadata_1.call_variants() { - for call_variant in call_variants { - let (e1, _) = calls.entry(&call_variant.name).or_default(); - *e1 = Some(call_variant); - } - } - if let Some(call_variants) = pallet_metadata_2.call_variants() { - for call_variant in call_variants { - let (e1, e2) = calls.entry(&call_variant.name).or_default(); - // skip all entries with the same hash: - if let Some(e1_inner) = e1 { - let e1_hash = pallet_metadata_1 - .call_hash(&e1_inner.name) - .expect("call should be present"); - let e2_hash = pallet_metadata_2 - .call_hash(&call_variant.name) - .expect("call should be present"); - if e1_hash == e2_hash { - calls.remove(call_variant.name.as_str()); - continue; - } - } - *e2 = Some(call_variant); - } - } - calls - .into_values() - .map(|tuple| Diff::try_from(tuple).unwrap()) - .collect() + return diff( + pallet_metadata_1.call_variants().unwrap_or_default(), + pallet_metadata_2.call_variants().unwrap_or_default(), + |e| { + pallet_metadata_1 + .call_hash(&e.name) + .expect("call should be present") + }, + |e| { + pallet_metadata_2 + .call_hash(&e.name) + .expect("call should be present") + }, + |e| &e.name, + ); } fn constants_differences<'a>( pallet_metadata_1: &'a PalletMetadata<'a>, pallet_metadata_2: &'a PalletMetadata<'a>, ) -> Vec> { - let mut constants: HashMap<&str, (Option<&'a ConstantMetadata>, Option<&'a ConstantMetadata>)> = - HashMap::new(); - for constant in pallet_metadata_1.constants() { - let (e1, _) = constants.entry(constant.name()).or_default(); - *e1 = Some(constant); - } - for constant in pallet_metadata_2.constants() { - let (e1, e2) = constants.entry(constant.name()).or_default(); - // skip all entries with the same hash: - if let Some(e1_inner) = e1 { - let e1_hash = pallet_metadata_1 - .constant_hash(e1_inner.name()) - .expect("constant should be present"); - let e2_hash = pallet_metadata_2 - .constant_hash(constant.name()) - .expect("constant should be present"); - if e1_hash == e2_hash { - constants.remove(constant.name()); - continue; - } - } - *e2 = Some(constant); - } - constants - .into_values() - .map(|tuple| Diff::try_from(tuple).unwrap()) - .collect() + diff( + pallet_metadata_1.constants(), + pallet_metadata_2.constants(), + |e| { + pallet_metadata_1 + .constant_hash(e.name()) + .expect("constant should be present") + }, + |e| { + pallet_metadata_2 + .constant_hash(e.name()) + .expect("constant should be present") + }, + |e| e.name(), + ) } fn runtime_api_differences<'a>( metadata_1: &'a Metadata, metadata_2: &'a Metadata, ) -> Vec>> { - let mut runtime_apis: HashMap< - &str, - ( - Option>, - Option>, - ), - > = HashMap::new(); - - for runtime_api in metadata_1.runtime_api_traits() { - let (e1, _) = runtime_apis.entry(runtime_api.name()).or_default(); - *e1 = Some(runtime_api); - } - - for runtime_api in metadata_2.runtime_api_traits() { - let (e1, e2) = runtime_apis.entry(runtime_api.name()).or_default(); - // skip all entries with the same hash: - if let Some(e1_inner) = e1 { - if e1_inner.hash() == runtime_api.hash() { - runtime_apis.remove(runtime_api.name()); - continue; - } - } - *e2 = Some(runtime_api); - } - runtime_apis - .into_values() - .map(|tuple| Diff::try_from(tuple).unwrap()) - .collect() + diff( + metadata_1.runtime_api_traits(), + metadata_2.runtime_api_traits(), + RuntimeApiMetadata::hash, + RuntimeApiMetadata::hash, + RuntimeApiMetadata::name, + ) } fn pallet_differences<'a>( metadata_1: &'a Metadata, metadata_2: &'a Metadata, ) -> Vec>> { - let mut pallets: HashMap<&str, (Option>, Option>)> = - HashMap::new(); - - for pallet_metadata in metadata_1.pallets() { - let (e1, _) = pallets.entry(pallet_metadata.name()).or_default(); - *e1 = Some(pallet_metadata); - } - - for pallet_metadata in metadata_2.pallets() { - let (e1, e2) = pallets.entry(pallet_metadata.name()).or_default(); - // skip all entries with the same hash: - if let Some(e1_inner) = e1 { - if e1_inner.hash() == pallet_metadata.hash() { - pallets.remove(pallet_metadata.name()); - continue; - } - } - *e2 = Some(pallet_metadata); - } - - pallets - .into_values() - .map(|tuple| { - Diff::try_from(tuple).expect("unreachable, fails only if two None values are present") - }) - .collect() + diff( + metadata_1.pallets(), + metadata_2.pallets(), + PalletMetadata::hash, + PalletMetadata::hash, + PalletMetadata::name, + ) } #[derive(Debug, Clone)] @@ -482,15 +397,42 @@ enum Diff { Removed(T), } -impl TryFrom<(Option, Option)> for Diff { - type Error = (); +fn diff( + items_a: impl IntoIterator, + items_b: impl IntoIterator, + hash_fn_a: impl Fn(&T) -> C, + hash_fn_b: impl Fn(&T) -> C, + key_fn: impl Fn(&T) -> I, +) -> Vec> { + let mut entries: HashMap, Option)> = HashMap::new(); + + for t1 in items_a { + let key = key_fn(&t1); + let (e1, _) = entries.entry(key).or_default(); + *e1 = Some(t1); + } - fn try_from(value: (Option, Option)) -> Result { - match value { - (None, None) => Err(()), - (Some(old), None) => Ok(Diff::Removed(old)), - (None, Some(new)) => Ok(Diff::Added(new)), - (Some(old), Some(new)) => Ok(Diff::Changed { from: old, to: new }), + for t2 in items_b { + let key = key_fn(&t2); + let (e1, e2) = entries.entry(key).or_default(); + // skip all entries with the same hash: + if let Some(e1_inner) = e1 { + let e1_hash = hash_fn_a(e1_inner); + let e2_hash = hash_fn_b(&t2); + if e1_hash == e2_hash { + entries.remove(&key_fn(&t2)); + continue; + } } + *e2 = Some(t2); } -} + entries + .into_values() + .map(|tuple| match tuple { + (None, None) => panic!("At least one value is inserted when the key exists; qed"), + (Some(old), None) => Diff::Removed(old), + (None, Some(new)) => Diff::Added(new), + (Some(old), Some(new)) => Diff::Changed { from: old, to: new }, + }) + .collect() +} \ No newline at end of file diff --git a/cli/src/commands/explore/storage.rs b/cli/src/commands/explore/storage.rs index a26778d317..692cb5c4c4 100644 --- a/cli/src/commands/explore/storage.rs +++ b/cli/src/commands/explore/storage.rs @@ -47,7 +47,7 @@ pub async fn explore_storage( }; // if specified call storage entry wrong, show user the storage entries to choose from (but this time as an error): - let Some(storage) = storage_metadata.entries().find(|entry| entry.name().to_lowercase() == entry_name.to_lowercase()) else { + let Some(storage) = storage_metadata.entries().iter().find(|entry| entry.name().to_lowercase() == entry_name.to_lowercase()) else { let storage_entries = print_available_storage_entries(storage_metadata, pallet_name); let description = format!("Usage:\n subxt explore {pallet_name} storage \n view details for a specific storage entry\n\n{storage_entries}"); return Err(eyre!("Storage entry \"{entry_name}\" not found in \"{pallet_name}\" pallet!\n\n{description}")); @@ -171,7 +171,11 @@ fn print_available_storage_entries( "Available 's in the \"{}\" pallet:", pallet_name ); - let mut strings: Vec<_> = storage_metadata.entries().map(|s| s.name()).collect(); + let mut strings: Vec<_> = storage_metadata + .entries() + .iter() + .map(|s| s.name()) + .collect(); strings.sort(); for entry in strings { write!(output, "\n {}", entry).unwrap(); diff --git a/codegen/src/api/storage.rs b/codegen/src/api/storage.rs index ca432082e8..25c9dcc6a0 100644 --- a/codegen/src/api/storage.rs +++ b/codegen/src/api/storage.rs @@ -30,11 +30,11 @@ pub fn generate_storage( should_gen_docs: bool, ) -> Result { let Some(storage) = pallet.storage() else { - return Ok(quote!()) + return Ok(quote!()); }; let storage_fns = storage - .entries() + .entries().iter() .map(|entry| { generate_storage_entry_fns(type_gen, pallet, entry, crate_path, should_gen_docs) }) @@ -104,7 +104,7 @@ fn generate_storage_entry_fns( let pallet_name = pallet.name(); let storage_name = storage_entry.name(); let Some(storage_hash) = pallet.storage_hash(storage_name) else { - return Err(CodegenError::MissingStorageMetadata(pallet_name.into(), storage_name.into())) + return Err(CodegenError::MissingStorageMetadata(pallet_name.into(), storage_name.into())); }; let fn_name = format_ident!("{}", storage_entry.name().to_snake_case()); @@ -157,7 +157,7 @@ fn generate_storage_entry_fns( // so expose a function to create this entry, too: let root_entry_fn = if is_map_type { let fn_name_root = format_ident!("{}_root", fn_name); - quote! ( + quote!( #docs pub fn #fn_name_root( &self, diff --git a/metadata/src/lib.rs b/metadata/src/lib.rs index 6019079b94..b9e4031cf1 100644 --- a/metadata/src/lib.rs +++ b/metadata/src/lib.rs @@ -78,7 +78,7 @@ impl Metadata { } /// An iterator over all of the available pallets. - pub fn pallets(&self) -> impl ExactSizeIterator> { + pub fn pallets(&self) -> impl ExactSizeIterator> { self.pallets.values().iter().map(|inner| PalletMetadata { inner, types: self.types(), @@ -109,7 +109,7 @@ impl Metadata { } /// An iterator over all of the runtime APIs. - pub fn runtime_api_traits(&self) -> impl ExactSizeIterator> { + pub fn runtime_api_traits(&self) -> impl ExactSizeIterator> { self.apis.values().iter().map(|inner| RuntimeApiMetadata { inner, types: self.types(), @@ -132,9 +132,9 @@ impl Metadata { /// Filter out any pallets that we don't want to keep, retaining only those that we do. pub fn retain(&mut self, pallet_filter: F, api_filter: G) - where - F: FnMut(&str) -> bool, - G: FnMut(&str) -> bool, + where + F: FnMut(&str) -> bool, + G: FnMut(&str) -> bool, { utils::retain::retain_metadata(self, pallet_filter, api_filter); } @@ -246,7 +246,7 @@ impl<'a> PalletMetadata<'a> { } /// An iterator over the constants in this pallet. - pub fn constants(&self) -> impl ExactSizeIterator { + pub fn constants(&self) -> impl ExactSizeIterator { self.inner.constants.values().iter() } @@ -313,8 +313,8 @@ impl StorageMetadata { } /// An iterator over the storage entries. - pub fn entries(&self) -> impl ExactSizeIterator { - self.entries.values().iter() + pub fn entries(&self) -> &[StorageEntryMetadata] { + self.entries.values() } /// Return a specific storage entry given its name. @@ -508,7 +508,7 @@ impl<'a> RuntimeApiMetadata<'a> { &self.inner.docs } /// An iterator over the trait methods. - pub fn methods(&self) -> impl ExactSizeIterator { + pub fn methods(&self) -> impl ExactSizeIterator { self.inner.methods.values().iter() } /// Get a specific trait method given its name. @@ -520,7 +520,7 @@ impl<'a> RuntimeApiMetadata<'a> { crate::utils::validation::get_runtime_api_hash(self, method_name) } - /// Return a hash for the entire pallet. + /// Return a hash for the runtime API trait. pub fn hash(&self) -> [u8; 32] { crate::utils::validation::get_runtime_trait_hash(*self) } @@ -559,7 +559,7 @@ impl RuntimeApiMethodMetadata { &self.docs } /// Method inputs. - pub fn inputs(&self) -> impl ExactSizeIterator { + pub fn inputs(&self) -> impl ExactSizeIterator { self.inputs.iter() } /// Method return type. diff --git a/metadata/src/utils/validation.rs b/metadata/src/utils/validation.rs index 003a312395..bc518611e4 100644 --- a/metadata/src/utils/validation.rs +++ b/metadata/src/utils/validation.rs @@ -379,7 +379,7 @@ pub fn get_pallet_hash(pallet: PalletMetadata) -> [u8; HASH_LEN] { let storage_bytes = match pallet.storage() { Some(storage) => { let prefix_hash = hash(storage.prefix().as_bytes()); - let entries_hash = storage.entries().fold([0u8; HASH_LEN], |bytes, entry| { + let entries_hash = storage.entries().iter().fold([0u8; HASH_LEN], |bytes, entry| { // We don't care what order the storage entries occur in, so XOR them together // to make the order irrelevant. xor( @@ -595,8 +595,8 @@ mod tests { meta_type::<()>(), vec![], ) - .try_into() - .expect("can build valid metadata") + .try_into() + .expect("can build valid metadata") } #[test] From 07281d18f932f5bc5e27a9ace30bb709d32e9903 Mon Sep 17 00:00:00 2001 From: Tadeo hepperle Date: Mon, 19 Jun 2023 12:52:01 +0200 Subject: [PATCH 15/17] clippy fix and fmt --- cli/src/commands/diff.rs | 37 ++++++++++++++--------------- cli/src/commands/explore/storage.rs | 2 +- codegen/src/api/storage.rs | 3 ++- metadata/src/lib.rs | 16 ++++++------- metadata/src/utils/validation.rs | 23 ++++++++++-------- 5 files changed, 42 insertions(+), 39 deletions(-) diff --git a/cli/src/commands/diff.rs b/cli/src/commands/diff.rs index 731193b2bd..a488445077 100644 --- a/cli/src/commands/diff.rs +++ b/cli/src/commands/diff.rs @@ -1,4 +1,4 @@ -use clap::{Args, Parser as ClapParser}; +use clap::Args; use codec::Decode; use frame_metadata::RuntimeMetadataPrefixed; @@ -13,7 +13,7 @@ use scale_info::Variant; use subxt_metadata::{ ConstantMetadata, Metadata, PalletMetadata, RuntimeApiMetadata, StorageEntryMetadata, - StorageEntryType, StorageMetadata, + StorageEntryType, }; /// Explore the differences between two nodes @@ -133,7 +133,7 @@ pub async fn run(opts: Opts, output: &mut impl std::io::Write) -> color_eyre::Re from.name(), storage_diff.to_strings().join(", ") ) - .yellow() + .yellow() )?; } } @@ -238,22 +238,22 @@ impl StorageEntryDiff { StorageEntryType::Plain(_) => None, StorageEntryType::Map { key_ty, .. } => Some(*key_ty), } - .map(|key_ty| { - metadata_1 - .type_hash(key_ty) - .expect("type should be present") - }) - .unwrap_or_default(); + .map(|key_ty| { + metadata_1 + .type_hash(key_ty) + .expect("type should be present") + }) + .unwrap_or_default(); let key_2_hash = match storage_entry_2.entry_type() { StorageEntryType::Plain(_) => None, StorageEntryType::Map { key_ty, .. } => Some(*key_ty), } - .map(|key_ty| { - metadata_2 - .type_hash(key_ty) - .expect("type should be present") - }) - .unwrap_or_default(); + .map(|key_ty| { + metadata_2 + .type_hash(key_ty) + .expect("type should be present") + }) + .unwrap_or_default(); let key_different = key_1_hash != key_2_hash; StorageEntryDiff { @@ -321,7 +321,6 @@ fn storage_differences<'a>( ) } - fn calls_differences<'a>( pallet_metadata_1: &'a PalletMetadata<'a>, pallet_metadata_2: &'a PalletMetadata<'a>, @@ -398,8 +397,8 @@ enum Diff { } fn diff( - items_a: impl IntoIterator, - items_b: impl IntoIterator, + items_a: impl IntoIterator, + items_b: impl IntoIterator, hash_fn_a: impl Fn(&T) -> C, hash_fn_b: impl Fn(&T) -> C, key_fn: impl Fn(&T) -> I, @@ -435,4 +434,4 @@ fn diff( (Some(old), Some(new)) => Diff::Changed { from: old, to: new }, }) .collect() -} \ No newline at end of file +} diff --git a/cli/src/commands/explore/storage.rs b/cli/src/commands/explore/storage.rs index 692cb5c4c4..0796386464 100644 --- a/cli/src/commands/explore/storage.rs +++ b/cli/src/commands/explore/storage.rs @@ -164,7 +164,7 @@ fn print_available_storage_entries( storage_metadata: &StorageMetadata, pallet_name: &str, ) -> String { - if storage_metadata.entries().len() == 0 { + if storage_metadata.entries().is_empty() { format!("No 's available in the \"{pallet_name}\" pallet.") } else { let mut output = format!( diff --git a/codegen/src/api/storage.rs b/codegen/src/api/storage.rs index 25c9dcc6a0..640234b698 100644 --- a/codegen/src/api/storage.rs +++ b/codegen/src/api/storage.rs @@ -34,7 +34,8 @@ pub fn generate_storage( }; let storage_fns = storage - .entries().iter() + .entries() + .iter() .map(|entry| { generate_storage_entry_fns(type_gen, pallet, entry, crate_path, should_gen_docs) }) diff --git a/metadata/src/lib.rs b/metadata/src/lib.rs index b9e4031cf1..4d6620d04e 100644 --- a/metadata/src/lib.rs +++ b/metadata/src/lib.rs @@ -78,7 +78,7 @@ impl Metadata { } /// An iterator over all of the available pallets. - pub fn pallets(&self) -> impl ExactSizeIterator> { + pub fn pallets(&self) -> impl ExactSizeIterator> { self.pallets.values().iter().map(|inner| PalletMetadata { inner, types: self.types(), @@ -109,7 +109,7 @@ impl Metadata { } /// An iterator over all of the runtime APIs. - pub fn runtime_api_traits(&self) -> impl ExactSizeIterator> { + pub fn runtime_api_traits(&self) -> impl ExactSizeIterator> { self.apis.values().iter().map(|inner| RuntimeApiMetadata { inner, types: self.types(), @@ -132,9 +132,9 @@ impl Metadata { /// Filter out any pallets that we don't want to keep, retaining only those that we do. pub fn retain(&mut self, pallet_filter: F, api_filter: G) - where - F: FnMut(&str) -> bool, - G: FnMut(&str) -> bool, + where + F: FnMut(&str) -> bool, + G: FnMut(&str) -> bool, { utils::retain::retain_metadata(self, pallet_filter, api_filter); } @@ -246,7 +246,7 @@ impl<'a> PalletMetadata<'a> { } /// An iterator over the constants in this pallet. - pub fn constants(&self) -> impl ExactSizeIterator { + pub fn constants(&self) -> impl ExactSizeIterator { self.inner.constants.values().iter() } @@ -508,7 +508,7 @@ impl<'a> RuntimeApiMetadata<'a> { &self.inner.docs } /// An iterator over the trait methods. - pub fn methods(&self) -> impl ExactSizeIterator { + pub fn methods(&self) -> impl ExactSizeIterator { self.inner.methods.values().iter() } /// Get a specific trait method given its name. @@ -559,7 +559,7 @@ impl RuntimeApiMethodMetadata { &self.docs } /// Method inputs. - pub fn inputs(&self) -> impl ExactSizeIterator { + pub fn inputs(&self) -> impl ExactSizeIterator { self.inputs.iter() } /// Method return type. diff --git a/metadata/src/utils/validation.rs b/metadata/src/utils/validation.rs index bc518611e4..a16f33fae9 100644 --- a/metadata/src/utils/validation.rs +++ b/metadata/src/utils/validation.rs @@ -379,14 +379,17 @@ pub fn get_pallet_hash(pallet: PalletMetadata) -> [u8; HASH_LEN] { let storage_bytes = match pallet.storage() { Some(storage) => { let prefix_hash = hash(storage.prefix().as_bytes()); - let entries_hash = storage.entries().iter().fold([0u8; HASH_LEN], |bytes, entry| { - // We don't care what order the storage entries occur in, so XOR them together - // to make the order irrelevant. - xor( - bytes, - get_storage_entry_hash(registry, entry, &mut visited_ids), - ) - }); + let entries_hash = storage + .entries() + .iter() + .fold([0u8; HASH_LEN], |bytes, entry| { + // We don't care what order the storage entries occur in, so XOR them together + // to make the order irrelevant. + xor( + bytes, + get_storage_entry_hash(registry, entry, &mut visited_ids), + ) + }); concat_and_hash2(&prefix_hash, &entries_hash) } None => [0u8; HASH_LEN], @@ -595,8 +598,8 @@ mod tests { meta_type::<()>(), vec![], ) - .try_into() - .expect("can build valid metadata") + .try_into() + .expect("can build valid metadata") } #[test] From df3e59f069955fd0c394f009560fb2080a95e83d Mon Sep 17 00:00:00 2001 From: Tadeo hepperle Date: Mon, 19 Jun 2023 18:45:45 +0200 Subject: [PATCH 16/17] add unit test and ordering --- cli/src/commands/diff.rs | 76 +++++++++++++++++++++++++++------------- 1 file changed, 51 insertions(+), 25 deletions(-) diff --git a/cli/src/commands/diff.rs b/cli/src/commands/diff.rs index a488445077..669046f65c 100644 --- a/cli/src/commands/diff.rs +++ b/cli/src/commands/diff.rs @@ -133,7 +133,7 @@ pub async fn run(opts: Opts, output: &mut impl std::io::Write) -> color_eyre::Re from.name(), storage_diff.to_strings().join(", ") ) - .yellow() + .yellow() )?; } } @@ -238,22 +238,22 @@ impl StorageEntryDiff { StorageEntryType::Plain(_) => None, StorageEntryType::Map { key_ty, .. } => Some(*key_ty), } - .map(|key_ty| { - metadata_1 - .type_hash(key_ty) - .expect("type should be present") - }) - .unwrap_or_default(); + .map(|key_ty| { + metadata_1 + .type_hash(key_ty) + .expect("type should be present") + }) + .unwrap_or_default(); let key_2_hash = match storage_entry_2.entry_type() { StorageEntryType::Plain(_) => None, StorageEntryType::Map { key_ty, .. } => Some(*key_ty), } - .map(|key_ty| { - metadata_2 - .type_hash(key_ty) - .expect("type should be present") - }) - .unwrap_or_default(); + .map(|key_ty| { + metadata_2 + .type_hash(key_ty) + .expect("type should be present") + }) + .unwrap_or_default(); let key_different = key_1_hash != key_2_hash; StorageEntryDiff { @@ -389,16 +389,16 @@ fn pallet_differences<'a>( ) } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq, Eq)] enum Diff { Added(T), Changed { from: T, to: T }, Removed(T), } -fn diff( - items_a: impl IntoIterator, - items_b: impl IntoIterator, +fn diff( + items_a: impl IntoIterator, + items_b: impl IntoIterator, hash_fn_a: impl Fn(&T) -> C, hash_fn_b: impl Fn(&T) -> C, key_fn: impl Fn(&T) -> I, @@ -425,13 +425,39 @@ fn diff( } *e2 = Some(t2); } - entries - .into_values() - .map(|tuple| match tuple { - (None, None) => panic!("At least one value is inserted when the key exists; qed"), - (Some(old), None) => Diff::Removed(old), - (None, Some(new)) => Diff::Added(new), - (Some(old), Some(new)) => Diff::Changed { from: old, to: new }, - }) + + // sort the values by key before returning + let mut diff_vec_with_keys: Vec<_> = entries + .into_iter().collect(); + diff_vec_with_keys.sort_by(|a, b| a.0.cmp(&b.0)); + diff_vec_with_keys.into_iter().map(|(_, tuple)| match tuple { + (None, None) => panic!("At least one value is inserted when the key exists; qed"), + (Some(old), None) => Diff::Removed(old), + (None, Some(new)) => Diff::Added(new), + (Some(old), Some(new)) => Diff::Changed { from: old, to: new }, + }) .collect() } + +#[cfg(test)] +mod test { + use crate::commands::diff::{diff, Diff}; + + #[test] + fn test_diff_fn() { + let old_pallets = [("Babe", 7), ("Claims", 9), ("Balances", 23)]; + let new_pallets = [("Claims", 9), ("Balances", 22), ("System", 3), ("NFTs", 5)]; + let hash_fn = |e: &(&str, i32)| e.0.len() as i32 * e.1; + let differences = diff(old_pallets, new_pallets, &hash_fn, &hash_fn, |e| e.0); + let expected_differences = vec![ + Diff::Removed(("Babe", 7)), + Diff::Changed { + from: ("Balances", 23), + to: ("Balances", 22), + }, + Diff::Added(("NFTs", 5)), + Diff::Added(("System", 3)), + ]; + assert_eq!(differences, expected_differences); + } +} From f0f14ab216456e69fd09e0970b4dc41e77522246 Mon Sep 17 00:00:00 2001 From: Tadeo hepperle Date: Mon, 19 Jun 2023 18:49:29 +0200 Subject: [PATCH 17/17] fix small issue --- cli/src/commands/diff.rs | 49 ++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/cli/src/commands/diff.rs b/cli/src/commands/diff.rs index 669046f65c..9200e85209 100644 --- a/cli/src/commands/diff.rs +++ b/cli/src/commands/diff.rs @@ -133,7 +133,7 @@ pub async fn run(opts: Opts, output: &mut impl std::io::Write) -> color_eyre::Re from.name(), storage_diff.to_strings().join(", ") ) - .yellow() + .yellow() )?; } } @@ -238,22 +238,22 @@ impl StorageEntryDiff { StorageEntryType::Plain(_) => None, StorageEntryType::Map { key_ty, .. } => Some(*key_ty), } - .map(|key_ty| { - metadata_1 - .type_hash(key_ty) - .expect("type should be present") - }) - .unwrap_or_default(); + .map(|key_ty| { + metadata_1 + .type_hash(key_ty) + .expect("type should be present") + }) + .unwrap_or_default(); let key_2_hash = match storage_entry_2.entry_type() { StorageEntryType::Plain(_) => None, StorageEntryType::Map { key_ty, .. } => Some(*key_ty), } - .map(|key_ty| { - metadata_2 - .type_hash(key_ty) - .expect("type should be present") - }) - .unwrap_or_default(); + .map(|key_ty| { + metadata_2 + .type_hash(key_ty) + .expect("type should be present") + }) + .unwrap_or_default(); let key_different = key_1_hash != key_2_hash; StorageEntryDiff { @@ -397,8 +397,8 @@ enum Diff { } fn diff( - items_a: impl IntoIterator, - items_b: impl IntoIterator, + items_a: impl IntoIterator, + items_b: impl IntoIterator, hash_fn_a: impl Fn(&T) -> C, hash_fn_b: impl Fn(&T) -> C, key_fn: impl Fn(&T) -> I, @@ -427,15 +427,16 @@ fn diff( } // sort the values by key before returning - let mut diff_vec_with_keys: Vec<_> = entries - .into_iter().collect(); + let mut diff_vec_with_keys: Vec<_> = entries.into_iter().collect(); diff_vec_with_keys.sort_by(|a, b| a.0.cmp(&b.0)); - diff_vec_with_keys.into_iter().map(|(_, tuple)| match tuple { - (None, None) => panic!("At least one value is inserted when the key exists; qed"), - (Some(old), None) => Diff::Removed(old), - (None, Some(new)) => Diff::Added(new), - (Some(old), Some(new)) => Diff::Changed { from: old, to: new }, - }) + diff_vec_with_keys + .into_iter() + .map(|(_, tuple)| match tuple { + (None, None) => panic!("At least one value is inserted when the key exists; qed"), + (Some(old), None) => Diff::Removed(old), + (None, Some(new)) => Diff::Added(new), + (Some(old), Some(new)) => Diff::Changed { from: old, to: new }, + }) .collect() } @@ -448,7 +449,7 @@ mod test { let old_pallets = [("Babe", 7), ("Claims", 9), ("Balances", 23)]; let new_pallets = [("Claims", 9), ("Balances", 22), ("System", 3), ("NFTs", 5)]; let hash_fn = |e: &(&str, i32)| e.0.len() as i32 * e.1; - let differences = diff(old_pallets, new_pallets, &hash_fn, &hash_fn, |e| e.0); + let differences = diff(old_pallets, new_pallets, hash_fn, hash_fn, |e| e.0); let expected_differences = vec![ Diff::Removed(("Babe", 7)), Diff::Changed {