op-service: Refactor EthClient with ReceiptsProvider abstraction

[sebastianst]

Description

This PR adds a ReceiptsProvider abstraction to the EthClient and extracts any receipts fetching logic into separate provider implementations.

It's an implementation of this proposed design.

Changes

• The RethDB rpc kind got removed, it's now encapsulated in its own provider.
  • If the reth db path is set in the eth client config, the reth db provider is statically set as the only provider and no attempt is made to create an rpc-based provider.
• Receipts validation is now skipped if we trust the rpc. Test added that receipts validation runs if not trusted.
• Concurrent receipt requests are now properly deduplicated by the caching layer.
• The different receipts provider implementations were split up into different files. The RPC receipts fetcher specific code was moved from file receipts.go to receipts_rpc.go, which increased the diff. But all types and functions around the rpc kind (RPCKind..., AvailableReceiptsFetchingMethods, PickBestReceiptsFetchingMethod, ...) were moved over but otherwise not modified.

Tests

All existing tests of receipts fetching pass. And a bunch of new tests were added

• Test that receipts validation runs if rpc not trusted.
• Tests that the basic rpc fetcher, which uses batched rpc calls,
  • is reusing an existing IterativeBatchCall,
  • can be called concurrently by many routines for the same block hash.
• Tests that the caching layer really caches and that it can be called concurrently by many routines for the same block hash and it deduplicates requests, resulting in only a single call to the underlying receipts provider.

Additional context

This refactor provides the basis for allowing for more modular implementations around receipts fetching.

• The RethDB receipts fetcher can be cleanly separated and its configuration doesn't need to leak into the EthClient (this isn't done in this PR yet, but now easily possible).
• In general, any future receipt fetchers can just be injected into the EthClient without leaking implementation details into it.
• Before we shifted from receipts pre-fetching to full EthClient block pre-fetching, this would also have created the basis for a pre-fetching receipts layer. This was the original motivation for this refactor.

Possible Follow-ups

• Add InfoAndTxHashesByHash method to EthClient (see one comment below).
• Improve RethDB fetcher to hold a reference to an opened database, instead of reopening it on every request. Persist RethDB instance in the RethDBReceiptsFetcher #8225
• The receipts provider caching layer now properly deduplicated receipt requests. We might also want to do this in the EthClient for header and block requests.

Metadata

• Fixes https://github.com/ethereum-optimism/client-pod/issues/193

[ajsutton]

Looks good generally, I just think we need to preserve the ability to reuse the receipts we do get before an error with the basic rpc kind. Everything else I think gets everything or nothing.

[sebastianst]

@coderabbitai review

[coderabbitai]

Walkthrough

Walkthrough

The changes revolve around the Ethereum client configuration and receipt fetching in an Ethereum-Optimism service. A new RethDBPath configuration option has been added, and receipt fetching now supports caching and concurrent operations. The EthClient struct has been refactored to use a ReceiptsProvider interface, and new types and methods for receipt fetching and caching have been introduced. Tests have been updated to cover new functionalities, and some existing functions have been removed or modified.

Changes

File Path	Change Summary
op-service/sources/eth_client.go	Added RethDBPath to EthClientConfig, refactored EthClient to use ReceiptsProvider, and updated FetchReceipts method.
op-service/sources/receipts.go	Added ReceiptsProvider interface, removed old receipt fetching types and functions.
op-service/sources/reth_db.go	Added RethDBReceiptsFetcher and related functions for fetching and caching receipts from Reth DB.
op-service/sources/reth_db_stub.go	Changed imports, added buildRethdb constant, replaced FetchRethReceipts with newRecProviderFromConfig.
op-service/sources/receipts_*.go	Introduced new types and methods for batch receipt fetching and caching.
op-service/sources/receipts_*.test.go	Added tests for new receipt fetching and caching functionalities.
op-service/sources/types.go	Added BlockID method to rpcHeader struct.
op-service/sources/eth_client_test.go	Added tests for EthClient receipt validation and caching.

---

Tips

Chat with CodeRabbit Bot (@coderabbitai)

• If you reply to a review comment from CodeRabbit, the bot will automatically respond.
• To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
• Note: Review comments are made on code diffs or files, not on the PR overview.
• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• The JSON schema for the configuration file is available here.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

[coderabbitai]

Review Status

Actionable comments generated: 2

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 0c62c8e and 5267327.

Files selected for processing (4)

• op-service/sources/eth_client.go (6 hunks)
• op-service/sources/receipts.go (8 hunks)
• op-service/sources/reth_db.go (2 hunks)
• op-service/sources/reth_db_stub.go (1 hunks)

Additional comments: 10

op-service/sources/eth_client.go (6)

• 63-70: The addition of the RethDBPath field in the EthClientConfig struct is a significant change that enables the use of the rethdb receipts fetcher. This is a good example of extending functionality while maintaining backward compatibility.

• 82-96: The conditional logic in the Check method ensures that the RethDBPath is only used when rethdb support is built into the binary. This is a good practice to prevent runtime errors due to misconfiguration.

• 107-120: The EthClient struct has been refactored to include a recProvider field, which is an instance of the ReceiptsProvider interface. This change improves modularity and allows for different implementations of receipt fetching.

• 126-128: The payloadsCache field in the EthClient struct is a cache for payloads by hash, which is consistent with the other caching strategies in the client.

• 134-150: The NewEthClient function has been updated to use the new recProvider field and the new configuration options. This change is necessary to integrate the new receipts fetching logic.

• 314-333: The FetchReceipts method has been updated to use the recProvider to fetch receipts. This change aligns with the refactor to use the ReceiptsProvider interface and improves the method's flexibility.

op-service/sources/receipts.go (4)

• 385-388: The new constants RPCKindBasic, RPCKindAny, and RPCKindStandard have been added to the RPCProviderKind type. Ensure that these new kinds are integrated and handled wherever RPCProviderKind is used throughout the codebase.

• 400-401: The RPCKindStandard constant has been added to the RPCProviderKinds slice. Verify that this addition is reflected in all relevant parts of the codebase, such as configuration parsing and validation.

• 558-569: The AvailableReceiptsFetchingMethods function has been updated to handle the new RPCKindBasic, RPCKindAny, and RPCKindStandard kinds. Verify that the logic for selecting available methods aligns with the intended behavior for these new kinds.

• 574-580: The PickBestReceiptsFetchingMethod function has been updated with logic specific to RPCKindAlchemy. Ensure that the cost-benefit analysis for using AlchemyGetTransactionReceipts over other methods is correct and that the threshold of txCount > 250/15 is appropriate.

[semgrep-app]

Semgrep found 6 sol-style-return-arg-fmt findings:

• packages/contracts-bedrock/scripts/Deployer.sol: L432, L422, L412, L402, L392, L373

Named return arguments to functions must be appended with an underscore (_)

_{Ignore this finding from sol-style-return-arg-fmt.}

Semgrep found 1 sol-style-input-arg-fmt finding:

• packages/contracts-bedrock/scripts/Deployer.sol: L373

Inputs to functions must be prepended with an underscore (_)

_{Ignore this finding from sol-style-input-arg-fmt.}

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 5267327 and e01f9ca.

Files selected for processing (1)

• op-service/sources/receipts_test.go (1 hunks)

Additional comments: 2

op-service/sources/receipts_test.go (2)

• 574-648: The TestBasicRPCReceiptsFetcherConcurrency function correctly tests the concurrency of fetching receipts as described in the summary. It sets up a mock RPC, starts multiple fetchers, and asserts the results, ensuring that the expected number of calls to BatchCallContext is less than the number of fetchers, which implies that some calls are being shared and not made concurrently. The use of a barrier channel to synchronize the start of the fetchers is a good practice for concurrency testing. The function also logs the JSON representation of the fetched receipts, which could be useful for debugging.

• 650-657: The requireEqualReceipt helper function is present and used within the TestBasicRPCReceiptsFetcherConcurrency function to compare receipts, as described in the summary. This function ensures that the expected and actual receipts are equal by comparing their JSON representations.

[ajsutton]

LGTM.