Skip to content

Commit

Permalink
fix(alloy-provider): get_block_by_number arg (#1582)
Browse files Browse the repository at this point in the history
<!--
Thank you for your Pull Request. Please provide a description above and review
the requirements below.

Bug fixes and new features should include tests.

Contributors guide: https://github.com/alloy-rs/core/blob/main/CONTRIBUTING.md

The contributors guide includes instructions for running rustfmt and building the
documentation.
-->

<!-- ** Please select "Allow edits from maintainers" in the PR Options ** -->

## Motivation
Different args in `alloy-provider` crate: 
```
    /// Gets a block by its [BlockHash], with full transactions or only hashes.
    async fn get_block_by_hash(
        &self,
        hash: BlockHash,
        kind: BlockTransactionsKind,    <--
    )
    ...
    /// Get a block by its number.
    // TODO: Network associate
    async fn get_block_by_number(
        &self,
        number: BlockNumberOrTag,
        hydrate: bool,                  <--
    )
```
<!--
Explain the context and why you're making that change. What is the problem
you're trying to solve? In some cases there is not a problem and this can be
thought of as being the motivation for your change.
-->

## Solution

<!--
Summarize the solution and provide any necessary context needed to understand
the code change.
-->
Changing `hydrate` argument to `kind` to make methods symmetrical.

## PR Checklist

- [ ] Added Tests
- [ ] Added Documentation
- [ ] Breaking changes
  • Loading branch information
steph-rs authored Oct 29, 2024
1 parent b227b17 commit abb4dd4
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 18 deletions.
4 changes: 2 additions & 2 deletions crates/provider/src/fillers/gas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use alloy_consensus::BlockHeader;
use alloy_eips::eip4844::BLOB_TX_MIN_BLOB_GASPRICE;
use alloy_json_rpc::RpcError;
use alloy_network::{Network, TransactionBuilder, TransactionBuilder4844};
use alloy_network_primitives::BlockResponse;
use alloy_network_primitives::{BlockResponse, BlockTransactionsKind};
use alloy_rpc_types_eth::BlockNumberOrTag;
use alloy_transport::{Transport, TransportResult};
use futures::FutureExt;
Expand Down Expand Up @@ -227,7 +227,7 @@ where
}

provider
.get_block_by_number(BlockNumberOrTag::Latest, false)
.get_block_by_number(BlockNumberOrTag::Latest, BlockTransactionsKind::Hashes)
.await?
.ok_or(RpcError::NullResp)?
.header()
Expand Down
11 changes: 8 additions & 3 deletions crates/provider/src/layers/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,11 +156,16 @@ where
async fn get_block_by_number(
&self,
number: BlockNumberOrTag,
hydrate: bool,
kind: BlockTransactionsKind,
) -> TransportResult<Option<N::BlockResponse>> {
let req = RequestType::new("eth_getBlockByNumber", (number, hydrate));
let full = match kind {
BlockTransactionsKind::Full => true,
BlockTransactionsKind::Hashes => false,
};

let req = RequestType::new("eth_getBlockByNumber", (number, full));

cache_get_or_fetch(&self.cache, req, self.inner.get_block_by_number(number, hydrate)).await
cache_get_or_fetch(&self.cache, req, self.inner.get_block_by_number(number, kind)).await
}

async fn get_block_by_hash(
Expand Down
28 changes: 15 additions & 13 deletions crates/provider/src/provider/trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ pub trait Provider<T: Transport + Clone = BoxTransport, N: Network = Ethereum>:
Some(base_fee) if base_fee != 0 => base_fee,
_ => {
// empty response, fetch basefee from latest block directly
self.get_block_by_number(BlockNumberOrTag::Latest, false)
self.get_block_by_number(BlockNumberOrTag::Latest, BlockTransactionsKind::Hashes)
.await?
.ok_or(RpcError::NullResp)?
.header()
Expand Down Expand Up @@ -285,10 +285,7 @@ pub trait Provider<T: Transport + Clone = BoxTransport, N: Network = Ethereum>:
) -> TransportResult<Option<N::BlockResponse>> {
match block {
BlockId::Hash(hash) => self.get_block_by_hash(hash.into(), kind).await,
BlockId::Number(number) => {
let full = matches!(kind, BlockTransactionsKind::Full);
self.get_block_by_number(number, full).await
}
BlockId::Number(number) => self.get_block_by_number(number, kind).await,
}
}

Expand Down Expand Up @@ -324,14 +321,19 @@ pub trait Provider<T: Transport + Clone = BoxTransport, N: Network = Ethereum>:
async fn get_block_by_number(
&self,
number: BlockNumberOrTag,
hydrate: bool,
kind: BlockTransactionsKind,
) -> TransportResult<Option<N::BlockResponse>> {
let full = match kind {
BlockTransactionsKind::Full => true,
BlockTransactionsKind::Hashes => false,
};

let block = self
.client()
.request::<_, Option<N::BlockResponse>>("eth_getBlockByNumber", (number, hydrate))
.request::<_, Option<N::BlockResponse>>("eth_getBlockByNumber", (number, full))
.await?
.map(|mut block| {
if !hydrate {
if !full {
// this ensures an empty response for `Hashes` has the expected form
// this is required because deserializing [] is ambiguous
block.transactions_mut().convert_to_hashes();
Expand Down Expand Up @@ -1515,7 +1517,7 @@ mod tests {
let provider = ProviderBuilder::new().on_anvil();
let num = 0;
let tag: BlockNumberOrTag = num.into();
let block = provider.get_block_by_number(tag, true).await.unwrap().unwrap();
let block = provider.get_block_by_number(tag, BlockTransactionsKind::Full).await.unwrap().unwrap();
let hash = block.header.hash;
let block =
provider.get_block_by_hash(hash, BlockTransactionsKind::Full).await.unwrap().unwrap();
Expand All @@ -1527,7 +1529,7 @@ mod tests {
let provider = ProviderBuilder::new().on_anvil();
let num = 0;
let tag: BlockNumberOrTag = num.into();
let block = provider.get_block_by_number(tag, true).await.unwrap().unwrap();
let block = provider.get_block_by_number(tag, BlockTransactionsKind::Full).await.unwrap().unwrap();
let hash = block.header.hash;
let block: Block = provider
.raw_request::<(B256, bool), Block>("eth_getBlockByHash".into(), (hash, true))
Expand All @@ -1541,7 +1543,7 @@ mod tests {
let provider = ProviderBuilder::new().on_anvil();
let num = 0;
let tag: BlockNumberOrTag = num.into();
let block = provider.get_block_by_number(tag, true).await.unwrap().unwrap();
let block = provider.get_block_by_number(tag, BlockTransactionsKind::Full).await.unwrap().unwrap();
assert_eq!(block.header.number, num);
}

Expand All @@ -1550,7 +1552,7 @@ mod tests {
let provider = ProviderBuilder::new().on_anvil();
let num = 0;
let tag: BlockNumberOrTag = num.into();
let block = provider.get_block_by_number(tag, true).await.unwrap().unwrap();
let block = provider.get_block_by_number(tag, BlockTransactionsKind::Full).await.unwrap().unwrap();
assert_eq!(block.header.number, num);
}

Expand Down Expand Up @@ -1774,7 +1776,7 @@ mod tests {
async fn test_empty_transactions() {
let provider = ProviderBuilder::new().on_anvil();

let block = provider.get_block_by_number(0.into(), false).await.unwrap().unwrap();
let block = provider.get_block_by_number(0.into(), BlockTransactionsKind::Hashes).await.unwrap().unwrap();
assert!(block.transactions.is_hashes());
}
}

0 comments on commit abb4dd4

Please sign in to comment.