fix: extend pending-block RPC filter to state-query methods

[operagxoksana]

eth_getBalance, eth_getTransactionCount, eth_getCode, eth_getStorageAt, eth_call and eth_estimateGas accept a block-tag param that can be "pending", but were not covered by is_pending_block_method(). Their block-tag is not the first positional param, so a new extract_param_at() helper is added to locate it.

[osr21]

Disclosure: I'm not affiliated with Circle — an external community contributor, not a maintainer. I have no write access to this repository, so the review state I set carries no merge authority and is advisory only. Please treat this as one contributor's technical assessment, and defer to Circle maintainers for the binding review. Rust findings below are source-review only (no cargo/rustc in my environment); everything else is executed or probed live and labelled as such.

---

The premise is real and I verified it against the live network, but I think the response shape blocks this as written: null is not a representable answer for these six methods, and one of them sits on the hot path of every viem transaction.

What I verified and agree with

The gap is genuine and reachable today. Against https://rpc.testnet.arc.network (chainId 0x4cef52, head 0x399f129), the methods this PR adds all answer a "pending" tag right now:

eth_getTransactionCount [addr,"pending"]  -> 0x0
eth_getBalance          [addr,"pending"]  -> 0xd14311c7eb2f752958d
eth_call                [{to},"pending"]  -> 0x
eth_estimateGas         [{to},"pending"]  -> 0x5f12
eth_getStorageAt  [addr,"0x0","pending"]  -> 0x0000...

The existing filter is confirmed live on the same endpoint, so this is a true gap in a deployed control rather than a theoretical one — eth_getBlockReceipts ["pending"] and eth_getBlockTransactionCountByNumber ["pending"] both return null, which is the middleware's null_response, not an upstream error.

The object keys are correct. I checked every signature against the pinned reth-rpc-eth-api (Cargo.lock → tag=v2.2.0, 88505c7), crates/rpc/rpc-eth-api/src/core.rs. All six bind block_number, and the positional indices in state_query_block_param_position match exactly, including eth_getStorageAt at index 2:

async fn balance(&self, address: Address, block_number: Option<BlockId>) -> RpcResult<U256>;
async fn storage_at(&self, address: Address, index: JsonStorageKey, block_number: Option<BlockId>) -> RpcResult<B256>;
async fn call(&self, request: TxReq, block_number: Option<BlockId>, state_overrides: ..., block_overrides: ...) -> RpcResult<Bytes>;

BlockId is also the right type — it picks up the EIP-1898 object form ({"blockNumber":"pending"}) as well as string tags. And the existing test_enabled_allows_non_pending_methods case survives, because it passes [] for eth_getBalance/eth_call, so index 1 is absent and optional_next yields None.

Blocking: null is not a valid response for any of these six

Every method the filter covered before this PR returns an Option in reth, so null is the method's own canonical "not found" value:

async fn block_receipts(...) -> RpcResult<Option<Vec<R>>>;
async fn header_by_number(...) -> RpcResult<Option<H>>;
async fn uncle_by_block_number_and_index(...) -> RpcResult<Option<B>>;

None of the six added here are optional — they are U256, Bytes, and B256. Returning null puts a value on the wire that the method's own schema cannot express, so clients don't degrade gracefully, they misparse.

I ran this against viem 2.52.2 with a transport stubbed to return JSON-RPC success with result: null:

getTransactionCount(pending) => THREW TypeError: Cannot convert null to a BigInt
getBalance(pending)          => THREW TypeError: Cannot convert null to a BigInt
estimateGas(pending)         => THREW EstimateGasExecutionError: An error occurred.
call(pending)                => RESOLVED: {"data":null}
getCode(pending)             => RESOLVED: null
getStorageAt(pending)        => RESOLVED: null

Two distinct failure modes, both bad: a raw TypeError that names nothing an operator could act on, and — worse — three methods that silently succeed with a null payload.

The consequential one is eth_getTransactionCount. A pending nonce is how wallets pick the next nonce, and in viem it is not an edge case — it is the default write path:

_esm/actions/wallet/prepareTransactionRequest.js:250   blockTag: 'pending'   <- every sendTransaction/writeContract without an explicit nonce
_esm/utils/nonceManager.js:76                          blockTag: 'pending'
_esm/actions/wallet/prepareAuthorization.js:74         blockTag: 'pending'   (EIP-7702)
_esm/actions/public/verifyHash.js:123                  blockTag: 'pending'   (ERC-6492)

