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

Storage: Support iterating over NMaps with partial keys #1079

Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
ddd2c2d
implement partial key iters
tadeohepperle Jul 21, 2023
52a01d2
Merge branch 'master' into tadeo-hepperle-support-iterating-over-n-ma…
tadeohepperle Jul 21, 2023
dae5bf9
format
tadeohepperle Jul 21, 2023
e8ad7c1
make tests compile
tadeohepperle Jul 21, 2023
198be19
fix docs and try example
tadeohepperle Jul 25, 2023
a2099bb
Merge branch 'master' into tadeo-hepperle-support-iterating-over-n-ma…
tadeohepperle Jul 25, 2023
e5af576
codegen: Fetch and decode metadata version then fallback (#1092)
lexnv Jul 25, 2023
5e95250
Bump darling from 0.20.1 to 0.20.3 (#1085)
dependabot[bot] Jul 25, 2023
86af7ad
Bump either from 1.8.1 to 1.9.0 (#1084)
dependabot[bot] Jul 25, 2023
57c5eb5
Bump clap from 4.3.11 to 4.3.19 (#1083)
dependabot[bot] Jul 25, 2023
2e8f684
Bump trybuild from 1.0.81 to 1.0.82 (#1082)
dependabot[bot] Jul 25, 2023
8da983c
Prep for 0.30.1 release (#1094)
jsdw Jul 25, 2023
0386a13
Set minimum supported `rust-version` to `1.70` (#1097)
ascjones Jul 27, 2023
a0be58b
Bump serde_json from 1.0.103 to 1.0.104 (#1100)
dependabot[bot] Jul 31, 2023
b2cd7ab
Bump serde from 1.0.175 to 1.0.179 (#1101)
dependabot[bot] Jul 31, 2023
2309e3a
Tests: support 'substrate-node' too and allow multiple binary paths (…
jsdw Jul 31, 2023
3313077
adjust book
tadeohepperle Aug 2, 2023
fc7bb8f
remove the partial iteration example. there was nothing good to show
tadeohepperle Aug 2, 2023
2c68b43
Merge branch 'master' into tadeo-hepperle-support-iterating-over-n-ma…
tadeohepperle Aug 2, 2023
d63002b
Merge branch 'master' into tadeo-hepperle-support-iterating-over-n-ma…
tadeohepperle Aug 7, 2023
3d1d877
Merge branch 'master' into tadeo-hepperle-support-iterating-over-n-ma…
tadeohepperle Aug 8, 2023
cab5e1e
revert spaces in changelog
tadeohepperle Aug 9, 2023
6ecd755
Support more types in Storage entry constructors (#1105)
tadeohepperle Aug 9, 2023
66b529c
Update subxt/src/book/usage/storage.rs
tadeohepperle Aug 9, 2023
95e9aea
remove dynamic_iter
tadeohepperle Aug 9, 2023
03857ca
Merge branch 'master' into tadeo-hepperle-support-iterating-over-n-ma…
tadeohepperle Aug 10, 2023
ee21534
fix example
tadeohepperle Aug 11, 2023
95293dc
format
tadeohepperle Aug 11, 2023
830ffa3
add example, adjust book
tadeohepperle Aug 11, 2023
57df713
Merge branch 'master' into tadeo-hepperle-support-iterating-over-n-ma…
tadeohepperle Aug 11, 2023
4d5abe6
Update subxt/src/book/usage/storage.rs
tadeohepperle Aug 11, 2023
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
149 changes: 54 additions & 95 deletions codegen/src/api/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
// This file is dual-licensed as Apache-2.0 or GPL-3.0.
// see LICENSE for license details.

use crate::types::TypePath;
use crate::{types::TypeGenerator, CratePath};
use heck::ToSnakeCase as _;
use proc_macro2::TokenStream as TokenStream2;
use proc_macro2::{Ident, TokenStream as TokenStream2};
use quote::{format_ident, quote};
use scale_info::TypeDef;
use subxt_metadata::{
Expand Down Expand Up @@ -61,147 +62,105 @@ fn generate_storage_entry_fns(
crate_path: &CratePath,
should_gen_docs: bool,
) -> Result<TokenStream2, CodegenError> {
let (fields, key_impl) = match storage_entry.entry_type() {
StorageEntryType::Plain(_) => (vec![], quote!(vec![])),
let keys: Vec<(Ident, TypePath)> = match storage_entry.entry_type() {
StorageEntryType::Plain(_) => vec![],
StorageEntryType::Map { key_ty, .. } => {
match &type_gen.resolve_type(*key_ty).type_def {
// An N-map; return each of the keys separately.
TypeDef::Tuple(tuple) => {
let fields = tuple
.fields
.iter()
.enumerate()
.map(|(i, f)| {
let field_name = format_ident!("_{}", syn::Index::from(i));
let field_type = type_gen.resolve_type_path(f.id);
(field_name, field_type)
})
.collect::<Vec<_>>();

let keys = fields
.iter()
.map(|(field_name, _)| {
quote!( #crate_path::storage::address::make_static_storage_map_key(#field_name.borrow()) )
});
let key_impl = quote! {
vec![ #( #keys ),* ]
};

(fields, key_impl)
}
TypeDef::Tuple(tuple) => tuple
.fields
.iter()
.enumerate()
.map(|(i, f)| {
let ident: Ident = format_ident!("_{}", syn::Index::from(i));
jsdw marked this conversation as resolved.
Show resolved Hide resolved
let ty_path = type_gen.resolve_type_path(f.id);
(ident, ty_path)
})
.collect::<Vec<_>>(),
// A map with a single key; return the single key.
_ => {
let ident = format_ident!("_0");
let ty_path = type_gen.resolve_type_path(*key_ty);
let fields = vec![(format_ident!("_0"), ty_path)];
let key_impl = quote! {
vec![ #crate_path::storage::address::make_static_storage_map_key(_0.borrow()) ]
};
(fields, key_impl)
vec![(ident, ty_path)]
}
}
}
};

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()));
};

let fn_name = format_ident!("{}", storage_entry.name().to_snake_case());
let snake_case_name = storage_entry.name().to_snake_case();
let storage_entry_ty = match storage_entry.entry_type() {
StorageEntryType::Plain(ty) => *ty,
StorageEntryType::Map { value_ty, .. } => *value_ty,
};
let storage_entry_value_ty = type_gen.resolve_type_path(storage_entry_ty);

let docs = storage_entry.docs();
let docs = should_gen_docs
.then_some(quote! { #( #[doc = #docs ] )* })
.unwrap_or_default();

let key_args = fields.iter().map(|(field_name, field_type)| {
// The field type is translated from `std::vec::Vec<T>` to `[T]`. We apply
// Borrow to all types, so this just makes it a little more ergonomic.
//
// TODO [jsdw]: Support mappings like `String -> str` too for better borrow
// ergonomics.
let field_ty = match field_type.vec_type_param() {
Some(ty) => quote!([#ty]),
_ => quote!(#field_type),
};
quote!( #field_name: impl ::std::borrow::Borrow<#field_ty> )
});

let is_map_type = matches!(storage_entry.entry_type(), StorageEntryType::Map { .. });

// Is the entry iterable?
let is_iterable_type = if is_map_type {
quote!(#crate_path::storage::address::Yes)
} else {
quote!(())
};

let has_default_value = match storage_entry.modifier() {
StorageEntryModifier::Default => true,
StorageEntryModifier::Optional => false,
let is_defaultable_type = match storage_entry.modifier() {
StorageEntryModifier::Default => quote!(#crate_path::storage::address::Yes),
StorageEntryModifier::Optional => quote!(()),
};

// Does the entry have a default value?
let is_defaultable_type = if has_default_value {
quote!(#crate_path::storage::address::Yes)
} else {
quote!(())
};
let all_fns = (0..=keys.len()).map(|n_keys| {
let keys_slice = &keys[..n_keys];
let (fn_name, is_fetchable, is_iterable) = if n_keys == keys.len() {
let fn_name = format_ident!("{snake_case_name}");
(fn_name, true, false)
} else {
let fn_name = if n_keys == 0 {
format_ident!("{snake_case_name}_iter")
} else {
format_ident!("{snake_case_name}_iter{}", n_keys)
};
(fn_name, false, true)
};
let is_fetchable_type = is_fetchable.then_some(quote!(#crate_path::storage::address::Yes)).unwrap_or(quote!(()));
let is_iterable_type = is_iterable.then_some(quote!(#crate_path::storage::address::Yes)).unwrap_or(quote!(()));
Comment on lines +113 to +125
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: I found this a bit hard to follow, maybe a match would state the intent a bit better.
And since the is_fetchable and is_iterable are mutually exclusive, we could probably do with just one of them, up to you

Copy link
Collaborator

@jsdw jsdw Aug 11, 2023

Choose a reason for hiding this comment

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

I had a go and came up with something like this

let is_iterator = n_keys != keys.len();

let fn_name = match (is_iterator, keys.len()) {
    (false, _) => format_ident!("{snake_case_name}"),
    (true, 0) => format_ident!("{snake_case_name}_iter"),
    (true, n) => format_ident!("{snake_case_name}_iter{}", n)
};

let yes_ty = || quote!(#crate_path::storage::address::Yes);
let no_ty = || quote!(());

let is_fetchable_type = is_iterator.then(no_ty).unwrap_or_else(yes_ty);
let is_iterable_type = is_iterator.then(yes_ty).unwrap_or_else(no_ty);

let key_impls = keys_slice.iter().map(|(field_name, _)| quote!( #crate_path::storage::address::make_static_storage_map_key(#field_name.borrow()) ));
let key_args = keys_slice.iter().map(|(field_name, field_type)| {
// The field type is translated from `std::vec::Vec<T>` to `[T]`. We apply
// Borrow to all types, so this just makes it a little more ergonomic.
// TODO [jsdw]: Support mappings like `String -> str` too for better borrow
// ergonomics.
let field_ty = match field_type.vec_type_param() {
Some(ty) => quote!([#ty]),
_ => quote!(#field_type),
};
quote!( #field_name: impl ::std::borrow::Borrow<#field_ty> )
});

// If the item is a map, we want a way to access the root entry to do things like iterate over it,
// 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!(
#docs
pub fn #fn_name_root(
pub fn #fn_name(
&self,
#(#key_args,)*
) -> #crate_path::storage::address::Address::<
#crate_path::storage::address::StaticStorageMapKey,
#storage_entry_value_ty,
(),
#is_fetchable_type,
#is_defaultable_type,
#is_iterable_type
> {
#crate_path::storage::address::Address::new_static(
#pallet_name,
#storage_name,
Vec::new(),
vec![#(#key_impls,)*],
[#(#storage_hash,)*]
)
}
)
} else {
quote!()
};
});

Ok(quote! {
// Access a specific value from a storage entry
#docs
pub fn #fn_name(
&self,
#( #key_args, )*
) -> #crate_path::storage::address::Address::<
#crate_path::storage::address::StaticStorageMapKey,
#storage_entry_value_ty,
#crate_path::storage::address::Yes,
#is_defaultable_type,
#is_iterable_type
> {
#crate_path::storage::address::Address::new_static(
#pallet_name,
#storage_name,
#key_impl,
[#(#storage_hash,)*]
)
}
#( #all_fns

#root_entry_fn
)*
})
}
2 changes: 1 addition & 1 deletion subxt/examples/storage_iterating.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
let api = OnlineClient::<PolkadotConfig>::new().await?;

// Build a storage query to iterate over account information.
let storage_query = polkadot::storage().system().account_root();
let storage_query = polkadot::storage().system().account_iter();

// Get back an iterator of results (here, we are fetching 10 items at
// a time from the node, but we always iterate over one at a time).
Expand Down
2 changes: 1 addition & 1 deletion subxt/examples/storage_iterating_dynamic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
let api = OnlineClient::<PolkadotConfig>::new().await?;

// Build a dynamic storage query to iterate account information.
let storage_query = subxt::dynamic::storage_root("System", "Account");
let storage_query = subxt::dynamic::storage_iter("System", "Account");

// Use that query to return an iterator over the results.
let mut results = api
Expand Down
46 changes: 46 additions & 0 deletions subxt/examples/storage_iterating_partial.rs
jsdw marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if this partial example should (also) be part of the integration-tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried that but had some issues. Let me see again, it is a good idea.

Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
use subxt::{OnlineClient, PolkadotConfig};
use subxt::utils::AccountId32;
use subxt_signer::sr25519::dev;

#[subxt::subxt(runtime_metadata_path = "../artifacts/polkadot_metadata_full.scale")]
pub mod polkadot {}


#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
// Create a new API client, configured to talk to Polkadot nodes.
let api = OnlineClient::<PolkadotConfig>::new().await?;

let alice: AccountId32 = dev::alice().public_key().into();

// alice sends two note_preimage requests to the chain:
let proposal_1 = polkadot::tx().preimage().note_preimage(vec![0,1,2]);
let proposal_2 = polkadot::tx().preimage().note_preimage(vec![0,1,2,3]);
for proposal in [proposal_1, proposal_2]{
api
.tx()
.sign_and_submit_then_watch_default(&proposal, &dev::alice())
.await?
.wait_for_finalized_success()
.await?;
}


// NOT WORKING YET! WORK IN PROGRESS!

// check with partial key iteration that the proposals are saved:
// let storage_query = polkadot::storage().preimage().preimage_for_iter();
// let mut results = api
// .storage()
// .at_latest()
// .await?
// .iter(storage_query, 10)
// .await?;
//
// while let Some((key, value)) = results.next().await? {
// println!("Key: 0x{}", hex::encode(&key));
// println!("Value: {:?}", value);
// }

Ok(())
}
4 changes: 2 additions & 2 deletions subxt/src/book/usage/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@
//! pub mod polkadot {}
//!
//! // A static query capable of iterating over accounts:
//! let storage_query = polkadot::storage().system().account_root();
//! let storage_query = polkadot::storage().system().account_iter();
//! // A dynamic query to do the same:
//! let storage_query = subxt::dynamic::storage_root("System", "Account");
//! let storage_query = subxt::dynamic::storage_iter("System", "Account");
//! ```
//!
//! All valid storage queries implement [`crate::storage::StorageAddress`]. As well as describing
Expand Down
2 changes: 1 addition & 1 deletion subxt/src/dynamic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ pub use crate::tx::dynamic as tx;
pub use crate::constants::dynamic as constant;

// Lookup storage values dynamically.
pub use crate::storage::{dynamic as storage, dynamic_root as storage_root};
pub use crate::storage::{dynamic as storage, dynamic_iter as storage_iter};

// Execute runtime API function call dynamically.
pub use crate::runtime_api::dynamic as runtime_api_call;
Expand Down
4 changes: 2 additions & 2 deletions subxt/src/storage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ pub use crate::rpc::types::StorageKey;
/// entry lives and how to properly decode it.
pub mod address {
pub use super::storage_address::{
dynamic, dynamic_root, make_static_storage_map_key, Address, DynamicAddress,
dynamic, dynamic_iter, make_static_storage_map_key, Address, DynamicAddress,
StaticStorageMapKey, StorageAddress, Yes,
};
}

// For consistency with other modules, also expose
// the basic address stuff at the root of the module.
pub use storage_address::{dynamic, dynamic_root, Address, DynamicAddress, StorageAddress};
pub use storage_address::{dynamic, dynamic_iter, Address, DynamicAddress, StorageAddress};
4 changes: 2 additions & 2 deletions subxt/src/storage/storage_address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ pub trait StorageAddress {
pub struct Yes;

/// A concrete storage address. This can be created from static values (ie those generated
/// via the `subxt` macro) or dynamic values via [`dynamic`] and [`dynamic_root`].
/// via the `subxt` macro) or dynamic values via [`dynamic`] and [`dynamic_iter`].
pub struct Address<StorageKey, ReturnTy, Fetchable, Defaultable, Iterable> {
pallet_name: Cow<'static, str>,
entry_name: Cow<'static, str>,
Expand Down Expand Up @@ -229,7 +229,7 @@ pub fn make_static_storage_map_key<T: codec::Encode>(t: T) -> StaticStorageMapKe
}

/// Construct a new dynamic storage lookup to the root of some entry.
pub fn dynamic_root(
pub fn dynamic_iter(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, but now the dynamic_iter doesn't actually provide the ability to iterate over subsets of keys like the static one does.

Maybe we should just remove this function entirely, since people can just use dynamic below with an empty vec to achieve the same, or with a subset of keys to hopefully be able to iterate whatever they like.

Copy link
Collaborator

Choose a reason for hiding this comment

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

(and if we remove this, remember to update the guide :)

pallet_name: impl Into<String>,
entry_name: impl Into<String>,
) -> DynamicAddress<Value> {
Expand Down
2 changes: 1 addition & 1 deletion subxt/src/storage/storage_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ where
/// let api = OnlineClient::<PolkadotConfig>::new().await.unwrap();
///
/// // Address to the root of a storage entry that we'd like to iterate over.
/// let address = polkadot::storage().xcm_pallet().version_notifiers_root();
/// let address = polkadot::storage().xcm_pallet().version_notifiers_iter();
///
/// // Iterate over keys and values at that address.
/// let mut iter = api
Expand Down
6 changes: 3 additions & 3 deletions testing/integration-tests/src/full_client/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ async fn fetch_keys() {
let ctx = test_context().await;
let api = ctx.client();

let addr = node_runtime::storage().system().account_root();
let addr = node_runtime::storage().system().account_iter();
let keys = api
.storage()
.at_latest()
Expand All @@ -124,7 +124,7 @@ async fn test_iter() {
let ctx = test_context().await;
let api = ctx.client();

let addr = node_runtime::storage().system().account_root();
let addr = node_runtime::storage().system().account_iter();
let mut iter = api
.storage()
.at_latest()
Expand Down Expand Up @@ -409,7 +409,7 @@ async fn unsigned_extrinsic_is_same_shape_as_polkadotjs() {
let expected_tx_bytes = hex::decode(
"b004060700d43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d0f0090c04bb6db2b"
)
.unwrap();
.unwrap();

// Make sure our encoding is the same as the encoding polkadot UI created.
assert_eq!(actual_tx_bytes, expected_tx_bytes);
Expand Down
2 changes: 1 addition & 1 deletion testing/integration-tests/src/full_client/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
/// Generate by running this at the root of the repository:
///
/// ```
/// cargo run --bin subxt -- codegen --file artifacts/polkadot_metadata_full.scale | rustfmt > testing/integration-tests/src/codegen/polkadot.rs
/// cargo run --bin subxt -- codegen --file artifacts/polkadot_metadata_full.scale | rustfmt > testing/integration-tests/src/full_client/codegen/polkadot.rs
/// ```
#[rustfmt::skip]
#[allow(clippy::all)]
Expand Down
Loading
Loading