Skip to content

Remove codec::Encode and codec::Decode derives from generated APIs by default #2008

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

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
9 changes: 4 additions & 5 deletions cli/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ impl FileOrUrl {
Ok(bytes)
}
(Some(PathOrStdIn::StdIn), None, None) => {
let res = std::io::stdin().bytes().collect::<Result<Vec<u8>, _>>();
let reader = std::io::BufReader::new(std::io::stdin());
let res = reader.bytes().collect::<Result<Vec<u8>, _>>();

match res {
Ok(bytes) => Ok(bytes),
Expand Down Expand Up @@ -191,13 +192,11 @@ fn time_based_seed() -> u64 {

pub fn first_paragraph_of_docs(docs: &[String]) -> String {
// take at most the first paragraph of documentation, such that it does not get too long.
let docs_str = docs
.iter()
docs.iter()
.map(|e| e.trim())
.take_while(|e| !e.is_empty())
.collect::<Vec<_>>()
.join("\n");
docs_str
.join("\n")
}

pub trait Indent: ToString {
Expand Down
3 changes: 3 additions & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# result_large_err lint complains if error variant is 128 bytes or more by default.
# Our error is. Let's up this limit a bit for now to avoid lots of warnings.
large-error-threshold = 256
98 changes: 7 additions & 91 deletions codegen/src/api/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use heck::{ToSnakeCase as _, ToUpperCamelCase};
use proc_macro2::{Ident, TokenStream as TokenStream2, TokenStream};
use quote::{format_ident, quote};
use scale_info::TypeDef;
use scale_typegen::{TypeGenerator, typegen::type_path::TypePath};
use scale_typegen::TypeGenerator;
use subxt_metadata::{
PalletMetadata, StorageEntryMetadata, StorageEntryModifier, StorageEntryType, StorageHasher,
};
Expand Down Expand Up @@ -92,7 +92,7 @@ fn generate_storage_entry_fns(
.expect("type is in metadata; qed");

let alias_name = format_ident!("Param{}", idx);
let alias_type = primitive_type_alias(&ty_path, type_gen.settings());
let alias_type = ty_path.to_token_stream(type_gen.settings());

let alias_type_def = quote!( pub type #alias_name = #alias_type; );
let alias_type_path = quote!( types::#alias_module_name::#alias_name );
Expand Down Expand Up @@ -206,7 +206,7 @@ fn generate_storage_entry_fns(
let key = &keys_slice[0];
if key.hasher.ends_with_key() {
let arg = &key.arg_name;
let keys = quote!(#static_storage_key::new(#arg.borrow()));
let keys = quote!(#static_storage_key::new(#arg));
Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't use references anymore because we need the self of the key to work with EncodeAsType and DecodeAsType?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The basic reason was just because previously we would SCALE encode the key before doing anything with it, but now we want to avoid SCALE encoding and so we keep the key itself, requiring us to have ownership of the key.

It might be that we can allow the stored key to be a reference as well or something like that, because we only need it for long enough to encode it given actual metadata!

let path = &key.alias_type_path;
let path = quote!(#static_storage_key<#path>);
(keys, path)
Expand All @@ -220,7 +220,7 @@ fn generate_storage_entry_fns(
arg_name, hasher, ..
}| {
if hasher.ends_with_key() {
quote!( #static_storage_key::new(#arg_name.borrow()) )
quote!( #static_storage_key::new(#arg_name) )
} else {
quote!(())
}
Expand Down Expand Up @@ -250,7 +250,7 @@ fn generate_storage_entry_fns(
arg_name,
alias_type_path,
..
}| quote!( #arg_name: impl ::core::borrow::Borrow<#alias_type_path> ),
}| quote!( #arg_name: #alias_type_path ),
);

quote!(
Expand Down Expand Up @@ -300,34 +300,14 @@ fn generate_storage_entry_fns(
))
}

fn primitive_type_alias(
type_path: &TypePath,
settings: &scale_typegen::TypeGeneratorSettings,
) -> TokenStream {
// Vec<T> is cast to [T]
if let Some(ty) = type_path.vec_type_param() {
let ty = ty.to_token_stream(settings);
return quote!([#ty]);
}
// String is cast to str
if type_path.is_string() {
return quote!(::core::primitive::str);
}
type_path.to_token_stream(settings)
}

#[cfg(test)]
mod tests {
use crate::RuntimeGenerator;
use frame_metadata::v15;
use heck::ToUpperCamelCase;
use quote::{format_ident, quote};
use scale_info::{MetaType, meta_type};

use std::borrow::Cow;

use subxt_metadata::Metadata;

// TODO: Think about adding tests for storage codegen which can use this sort of function.
#[allow(dead_code)]
fn metadata_with_storage_entries(
storage_entries: impl IntoIterator<Item = (&'static str, MetaType)>,
) -> Metadata {
Expand Down Expand Up @@ -387,68 +367,4 @@ mod tests {
.expect("can build valid metadata");
metadata
}

#[test]
fn borrow_type_replacements() {
let storage_entries = [
("vector", meta_type::<Vec<u8>>()),
("boxed", meta_type::<Box<u16>>()),
("string", meta_type::<String>()),
("static_string", meta_type::<&'static str>()),
("cow_string", meta_type::<Cow<'_, str>>()),
];

let expected_borrowed_types = [
quote!([::core::primitive::u8]),
quote!(::core::primitive::u16),
quote!(::core::primitive::str),
quote!(::core::primitive::str),
quote!(::core::primitive::str),
];

let metadata = metadata_with_storage_entries(storage_entries);

let item_mod = syn::parse_quote!(
pub mod api {}
);
let generator = RuntimeGenerator::new(metadata);
let generated = generator
.generate_runtime(
item_mod,
Default::default(),
Default::default(),
syn::parse_str("::subxt_path").unwrap(),
false,
)
.expect("should be able to generate runtime");
let generated_str = generated.to_string();

for ((name, _), expected_type) in storage_entries
.into_iter()
.zip(expected_borrowed_types.into_iter())
{
let name_ident = format_ident!("{}", name);
let expected_storage_constructor = quote!(
fn #name_ident(
&self,
_0: impl ::core::borrow::Borrow<types::#name_ident::Param0>,
)
);
dbg!(&generated_str);
dbg!(&expected_storage_constructor.to_string());
assert!(generated_str.contains(&expected_storage_constructor.to_string()));

let alias_name = format_ident!("{}", name.to_upper_camel_case());
let expected_alias_module = quote!(
pub mod #name_ident {
use super::runtime_types;

pub type #alias_name = ::core::primitive::bool;
pub type Param0 = #expected_type;
}
);

assert!(generated_str.contains(&expected_alias_module.to_string()));
}
}
}
32 changes: 24 additions & 8 deletions codegen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,15 +312,35 @@ fn subxt_type_gen_settings(
crate_path: &syn::Path,
should_gen_docs: bool,
) -> TypeGeneratorSettings {
// If we're using codec::Encode or codec::Decode derives, then we want to
// output #[codec(index = N)] and #[codec(compact)] attrs, else we don't.
let insert_codec_attributes = derives.default_derives().derives().iter().any(|path| {
let mut segments_backwards = path.segments.iter().rev();
let ident = segments_backwards.next();
let module = segments_backwards.next();

let is_ident_match = ident.is_some_and(|s| s.ident == "Encode" || s.ident == "Decode");
let is_module_match = module.is_some_and(|s| s.ident == "codec");

is_ident_match && is_module_match
});

// If we're inserting the codec attributes, we also should use `CompactAs` where necessary.
let compact_as_type_path = if insert_codec_attributes {
Some(parse_quote!(#crate_path::ext::codec::CompactAs))
} else {
None
};

TypeGeneratorSettings {
types_mod_ident: parse_quote!(runtime_types),
should_gen_docs,
derives,
substitutes,
decoded_bits_type_path: Some(parse_quote!(#crate_path::utils::bits::DecodedBits)),
compact_as_type_path: Some(parse_quote!(#crate_path::ext::codec::CompactAs)),
compact_as_type_path,
compact_type_path: Some(parse_quote!(#crate_path::ext::codec::Compact)),
insert_codec_attributes: true,
insert_codec_attributes,
alloc_crate_path: AllocCratePath::Custom(parse_quote!(#crate_path::alloc)),
}
}
Expand All @@ -329,19 +349,15 @@ fn default_derives(crate_path: &syn::Path) -> DerivesRegistry {
let encode_crate_path = quote::quote! { #crate_path::ext::scale_encode }.to_string();
let decode_crate_path = quote::quote! { #crate_path::ext::scale_decode }.to_string();

let derives: [syn::Path; 5] = [
let derives: [syn::Path; 3] = [
parse_quote!(#crate_path::ext::scale_encode::EncodeAsType),
parse_quote!(#crate_path::ext::scale_decode::DecodeAsType),
parse_quote!(#crate_path::ext::codec::Encode),
parse_quote!(#crate_path::ext::codec::Decode),
parse_quote!(Debug),
];

let attributes: [syn::Attribute; 4] = [
let attributes: [syn::Attribute; 2] = [
parse_quote!(#[encode_as_type(crate_path = #encode_crate_path)]),
parse_quote!(#[decode_as_type(crate_path = #decode_crate_path)]),
parse_quote!(#[codec(crate = #crate_path::ext::codec)]),
parse_quote!(#[codec(dumb_trait_bound)]),
];

let mut derives_registry = DerivesRegistry::new();
Expand Down
2 changes: 1 addition & 1 deletion core/src/storage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
//!
//! // Build a storage query to access account information.
//! let account = dev::alice().public_key().into();
//! let address = polkadot::storage().system().account(&account);
//! let address = polkadot::storage().system().account(account);
//!
//! // We can validate that the address is compatible with the given metadata.
//! storage::validate(&address, &metadata).unwrap();
Expand Down
76 changes: 28 additions & 48 deletions core/src/storage/storage_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,10 @@
// see LICENSE for license details.

use super::utils::hash_bytes;
use crate::{
error::{Error, MetadataError, StorageAddressError},
utils::{Encoded, Static},
};
use crate::error::{Error, MetadataError, StorageAddressError};
use alloc::vec;
use alloc::vec::Vec;
use derive_where::derive_where;
use scale_decode::visitor::IgnoreVisitor;
use scale_decode::{DecodeAsType, visitor::IgnoreVisitor};
use scale_encode::EncodeAsType;
use scale_info::{PortableRegistry, TypeDef};
use scale_value::Value;
Expand Down Expand Up @@ -164,49 +160,35 @@ impl StorageKey for () {
}
}

/// A storage key for static encoded values.
/// The original value is only present at construction, but can be decoded from the contained bytes.
#[derive_where(Clone, Debug, PartialOrd, PartialEq, Eq)]
pub struct StaticStorageKey<K: ?Sized> {
bytes: Static<Encoded>,
_marker: core::marker::PhantomData<K>,
}

impl<K: codec::Encode + ?Sized> StaticStorageKey<K> {
/// Creates a new static storage key
pub fn new(key: &K) -> Self {
StaticStorageKey {
bytes: Static(Encoded(key.encode())),
_marker: core::marker::PhantomData,
}
}
/// A storage key used as part of the static codegen.
#[derive(Clone, Debug, PartialOrd, PartialEq, Eq)]
pub struct StaticStorageKey<K> {
key: K,
}

impl<K: codec::Decode> StaticStorageKey<K> {
/// Decodes the encoded inner bytes into the type `K`.
pub fn decoded(&self) -> Result<K, Error> {
let decoded = K::decode(&mut self.bytes())?;
Ok(decoded)
impl<K> StaticStorageKey<K> {
/// Creates a new static storage key.
pub fn new(key: K) -> Self {
StaticStorageKey { key }
}
}

impl<K: ?Sized> StaticStorageKey<K> {
/// Returns the scale-encoded bytes that make up this key
pub fn bytes(&self) -> &[u8] {
&self.bytes.0.0
impl<K: Clone> StaticStorageKey<K> {
/// Returns the decoded storage key.
pub fn into_key(self) -> K {
self.key
}
}

// Note: The ?Sized bound is necessary to support e.g. `StorageKey<[u8]>`.
impl<K: ?Sized> StorageKey for StaticStorageKey<K> {
impl<K: EncodeAsType + DecodeAsType> StorageKey for StaticStorageKey<K> {
fn encode_storage_key(
&self,
bytes: &mut Vec<u8>,
hashers: &mut StorageHashersIter,
types: &PortableRegistry,
) -> Result<(), Error> {
let (hasher, ty_id) = hashers.next_or_err()?;
let encoded_value = self.bytes.encode_as_type(ty_id, types)?;
let encoded_value = self.key.encode_as_type(ty_id, types)?;
hash_bytes(&encoded_value, hasher, bytes);
Ok(())
}
Expand All @@ -227,11 +209,9 @@ impl<K: ?Sized> StorageKey for StaticStorageKey<K> {
return Err(StorageAddressError::HasherCannotReconstructKey { ty_id, hasher }.into());
};

// Return the key bytes.
let key = StaticStorageKey {
bytes: Static(Encoded(key_bytes.to_vec())),
_marker: core::marker::PhantomData::<K>,
};
// Decode and return the key.
let key = K::decode_as_type(&mut &*key_bytes, ty_id, types)?;
let key = StaticStorageKey { key };
Ok(key)
}
}
Expand Down Expand Up @@ -462,16 +442,16 @@ mod tests {
T4C::decode_storage_key(&mut &bytes[..], &mut hashers.iter(), &types)
.unwrap();

assert_eq!(keys_a.1.decoded().unwrap(), 13);
assert_eq!(keys_b.1.0.decoded().unwrap(), 13);
assert_eq!(keys_c.0.1.decoded().unwrap(), 13);
assert_eq!(keys_a.1.into_key(), 13);
assert_eq!(keys_b.1.0.into_key(), 13);
assert_eq!(keys_c.0.1.into_key(), 13);

assert_eq!(keys_a.2.decoded().unwrap(), "Hello");
assert_eq!(keys_b.1.1.decoded().unwrap(), "Hello");
assert_eq!(keys_c.1.0.decoded().unwrap(), "Hello");
assert_eq!(keys_a.3.decoded().unwrap(), era);
assert_eq!(keys_b.2.decoded().unwrap(), era);
assert_eq!(keys_c.1.1.decoded().unwrap(), era);
assert_eq!(keys_a.2.into_key(), "Hello");
assert_eq!(keys_b.1.1.into_key(), "Hello");
assert_eq!(keys_c.1.0.into_key(), "Hello");
assert_eq!(keys_a.3.into_key(), era);
assert_eq!(keys_b.2.into_key(), era);
assert_eq!(keys_c.1.1.into_key(), era);
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions lightclient/src/platform/wasm_platform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,7 @@ impl PlatformRef for SubxtPlatform {

let socket_future = async move {
tracing::debug!(target: LOG_TARGET, "Connecting to addr={addr}");
WasmSocket::new(addr.as_str())
.map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err.to_string()))
WasmSocket::new(addr.as_str()).map_err(|err| std::io::Error::other(err.to_string()))
};

future::ready(super::wasm_helpers::Stream(with_buffers::WithBuffers::new(
Expand Down
Loading
Loading