filter_pending_txs defaults to true (node.rs:150, rpc_middleware.rs:109), so on a default-configured node this turns "send a transaction from a viem dapp" into TypeError: Cannot convert null to a BigInt. That is a much larger blast radius than the block-content methods the filter covered previously, none of which sit on a signing path.

Severity of the leak itself, calibrated honestly

Worth weighing against that cost: on the live node the exposure is currently nil in steady state. For an active address at head 0x399f129, pending and latest are byte-identical:

eth_getBalance          pending=0x16c0a26072333adc08ba  latest=0x16c0a26072333adc08ba  same
eth_getTransactionCount pending=0x0                     latest=0x0                     same

That is consistent with eth_getBlockByNumber ["pending"] returning -32014 requested data not available — with no pending block, reth's state provider already falls back to latest. So the real exposure is the narrow, racy window your doc comment describes, when the consensus engine briefly publishes a proposed block. Real, worth closing, but not a standing disclosure — which argues for closing it in a way that costs clients nothing.

Suggested alternative: coerce pending → latest instead of nulling

Since reth already resolves pending state to latest whenever no pending block exists (demonstrated above), making that mapping explicit for these six methods would:

• close the transient window deterministically, which is the actual goal;
• leak nothing, since with pending transactions hidden by default a pending nonce could never have included other senders' transactions anyway;
• keep every value schema-valid, so viem, ethers, and wallets keep working unchanged.

In other words the observable behaviour for clients stays exactly what it already is in the common case, and the pre-finalization read disappears. If you prefer to reject rather than coerce, an explicit JSON-RPC error — mirroring PENDING_TX_SUBSCRIPTION_ERROR_CODE in the subscription path — is still far better than null, because at least it surfaces an actionable message instead of a TypeError deep inside a client library. What I'd avoid is null, which is the one option that is both invalid per schema and silent for three of the six.

This is a product call as much as a technical one, so I'd defer to maintainers on which of the two to take.

Coverage gaps, if the intent is to close the class

From the same pinned core.rs, these also take a block parameter and remain uncovered after this PR:

method	index	object key	note
eth_getProof	2	block_number	state proof at pending state
eth_createAccessList	1	block_number
eth_simulateV1	1	block_number
eth_getStorageValues	1	block_number
eth_getAccount	1	block	different key
eth_getAccountInfo	1	block	different key
eth_feeHistory	1	newest_block	BlockNumberOrTag, not BlockId
eth_getUncleByBlockNumberAndIndex	0	number	block-content class, missed by is_pending_block_method
eth_getBlockAccessListByBlockNumber	0	number	same
eth_getBlockAccessListRaw	0	block	same

Calibrating that down: eth_getProof, eth_createAccessList, and eth_getHeaderByNumber all return -32601 method not supported on the public endpoints I probed, so they are not exposed there. That is provider namespace configuration though, not something arc-node enforces, so a self-hosted node with the eth namespace enabled would still expose them. The block and newest_block keys are worth noting because a mechanical copy of the block_number entry would silently miss them — your helper returns the key per method, which is exactly the right shape to extend.

Smaller points

1. Named-param key list is narrower than its neighbour. extract_param_at is called with &[key] only, while the adjacent eth_getBlockReceipts branch passes both BLOCK_ID_OBJECT_KEY_SNAKE and BLOCK_ID_OBJECT_KEY_CAMEL. jsonrpsee binds snake-case proc-macro field names, so blockNumber shouldn't occur — but the file already chose to be defensive one branch above, and the inconsistency will read as an oversight later. Either add the camel variant or drop a comment saying why it isn't needed here.
2. Changelog. Repo convention covers this exact class — v0.8.0 carries "[EL] Complete the pending-block RPC filter…" under ### Fixes. This PR adds no entry, and given the client-visible impact above it may warrant a BREAKING_CHANGES.md note too, depending on which response strategy you land on.
3. Match guards are unnecessary. const &str values are valid match patterns, so ETH_GET_BALANCE_METHOD => Some((1, "block_number")), works directly without the m if m == … guard.
4. Test gap. The new tests cover array form only. Given extract_param_at has a distinct object-params branch, an object-form case ({"address":"0x…","block_number":"pending"}) and an EIP-1898 case ([addr, {"blockNumber":"pending"}]) would pin the two paths that are easiest to regress.

