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

added at_latest #900

Merged
merged 3 commits into from
Apr 11, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
4 changes: 2 additions & 2 deletions examples/examples/concurrent_storage_requests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {

// For storage requests, we can join futures together to
// await multiple futures concurrently:
let a_fut = api.storage().at(None).await?.fetch(&staking_bonded);
let b_fut = api.storage().at(None).await?.fetch(&staking_ledger);
let a_fut = api.storage().at_latest().await?.fetch(&staking_bonded);
let b_fut = api.storage().at_latest().await?.fetch(&staking_ledger);
let (a, b) = join!(a_fut, b_fut);

println!("{a:?}, {b:?}");
Expand Down
4 changes: 2 additions & 2 deletions examples/examples/dynamic_queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
);
let account = api
.storage()
.at(None)
.at_latest()
.await?
.fetch_or_default(&storage_address)
.await?
Expand All @@ -73,7 +73,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
let storage_address = subxt::dynamic::storage_root("System", "Account");
let mut iter = api
.storage()
.at(None)
.at_latest()
.await?
.iter(storage_address, 10)
.await?;
Expand Down
2 changes: 1 addition & 1 deletion examples/examples/fetch_all_accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {

let address = polkadot::storage().system().account_root();

let mut iter = api.storage().at(None).await?.iter(address, 10).await?;
let mut iter = api.storage().at_latest().await?.iter(address, 10).await?;

while let Some((key, account)) = iter.next().await? {
println!("{}: {}", hex::encode(key), account.data.free);
Expand Down
6 changes: 3 additions & 3 deletions examples/examples/fetch_staking_details.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
let active_era_addr = polkadot::storage().staking().active_era();
let era = api
.storage()
.at(None)
.at_latest()
.await?
.fetch(&active_era_addr)
.await?
Expand All @@ -51,7 +51,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
let controller_acc_addr = polkadot::storage().staking().bonded(&alice_stash_id);
let controller_acc = api
.storage()
.at(None)
.at_latest()
.await?
.fetch(&controller_acc_addr)
.await?
Expand All @@ -61,7 +61,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
let era_reward_addr = polkadot::storage().staking().eras_reward_points(era.index);
let era_result = api
.storage()
.at(None)
.at_latest()
.await?
.fetch(&era_reward_addr)
.await?;
Expand Down
10 changes: 5 additions & 5 deletions examples/examples/storage_iterating.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
{
let key_addr = polkadot::storage().xcm_pallet().version_notifiers_root();

let mut iter = api.storage().at(None).await?.iter(key_addr, 10).await?;
let mut iter = api.storage().at_latest().await?.iter(key_addr, 10).await?;

println!("\nExample 1. Obtained keys:");
while let Some((key, value)) = iter.next().await? {
Expand All @@ -45,7 +45,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
// Fetch at most 10 keys from below the prefix XcmPallet' VersionNotifiers.
let keys = api
.storage()
.at(None)
.at_latest()
.await?
.fetch_keys(&key_addr.to_root_bytes(), 10, None)
.await?;
Expand All @@ -54,7 +54,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
for key in keys.iter() {
println!("Key: 0x{}", hex::encode(key));

if let Some(storage_data) = api.storage().at(None).await?.fetch_raw(&key.0).await? {
if let Some(storage_data) = api.storage().at_latest().await?.fetch_raw(&key.0).await? {
// We know the return value to be `QueryId` (`u64`) from inspecting either:
// - polkadot code
// - polkadot.rs generated file under `version_notifiers()` fn
Expand Down Expand Up @@ -85,7 +85,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {

let keys = api
.storage()
.at(None)
.at_latest()
.await?
.fetch_keys(&query_key, 10, None)
.await?;
Expand All @@ -94,7 +94,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
for key in keys.iter() {
println!("Key: 0x{}", hex::encode(key));

if let Some(storage_data) = api.storage().at(None).await?.fetch_raw(&key.0).await? {
if let Some(storage_data) = api.storage().at_latest().await?.fetch_raw(&key.0).await? {
// We know the return value to be `QueryId` (`u64`) from inspecting either:
// - polkadot code
// - polkadot.rs generated file under `version_notifiers()` fn
Expand Down
2 changes: 1 addition & 1 deletion subxt/src/blocks/block_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ where
Some(events) => events.clone(),
None => {
events::EventsClient::new(client.clone())
.at(Some(block_hash))
.at(block_hash)
.await?
}
};
Expand Down
30 changes: 30 additions & 0 deletions subxt/src/blocks/blocks_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,36 @@ where
/// runtime upgrade. You can attempt to retrieve older blocks,
/// but may run into errors attempting to work with them.
pub fn at(
&self,
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could also change the documentation of this function to state /// Obtain block details given the block hash.

block_hash: T::Hash,
) -> impl Future<Output = Result<Block<T, Client>, Error>> + Send + 'static {
self.at_optional_block_hash(Some(block_hash))
}

/// Obtain block details given the provided block hash, or the latest block if `None` is
/// provided.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/// Obtain block details given the provided block hash, or the latest block if `None` is
/// provided.
/// Obtain block details of the latest block hash.

///
/// # Warning
///
/// This call only supports blocks produced since the most recent
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can remove this warning from the documentation since we are always operating with the latest block.
And therefore, users have no way to utilize an older and incompatible block here.

/// runtime upgrade. You can attempt to retrieve older blocks,
/// but may run into errors attempting to work with them.
pub fn at_latest(
&self,
) -> impl Future<Output = Result<Block<T, Client>, Error>> + Send + 'static {
self.at_optional_block_hash(None)
}

/// Obtain block details given the provided block hash, or the latest block if `None` is
/// provided.
///
/// # Warning
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Since this function is not publically exposed by subxt, we could also remove the warning message.
I believe we placed that warning to explicitly state in the documentation that older blocks may not be supported, just to set up the user expectations.

///
/// This call only supports blocks produced since the most recent
/// runtime upgrade. You can attempt to retrieve older blocks,
/// but may run into errors attempting to work with them.
#[inline]
Copy link
Member

Choose a reason for hiding this comment

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

I would like some consistency here w.r.t. to inline here as this method is marked as "inline" but the wrappers on top of this are not.

@lexnv what do you prefer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@niklasad1 I wanted to keep consistency to how it was before. Before, the pub method was also not inlined. Now the inline will just generate two pub functions at() and at_latest()that are both not inlined (like before).

Copy link
Member

Choose a reason for hiding this comment

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

I see, makes sense but let's hear what @lexnv thinks :)

Copy link
Collaborator

@jsdw jsdw Apr 11, 2023

Choose a reason for hiding this comment

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

I wouldn't have the #[inline] here myself (if anything I'd have it on at() and at_latest() because they are trivially inlineable at the call site and would have a similar effect with less code duplication).

But more to the point, at() no longer needs to be async, so I think that this implementation should be split up and the relevant part of it put in each of at() and at_latest(), so that only the latter needs to be declared async now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah I see; it would have to be async anyway! In that case let';s just remove the #[inline] (if anything it's be best on at() and at_latest() I think) and this looks good to me!

fn at_optional_block_hash(
&self,
block_hash: Option<T::Hash>,
) -> impl Future<Output = Result<Block<T, Client>, Error>> + Send + 'static {
Expand Down
26 changes: 26 additions & 0 deletions subxt/src/events/events_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,32 @@ where
/// runtime upgrade. You can attempt to retrieve events from older blocks,
/// but may run into errors attempting to work with them.
pub fn at(
&self,
block_hash: T::Hash,
) -> impl Future<Output = Result<Events<T>, Error>> + Send + 'static {
self.at_optional_block_hash(Some(block_hash))
}

/// Obtain events at the latest block hash.
///
/// # Warning
///
/// This call only supports blocks produced since the most recent
/// runtime upgrade. You can attempt to retrieve events from older blocks,
/// but may run into errors attempting to work with them.
pub fn at_latest(&self) -> impl Future<Output = Result<Events<T>, Error>> + Send + 'static {
self.at_optional_block_hash(None)
}

/// Obtain events at some block hash.
///
/// # Warning
///
/// This call only supports blocks produced since the most recent
/// runtime upgrade. You can attempt to retrieve events from older blocks,
/// but may run into errors attempting to work with them.
#[inline]
fn at_optional_block_hash(
Copy link
Collaborator

Choose a reason for hiding this comment

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

As above, let's put this code directly into at() and at_latest() so that the former no longer needs to be async, and no need for #[inline] :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah I see; both have to be async anyway actually!

In this case, let's just remove the #[inline] and it's all good :)

&self,
block_hash: Option<T::Hash>,
) -> impl Future<Output = Result<Events<T>, Error>> + Send + 'static {
Expand Down
24 changes: 12 additions & 12 deletions subxt/src/storage/storage_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,24 +71,24 @@ where
Client: OnlineClientT<T>,
{
/// Obtain storage at some block hash.
pub fn at(
pub fn at(&self, block_hash: T::Hash) -> Storage<T, Client> {
Storage::new(self.client.clone(), block_hash)
}

/// Obtain storage at the latest block hash.
pub fn at_latest(
&self,
block_hash: Option<T::Hash>,
) -> impl Future<Output = Result<Storage<T, Client>, Error>> + Send + 'static {
// Clone and pass the client in like this so that we can explicitly
// return a Future that's Send + 'static, rather than tied to &self.
let client = self.client.clone();
async move {
// If block hash is not provided, get the hash
// for the latest block and use that.
let block_hash = match block_hash {
Some(hash) => hash,
None => client
.rpc()
.block_hash(None)
.await?
.expect("didn't pass a block number; qed"),
};
// get the hash for the latest block and use that.
let block_hash = client
.rpc()
.block_hash(None)
.await?
.expect("didn't pass a block number; qed");

Ok(Storage::new(client, block_hash))
}
Expand Down
4 changes: 2 additions & 2 deletions subxt/src/storage/storage_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ where
/// // Fetch just the keys, returning up to 10 keys.
/// let value = api
/// .storage()
/// .at(None)
/// .at_latest()
/// .await
/// .unwrap()
/// .fetch(&address)
Expand Down Expand Up @@ -185,7 +185,7 @@ where
/// // Iterate over keys and values at that address.
/// let mut iter = api
/// .storage()
/// .at(None)
/// .at_latest()
/// .await
/// .unwrap()
/// .iter(address, 10)
Expand Down
2 changes: 1 addition & 1 deletion subxt/src/tx/tx_progress.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ impl<T: Config, C: OnlineClientT<T>> TxInBlock<T, C> {
.ok_or(Error::Transaction(TransactionError::BlockNotFound))?;

let events = EventsClient::new(self.client.clone())
.at(Some(self.block_hash))
.at(self.block_hash)
.await?;

Ok(crate::blocks::ExtrinsicEvents::new(
Expand Down
4 changes: 2 additions & 2 deletions testing/integration-tests/src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ async fn fetch_keys() {
let addr = node_runtime::storage().system().account_root();
let keys = api
.storage()
.at(None)
.at_latest()
.await
.unwrap()
.fetch_keys(&addr.to_root_bytes(), 4, None)
Expand All @@ -128,7 +128,7 @@ async fn test_iter() {
let addr = node_runtime::storage().system().account_root();
let mut iter = api
.storage()
.at(None)
.at_latest()
.await
.unwrap()
.iter(addr, 10)
Expand Down
24 changes: 12 additions & 12 deletions testing/integration-tests/src/frame/balances.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ async fn tx_basic_transfer() -> Result<(), subxt::Error> {

let alice_pre = api
.storage()
.at(None)
.at_latest()
.await?
.fetch_or_default(&alice_account_addr)
.await?;
let bob_pre = api
.storage()
.at(None)
.at_latest()
.await?
.fetch_or_default(&bob_account_addr)
.await?;
Expand Down Expand Up @@ -64,13 +64,13 @@ async fn tx_basic_transfer() -> Result<(), subxt::Error> {

let alice_post = api
.storage()
.at(None)
.at_latest()
.await?
.fetch_or_default(&alice_account_addr)
.await?;
let bob_post = api
.storage()
.at(None)
.at_latest()
.await?
.fetch_or_default(&bob_account_addr)
.await?;
Expand Down Expand Up @@ -102,13 +102,13 @@ async fn tx_dynamic_transfer() -> Result<(), subxt::Error> {

let alice_pre = api
.storage()
.at(None)
.at_latest()
.await?
.fetch_or_default(&alice_account_addr)
.await?;
let bob_pre = api
.storage()
.at(None)
.at_latest()
.await?
.fetch_or_default(&bob_account_addr)
.await?;
Expand Down Expand Up @@ -152,13 +152,13 @@ async fn tx_dynamic_transfer() -> Result<(), subxt::Error> {

let alice_post = api
.storage()
.at(None)
.at_latest()
.await?
.fetch_or_default(&alice_account_addr)
.await?;
let bob_post = api
.storage()
.at(None)
.at_latest()
.await?
.fetch_or_default(&bob_account_addr)
.await?;
Expand Down Expand Up @@ -211,7 +211,7 @@ async fn multiple_transfers_work_nonce_incremented() -> Result<(), subxt::Error>

let bob_pre = api
.storage()
.at(None)
.at_latest()
.await?
.fetch_or_default(&bob_account_addr)
.await?;
Expand All @@ -231,7 +231,7 @@ async fn multiple_transfers_work_nonce_incremented() -> Result<(), subxt::Error>

let bob_post = api
.storage()
.at(None)
.at_latest()
.await?
.fetch_or_default(&bob_account_addr)
.await?;
Expand All @@ -248,7 +248,7 @@ async fn storage_total_issuance() {
let addr = node_runtime::storage().balances().total_issuance();
let total_issuance = api
.storage()
.at(None)
.at_latest()
.await
.unwrap()
.fetch_or_default(&addr)
Expand Down Expand Up @@ -283,7 +283,7 @@ async fn storage_balance_lock() -> Result<(), subxt::Error> {

let locks = api
.storage()
.at(None)
.at_latest()
.await?
.fetch_or_default(&locks_addr)
.await?;
Expand Down
4 changes: 2 additions & 2 deletions testing/integration-tests/src/frame/contracts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ async fn tx_call() {
let contract_info = cxt
.client()
.storage()
.at(None)
.at_latest()
.await
.unwrap()
.fetch(&info_addr)
Expand All @@ -222,7 +222,7 @@ async fn tx_call() {
let keys = cxt
.client()
.storage()
.at(None)
.at_latest()
.await
.unwrap()
.fetch_keys(&info_addr_bytes, 10, None)
Expand Down
Loading