Macro error messages validating metadata and making suggestions (#1339)

* integrate scale-typegen, remove types mod

* reintroduce default substitutes and derives

* support runtime_types only again

* generating polkadot.rs ok

* update scale-typegen to discrete error types

* scale-typegen-api-changes

* add note about UncheckedExtrinsic in default substitutes

* add resursive attributes and derives

* adjust example where Clone bound recursive

* move scale-typegen dependency to workspace

* expose default typegen settings

* lightclient: Fix wasm socket closure called after being dropped (#1289)

* lightclient: Close wasm socket while dropping from connecting state

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* lightclient: Construct one time only closures

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* testing: Enable console logs for lightclient WASM testing

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* lightclient: Separate wakes and check connectivity on poll_read

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* lightclient: Close the socket depending on internal state

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* Revert "lightclient: Separate wakes and check connectivity on poll_read"

This reverts commit 866094001d4c0b119a80ed681a74b323f74eae1b.

* lightclient: Return pending if socket is opening from poll_read

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* lightclient: Close the socket on `poll_close`

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* lightclient: Reset closures on Drop to avoid recursive invokation

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* lightclient: Close the socket if not already closing

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

---------

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* workflows: Install rustup component for building substrate (#1295)

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* cli: Command to fetch chainSpec and optimise its size (#1278)

* cli: Add chainSpec command

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* cli/chainSpec: Move to dedicated module

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* cli: Compute the state root hash

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* cli: Remove code substitutes

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* artifacts: Update polkadot.json

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* scripts: Generate the chain spec

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* cli: Remove testing artifacts

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* cli: Fix clippy

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* cli: Apply rustfmt

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* cli: Introduce feature flag for smoldot dependency

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* cli: Rename chain-spec to chain-spec-pruning

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* scripts: Update chain-spec command

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

---------

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>

* remove comments and unused args

* Update substrate- and signer-related dependencies (#1297)

* update crypto dependencies, adjust keypair

* add scale_info::TypeInfo derive in some places

* add multi signature derive

* fix lock file

* fix lock file again :|

* adjust to new interface in scale-typegen

* use released scale typegen

* reintroduce type aliases

* introduce type aliases again using scale-typegen

* cargo fmt and clippy

* reconcile changes with master branch

* update polkadot.rs

* bump scale-typgen to fix substitution

* subxt macro, helpful error messages

* adjust ui tests

* fix lock file

* format

* Update macro/src/lib.rs

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>

* incorperate nits

* update Cargo.lock to avoid compatibility issues

---------

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Co-authored-by: Alexandru Vasile <60601340+lexnv@users.noreply.github.com>
Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>

Cargo.lock

@@ -4538,6 +4538,8 @@ dependencies = [
  "darling 0.20.3",
  "parity-scale-codec",
  "proc-macro-error",
+ "quote",
+ "scale-typegen",
  "subxt-codegen",
  "syn 2.0.48",
 ]

macro/Cargo.toml

@@ -23,5 +23,7 @@ proc-macro = true
 codec = { package = "parity-scale-codec", workspace = true }
 darling = { workspace = true }
 proc-macro-error = { workspace = true }
-syn = { workspace = true }
+syn.workspace = { workspace = true }
+quote = { workspace = true }
 subxt-codegen = { workspace = true, features = ["fetch-metadata"] }
+scale-typegen = { workspace = true }

macro/src/lib.rs

@@ -10,11 +10,16 @@ use codec::Decode;
 use darling::{ast::NestedMeta, FromMeta};
 use proc_macro::TokenStream;
 use proc_macro_error::{abort_call_site, proc_macro_error};
+use quote::ToTokens;
+use scale_typegen::typegen::{
+    settings::substitutes::path_segments,
+    validation::{registry_contains_type_path, similar_type_paths_in_registry},
+};
 use subxt_codegen::{
     fetch_metadata::{
         fetch_metadata_from_file_blocking, fetch_metadata_from_url_blocking, MetadataVersion, Url,
     },
-    CodegenBuilder, CodegenError,
+    CodegenBuilder, CodegenError, Metadata,
 };
 use syn::{parse_macro_input, punctuated::Punctuated};
 
@@ -83,17 +88,21 @@ struct SubstituteType {
 #[proc_macro_attribute]
 #[proc_macro_error]
 pub fn subxt(args: TokenStream, input: TokenStream) -> TokenStream {
-    let attr_args = match NestedMeta::parse_meta_list(args.into()) {
-        Ok(v) => v,
-        Err(e) => {
-            return TokenStream::from(darling::Error::from(e).write_errors());
-        }
-    };
-    let item_mod = parse_macro_input!(input as syn::ItemMod);
-    let args = match RuntimeMetadataArgs::from_list(&attr_args) {
-        Ok(v) => v,
-        Err(e) => return TokenStream::from(e.write_errors()),
-    };
+    match subxt_inner(args, parse_macro_input!(input as syn::ItemMod)) {
+        Ok(e) => e,
+        Err(e) => e,
+    }
+}
+
+// Note: just an additional function to make early returns easier.
+fn subxt_inner(args: TokenStream, item_mod: syn::ItemMod) -> Result<TokenStream, TokenStream> {
+    let attr_args = NestedMeta::parse_meta_list(args.into())
+        .map_err(|e| TokenStream::from(darling::Error::from(e).write_errors()))?;
+    let args = RuntimeMetadataArgs::from_list(&attr_args)
+        .map_err(|e| TokenStream::from(e.write_errors()))?;
+
+    // Fetch metadata first, because we need it to validate some of the chosen codegen options.
+    let metadata = fetch_metadata(&args)?;
 
     let mut codegen = CodegenBuilder::new();
 
@@ -127,6 +136,7 @@ pub fn subxt(args: TokenStream, input: TokenStream) -> TokenStream {
             .collect(),
     );
     for d in args.derive_for_type {
+        validate_type_path(&d.path.path, &metadata);
         codegen.add_derives_for_type(d.path, d.derive.into_iter(), d.recursive);
     }
 
@@ -139,20 +149,65 @@ pub fn subxt(args: TokenStream, input: TokenStream) -> TokenStream {
             .collect(),
     );
     for d in args.attributes_for_type {
+        validate_type_path(&d.path.path, &metadata);
         codegen.add_attributes_for_type(d.path, d.attributes.into_iter().map(|a| a.0), d.recursive)
     }
 
     // Insert type substitutions:
     for sub in args.substitute_type.into_iter() {
+        validate_type_path(&sub.path, &metadata);
         codegen.set_type_substitute(sub.path, sub.with);
     }
 
+    let code = codegen
+        .generate(metadata)
+        .map_err(|e| e.into_compile_error())?;
+
+    Ok(code.into())
+}
+
+/// Checks that a type is present in the type registry. If it is not found, abort with a
+/// helpful error message, showing the user alternative types, that have the same name, but are at different locations in the metadata.
+fn validate_type_path(path: &syn::Path, metadata: &Metadata) {
+    let path_segments = path_segments(path);
+    let ident = &path
+        .segments
+        .last()
+        .expect("Empty path should be filtered out before already")
+        .ident;
+    if !registry_contains_type_path(metadata.types(), &path_segments) {
+        let alternatives = similar_type_paths_in_registry(metadata.types(), path);
+        let alternatives: String = if alternatives.is_empty() {
+            format!("There is no Type with name `{ident}` in the provided metadata.")
+        } else {
+            let mut s = "A type with the same name is present at: ".to_owned();
+            for p in alternatives {
+                s.push('\n');
+                s.push_str(&pretty_path(&p));
+            }
+            s
+        };
+
+        abort_call_site!(
+            "Type `{}` does not exist at path `{}`\n\n{}",
+            ident.to_string(),
+            pretty_path(path),
+            alternatives
+        );
+    }
+
+    fn pretty_path(path: &syn::Path) -> String {
+        path.to_token_stream().to_string().replace(' ', "")
+    }
+}
+
+/// Fetches metadata in a blocking manner, from a url or file path.
+fn fetch_metadata(args: &RuntimeMetadataArgs) -> Result<subxt_codegen::Metadata, TokenStream> {
     // Do we want to fetch unstable metadata? This only works if fetching from a URL.
     let unstable_metadata = args.unstable_metadata.is_present();
-
-    match (
-        args.runtime_metadata_path,
-        args.runtime_metadata_insecure_url,
+    let metadata = match (
+        &args.runtime_metadata_path,
+        &args.runtime_metadata_insecure_url,
     ) {
         (Some(rest_of_path), None) => {
             if unstable_metadata {
@@ -164,16 +219,12 @@ pub fn subxt(args: TokenStream, input: TokenStream) -> TokenStream {
             let root = std::env::var("CARGO_MANIFEST_DIR").unwrap_or_else(|_| ".".into());
             let root_path = std::path::Path::new(&root);
             let path = root_path.join(rest_of_path);
-            let generated_code = fetch_metadata_from_file_blocking(&path)
-                .map_err(CodegenError::from)
+            fetch_metadata_from_file_blocking(&path)
                 .and_then(|b| subxt_codegen::Metadata::decode(&mut &*b).map_err(Into::into))
-                .and_then(|m| codegen.generate(m).map_err(Into::into))
-                .unwrap_or_else(|e| e.into_compile_error());
-
-            generated_code.into()
+                .map_err(|e| CodegenError::from(e).into_compile_error())?
         }
         (None, Some(url_string)) => {
-            let url = Url::parse(&url_string).unwrap_or_else(|_| {
+            let url = Url::parse(url_string).unwrap_or_else(|_| {
                 abort_call_site!("Cannot download metadata; invalid url: {}", url_string)
             });
 
@@ -182,13 +233,10 @@ pub fn subxt(args: TokenStream, input: TokenStream) -> TokenStream {
                 false => MetadataVersion::Latest,
             };
 
-            let generated_code = fetch_metadata_from_url_blocking(url, version)
+            fetch_metadata_from_url_blocking(url, version)
                 .map_err(CodegenError::from)
                 .and_then(|b| subxt_codegen::Metadata::decode(&mut &*b).map_err(Into::into))
-                .and_then(|m| codegen.generate(m).map_err(Into::into))
-                .unwrap_or_else(|e| e.into_compile_error());
-
-            generated_code.into()
+                .map_err(|e| e.into_compile_error())?
         }
         (None, None) => {
             abort_call_site!(
@@ -200,5 +248,6 @@ pub fn subxt(args: TokenStream, input: TokenStream) -> TokenStream {
                 "Only one of 'runtime_metadata_path' or 'runtime_metadata_insecure_url' can be provided"
             )
         }
-    }
+    };
+    Ok(metadata)
 }

testing/ui-tests/src/incorrect/substitute_at_wrong_path.rs

@@ -0,0 +1,10 @@
+#[subxt::subxt(
+    runtime_metadata_path = "../../../../artifacts/polkadot_metadata_small.scale",
+    substitute_type(
+        path = "sp_runtime::multiaddress::Event",
+        with = "crate::MyEvent"
+    )
+)]
+pub mod node_runtime {}
+
+fn main() {}

testing/ui-tests/src/incorrect/substitute_at_wrong_path.stderr

@@ -0,0 +1,18 @@
+error: Type `Event` does not exist at path `sp_runtime::multiaddress::Event`
+
+       A type with the same name is present at:
+       frame_system::pallet::Event
+       pallet_balances::pallet::Event
+       pallet_multisig::pallet::Event
+ --> src/incorrect/substitute_at_wrong_path.rs:1:1
+  |
+1 | / #[subxt::subxt(
+2 | |     runtime_metadata_path = "../../../../artifacts/polkadot_metadata_small.scale",
+3 | |     substitute_type(
+4 | |         path = "sp_runtime::multiaddress::Event",
+5 | |         with = "crate::MyEvent"
+6 | |     )
+7 | | )]
+  | |__^
+  |
+  = note: this error originates in the attribute macro `subxt::subxt` (in Nightly builds, run with -Z macro-backtrace for more info)

testing/ui-tests/src/incorrect/substitute_path_not_absolute.rs

@@ -1,8 +1,8 @@
 #[subxt::subxt(
-    runtime_metadata_path = "../../../../artifacts/polkadot_metadata_tiny.scale",
+    runtime_metadata_path = "../../../../artifacts/polkadot_metadata_small.scale",
     substitute_type(
-        path = "sp_arithmetic::per_things::Perbill",
-        with = "sp_runtime::Perbill"
+        path = "frame_support::dispatch::DispatchInfo",
+        with = "my_mod::DispatchInfo"
     )
 )]
 pub mod node_runtime {}

testing/ui-tests/src/incorrect/substitute_path_not_absolute.stderr

@@ -1,5 +1,5 @@
 error: Type Generation failed: Type substitution error: `substitute_type(with = <path>)` must be a path prefixed with 'crate::' or '::'
  --> src/incorrect/substitute_path_not_absolute.rs:5:16
   |
-5 |         with = "sp_runtime::Perbill"
-  |                ^^^^^^^^^^^^^^^^^^^^^
+5 |         with = "my_mod::DispatchInfo"
+  |                ^^^^^^^^^^^^^^^^^^^^^^