Structurally the change is sound — the helper is the right abstraction, the indices and keys are right, and the doc comment on filter_pending_txs was kept in sync. My concern is only the value it returns.

[operagxoksana]

Thanks for the detailed review and for testing this on testnet. You're right returning null isn't correct for these methods and can break clients like viem. I'll fix the response and update the tests accordingly.

[osr21]

Disclosure: I'm not affiliated with Circle — I'm an external community contributor, not a maintainer, and I have no write access to this repository. Any review state I set is advisory only and carries no merge authority; please defer to Circle maintainers for the binding review. Rust findings below are source-review only (no cargo/rustc in my environment) and CI is authoritative; everything labelled live was executed against https://rpc.testnet.arc.network today.

Thanks for the quick turnaround, and agreed on the direction.

One procedural note first: the head commit is still f448ea4b, the same tree I reviewed, and the re-review request arrived a few seconds after your comment — so there's nothing new for me to look at yet. I'm submitting this as a comment rather than a new state, so the existing changes-requested stands on its own until you push. Everything below is aimed at making that push land in one go.

Re-verified live today

The premise still holds on the deployed node, so nothing has moved under you:

eth_getBlockReceipts ["pending"]                 -> null        (filter is live)
eth_getBlockTransactionCountByNumber ["pending"] -> null        (filter is live)
eth_getBalance          [addr,"pending"]         -> 0x0
eth_getTransactionCount [addr,"pending"]         -> 0x0
eth_getCode             [addr,"pending"]         -> 0x
eth_getStorageAt  [addr,"0x0","pending"]         -> 0x00…00
eth_call          [{to},"pending"]               -> 0x
eth_estimateGas   [{to},"pending"]               -> 0x5208
eth_getBalance    [addr,{"blockNumber":"pending"}] -> 0x0       (EIP-1898 form also reaches it)

Two things worth deciding before you push

1. Which response shape. My recommendation is still coercing pending → latest rather than an error, because on this node they are already the same value in steady state, so coercion is invisible to clients while still closing the transient window. If you prefer to reject, use a distinct error code in the spirit of PENDING_TX_SUBSCRIPTION_ERROR_CODE (-32001) — viem surfaces that as a normal RpcError with your message intact, which is at least actionable. Either is defensible; null is the only option that is both schema-invalid and silent.

2. If you coerce, don't rebuild the Request. This one is easy to get wrong and the compiler won't catch it. In the pinned jsonrpsee (0.26.0, workspace Cargo.toml:142), Request::params is a public field:

pub struct Request<'a> {
    pub jsonrpc: TwoPointZero,
    pub id: Id<'a>,
    pub method: Cow<'a, str>,
    pub params: Option<Cow<'a, RawValue>>,
    #[serde(skip)]
    pub extensions: Extensions,
}

so rewriting the tag is an assignment to req.params on an owned Request — but the obvious-looking Request::owned(method, new_params, id) constructor sets extensions: Extensions::new(), silently dropping whatever the server put there. Mutate the field in place and forward the same request; don't reconstruct it.

Also worth pinning in tests if you go this route: the rewrite has to handle the string tag, the EIP-1898 object form ({"blockNumber":"pending"}), and the absent-param case (leave it alone — reth already defaults to latest).

New finding: eth_getLogs is the same class, and this PR doesn't reach it

This is the one I'd most like you to consider, because it changes the shape of the helper. eth_getLogs accepts pending in its filter object and is not covered by is_pending_block_method or by state_query_block_param_position. Live, it resolves pending to head + 1, which makes it visibly racy:

head=60660590   {"fromBlock":"pending","toBlock":"pending"} -> 50 logs, all from block 60660591
head=60660595   {"fromBlock":"pending","toBlock":"pending"} -> error -32602
                "block range extends beyond current head block: requested 60660596, head 60660595"
head=60660599   {"fromBlock":"pending","toBlock":"pending"} -> 10 logs, all from block 60660601

Two consequences:

• It belongs in the same bucket as the six you added — eth_getLogs returns Vec<Log>, not an Option, so null is unrepresentable there too. Confirmed against viem 2.52.2 with a transport stubbed to return result: null: TypeError: Cannot read properties of null (reading 'map').
• The helper can't express it. state_query_block_param_position returns "one index, one key", but getLogs carries two tags (fromBlock, toBlock) nested one level inside the object at index 0. If the intent is to close the class rather than six specific methods, that's a third param shape, alongside "first positional" and "nth positional".

Also live: eth_feeHistory ["0x1","pending",[]] returns real data (oldestBlock: 0x39d9bf7), so that gap from my earlier list is reachable too. By contrast eth_getProof, eth_createAccessList, and eth_simulateV1 all answer -32601 method not supported on the public endpoint — that's provider namespace configuration, not something arc-node enforces, so a self-hosted node with the full eth namespace still exposes them.

Two smaller things I can now be concrete about

Skipping positional params allocates on the hottest path. extract_param_at advances the sequence with let _: Option<serde_json::Value> = seq.optional_next()…, which builds a full Value tree for each skipped argument. For eth_call and eth_estimateGas the skipped argument is the transaction object, calldata included, and eth_call is typically the busiest method on the node. ParamsSequence::optional_next is bounded T: Deserialize<'a> — a borrowed lifetime, not DeserializeOwned (jsonrpsee-types 0.26.0, src/params.rs) — so you can skip with Option<&serde_json::value::RawValue> and get a borrow of the raw slice instead of a parsed tree. No behavioural change; the outer T: DeserializeOwned bound on the function is unaffected.

The object-params branch can't be validated live, so unit tests have to carry it. Through the public endpoint, by-name params are rejected before dispatch — -32700 parse error for every method I tried, including "params": {} on eth_blockNumber. I can't tell from outside whether that's the gateway or the node itself, so I'm not claiming the branch is dead code, only that nothing on that path is reachable for live verification. It makes the object-form test case from my earlier review more load-bearing than it looked.

Still open from the earlier review, unchanged: the camel-case key asymmetry against the adjacent eth_getBlockReceipts branch, the missing CHANGELOG entry (v0.8.0 has a precedent line for exactly this filter under ### Fixes), the unnecessary m if m == … match guards, and the array-only test coverage.

Happy to re-review once the fix is pushed.

[operagxoksana]

Pushed the fix pending-state queries now reject with an explicit error (-32002) instead of null, and coverage extends to eth_getLogs and eth_feeHistory as you found. Also fixed the filter_pending_txs guard concern, removed the unnecessary match guards, added the CHANGELOG entry, and added object-form + asymmetric eth_getLogs test cases. Ready for re-review.

[osr21]

Disclosure: I'm not affiliated with Circle — I'm an external community contributor, not a maintainer, and I have no write access to this repository. This is one contributor's technical assessment, advisory only; a "request changes" from me blocks nothing and Circle maintainers decide. Rust findings here are source-review only (I can't compile the tree); CI is authoritative. Live probes were run against the public https://rpc.testnet.arc.io/ endpoint.

Thanks for turning this around quickly — the mechanical review items are all addressed: null is gone, the filter_pending_txs gate is right (the layer is only wrapped at rpc_middleware.rs:185, so the new check inherits it), the match guards are gone, the CHANGELOG entry is there, the skip in extract_param_at now uses borrowed &RawValue instead of an owned Value tree, and object-form params have unit coverage.

I have to request changes anyway, because verifying the new behaviour turned up something I got wrong in my last review, and it invalidates most of this PR's scope.

I was wrong, and so is the premise: pending already resolves to latest

My previous review argued about how to answer these six methods (null vs. error vs. coercion) and never checked whether they were broken at all. They aren't. Live, against an account with a non-zero nonce:

nonce(pending)   = 0x1e8      nonce(latest)   = 0x1e8
balance(pending) = 0x8b291213354eb52   balance(latest) = 0x8b291213354eb52
eth_call([{to:0x0}, "pending"])  -> 0x

Reth already coerces pending → latest for these, which is exactly the coercion I suggested. The middleware doesn't need to implement it, and this PR replaces a correct answer with an error.

This contradicts the repo's own conformance suite

crates/test/checks/src/mev.rs is not neutral on this — it documents the fallback as the intended, MEV-safe behaviour:

//! - **Pending state fallback** — state methods with `"pending"` tag match `"latest"`

