Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the generated DispatchError instead of the hardcoded Substrate one #394

Merged
merged 23 commits into from
Jan 20, 2022
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
92c2c1f
WIP DispatchError generic param
jsdw Jan 13, 2022
55c1dcb
main crate now compiling with new E generic param for DispatchError
jsdw Jan 13, 2022
2a8f094
Remove E param from RpcClient since it doesn't really need it
jsdw Jan 14, 2022
2f19f9c
Point to generated DispatchError in codegen so no need for additional…
jsdw Jan 14, 2022
5d0a56a
More error hunting; cargo check --all-targets passes now
jsdw Jan 14, 2022
940c770
Use our own RuntimeVersion struct (for now) to avoid error decoding i…
jsdw Jan 14, 2022
8235ddc
cargo fmt
jsdw Jan 14, 2022
8425253
Merge branch 'master' into jsdw-dispatch-error
jsdw Jan 17, 2022
3748160
fix docs and expose private documented thing that I think should be pub
jsdw Jan 17, 2022
63fbde8
move error info to compile time so that we can make DispatchError a l…
jsdw Jan 17, 2022
b6e13bd
cargo fmt
jsdw Jan 17, 2022
a458565
clippy
jsdw Jan 17, 2022
ad38bdb
Rework error handling to remove <E> param in most cases
jsdw Jan 19, 2022
8c7a207
fix Error doc ambiguity (hopefully)
jsdw Jan 19, 2022
6309a1f
doc tweaks
jsdw Jan 19, 2022
f6f27db
Merge branch 'master' into jsdw-dispatch-error
jsdw Jan 19, 2022
19fe205
docs: remove dismbiguation thing that isn't needed now
jsdw Jan 20, 2022
5e6b82f
One more Error<E> that can be a BasicError
jsdw Jan 20, 2022
19df387
rewrite pallet errors thing into normal loops to tidy
jsdw Jan 20, 2022
e1918eb
tidy errors codegen a little
jsdw Jan 20, 2022
d8e40fa
tidy examples/custom_type_derives.rs a little
jsdw Jan 20, 2022
424acb5
cargo fmt
jsdw Jan 20, 2022
d7e5914
silcnce clippy in example
jsdw Jan 20, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions codegen/src/api/calls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ pub fn generate_calls(
pub fn #fn_name(
&self,
#( #call_fn_args, )*
) -> ::subxt::SubmittableExtrinsic<'a, T, E, A, #call_struct_name> {
) -> ::subxt::SubmittableExtrinsic<'a, T, X, A, #call_struct_name, DispatchError> {
let call = #call_struct_name { #( #call_args, )* };
::subxt::SubmittableExtrinsic::new(self.client, call)
}
Expand All @@ -80,17 +80,20 @@ pub fn generate_calls(
quote! {
pub mod calls {
use super::#types_mod_ident;

type DispatchError = #types_mod_ident::sp_runtime::DispatchError;

#( #call_structs )*

pub struct TransactionApi<'a, T: ::subxt::Config, E, A> {
pub struct TransactionApi<'a, T: ::subxt::Config, X, A> {
client: &'a ::subxt::Client<T>,
marker: ::core::marker::PhantomData<(E, A)>,
marker: ::core::marker::PhantomData<(X, A)>,
}

impl<'a, T, E, A> TransactionApi<'a, T, E, A>
impl<'a, T, X, A> TransactionApi<'a, T, X, A>
where
T: ::subxt::Config,
E: ::subxt::SignedExtra<T>,
X: ::subxt::SignedExtra<T>,
A: ::subxt::AccountData<T>,
{
pub fn new(client: &'a ::subxt::Client<T>) -> Self {
Expand Down
171 changes: 171 additions & 0 deletions codegen/src/api/errors.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
// Copyright 2019-2022 Parity Technologies (UK) Ltd.
// This file is part of subxt.
//
// subxt is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// subxt is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with subxt. If not, see <http://www.gnu.org/licenses/>.

use frame_metadata::v14::RuntimeMetadataV14;
use proc_macro2::{
Span as Span2,
TokenStream as TokenStream2,
};
use quote::quote;

pub struct ErrorDetails {
pub type_def: TokenStream2,
pub dispatch_error_impl_fn: TokenStream2,
}

impl ErrorDetails {
fn emit_compile_error(err: &str) -> ErrorDetails {
let err_lit_str = syn::LitStr::new(err, Span2::call_site());
ErrorDetails {
type_def: quote!(),
dispatch_error_impl_fn: quote!(compile_error!(#err_lit_str)),
}
}
}

/// The purpose of this is to enumerate all of the possible `(module_index, error_index)` error
/// variants, so that we can convert `u8` error codes inside a generated `DispatchError` into
/// nicer error strings with documentation. To do this, we emit the type we'll return instances of,
/// and a function that returns such an instance for all of the error codes seen in the metadata.
pub fn generate_error_details(metadata: &RuntimeMetadataV14) -> ErrorDetails {
let errors = match pallet_errors(metadata) {
Ok(errors) => errors,
Err(e) => {
let err_string =
format!("Failed to generate error details from metadata: {}", e);
return ErrorDetails::emit_compile_error(&err_string)
}
};

let match_body_items = errors.into_iter().map(|err| {
let docs = err.description();
let pallet_index = err.pallet_index;
let error_index = err.error_index;
let pallet_name = err.pallet;
let error_name = err.error;

quote! {
(#pallet_index, #error_index) => Some(ErrorDetails {
pallet: #pallet_name,
error: #error_name,
docs: #docs
})
}
});

ErrorDetails {
// A type we'll be returning that needs defining at the top level:
type_def: quote! {
pub struct ErrorDetails {
pub pallet: &'static str,
pub error: &'static str,
pub docs: &'static str,
}
},
// A function which will live in an impl block for our DispatchError,
// to statically return details for known error types:
dispatch_error_impl_fn: quote! {
pub fn details(&self) -> Option<ErrorDetails> {
if let Self::Module { index, error } = self {
match (index, error) {
#( #match_body_items ),*,
_ => None
}
} else {
None
}
}
},
}
}

fn pallet_errors(
metadata: &RuntimeMetadataV14,
) -> Result<Vec<ErrorMetadata>, InvalidMetadataError> {
let get_type_def_variant = |type_id: u32| {
let ty = metadata
.types
.resolve(type_id)
.ok_or(InvalidMetadataError::MissingType(type_id))?;
if let scale_info::TypeDef::Variant(var) = ty.type_def() {
Ok(var)
} else {
Err(InvalidMetadataError::TypeDefNotVariant(type_id))
}
};

let pallet_errors = metadata
.pallets
.iter()
.filter_map(|pallet| {
pallet.error.as_ref().map(|error| {
let type_def_variant = get_type_def_variant(error.ty.id())?;
Ok((pallet, type_def_variant))
})
})
.collect::<Result<Vec<(_, _)>, _>>()?;

let errors = pallet_errors
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can unify these two chains into a single? Like, why collect the pallet_errors first and then build the errors Vec?

Copy link
Collaborator Author

@jsdw jsdw Jan 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be a bit tricky; we end up with Result<_> Items just prior to that collect, so the collecting I think is just there to give a chance to bail if we encounter any errors! I'll see whether I can do it in a clean way!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I settled on standard for loops; it felt tidier:

let mut pallet_errors = vec![];
for pallet in &metadata.pallets {
    let error = match &pallet.error {
        Some(err) => err,
        None => continue
    };

    let type_def_variant = get_type_def_variant(error.ty.id())?;
    for var in type_def_variant.variants().iter() {
        pallet_errors.push(ErrorMetadata {
            pallet_index: pallet.index,
            error_index: var.index(),
            pallet: pallet.name.clone(),
            error: var.name().clone(),
            variant: var.clone(),
        });
    }
}

.iter()
.flat_map(|(pallet, type_def_variant)| {
type_def_variant.variants().iter().map(move |var| {
ErrorMetadata {
pallet_index: pallet.index,
error_index: var.index(),
pallet: pallet.name.clone(),
error: var.name().clone(),
variant: var.clone(),
}
})
})
.collect();

Ok(errors)
}

#[derive(Clone, Debug)]
pub struct ErrorMetadata {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing docs here. Also, what do you think about moving this to the top?

pub pallet_index: u8,
pub error_index: u8,
pub pallet: String,
pub error: String,
variant: scale_info::Variant<scale_info::form::PortableForm>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dq: what is the reason for storing the variant for?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, for the description()?

Copy link
Collaborator Author

@jsdw jsdw Jan 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, that's it! I'm not sure why I did/left this now. I've made it be just the doc string, which simplifies things a bit!

}

impl ErrorMetadata {
pub fn description(&self) -> String {
self.variant.docs().join("\n")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if the node is compiled to strip the docs, this will be empty? Worth detecting and add some kind of default, "No docs available in the metadata"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the empty case docs will just be "", which seems good enough (that can be detected and handled down the line in whichever way things prefer)

}
}

#[derive(Debug)]
pub enum InvalidMetadataError {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing docs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah; doesn't need to be pub :)

MissingType(u32),
TypeDefNotVariant(u32),
}

impl std::fmt::Display for InvalidMetadataError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
InvalidMetadataError::MissingType(n) => {
write!(f, "Type {} missing from type registry", n)
}
InvalidMetadataError::TypeDefNotVariant(n) => {
write!(f, "Type {} was not a variant/enum type", n)
}
}
}
}
38 changes: 26 additions & 12 deletions codegen/src/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
// along with subxt. If not, see <http://www.gnu.org/licenses/>.

mod calls;
mod errors;
mod events;
mod storage;

Expand Down Expand Up @@ -216,6 +217,10 @@ impl RuntimeGenerator {
pallet.calls.as_ref().map(|_| pallet_mod_name)
});

let error_details = errors::generate_error_details(&self.metadata);
let error_type = error_details.type_def;
let error_fn = error_details.dispatch_error_impl_fn;

quote! {
#[allow(dead_code, unused_imports, non_camel_case_types)]
pub mod #mod_ident {
Expand All @@ -227,6 +232,15 @@ impl RuntimeGenerator {
/// constructing a transaction.
pub type DefaultAccountData = self::system::storage::Account;

/// The default error type returned when there is a runtime issue.
pub type DispatchError = self::runtime_types::sp_runtime::DispatchError;

// Statically generate error information so that we don't need runtime metadata for it.
#error_type
impl DispatchError {
#error_fn
}

impl ::subxt::AccountData<::subxt::DefaultConfig> for DefaultAccountData {
fn nonce(result: &<Self as ::subxt::StorageEntry>::Value) -> <::subxt::DefaultConfig as ::subxt::Config>::Index {
result.nonce
Expand All @@ -236,31 +250,31 @@ impl RuntimeGenerator {
}
}

pub struct RuntimeApi<T: ::subxt::Config, E> {
pub struct RuntimeApi<T: ::subxt::Config, X> {
pub client: ::subxt::Client<T>,
marker: ::core::marker::PhantomData<E>,
marker: ::core::marker::PhantomData<X>,
}

impl<T, E> ::core::convert::From<::subxt::Client<T>> for RuntimeApi<T, E>
impl<T, X> ::core::convert::From<::subxt::Client<T>> for RuntimeApi<T, X>
where
T: ::subxt::Config,
E: ::subxt::SignedExtra<T>,
X: ::subxt::SignedExtra<T>,
{
fn from(client: ::subxt::Client<T>) -> Self {
Self { client, marker: ::core::marker::PhantomData }
}
}

impl<'a, T, E> RuntimeApi<T, E>
impl<'a, T, X> RuntimeApi<T, X>
where
T: ::subxt::Config,
E: ::subxt::SignedExtra<T>,
X: ::subxt::SignedExtra<T>,
{
pub fn storage(&'a self) -> StorageApi<'a, T> {
StorageApi { client: &self.client }
}

pub fn tx(&'a self) -> TransactionApi<'a, T, E, DefaultAccountData> {
pub fn tx(&'a self) -> TransactionApi<'a, T, X, DefaultAccountData> {
TransactionApi { client: &self.client, marker: ::core::marker::PhantomData }
}
}
Expand All @@ -280,19 +294,19 @@ impl RuntimeGenerator {
)*
}

pub struct TransactionApi<'a, T: ::subxt::Config, E, A> {
pub struct TransactionApi<'a, T: ::subxt::Config, X, A> {
client: &'a ::subxt::Client<T>,
marker: ::core::marker::PhantomData<(E, A)>,
marker: ::core::marker::PhantomData<(X, A)>,
}

impl<'a, T, E, A> TransactionApi<'a, T, E, A>
impl<'a, T, X, A> TransactionApi<'a, T, X, A>
where
T: ::subxt::Config,
E: ::subxt::SignedExtra<T>,
X: ::subxt::SignedExtra<T>,
A: ::subxt::AccountData<T>,
{
#(
pub fn #pallets_with_calls(&self) -> #pallets_with_calls::calls::TransactionApi<'a, T, E, A> {
pub fn #pallets_with_calls(&self) -> #pallets_with_calls::calls::TransactionApi<'a, T, X, A> {
#pallets_with_calls::calls::TransactionApi::new(self.client)
}
)*
Expand Down
5 changes: 3 additions & 2 deletions codegen/src/api/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ pub fn generate_storage(
quote! {
pub mod storage {
use super::#types_mod_ident;

#( #storage_structs )*

pub struct StorageApi<'a, T: ::subxt::Config> {
Expand Down Expand Up @@ -195,7 +196,7 @@ fn generate_storage_entry_fns(
pub async fn #fn_name_iter(
&self,
hash: ::core::option::Option<T::Hash>,
) -> ::core::result::Result<::subxt::KeyIter<'a, T, #entry_struct_ident>, ::subxt::Error> {
) -> ::core::result::Result<::subxt::KeyIter<'a, T, #entry_struct_ident>, ::subxt::BasicError> {
self.client.storage().iter(hash).await
}
)
Expand All @@ -211,7 +212,7 @@ fn generate_storage_entry_fns(
&self,
#( #key_args, )*
hash: ::core::option::Option<T::Hash>,
) -> ::core::result::Result<#return_ty, ::subxt::Error> {
) -> ::core::result::Result<#return_ty, ::subxt::BasicError> {
let entry = #constructor;
self.client.storage().#fetch(&entry, hash).await
}
Expand Down
1 change: 1 addition & 0 deletions codegen/src/derives.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ impl Default for GeneratedTypeDerives {
let mut derives = Punctuated::new();
derives.push(syn::parse_quote!(::subxt::codec::Encode));
derives.push(syn::parse_quote!(::subxt::codec::Decode));
derives.push(syn::parse_quote!(Debug));
Self::new(derives)
}
}
Expand Down
Loading