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

eth_call targeting an unknown block hash returns "unknown block number" #7368

Closed
1 task done
mfw78 opened this issue Mar 28, 2024 · 9 comments · Fixed by #7416
Closed
1 task done

eth_call targeting an unknown block hash returns "unknown block number" #7368

mfw78 opened this issue Mar 28, 2024 · 9 comments · Fixed by #7416
Labels
C-bug An unexpected or incorrect behavior D-good-first-issue Nice and easy! A great choice to get started M-prevent-stale Prevents old inactive issues/PRs from being closed due to inactivity

Comments

@mfw78
Copy link

mfw78 commented Mar 28, 2024

Describe the bug

When making an eth_call targeting a block by hash that does not exist returns unknown block number, instead of reporting the block hash as not found.

Steps to reproduce

  1. Start the reth node.
  2. curl -X POST -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","method":"eth_call","params":[{"to":"0x9 008d19f58aabd9ed0d60971565aa8510560ab41","data":"0xf698da25","value":"0x0"},"0x230d842afd1faceaa3c5c0dacd24228747565c4 41ed5e1e3a9306c2c6b55d619"],"id":1}' http://localhost:8545
  3. Observe error: {"jsonrpc":"2.0","error":{"code":-32001,"message":"unknown block number"},"id":1}

Compare the above with:

  1. curl -X POST -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","method":"eth_call","params":[{"to":"0x9 008d19f58aabd9ed0d60971565aa8510560ab41","data":"0xf698da25","value":"0x0"},"0x230d842afd1faceaa3c5c0dacd24228747565c4 41ed5e1e3a9306c2c6b55d619"],"id":1}' https://rpc.mevblocker.io
  2. Observe error: {"jsonrpc":"2.0","id":1,"error":{"code":-32000,"message":"block 230d842afd1faceaa3c5c0dacd24228747565c441ed5e1e3a9306c2c6b55d619 not found"}}

Node logs

No response

Platform(s)

Linux (x86)

What version/commit are you on?

v0.2.0-beta.3

What database version are you on?

Current database version: 2
Local database version: 2

What type of node are you running?

Archive (default)

What prune config do you use, if any?

No response

If you've built Reth from source, provide the full command you used

No response

Code of Conduct

  • I agree to follow the Code of Conduct
@mfw78 mfw78 added C-bug An unexpected or incorrect behavior S-needs-triage This issue needs to be labelled labels Mar 28, 2024
@mfw78 mfw78 changed the title eth_call targeting an unknown block hash returns "unknown block number" eth_call targeting an unknown block hash returns "unknown block number" Mar 28, 2024
@mattsse mattsse added D-good-first-issue Nice and easy! A great choice to get started and removed S-needs-triage This issue needs to be labelled labels Mar 28, 2024
@mattsse
Copy link
Collaborator

mattsse commented Mar 28, 2024

we can map the error to a more specific error, but those are also not consistent across clients

@ryanschneider
Copy link
Contributor

What about something like changing EthApiError::UnknownBlockNumber (https://github.com/ryanschneider/reth/blob/main/crates/rpc/rpc/src/eth/error.rs#L36-L37) to

    /// When an unknown block is encountered
    #[error("unknown block id: {0:?}")]
    UnknownBlockId(BlockId),

Since that error is used in a lot of spots I'd probably first add the new error and deprecate the old error just to keep the initial PR smaller, then do a follow-on to update everywhere it's used. If that sounds like a reasonable plan assign this to me, I just setup a new system for reth development so need something simple to test out my developer processes. :)

@ryanschneider
Copy link
Contributor

Hmm, actually my suggestion above doesn't quite work, for a couple reasons:

First off, this generates errors like:

{
  "jsonrpc": "2.0",
  "error": {
    "code": -32001,
    "message": "unknown block id: Hash(RpcBlockHash { block_hash: 0x230d842afd1faceaa3c5c0dacd24228747565c441ed5e1e3a9306c2c6b55d619, require_canonical: None })"
  },
  "id": 1
}

