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

Make storage futures only borrow client, not self, for better ergonomics #561

Merged
merged 7 commits into from
Jun 14, 2022

Conversation

jsdw
Copy link
Collaborator

@jsdw jsdw commented Jun 7, 2022

Just a small qol improvement that allows one to get a future back from a storage query and await it later without needing to store any intermediate variables due to unnecessarily large lifetimes being captured by said futures.

Without this you'd need to write:

let staking = api.storage().staking();
let a_fut = staking.bonded(&addr, None);
let b_fut = staking.ledger(&addr, None);
let (a, b) = join!(a_fut, b_fut);

But with it you can chain all the way up to the future like so:

let a_fut = api.storage().staking().bonded(&addr, None);
let b_fut = api.storage().staking().ledger(&addr, None);
let (a, b) = join!(a_fut, b_fut);

Closes #558

Copy link
Member

@niklasad1 niklasad1 left a comment

Choose a reason for hiding this comment

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

👍

@@ -300,21 +315,32 @@ fn generate_storage_entry_fns(

let client_fns = quote! {
#docs_token
pub async fn #fn_name(
pub fn #fn_name(
&self,
#( #key_args, )*
Copy link
Collaborator Author

@jsdw jsdw Jun 10, 2022

Choose a reason for hiding this comment

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

Note to self; key args have a lifetime of 'a, which is the same as the lifetime of the client struct.

Should key args have a lifetime of 'b, and the returned future rely on both 'a and 'b, to avoid any unnecessary restrictions on how the future is used in conjunction with these references?

It would be good to test and see whether a single bound is overly restrictive (as well as to test that we can create and later use futures so that we confirm it's all good in one place.

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 did a little testing and it seems OK to use the 'a lifetime for key_args and client; from reading around I think Rust will pick the correct smallest lifetime that all params with 'a use anyway, so no need to add other lifetimes.

@jsdw jsdw requested review from a team and removed request for a team June 13, 2022 09:27
jsdw and others added 5 commits June 13, 2022 14:32
Capitalise comment

Co-authored-by: Tarik Gul <47201679+TarikGul@users.noreply.github.com>
Capitalise comment

Co-authored-by: Tarik Gul <47201679+TarikGul@users.noreply.github.com>
@jsdw jsdw merged commit 7ba95c2 into master Jun 14, 2022
@jsdw jsdw deleted the jsdw-storage-futs branch June 14, 2022 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

futures::join! two Futures from api
3 participants