8000 feat(supervisor/storage): chaindb and chaindb factory by dhyaniarun1993 · Pull Request #1864 · op-rs/kona · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat(supervisor/storage): chaindb and chaindb factory #1864

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
May 27, 2025
Merged

Conversation

dhyaniarun1993
Copy link
Collaborator
@dhyaniarun1993 dhyaniarun1993 commented May 26, 2025

Closes #1843
Ref #1684

@dhyaniarun1993 dhyaniarun1993 self-assigned this May 26, 2025
@dhyaniarun1993 dhyaniarun1993 added W-supervisor Workstream: supervisor A-db Area: database labels May 26, 2025
@dhyaniarun1993 dhyaniarun1993 marked this pull request as ready for review May 26, 2025 11:27
@Copilot Copilot AI review requested due to automatic review settings May 26, 2025 11:27
Copy link
Contributor
@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces new functionality for managing chain-specific databases by adding a Chaindb implementation and a corresponding factory, as well as exposing a derivation storage trait for supervisor storage.

  • Added the DerivationStorage trait
  • Introduced new modules: ChainDb and ChainDbFactory
  • Updated error handling to include a new database initialization error

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
crates/supervisor/storage/src/traits.rs Added the derivation storage interface
crates/supervisor/storage/src/providers/mod.rs Cleaned up unused imports
crates/supervisor/storage/src/providers/derivation_provider.rs Removed dead_code attributes from functions
crates/supervisor/storage/src/lib.rs Exposed new ChainDb and ChainDbFactory modules
crates/supervisor/storage/src/error.rs Added a DatabaseInit error variant
crates/supervisor/storage/src/chaindb_factory.rs Introduced chain database factory logic
crates/supervisor/storage/src/chaindb.rs Implemented ChainDb and integrated DerivationStorage
crates/supervisor/storage/Cargo.toml Added dependency configuration for eyre
Cargo.toml Added eyre dependency

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@dhyaniarun1993 dhyaniarun1993 requested a review from Copilot May 26, 2025 11:33
Copy link
Contributor
@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces ChainDb and ChainDbFactory implementations, along with a new DerivationStorage trait to integrate database operations for block derivation. It also adjusts provider annotations, updates error handling with a new DatabaseInit variant, and includes related dependency changes in Cargo.toml.

  • Added the DerivationStorage trait and its implementation for ChainDb.
  • Introduced ChainDbFactory for managing per-chain databases.
  • Updated Cargo.toml files to include the eyre dependency for improved error reporting.

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
crates/supervisor/storage/src/traits.rs Added a new trait defining methods for block derivation storage.
crates/supervisor/storage/src/providers/mod.rs Removed unused import annotations for provider usage.
crates/supervisor/storage/src/providers/derivation_provider.rs Removed dead-code attribute from provider methods.
crates/supervisor/storage/src/lib.rs Re-exported modules and traits to integrate new functionalities.
crates/supervisor/storage/src/error.rs Added a new error variant (DatabaseInit) for initialization errors.
crates/supervisor/storage/src/chaindb_factory.rs Introduced a factory to manage per-chain ChainDb instances with tests.
crates/supervisor/storage/src/chaindb.rs Implemented ChainDb with database initialization and trait support.
crates/supervisor/storage/Cargo.toml Added the eyre dependency to the workspace.
Cargo.toml Added the eyre dependency for error handling in the main project.

}

// Not found, create and insert
let mut dbs = self.dbs.write().unwrap();
Copy link
Preview
Copilot AI May 26, 2025

Choose a reason for hiding this comment

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

Consider handling potential lock poisoning on the write lock instead of using unwrap(), to ensure graceful error recovery in concurrent scenarios.

Copilot uses AI. Check for mistakes.

Copy link
codecov bot commented May 26, 2025

Codecov Report

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

Project coverage is 83.6%. Comparing base (65960dd) to head (6462776).
Report is 1 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
crates/supervisor/storage/src/chaindb.rs 98.0% 2 Missing ⚠️
crates/supervisor/storage/src/chaindb_factory.rs 98.4% 1 Missing ⚠️

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dhyaniarun1993 dhyaniarun1993 requested a review from sadiq1971 May 26, 2025 15:51
/// Finds a [`BlockInfo`] by a specific [`Log`].
///
/// # Arguments
/// * `block_number` - The block number to search for the log.
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we need to pass block number as param, wouldn't it be better to simply get block info by number? Or am I misunderstanding something here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This method is more like searching the block by logs information. This will return the block only if it contains the provided log.

I have added a todo on the method. I think we need to structure it better for the understanding.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have updated the doc as well to explain this.

@dhyaniarun1993 dhyaniarun1993 changed the title feat: chaindb and chaindb factory feat(supervisor/storage): chaindb and chaindb factory May 27, 2025
@emhane emhane merged commit df56250 into main May 27, 2025
21 of 22 checks passed
@emhane emhane deleted the feat/chain-db branch May 27, 2025 12:20
@github-project-automation github-project-automation bot moved this from In Review to Done in Project Tracking May 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-db Area: database W-supervisor Workstream: supervisor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(supervisor/storage): Implement ChainDB
4 participants
0