fix(tests): replace alchemy-dependant test with mock server

[anaPerezGhiglia]

Context

The get_logs_future_to_block integration test (in edr_rpc_eth) started failing on the CI matrix nodes:
See job logs:

  JsonRpcError { code: -32000, message: "query block range exceeds server limit, narrow your filter: 10000" }

The test queried eth_getLogs from a fixed block to MAX_BLOCK_NUMBER against live Alchemy and asserted it returned []. That premise no longer holds: Alchemy enforces a block-range cap on eth_getLogs and measures the span against toBlock as provided (it doesn't clamp a far-future toBlock to the head first), so an unbounded toBlock is rejected. The behavior also varies by plan tier. See Alchemy documentation on this method.

Rationale

I decided to convert this into a mocked test because it never actually needed a live JSON-RPC node: get_logs_by_range forwards the range verbatim and does no clamping of its own, so the only EDR behavior worth testing is that the request serializes correctly and the response parses. The flakiness came entirely from specific Alchemy behavior, and we don't want our test suite to be testing Alchemy.
The clearest evidence of this was that the range cap differed depending on the plan tier of the API key (e.g. a 10-block limit on the free tier vs. ~10,000 on higher tiers), so the outcome depended on the key and backend node rather than on anything in our code.

Changes

• Remove the live get_logs_future_to_block and get_logs_future_from_block tests.

• Replace them with get_logs_by_range_serializes_request_and_parses_response, a deterministic test backed by a mockito server. It pins the part EDR actually owns: that the request (including a future toBlock) serializes to the expected eth_getLogs params, and that the response deserializes into the expected FilterLog.

  • Runs with no network and no test-remote feature.

• The mock also stubs eth_chainId/eth_blockNumber, which EDR's caching layer fires around the eth_getLogs request; they're optional responders so the test stays decoupled from caching internals.

[changeset-bot]

⚠️ No Changeset found

Latest commit: aa6d197

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[Copilot]

Pull request overview

This PR makes the edr_rpc_eth integration test suite deterministic by replacing previously live Alchemy-dependent eth_getLogs tests (which became flaky due to provider-specific block-range limits) with a local mockito-backed test that validates EDR-owned behavior (request serialization + response parsing).

Changes:

• Remove the two test-remote Alchemy integration tests that relied on provider behavior for future block ranges.
• Add a new mockito-based test that asserts eth_getLogs is serialized with an un-clamped future toBlock, and that a mocked log response deserializes into FilterLog.
• Stub eth_chainId / eth_blockNumber to satisfy the RPC client caching layer without coupling the test to caching internals.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.40%. Comparing base (7491446) to head (aa6d197).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1501      +/-   ##
==========================================
- Coverage   79.40%   79.40%   -0.01%     
==========================================
  Files         446      446              
  Lines       76607    76607              
  Branches    76607    76607              
==========================================
- Hits        60831    60826       -5     
- Misses      13656    13658       +2     
- Partials     2120     2123       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Wodann]

The original intent of async fn get_logs_future_from_block() was to validate that the remote provider returns an error when a future from block is queried. Conversely, async fn get_logs_future_to_block() was meant to test that having a valid "from" block and a future "to" block number does not throw an error.

Apparently, that behaviour already changed (depending on the client used by Alchemy) at some point. With the new range limit, we apparently never get the expected "a future from block is not allowed" error any more, due to the range limit.

To adhere to the original intent of the test, we'd need to pick a future block that's guaranteed to be in the future, but is not too far in the future to exceed the range limit. With a narrow range, that's quite tricky as the fork block is behind the actual blockchain (we build in a safety buffer when forking to ensure blocks are finalised and blocks continue to be mined every 12 seconds). One possibility would be to do something like forkBlockNumber + CONSTANT_BLOCK_OFFSET, where CONSTANT_BLOCK_OFFSET is guaranteed to be far enough in the future to not exist but small enough to not exceed the range limit.

The way the test has been rewritten, we're merely testing serialization. We already have serde tests for requests in crates/edr_provider/tests/integration/eth_request_serialization.rs, so I'm not sure that we're adhering to original spirit of the tests, nor adding much value by having the modified version of the tests.

If the tests are this flaky and we're not testing a consistent behaviour of the JSON-RPC layer but rather a subjective implementation detail of Alchemy; I'd choose to remove the tests altogether than mock past behaviour.

WDYT?

[anaPerezGhiglia]

I wasn't aware of the eth_request_serialization tests, thanks for pointing those out.
To be honest, I assumed serialization was the thing worth testing here precisely because the original test didn't explain its intent through either its name or its code, so it was hard to tell what the expected result was and why.

Given that serialization is already covered there, I agree the mocked version doesn't add much it'd just be duplicating that coverage, so I'd vote for removing it.

On the original intent: do we actually rely on EDR surfacing an error when a from block is in the future? That seems like behavior owned by the remote provider rather than anything EDR guarantees. I'm trying to make sure I understand what contract we were trying to pin down in the test before deciding if it's worth preserving it.

[Wodann]

do we actually rely on EDR surfacing an error when a from block is in the future?

If you call EDR's JSON-RPC method eth_getLogs, the provider would forward the remote provider's error to the user. We don't rely on any specific error being surfaced; nor did this test explicitly test the E2E pipeline. These are tests validating success and failure modes of different JSON-RPC requests.

I think at some point we already observed that testing failure modes is flaky, as you're tied to changes in the upstream API; so I think these tests are honestly not very valuable to keep around.