Add proper test Custom values (#1147)

* add proper tests for custom values

* remove try operators

* use sustrate compat for import of hash

* add license and hex

* add script to artifacts.sh

* custom values with ids not in metadata can be accessed in static interface

* fmt and clippy

* access bytes of custom values directly, even if type id wrong

* final fixes

* removing substrate-compat flag from ui tests

* Update subxt/src/custom_values/custom_values_client.rs

Co-authored-by: James Wilson <james@jsdw.me>

* remove types access in type generator

* 2 extra lines

---------

Co-authored-by: James Wilson <james@jsdw.me>

Cargo.lock

@@ -1520,6 +1520,16 @@ dependencies = [
  "slab",
 ]
 
+[[package]]
+name = "generate-custom-metadata"
+version = "0.31.0"
+dependencies = [
+ "frame-metadata 16.0.0",
+ "parity-scale-codec",
+ "scale-info",
+ "subxt",
+]
+
 [[package]]
 name = "generic-array"
 version = "0.12.4"
@@ -4774,6 +4784,8 @@ name = "ui-tests"
 version = "0.31.0"
 dependencies = [
  "frame-metadata 16.0.0",
+ "generate-custom-metadata",
+ "hex",
  "parity-scale-codec",
  "scale-info",
  "subxt",

Cargo.toml

@@ -7,6 +7,7 @@ members = [
     "testing/test-runtime",
     "testing/integration-tests",
     "testing/ui-tests",
+    "testing/generate-custom-metadata",
     "macro",
     "metadata",
     "signer",

codegen/src/api/custom_values.rs

@@ -9,7 +9,7 @@ use heck::ToSnakeCase as _;
 use subxt_metadata::{CustomValueMetadata, Metadata};
 
 use proc_macro2::TokenStream as TokenStream2;
-use quote::{format_ident, quote};
+use quote::{quote, ToTokens};
 
 /// Generate the custom values mod, if there are any custom values in the metadata. Else returns None.
 pub fn generate_custom_values<'a>(
@@ -43,18 +43,33 @@ fn generate_custom_value_fn(
     // names are transformed to snake case to make for good function identifiers.
     let name = custom_value.name();
     let fn_name = name.to_snake_case();
-    // Skip elements where the fn name is already occupied. E.g. if you have custom values with names "Foo" and "foo" in the metadata.
     if fn_names_taken.contains(&fn_name) {
         return None;
     }
-    let fn_name_ident = format_ident!("{fn_name}");
+    // if the fn_name would be an invalid ident, return None:
+    let fn_name_ident = syn::parse_str::<syn::Ident>(&fn_name).ok()?;
     fn_names_taken.insert(fn_name);
 
     let custom_value_hash = custom_value.hash();
-    let return_ty = type_gen.resolve_type_path(custom_value.type_id());
+
+    // for custom values it is important to check if the type id is actually in the metadata:
+    let type_is_valid = custom_value
+        .types()
+        .resolve(custom_value.type_id())
+        .is_some();
+    let (return_ty, decodable) = if type_is_valid {
+        let return_ty = type_gen
+            .resolve_type_path(custom_value.type_id())
+            .to_token_stream();
+        let decodable = quote!(#crate_path::custom_values::Yes);
+        (return_ty, decodable)
+    } else {
+        // if type registry does not contain the type, we can just return the Encoded scale bytes.
+        (quote!(()), quote!(()))
+    };
 
     Some(quote!(
-        pub fn #fn_name_ident() -> #crate_path::custom_values::StaticAddress<#return_ty> {
+        pub fn #fn_name_ident(&self) -> #crate_path::custom_values::StaticAddress<#return_ty, #decodable> {
             #crate_path::custom_values::StaticAddress::new_static(#name, [#(#custom_value_hash,)*])
         }
     ))

codegen/src/types/derives.rs

@@ -59,10 +59,7 @@ impl DerivesRegistry {
         derives: impl IntoIterator<Item = syn::Path>,
         attributes: impl IntoIterator<Item = syn::Attribute>,
     ) {
-        let type_derives = self
-            .specific_type_derives
-            .entry(ty)
-            .or_insert_with(Derives::new);
+        let type_derives = self.specific_type_derives.entry(ty).or_default();
         type_derives.derives.extend(derives);
         type_derives.attributes.extend(attributes);
     }

metadata/src/lib.rs

@@ -688,6 +688,11 @@ pub struct CustomValueMetadata<'a> {
 }
 
 impl<'a> CustomValueMetadata<'a> {
+    /// Access the underlying type registry.
+    pub fn types(&self) -> &PortableRegistry {
+        self.types
+    }
+
     /// The scale encoded value
     pub fn bytes(&self) -> &'a [u8] {
         self.data

metadata/src/utils/validation.rs

@@ -405,15 +405,22 @@ pub fn get_custom_metadata_hash(custom_metadata: &CustomMetadata) -> [u8; HASH_L
 }
 
 /// Obtain the hash of some custom value in the metadata including it's name/key.
+///
+/// If the `custom_value` has a type id that is not present in the metadata,
+/// only the name and bytes are used for hashing.
 pub fn get_custom_value_hash(
     custom_value: &CustomValueMetadata,
     cache: &mut HashMap<u32, CachedHash>,
 ) -> [u8; HASH_LEN] {
-    concat_and_hash3(
-        &hash(custom_value.name.as_bytes()),
-        &get_type_hash(custom_value.types, custom_value.type_id(), cache),
-        &hash(custom_value.bytes()),
-    )
+    let name_hash = hash(custom_value.name.as_bytes());
+    if custom_value.types.resolve(custom_value.type_id()).is_none() {
+        hash(&name_hash)
+    } else {
+        concat_and_hash2(
+            &name_hash,
+            &get_type_hash(custom_value.types, custom_value.type_id(), cache),
+        )
+    }
 }
 
 /// Obtain the hash for a specific storage item, or an error if it's not found.

scripts/artifacts.sh

@@ -17,5 +17,7 @@ cargo run --bin subxt metadata --version 15 > artifacts/polkadot_metadata_full.s
 cargo run --bin subxt codegen --file artifacts/polkadot_metadata_full.scale | rustfmt > testing/integration-tests/src/full_client/codegen/polkadot.rs
 # generate a metadata file that only contains a few pallets that we need for our examples.
 cargo run --bin subxt metadata --file artifacts/polkadot_metadata_full.scale --pallets "Balances,Staking,System,Multisig,Timestamp,ParaInherent" > artifacts/polkadot_metadata_small.scale
-# generate a metadata file that only contains no pallets
+# generate a metadata file that contains no pallets
 cargo run --bin subxt metadata --file artifacts/polkadot_metadata_full.scale --pallets "" > artifacts/polkadot_metadata_tiny.scale
+# generate a metadata file that only contains some custom metadata
+cargo run --bin generate-custom-metadata > artifacts/metadata_with_custom_values.scale 

subxt/src/custom_values/custom_value_address.rs

@@ -10,6 +10,9 @@ use crate::metadata::DecodeWithMetadata;
 pub trait CustomValueAddress {
     /// The type of the custom value.
     type Target: DecodeWithMetadata;
+    /// Should be set to `Yes` for Dynamic values and static values that have a valid type.
+    /// Should be `()` for custom values, that have an invalid type id.
+    type IsDecodable;
 
     /// the name (key) by which the custom value can be accessed in the metadata.
     fn name(&self) -> &str;
@@ -22,24 +25,28 @@ pub trait CustomValueAddress {
 
 impl CustomValueAddress for str {
     type Target = DecodedValueThunk;
+    type IsDecodable = Yes;
 
     fn name(&self) -> &str {
         self
     }
 }
 
+/// Used to signal whether a [`CustomValueAddress`] can be decoded.
+pub struct Yes;
+
 /// A static address to a custom value.
-pub struct StaticAddress<R> {
+pub struct StaticAddress<ReturnTy, IsDecodable> {
     name: &'static str,
     hash: Option<[u8; 32]>,
-    phantom: PhantomData<R>,
+    phantom: PhantomData<(ReturnTy, IsDecodable)>,
 }
 
-impl<R> StaticAddress<R> {
+impl<ReturnTy, IsDecodable> StaticAddress<ReturnTy, IsDecodable> {
     #[doc(hidden)]
     /// Creates a new StaticAddress.
-    pub fn new_static(name: &'static str, hash: [u8; 32]) -> Self {
-        StaticAddress {
+    pub fn new_static(name: &'static str, hash: [u8; 32]) -> StaticAddress<ReturnTy, IsDecodable> {
+        StaticAddress::<ReturnTy, IsDecodable> {
             name,
             hash: Some(hash),
             phantom: PhantomData,
@@ -56,8 +63,9 @@ impl<R> StaticAddress<R> {
     }
 }
 
-impl<R: DecodeWithMetadata> CustomValueAddress for StaticAddress<R> {
+impl<R: DecodeWithMetadata, Y> CustomValueAddress for StaticAddress<R, Y> {
     type Target = R;
+    type IsDecodable = Y;
 
     fn name(&self) -> &str {
         self.name

subxt/src/custom_values/custom_values_client.rs

@@ -1,5 +1,5 @@
 use crate::client::OfflineClientT;
-use crate::custom_values::custom_value_address::CustomValueAddress;
+use crate::custom_values::custom_value_address::{CustomValueAddress, Yes};
 use crate::error::MetadataError;
 use crate::metadata::DecodeWithMetadata;
 use crate::{Config, Error};
@@ -26,7 +26,7 @@ impl<T, Client> CustomValuesClient<T, Client> {
 impl<T: Config, Client: OfflineClientT<T>> CustomValuesClient<T, Client> {
     /// Access a custom value by the address it is registered under. This can be just a [str] to get back a dynamic value,
     /// or a static address from the generated static interface to get a value of a static type returned.
-    pub fn at<Address: CustomValueAddress + ?Sized>(
+    pub fn at<Address: CustomValueAddress<IsDecodable = Yes> + ?Sized>(
         &self,
         address: &Address,
     ) -> Result<Address::Target, Error> {
@@ -48,6 +48,24 @@ impl<T: Config, Client: OfflineClientT<T>> CustomValuesClient<T, Client> {
         Ok(value)
     }
 
+    /// Access the bytes of a custom value by the address it is registered under.
+    pub fn bytes_at<Address: CustomValueAddress + ?Sized>(
+        &self,
+        address: &Address,
+    ) -> Result<Vec<u8>, Error> {
+        // 1. Validate custom value shape if hash given:
+        self.validate(address)?;
+
+        // 2. Return the underlying bytes:
+        let metadata = self.client.metadata();
+        let custom = metadata.custom();
+        let custom_value = custom
+            .get(address.name())
+            .ok_or_else(|| MetadataError::CustomValueNameNotFound(address.name().to_string()))?;
+
+        Ok(custom_value.bytes().to_vec())
+    }
+
     /// Run the validation logic against some custom value address you'd like to access. Returns `Ok(())`
     /// if the address is valid (or if it's not possible to check since the address has no validation hash).
     /// Returns an error if the address was not valid (wrong name, type or raw bytes)

subxt/src/custom_values/mod.rs

@@ -7,5 +7,5 @@
 mod custom_value_address;
 mod custom_values_client;
 
-pub use custom_value_address::{CustomValueAddress, StaticAddress};
+pub use custom_value_address::{CustomValueAddress, StaticAddress, Yes};
 pub use custom_values_client::CustomValuesClient;

subxt/src/utils/mod.rs

@@ -30,7 +30,7 @@ pub use primitive_types::{H160, H256, H512};
 
 /// Wraps an already encoded byte vector, prevents being encoded as a raw byte vector as part of
 /// the transaction payload
-#[derive(Clone, Debug, Eq, PartialEq, Decode)]
+#[derive(Clone, Debug, Eq, PartialEq)]
 pub struct Encoded(pub Vec<u8>);
 
 impl codec::Encode for Encoded {

testing/generate-custom-metadata/Cargo.toml

@@ -0,0 +1,18 @@
+[package]
+name = "generate-custom-metadata"
+authors.workspace = true
+edition.workspace = true
+version.workspace = true
+rust-version.workspace = true
+license.workspace = true
+repository.workspace = true
+documentation.workspace = true
+homepage.workspace = true
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]
+subxt = { workspace = true, features = ["native"] }
+scale-info = { workspace = true, features = ["bit-vec"] }
+frame-metadata = { workspace = true }
+codec = { package = "parity-scale-codec", workspace = true, features = ["derive", "bit-vec"] }

testing/generate-custom-metadata/README.md

@@ -0,0 +1,5 @@
+# generate-custom-metadata
+
+A small crate with a binary that creates scale encoded metadata with custom values and writes it to stdout (as raw bytes).
+
+It also provides dispatch error types that are used in `../ui_tests`.

testing/generate-custom-metadata/src/lib.rs

@@ -5,11 +5,14 @@
 use codec::Encode;
 use frame_metadata::v15::{CustomMetadata, ExtrinsicMetadata, OuterEnums, RuntimeMetadataV15};
 use frame_metadata::RuntimeMetadataPrefixed;
+
 use scale_info::form::PortableForm;
 use scale_info::TypeInfo;
 use scale_info::{meta_type, IntoPortable};
 use std::collections::BTreeMap;
 
+pub mod dispatch_error;
+
 /// Generate metadata which contains a `Foo { a: u8, b: &str }` custom value.
 pub fn metadata_custom_values_foo() -> RuntimeMetadataPrefixed {
     let mut registry = scale_info::Registry::new();
@@ -23,7 +26,10 @@ pub fn metadata_custom_values_foo() -> RuntimeMetadataPrefixed {
     }
 
     let foo_value_metadata: frame_metadata::v15::CustomValueMetadata<PortableForm> = {
-        let value = Foo { a: 0, b: "Hello" };
+        let value = Foo {
+            a: 42,
+            b: "Have a great day!",
+        };
         let foo_ty = scale_info::MetaType::new::<Foo>();
         let foo_ty_id = registry.register_type(&foo_ty);
         frame_metadata::v15::CustomValueMetadata {
@@ -32,6 +38,13 @@ pub fn metadata_custom_values_foo() -> RuntimeMetadataPrefixed {
         }
     };
 
+    let invalid_type_id_metadata: frame_metadata::v15::CustomValueMetadata<PortableForm> = {
+        frame_metadata::v15::CustomValueMetadata {
+            ty: u32::MAX.into(),
+            value: vec![0, 1, 2, 3],
+        }
+    };
+
     // We don't care about the extrinsic type.
     let extrinsic = ExtrinsicMetadata {
         version: 0,
@@ -48,7 +61,7 @@ pub fn metadata_custom_values_foo() -> RuntimeMetadataPrefixed {
     let unit_ty = registry.register_type(&meta_type::<()>());
 
     // Metadata needs to contain this DispatchError, since codegen looks for it.
-    registry.register_type(&meta_type::<crate::utils::dispatch_error::ArrayDispatchError>());
+    registry.register_type(&meta_type::<dispatch_error::ArrayDispatchError>());
 
     let metadata = RuntimeMetadataV15 {
         types: registry.into(),
@@ -68,6 +81,7 @@ pub fn metadata_custom_values_foo() -> RuntimeMetadataPrefixed {
                 ("foo".into(), foo_value_metadata.clone()),
                 ("12".into(), foo_value_metadata.clone()),
                 ("&Hello".into(), foo_value_metadata),
+                ("InvalidTypeId".into(), invalid_type_id_metadata),
             ]),
         },
     };

testing/generate-custom-metadata/src/main.rs

@@ -0,0 +1,17 @@
+// Copyright 2019-2023 Parity Technologies (UK) Ltd.
+// This file is dual-licensed as Apache-2.0 or GPL-3.0.
+// see LICENSE for license details.
+
+use codec::Encode;
+use std::io::{self, Write};
+
+/// Creates some scale encoded metadata with custom values and writes it out to stdout (as raw bytes)
+///
+/// Can be called from the root of the project with: `cargo run --bin generate-custom-metadata > output.scale`.
+fn main() -> io::Result<()> {
+    let metadata_prefixed = generate_custom_metadata::metadata_custom_values_foo();
+    let stdout = io::stdout();
+    let mut handle = stdout.lock();
+    handle.write_all(&metadata_prefixed.encode())?;
+    Ok(())
+}

testing/ui-tests/Cargo.toml

@@ -10,8 +10,13 @@ publish = false
 
 [dev-dependencies]
 trybuild = { workspace = true }
+hex = { workspace = true }
 scale-info = { workspace = true, features = ["bit-vec"] }
-frame-metadata ={ workspace = true }
-codec = { package = "parity-scale-codec", workspace = true, features = ["derive", "bit-vec"] }
+frame-metadata = { workspace = true }
+codec = { package = "parity-scale-codec", workspace = true, features = [
+    "derive",
+    "bit-vec",
+] }
 subxt = { workspace = true, features = ["native", "jsonrpsee"] }
 subxt-metadata = { workspace = true }
+generate-custom-metadata = { path = "../generate-custom-metadata" }

testing/ui-tests/src/correct/custom_values.rs

@@ -0,0 +1,47 @@
+use codec::{Decode};
+use subxt::{config::substrate::H256, OfflineClient, PolkadotConfig};
+use subxt_metadata::Metadata;
+
+#[subxt::subxt(runtime_metadata_path = "../../../../artifacts/metadata_with_custom_values.scale", derive_for_all_types = "Eq, PartialEq")]
+pub mod node {}
+use node::runtime_types::generate_custom_metadata::Foo;
+
+fn main() {
+    let api = construct_offline_client();
+
+    let expected_foo = Foo {
+        a: 42,
+        b: "Have a great day!".into(),
+    };
+
+    // static query:
+    let foo_address = node::custom().foo();
+    let foo = api.custom_values().at(&foo_address).unwrap();
+    assert_eq!(foo, expected_foo);
+
+    // dynamic query:
+    let foo_value = api.custom_values().at("Foo").unwrap();
+    let foo: Foo = foo_value.as_type().unwrap();
+    assert_eq!(foo, expected_foo);
+
+    // static query for some custom value that has an invalid type id: (we can still access the bytes)
+    let custom_bytes = api.custom_values().bytes_at("InvalidTypeId").unwrap();
+    assert_eq!(vec![0,1,2,3], custom_bytes);
+}
+
+fn construct_offline_client() -> OfflineClient<PolkadotConfig> {
+    let genesis_hash = {
+        let h = "91b171bb158e2d3848fa23a9f1c25182fb8e20313b2c1eb49219da7a70ce90c3";
+        let bytes = hex::decode(h).unwrap();
+        H256::from_slice(&bytes)
+    };
+    let runtime_version = subxt::backend::RuntimeVersion {
+        spec_version: 9370,
+        transaction_version: 20,
+    };
+    let metadata = {
+        let bytes = std::fs::read("../../../../artifacts/metadata_with_custom_values.scale").unwrap();
+        Metadata::decode(&mut &*bytes).unwrap()
+    };
+    OfflineClient::<PolkadotConfig>::new(genesis_hash, runtime_version, metadata)
+}

testing/ui-tests/src/dispatch_errors.rs

@@ -2,10 +2,12 @@
 // This file is dual-licensed as Apache-2.0 or GPL-3.0.
 // see LICENSE for license details.
 
-use crate::utils::{
-    dispatch_error::{ArrayDispatchError, LegacyDispatchError, NamedFieldDispatchError},
-    generate_metadata_from_pallets_custom_dispatch_error,
+use crate::utils::generate_metadata_from_pallets_custom_dispatch_error;
+
+use generate_custom_metadata::dispatch_error::{
+    ArrayDispatchError, LegacyDispatchError, NamedFieldDispatchError,
 };
+
 use frame_metadata::RuntimeMetadataPrefixed;
 
 pub fn metadata_array_dispatch_error() -> RuntimeMetadataPrefixed {

testing/ui-tests/src/lib.rs

@@ -11,7 +11,6 @@
 //! Use with `TRYBUILD=overwrite` after updating codebase (see `trybuild` docs for more details on that)
 //! to automatically regenerate `stderr` files, but don't forget to check that new files make sense.
 
-mod custom_values;
 mod dispatch_errors;
 mod storage;
 mod utils;
@@ -21,13 +20,13 @@ use crate::utils::MetadataTestRunner;
 // Each of these tests leads to some rust code being compiled and
 // executed to test that compilation is successful (or errors in the
 // way that we'd expect).
+
 #[test]
 fn ui_tests() {
     let mut m = MetadataTestRunner::default();
     let t = trybuild::TestCases::new();
 
     t.pass("src/correct/*.rs");
-
     // Check that storage maps with no keys are handled properly.
     t.pass(
         m.new_test_case()
@@ -52,12 +51,6 @@ fn ui_tests() {
             .build(dispatch_errors::metadata_array_dispatch_error()),
     );
 
-    t.pass(
-        m.new_test_case()
-            .name("custom_values_foo")
-            .build(custom_values::metadata_custom_values_foo()),
-    );
-
     // Test retaining only specific pallets and ensure that works.
     for pallet in ["Babe", "Claims", "Grandpa", "Balances"] {
         let mut metadata = MetadataTestRunner::load_metadata();

testing/ui-tests/src/utils/mod.rs

@@ -2,7 +2,6 @@
 // This file is dual-licensed as Apache-2.0 or GPL-3.0.
 // see LICENSE for license details.
 
-pub mod dispatch_error;
 mod metadata_test_runner;
 
 use frame_metadata::{
@@ -12,6 +11,7 @@ use frame_metadata::{
     },
     RuntimeMetadataPrefixed,
 };
+use generate_custom_metadata::dispatch_error::ArrayDispatchError;
 use scale_info::{meta_type, IntoPortable, TypeInfo};
 
 pub use metadata_test_runner::MetadataTestRunner;
@@ -78,9 +78,7 @@ pub fn generate_metadata_from_pallets_custom_dispatch_error<DispatchError: TypeI
 /// We default to a useless extrinsic type, and register a fake `DispatchError`
 /// type so that codegen is happy with the metadata generated.
 pub fn generate_metadata_from_pallets(pallets: Vec<PalletMetadata>) -> RuntimeMetadataPrefixed {
-    generate_metadata_from_pallets_custom_dispatch_error::<dispatch_error::ArrayDispatchError>(
-        pallets,
-    )
+    generate_metadata_from_pallets_custom_dispatch_error::<ArrayDispatchError>(pallets)
 }
 
 /// Given some storage entries, generate a [`RuntimeMetadataPrefixed`] struct.