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

Add at_latest() to use instead of .at(None) in various places. #899

Closed
jsdw opened this issue Apr 6, 2023 · 1 comment · Fixed by #900
Closed

Add at_latest() to use instead of .at(None) in various places. #899

jsdw opened this issue Apr 6, 2023 · 1 comment · Fixed by #900
Assignees
Labels
ready to implement No significant design questions remain; this can be implemented now

Comments

@jsdw
Copy link
Collaborator

jsdw commented Apr 6, 2023

Currently we have APIs like:

let client = OnlineClient::<PolkadotConfig>::new();

// fetch latest block:
let block = client.blocks().at(None).await?;
// fetch latest events:
let events = client.events().at(None).await?;
// fetch latest storage entries:
let storage = client.storage().at(None).await?;

Fetching the latest block/event/storage seems like a common need, and moreso, we only need the async/await + Result for one of the paths (ie when None is provided). So how about we split into two code paths:

let client = OnlineClient::<PolkadotConfig>::new();

// fetch latest block:
let block = client.blocks().at_latest().await?;
// fetch block at `block_hash`:
let block = client.blocks().at(block_hash); // no more Option

// fetch latest events:
let events = client.events().at_latest().await?;
// fetch events at `block_hash`:
let events = client.events().at(block_hash); // no more Option

// fetch latest storage entries:
let storage = client.storage().at_latest().await?;
// fetch storage at block hash:
let storage = client.storage().at(block_hash); // no more Option

That way, the interface is a bit clearer, and we avoid unnecessary result unwrapping etc where it's not needed (ie somebody could get a block hash via client.blocks().at_latest().await?.hash() and then pass that to subsequent calls and avoid the awaiting/unwrapping.

@jsdw jsdw added the ready to implement No significant design questions remain; this can be implemented now label Apr 6, 2023
@tadeohepperle tadeohepperle self-assigned this Apr 6, 2023
@tadeohepperle
Copy link
Contributor

@jsdw I created a PR for this here: #900
Unfortunately I think the alleviation of async + Result is only possible for the storage_client.at(hash) function, not for the other two, because they still make rpc requests in case the hash is already known. So for the blocks_client and the events_client I just created wrapper functions that make the API simpler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to implement No significant design questions remain; this can be implemented now
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants