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

fix: use BlockId superset over BlockNumberOrTag where applicable #1135

Merged
merged 6 commits into from
Aug 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 2 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ jobs:
- "--no-default-features"
# Default features
- ""
# All features
- "--all-features"
Comment on lines +33 to +34
Copy link
Member Author

@zerosnacks zerosnacks Aug 8, 2024

Choose a reason for hiding this comment

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

Test cases in trace, debug including others were actually not being tested by the CI

I think opting for an --all-features CI job makes sense here as a general solution, solving issues that may arise on a case-by-case basis

exclude:
# All features on MSRV
- rust: "1.76" # MSRV
Expand Down
35 changes: 20 additions & 15 deletions crates/provider/src/ext/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
use crate::Provider;
use alloy_network::Network;
use alloy_primitives::{hex, Bytes, TxHash, B256};
use alloy_rpc_types_eth::{Block, BlockNumberOrTag, TransactionRequest};
use alloy_rpc_types_eth::{Block, BlockId, BlockNumberOrTag, TransactionRequest};
use alloy_rpc_types_trace::geth::{
BlockTraceResult, GethDebugTracingCallOptions, GethDebugTracingOptions, GethTrace, TraceResult,
};
Expand All @@ -13,16 +13,16 @@ use alloy_transport::{Transport, TransportResult};
#[cfg_attr(not(target_arch = "wasm32"), async_trait::async_trait)]
pub trait DebugApi<N, T>: Send + Sync {
/// Returns an RLP-encoded header.
async fn debug_get_raw_header(&self, block: BlockNumberOrTag) -> TransportResult<Bytes>;
async fn debug_get_raw_header(&self, block: BlockId) -> TransportResult<Bytes>;

/// Retrieves and returns the RLP encoded block by number, hash or tag.
async fn debug_get_raw_block(&self, block: BlockNumberOrTag) -> TransportResult<Bytes>;
async fn debug_get_raw_block(&self, block: BlockId) -> TransportResult<Bytes>;

/// Returns an EIP-2718 binary-encoded transaction.
async fn debug_get_raw_transaction(&self, hash: TxHash) -> TransportResult<Bytes>;

/// Returns an array of EIP-2718 binary-encoded receipts.
async fn debug_get_raw_receipts(&self, block: BlockNumberOrTag) -> TransportResult<Vec<Bytes>>;
async fn debug_get_raw_receipts(&self, block: BlockId) -> TransportResult<Vec<Bytes>>;

/// Returns an array of recent bad blocks that the client has seen on the network.
async fn debug_get_bad_blocks(&self) -> TransportResult<Vec<Block>>;
Expand Down Expand Up @@ -109,7 +109,7 @@ pub trait DebugApi<N, T>: Send + Sync {
async fn debug_trace_call(
&self,
tx: TransactionRequest,
block: BlockNumberOrTag,
block: BlockId,
trace_options: GethDebugTracingCallOptions,
) -> TransportResult<GethTrace>;

Expand All @@ -123,7 +123,7 @@ pub trait DebugApi<N, T>: Send + Sync {
async fn debug_trace_call_many(
&self,
txs: Vec<TransactionRequest>,
block: BlockNumberOrTag,
block: BlockId,
trace_options: GethDebugTracingCallOptions,
) -> TransportResult<Vec<GethTrace>>;
}
Expand All @@ -136,19 +136,19 @@ where
T: Transport + Clone,
P: Provider<T, N>,
{
async fn debug_get_raw_header(&self, block: BlockNumberOrTag) -> TransportResult<Bytes> {
async fn debug_get_raw_header(&self, block: BlockId) -> TransportResult<Bytes> {
self.client().request("debug_getRawHeader", (block,)).await
}

async fn debug_get_raw_block(&self, block: BlockNumberOrTag) -> TransportResult<Bytes> {
async fn debug_get_raw_block(&self, block: BlockId) -> TransportResult<Bytes> {
self.client().request("debug_getRawBlock", (block,)).await
}

async fn debug_get_raw_transaction(&self, hash: TxHash) -> TransportResult<Bytes> {
self.client().request("debug_getRawTransaction", (hash,)).await
}

async fn debug_get_raw_receipts(&self, block: BlockNumberOrTag) -> TransportResult<Vec<Bytes>> {
async fn debug_get_raw_receipts(&self, block: BlockId) -> TransportResult<Vec<Bytes>> {
self.client().request("debug_getRawReceipts", (block,)).await
}

Expand Down Expand Up @@ -200,7 +200,7 @@ where
async fn debug_trace_call(
&self,
tx: TransactionRequest,
block: BlockNumberOrTag,
block: BlockId,
trace_options: GethDebugTracingCallOptions,
) -> TransportResult<GethTrace> {
self.client().request("debug_traceCall", (tx, block, trace_options)).await
Expand All @@ -209,7 +209,7 @@ where
async fn debug_trace_call_many(
&self,
txs: Vec<TransactionRequest>,
block: BlockNumberOrTag,
block: BlockId,
trace_options: GethDebugTracingCallOptions,
) -> TransportResult<Vec<GethTrace>> {
self.client().request("debug_traceCallMany", (txs, block, trace_options)).await
Expand Down Expand Up @@ -268,7 +268,11 @@ mod test {
.max_priority_fee_per_gas(gas_price + 1);

let trace = provider
.debug_trace_call(tx, BlockNumberOrTag::Latest, GethDebugTracingCallOptions::default())
.debug_trace_call(
tx,
BlockNumberOrTag::Latest.into(),
GethDebugTracingCallOptions::default(),
)
.await
.unwrap();

Expand All @@ -284,7 +288,7 @@ mod test {
let provider = ProviderBuilder::new().on_http(geth.endpoint_url());

let rlp_header = provider
.debug_get_raw_header(BlockNumberOrTag::default())
.debug_get_raw_header(BlockId::Number(BlockNumberOrTag::Latest))
.await
.expect("debug_getRawHeader call should succeed");

Expand All @@ -298,7 +302,7 @@ mod test {
let provider = ProviderBuilder::new().on_http(geth.endpoint_url());

let rlp_block = provider
.debug_get_raw_block(BlockNumberOrTag::default())
.debug_get_raw_block(BlockId::Number(BlockNumberOrTag::Latest))
.await
.expect("debug_getRawBlock call should succeed");

Expand All @@ -311,7 +315,8 @@ mod test {
let geth = Geth::new().disable_discovery().data_dir(temp_dir.path()).spawn();
let provider = ProviderBuilder::new().on_http(geth.endpoint_url());

let result = provider.debug_get_raw_receipts(BlockNumberOrTag::default()).await;
let result =
provider.debug_get_raw_receipts(BlockId::Number(BlockNumberOrTag::Latest)).await;
assert!(result.is_ok());
}

Expand Down
39 changes: 17 additions & 22 deletions crates/provider/src/ext/trace.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! This module extends the Ethereum JSON-RPC provider with the Trace namespace's RPC methods.
use crate::{Provider, RpcWithBlock};
use alloy_eips::BlockNumberOrTag;
use alloy_eips::BlockId;
use alloy_network::Network;
use alloy_primitives::TxHash;
use alloy_rpc_types_eth::Index;
Expand Down Expand Up @@ -81,23 +81,20 @@ where
/// # Note
///
/// Not all nodes support this call.
async fn trace_block(
&self,
block: BlockNumberOrTag,
) -> TransportResult<Vec<LocalizedTransactionTrace>>;
async fn trace_block(&self, block: BlockId) -> TransportResult<Vec<LocalizedTransactionTrace>>;

/// Replays a transaction.
async fn trace_replay_transaction(
&self,
hash: TxHash,
trace_type: &[TraceType],
trace_types: &[TraceType],
) -> TransportResult<TraceResults>;

/// Replays all transactions in the given block.
async fn trace_replay_block_transactions(
&self,
block: BlockNumberOrTag,
trace_type: &[TraceType],
block: BlockId,
trace_types: &[TraceType],
) -> TransportResult<Vec<TraceResultsWithTransactionHash>>;
}

Expand All @@ -112,10 +109,10 @@ where
fn trace_call<'a, 'b>(
&self,
request: &'a <N as Network>::TransactionRequest,
trace_type: &'b [TraceType],
trace_types: &'b [TraceType],
) -> RpcWithBlock<T, (&'a <N as Network>::TransactionRequest, &'b [TraceType]), TraceResults>
{
RpcWithBlock::new(self.weak_client(), "trace_call", (request, trace_type))
RpcWithBlock::new(self.weak_client(), "trace_call", (request, trace_types))
}

fn trace_call_many<'a>(
Expand Down Expand Up @@ -144,9 +141,9 @@ where
async fn trace_raw_transaction(
&self,
data: &[u8],
trace_type: &[TraceType],
trace_types: &[TraceType],
) -> TransportResult<TraceResults> {
self.client().request("trace_rawTransaction", (data, trace_type)).await
self.client().request("trace_rawTransaction", (data, trace_types)).await
}

async fn trace_filter(
Expand All @@ -156,33 +153,31 @@ where
self.client().request("trace_filter", (tracer,)).await
}

async fn trace_block(
&self,
block: BlockNumberOrTag,
) -> TransportResult<Vec<LocalizedTransactionTrace>> {
async fn trace_block(&self, block: BlockId) -> TransportResult<Vec<LocalizedTransactionTrace>> {
self.client().request("trace_block", (block,)).await
}

async fn trace_replay_transaction(
&self,
hash: TxHash,
trace_type: &[TraceType],
trace_types: &[TraceType],
) -> TransportResult<TraceResults> {
self.client().request("trace_replayTransaction", (hash, trace_type)).await
self.client().request("trace_replayTransaction", (hash, trace_types)).await
}

async fn trace_replay_block_transactions(
&self,
block: BlockNumberOrTag,
trace_type: &[TraceType],
block: BlockId,
trace_types: &[TraceType],
) -> TransportResult<Vec<TraceResultsWithTransactionHash>> {
self.client().request("trace_replayBlockTransactions", (block, trace_type)).await
self.client().request("trace_replayBlockTransactions", (block, trace_types)).await
}
}

#[cfg(test)]
mod test {
use crate::ProviderBuilder;
use alloy_eips::BlockNumberOrTag;

use super::*;

Expand All @@ -194,7 +189,7 @@ mod test {
async fn test_trace_block() {
init_tracing();
let provider = ProviderBuilder::new().on_anvil();
let traces = provider.trace_block(BlockNumberOrTag::Latest).await.unwrap();
let traces = provider.trace_block(BlockId::Number(BlockNumberOrTag::Latest)).await.unwrap();
assert_eq!(traces.len(), 0);
}
}
15 changes: 8 additions & 7 deletions crates/provider/src/provider/trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,10 +259,10 @@ pub trait Provider<T: Transport + Clone = BoxTransport, N: Network = Ethereum>:
/// Gets a block by either its hash, tag, or number, with full transactions or only hashes.
async fn get_block(
&self,
id: BlockId,
block: BlockId,
kind: BlockTransactionsKind,
) -> TransportResult<Option<Block>> {
match id {
match block {
BlockId::Hash(hash) => self.get_block_by_hash(hash.into(), kind).await,
BlockId::Number(number) => {
let full = matches!(kind, BlockTransactionsKind::Full);
Expand Down Expand Up @@ -320,10 +320,10 @@ pub trait Provider<T: Transport + Clone = BoxTransport, N: Network = Ethereum>:
Ok(block)
}

/// Gets the selected block [BlockNumberOrTag] receipts.
/// Gets the selected block [BlockId] receipts.
async fn get_block_receipts(
&self,
block: BlockNumberOrTag,
block: BlockId,
) -> TransportResult<Option<Vec<N::ReceiptResponse>>> {
self.client().request("eth_getBlockReceipts", (block,)).await
}
Expand Down Expand Up @@ -1086,7 +1086,7 @@ mod tests {
}
}

#[cfg(feature = "ws")]
#[cfg(all(feature = "ws", not(windows)))]
#[tokio::test]
async fn subscribe_blocks_ws() {
use futures::stream::StreamExt;
Expand All @@ -1107,7 +1107,7 @@ mod tests {
}
}

#[cfg(feature = "ws")]
#[cfg(all(feature = "ws", not(windows)))]
Copy link
Member Author

Choose a reason for hiding this comment

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

Note ticket: #1136

#[tokio::test]
async fn subscribe_blocks_ws_boxed() {
use futures::stream::StreamExt;
Expand Down Expand Up @@ -1433,7 +1433,8 @@ mod tests {
async fn gets_block_receipts() {
init_tracing();
let provider = ProviderBuilder::new().on_anvil();
let receipts = provider.get_block_receipts(BlockNumberOrTag::Latest).await.unwrap();
let receipts =
provider.get_block_receipts(BlockId::Number(BlockNumberOrTag::Latest)).await.unwrap();
assert!(receipts.is_some());
}

Expand Down
2 changes: 1 addition & 1 deletion crates/provider/src/provider/with_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use std::{
task::Poll,
};

/// States of the
/// States of the [`RpcWithBlock`] future.
#[derive(Clone)]
enum States<T, Params, Resp, Output = Resp, Map = fn(Resp) -> Output>
where
Expand Down
2 changes: 1 addition & 1 deletion crates/rpc-client/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ mod test {
}

#[test]
#[cfg(feature = "ipc")]
#[cfg(all(feature = "ipc", not(windows)))]
fn test_parsing_ipc() {
use alloy_node_bindings::Anvil;

Expand Down