feat(op-service):Persist RethDB instance in the global variable

[Nickqiaoo]

A solution for https://github.com/ethereum-optimism/optimism/issues/8225, but persist the instance in the rust code.

[coderabbitai[bot]]

Walkthrough

The update streamlines the initialization of the DatabaseEnv instance in the rethdb-reader project by integrating the once_cell crate. This enhancement ensures that the DatabaseEnv is initialized only once, optimizing system efficiency and correctness.

Changes

Files	Change Summary
.../rethdb-reader/Cargo.toml, Cargo.lock	Added the once_cell crate as a dependency.
.../rethdb-reader/src/receipts.rs	Utilized once_cell for lazy initialization of DatabaseEnv, preventing multiple initializations.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

• X
• Mastodon
• Reddit
• LinkedIn

---

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit-tests for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit tests for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit tests.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

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.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

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

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

[Nickqiaoo]

A better way is export a new init rust function to set the global variable, call the init function in NewRethDBReceiptsFetcher in Go.But the current solution can already solve the problem of repeated initialization.

[clabby]

This LGTM, nice idea. We need to re-enable the rethdb-reader tests in CI, will approve once we've re-enabled and rebased on develop 👍

[github-actions[bot]]

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 5 days.

[Nickqiaoo]

@clabby Hi! I see that this PR is going to be closed, can it be merged?🥹

[clabby]

@clabby Hi! I see that this PR is going to be closed, can it be merged?🥹

Apologies for the delay here! Once #9692 is merged, we can rebase this and get it in :)

[clabby]

h/t @sebastianst, I think we should take a slightly different direction here. Right now, even though the op-node only needs 1 DB, the Go API allows for creating multiple RethDBReceiptsFetchers. With the DB instance being a static global, each instance of the RethDBReceiptsFetcher would re-use the same DB instance after it was first initiated. Can we export an open_db_read_only function from the library and store it in the Go struct, so that each receipts fetcher holds on to a single instance of the DB?

[sebastianst]

Thanks @Nickqiaoo for working on this! And thanks @clabby yes a DB instance stored inside the RethDBReceiptsFetchers would be cleaner, to give better encapsulation and theoretically allow the creation of multiple readers for different db backends. Because that's what the API currently allows. I think it's also described like this in the original ticket at https://github.com/ethereum-optimism/optimism/issues/8225.

[Nickqiaoo]

h/t @sebastianst, I think we should take a slightly different direction here. Right now, even though the op-node only needs 1 DB, the Go API allows for creating multiple RethDBReceiptsFetchers. With the DB instance being a static global, each instance of the RethDBReceiptsFetcher would re-use the same DB instance after it was first initiated. Can we export an open_db_read_only function from the library and store it in the Go struct, so that each receipts fetcher holds on to a single instance of the DB?

Instance reuse is indeed a problem, I'll do it.

[github-actions[bot]]

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 5 days.

[sebastianst]

@Nickqiaoo are you still working on this?

[Nickqiaoo]

@Nickqiaoo are you still working on this?

Sorry, i'am a little busy recently.I will do it this week.🫡

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 72.72727% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 28.46%. Comparing base (afb2048) to head (5f0c459). Report is 345 commits behind head on develop.

:exclamation: Current head 5f0c459 differs from pull request most recent head 08f06c6. Consider uploading reports for the commit 08f06c6 to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #9349      +/-   ##
===========================================
+ Coverage    27.85%   28.46%   +0.60%     
===========================================
  Files          167      165       -2     
  Lines         7362     7229     -133     
  Branches      1272     1324      +52     
===========================================
+ Hits          2051     2058       +7     
+ Misses        5190     5065     -125     
+ Partials       121      106      -15     

Flag	Coverage Δ
cannon-go-tests	82.29% <72.72%> (+19.73%)	:arrow_up:
chain-mon-tests	27.14% <ø> (ø)
common-ts-tests	26.72% <ø> (ø)
contracts-bedrock-tests	0.60% <ø> (-0.05%)	:arrow_down:
contracts-ts-tests	12.25% <ø> (ø)
core-utils-tests	44.03% <ø> (ø)
sdk-next-tests	41.94% <ø> (+0.42%)	:arrow_up:
sdk-tests	41.94% <ø> (+0.42%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
cannon/mipsevm/evm.go	93.93% <100.00%> (+0.49%)	:arrow_up:
cannon/mipsevm/state.go	87.23% <0.00%> (-1.90%)	:arrow_down:
cannon/mipsevm/instrumented.go	84.84% <0.00%> (ø)

... and 35 files with indirect coverage changes

[Nickqiaoo]

@sebastianst @clabby Hi, I create a new PR here https://github.com/ethereum-optimism/optimism/pull/9904