So would probably need to implement Display for BlockId. But actually I think we don't want to remove EthApiError::UnknownBlockNumber, but instead map from it to UnknownBlockId at the "call site", kind of like so in eth_call handler:

    /// Executes the call request (`eth_call`) and returns the output
    pub async fn call(
        &self,
        request: TransactionRequest,
        at: Option<BlockId>,
        overrides: EvmOverrides,
    ) -> EthResult<Bytes> {
        let at = at.unwrap_or(BlockId::Number(BlockNumberOrTag::Latest));
        let (res, _env) =
            self.transact_call_at(request, at, overrides).await.map_err(|e| match e {
                EthApiError::UnknownBlockNumber => EthApiError::UnknownBlockId(at),
                _ => e,
            })?;

        ensure_success(res.result)
    }

The reason is that often when we are returning EthApiError::UnknownBlockNumber we don't have access to the "full" BlockId. Does that make sense? Or is there a cleaner way?

@ryanschneider
Copy link
Contributor

Hmm, but BlockId is from alloy/crates/rpc-types, I guess we could add a wrapper type UnknownBlockId and do the formatting there, but the scope is definitely getting larger; I'll play around with it just to get reacclimated w/ the reth codebase but would appreciate feedback on this approach when you get a chance.

@mattsse
Copy link
Collaborator

mattsse commented Apr 1, 2024

checked geth error for reference: https://github.com/ethereum/go-ethereum/blob/767b00b0b514771a663f3362dd0310fc28d40c25/eth/api_backend.go#L216-L233

also relevant https://eips.ethereum.org/EIPS/eip-1474#block-identifier but idk if this is enforced by other clients tbh

@ryanschneider you can solve this with a wrapper error type, or customize the into ErrorObject impl

@ryanschneider
Copy link
Contributor

@mattsse RE: the EIP-1474 link, are you referring to returning -32000: Invalid input when requireCanonical is true but the block targetted isn't canonical? If so, I think that'll need to be a new EthApiError::BlockIdNotCanonical or similar.

I went with customizing the Into<ErrorObject> and now have:

{
  "jsonrpc": "2.0",
  "error": {
    "code": -32001,
    "message": "unknown block id: hash 0x230d842afd1faceaa3c5c0dacd24228747565c441ed5e1e3a9306c2c6b55d619"
  },
  "id": 1
}

And:

{
  "jsonrpc": "2.0",
  "error": {
    "code": -32001,
    "message": "unknown block id: number 0x230d842afd1fa"
  },
  "id": 1
}

I'll start a PR hopefully later today.

@ryanschneider
Copy link
Contributor

Draft PR: #7416

ryanschneider added a commit to ryanschneider/reth that referenced this issue Apr 1, 2024
ryanschneider added a commit to ryanschneider/reth that referenced this issue Apr 1, 2024
ryanschneider added a commit to ryanschneider/reth that referenced this issue Apr 1, 2024
ryanschneider added a commit to ryanschneider/reth that referenced this issue Apr 2, 2024
Copy link
Contributor

This issue is stale because it has been open for 21 days with no activity.

@github-actions github-actions bot added the S-stale This issue/PR is stale and will close with no further activity label Apr 23, 2024
@mattsse mattsse added M-prevent-stale Prevents old inactive issues/PRs from being closed due to inactivity and removed S-stale This issue/PR is stale and will close with no further activity labels Apr 23, 2024
@chaimaanairi
Copy link

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

I have experience in Ethereum development and smart contract deployment, with a focus on Solidity and Rust. I have worked on deploying contracts on both Ethereum and Layer 2 solutions, and I have experience with blockchain node implementation and RPC protocols.

How I plan on tackling this issue

To address the issue where eth_call targeting an unknown block hash returns "unknown block number" instead of "block hash not found," I would first review the current implementation of the eth_call method in the Reth codebase to understand the existing error handling mechanism. Then, I would implement logic to differentiate between block numbers and block hashes based on their format and update the error handling code to provide specific error messages for unknown block hashes and block numbers. Finally, I would test the changes thoroughly to ensure the correct error messages are returned in each scenario.

@github-project-automation github-project-automation bot moved this from Todo to Done in Reth Tracker Sep 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug An unexpected or incorrect behavior D-good-first-issue Nice and easy! A great choice to get started M-prevent-stale Prevents old inactive issues/PRs from being closed due to inactivity
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants