Skip to content

fix(eth_getLogs): return EIP-4444 error code for pruned receipts#12090

Open
manusw7 wants to merge 11 commits into
masterfrom
fix/rpc-eth-getlogs-pruned-history-error-code
Open

fix(eth_getLogs): return EIP-4444 error code for pruned receipts#12090
manusw7 wants to merge 11 commits into
masterfrom
fix/rpc-eth-getlogs-pruned-history-error-code

Conversation

@manusw7

@manusw7 manusw7 commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Closes #12089

Changes

  • In EthRpcModule.cs: catch ResourceNotFoundException in eth_getLogs and return error code 4444 with message "pruned history unavailable" instead of the generic -32000 path
  • In EthRpcModuleTests.cs: update the expected response in Eth_get_logs_with_resourceNotFound to match the new code and message

Types of changes

  • Bugfix (a non-breaking change that fixes an issue)

Testing

Requires testing

  • Yes

If yes, did you write tests?

  • Yes

Notes on testing

Updated existing unit test Eth_get_logs_with_resourceNotFound to assert the new 4444 / "pruned history unavailable" response.

Documentation

Requires documentation update

  • No

Requires explanation in Release Notes

  • No

@claude

claude Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Claude finished @manusw7's task in 2m 39s —— View job


PR Review

  • Gather context
  • Read changed files
  • Review correctness and edge cases
  • Check adherence to repo rules
  • Post findings

Summary

The core fix is correct: eth_getLogs(Filter) now returns error code 4444 with a standard message when receipts are pruned, aligning with EIP-4444. The ErrorCodes.PrunedHistoryUnavailable constant and the test update are appropriate.

Two issues found:


Medium — eth_getFilterLogs not updated

eth_getFilterLogs(UInt256 filterId) has its own catch (ResourceNotFoundException) block at EthRpcModule.cs:830–833 that still calls GetFailureResult(exception, ...), returning error code -32000. Since GetLogsResponse lazily iterates the log enumerable, a ResourceNotFoundException from pruned storage reaches this handler too — and a caller will get inconsistent error codes depending on whether they use a stored filter ID or a filter object.

The fix should mirror the eth_getLogs path:

catch (ResourceNotFoundException)
{
    return ResultWrapper<IEnumerable<FilterLog>>.Fail("Pruned history unavailable", ErrorCodes.PrunedHistoryUnavailable);
}

A regression test for eth_getFilterLogs with a pruned-history bridge mock would also be needed. Fix this →


Low — message casing inconsistency

See inline comment on line 912. The new message "pruned history unavailable" is all-lowercase, while the existing "Pruned history unavailable" at EthRpcModule.cs:1157 and DebugRpcModule.cs:586 use a capital P. The two sites within the same file should be consistent.


Verdict: not mergeable — the eth_getFilterLogs gap is a real EIP-4444 compliance inconsistency that should be closed in this PR.

Comment thread src/Nethermind/Nethermind.JsonRpc/Modules/Eth/EthRpcModule.cs Outdated
@manusw7

manusw7 commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

Lowercase is intentional — matching Geth's all-lowercase EIP-4444 messages. Also normalised the other "Pruned history unavailable" sites in the file for consistency.
Fixed eth_getFilterLogs to return ErrorCodes.PrunedHistoryUnavailable and added a regression test.

@claude review again

@claude

claude Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Claude finished @manusw7's task in 2m 47s —— View job


Claude finished @manusw7's task ——


PR Review (re-review)

  • Gather context
  • Read updated changed files
  • Check eth_getFilterLogs fix and new regression test
  • Verify casing normalization
  • Post findings

Summary

Both issues from my previous review are resolved:

  • eth_getFilterLogs fix ✅ — catch (ResourceNotFoundException) at EthRpcModule.cs:830 now returns 4444 / "pruned history unavailable". The new Eth_get_filter_logs_with_resourceNotFound test correctly exercises the lazy-enumerable path by having TryGetLogs set the out parameter to an enumerable that throws on first iteration, which propagates through GetLogsResponse's foreach loop into the catch block.

  • Casing normalization ✅ — All three EthRpcModule.cs sites (eth_getFilterLogs, eth_getLogs, and the BAL path at line 1157) now consistently use all-lowercase, matching Geth's EIP-4444 error messages. The two updated EthRpcModuleTests.cs BAL tests confirm the lowercase expectation.


Low — dead code left behind

EthRpcModule.cs:1071–1072 — the private GetFailureResult<TResult>(ResourceNotFoundException exception, bool isTemporary) overload had exactly two callers (both catch (ResourceNotFoundException exception) blocks), both of which this PR replaced with direct ResultWrapper.Fail(...) calls. The method now has no callers and should be removed.


Informational — DebugRpcModule.cs:586 still uses capital P

Out of scope for this PR, but DebugRpcModule.cs:586 still emits "Pruned history unavailable" while BlockFinderExtensions.cs and EthRpcModule.cs now use all-lowercase. Worth a follow-up for consistency.


Verdict: mergeable — the dead-code note is a minor cleanup that can be done in a follow-up. The EIP-4444 compliance gap is closed.

@LukaszRozmej LukaszRozmej left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we move "pruned history unavailable" string to some global constant?

Comment thread src/Nethermind/Nethermind.JsonRpc/Modules/Eth/EthRpcModule.cs Outdated
Comment thread src/Nethermind/Nethermind.JsonRpc/Modules/Eth/EthRpcModule.cs Outdated
Comment thread src/Nethermind/Nethermind.JsonRpc/ErrorCodes.cs Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

eth_getLogs (forked blocks): NM returns -32000 instead of EIP-4444 error code 4444

5 participants