and check_node asserts it for precisely the six methods this PR now rejects — eth_getBalance, eth_getTransactionCount, eth_getCode, eth_getStorageAt, eth_call, eth_estimateGas (mev.rs:385-423), via check_pending_eq_latest. It's exported from the crate as check_pending_state (checks/src/lib.rs:42).

This PR touches two files and neither is that one. As it stands, merging makes the project's own MEV conformance check fail against a default-configured node. That check is also the argument against needing this change: pending == latest leaks no mempool state, so rejection buys nothing for the threat model the filter exists to serve.

It breaks the canonical nonce fetch — including this repo's own tooling

eth_getTransactionCount(addr, "pending") is how essentially every client gets the next nonce. viem 2.52.2, default write path, against a stub that behaves like this PR:

=== baseline (pending allowed) ===
sendTransaction OK
  eth_getTransactionCount calls: ["pending"]

=== with this PR (pending nonce -> -32002) ===
sendTransaction FAILED: Requested resource not available.
  details: queries against pending block state are not allowed
  eth_getTransactionCount calls: ["pending"]

viem doesn't fall back to latest; the send just fails. Note this is the same class of breakage as the null version — my earlier point wasn't "null specifically is wrong", it was "don't interfere with this call", and I didn't state that clearly enough.

Two in-repo callers break the same way against a node with the default flag:

• crates/spammer/src/generator.rs:986 — eth_getTransactionCount(address, "pending"), deliberately, per its doc comment at :675 ("to skip nonces already accepted by the pool")
• crates/quake/src/rpc/mod.rs:185 — same call in get_transaction_count

eth_feeHistory isn't broken either

eth_feeHistory ["0x1","pending",[]] ->
  {"baseFeePerGas":["0x4a817c800","0x4a817c800"], "gasUsedRatio":[0.0267...],
   "oldestBlock":"0x39ed2aa", "reward":[[]]}

Real data, live. Same story as the six: no need to intercept.

eth_getLogs is the one genuine bug, and I'd still not reject it

You were right to pick this up from my last review — it reproduces. Three identical back-to-back calls with {"fromBlock":"pending","toBlock":"pending"}:

1) {"error":{"code":-32602,"message":"block range extends beyond current head block:
              requested 60740330, head 60740329"}}
2) {"result":[{"address":"0xffff...fffe", ...}]}
3) {"result":[{"address":"0xffff...fffe", ...}]}

pending resolves to head + 1 here, not to latest, so the same request non-deterministically errors or returns the next block's logs depending on where the head is. That's a real inconsistency worth fixing.

But rejecting it makes eth_getLogs the only method on the node where pending is an error rather than an alias for latest — three different behaviours across the RPC surface (null for block-content, error for logs, fallback for state). Clamping fromBlock/toBlock pending → latest in the middleware would make it consistent with what reth already does everywhere else, keep mev.rs's stated model intact, and fix the flip-flop. If maintainers prefer an error, that's a defensible call — but then it's a deliberate API decision that needs a BREAKING_CHANGES note, not a ### Fixes line.

Suggested scope

Drop the six state-query methods and eth_feeHistory entirely (state_query_block_param_position, the ETH_FEE_HISTORY_METHOD branch, and their tests), keep only the eth_getLogs handling, and reword the CHANGELOG entry to match. That removes extract_param_at's positional-skip path along with the two heaviest-traffic methods (eth_call, eth_estimateGas) from the middleware's hot path, which also disposes of the perf concern from my last review.

Smaller notes

• -32002 is EIP-1474 "Resource unavailable", which viem surfaces as Requested resource not available. — reasonable, and it doesn't collide with the -32004/-32005 already used in crates/evm-node/src/rpc/common.rs. Worth knowing that -32002 is also what MetaMask uses for "request already pending", so wallet-side logs may read oddly.
• The ## [Unreleased] heading is new to CHANGELOG.md — every prior entry arrived through a release sync commit rather than being staged unreleased. Worth a maintainer confirming the release tooling tolerates it.
• The comment on state_query_block_param_position says "only the snake_case key is tried here" and justifies it, which is the asymmetry I raised — good, that answers it. If that block goes away with the scope reduction, the eth_getLogs filter key deserves the same one-line justification.

Happy to re-check anything here if you think a probe was mis-set-up — I'd rather be corrected twice than have this land on the wrong premise.