feat: add get transaction by index methods #4579
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| @@ -76,14 +82,4 @@ pub enum ProviderError { | |||
| /// The address is not owned by this node. | |||
| #[error("{address} is not owned by this node")] | |||
| UnknownAddress { address: Address }, | |||
| /// Block number doesn't exist in blockchain | |||
There was a problem hiding this comment.
I replaced these with a single InvalidBlockNumberOrHash as we only check if any of these match to return None in the RPC and there will be many methods making this check.
| pub async fn handle_get_transaction_by_block_hash_and_index( | ||
| data: &ProviderData, | ||
| block_hash: B256, | ||
| index: U256, |
There was a problem hiding this comment.
I'm curious whether the type used in the JSON-RPC type was correct. Does the standard specifically call for a U256? I know I made some mistakes with other types where I used U256 but it turned out it's just a quantity type with 1 or more values after the 0x..
There was a problem hiding this comment.
Good question. I went with U256, because Hardhat accepts U256 as its rpcQuantity type.
There was a problem hiding this comment.
Created follow up issue: NomicFoundation/edr#236
crates/edr_provider/src/data.rs
Outdated
| } else { | ||
| None | ||
| }; | ||
|
|
||
| Ok(transaction) | ||
| } | ||
|
|
||
| pub fn transaction_from_block( |
There was a problem hiding this comment.
I would have expected this logic to happen in the rpc handler, not here.
PS/Disclaimer: I didn't review the entire PR
There was a problem hiding this comment.
My thinking with the GetTransactionResult (for which transaction_from_block is a helper method) is this:
The transaction getter RPC methods need some data from the block (header stuff and the transaction index) in addition to the signed transaction. To avoid cloning entire blocks just to get a transaction, the GetTransactionResult type exposes just the data needed by the transaction getter RPC methods in the internal representation (e.g. block number as u64) and then lets the RPC methods convert to the data types expected in the RPC layer.
There was a problem hiding this comment.
Actually on second thought we cloning the block would be cheap, because it's behind an Arc. But we still need a structure to return both the signed transaction and the block and I prefer explicit types.
Co-authored-by: Wodann <Wodann@users.noreply.github.com>
Co-authored-by: Wodann <Wodann@users.noreply.github.com>
Co-authored-by: Wodann <Wodann@users.noreply.github.com>
agostbiro
force-pushed
the
edr/feat/get-transaction-by-index-methods
branch
from
37d8a9f to
90e252d
Compare
Add support for
eth_getTransactionByBlockHashAndIndexandeth_getTransactionByBlockNumberAndIndex: