refactor: replace generic db with enum in ChainIndex#7044
Conversation
WalkthroughRefactors the codebase to introduce a concrete DbImpl delegating enum, remove DB generics from core types (ChainIndex, VM/FVM externs, StateManager), change StateTree to accept stores directly with ShallowClone, and update tooling, RPC, and migration APIs and trait delegations. ChangesDatabase Abstraction and Type System Refactoring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/db/db_impl.rs (1)
17-31: ⚡ Quick winAdd documentation for the public
DbImplenum.The
DbImplenum is a central public type in this refactor but lacks doc comments. As per coding guidelines, public structs and enums should be documented.📝 Suggested documentation
+/// Concrete database implementation enum that delegates operations to underlying backends. +/// +/// This type replaces generic database parameters throughout the codebase, providing +/// a unified interface to various storage backends including ManyCar, ParityDb, MemoryDB, +/// and specialized wrappers like ReadOpsTrackingStore. #[derive(Delegate)] #[delegate(SettingsStore)]As per coding guidelines: Document public functions and structs with doc comments.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/db/db_impl.rs` around lines 17 - 31, Add a doc comment for the public enum DbImpl describing its role as the central database implementation selector used throughout the crate, explain that each variant wraps a concrete store type (ManyCarWithGarbageCollectableParityDb, ManyCarWithMemoryDB, ManyCarParityDb, Memory, ReadOpsTrackingManyCarParityDb and the test-only Chain4U) and note any important behavior (e.g., which variants are used for testing or track read ops). Place the comment immediately above the pub enum DbImpl declaration and keep it concise but informative for users and maintainers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/db/db_impl.rs`:
- Around line 17-31: Add a doc comment for the public enum DbImpl describing its
role as the central database implementation selector used throughout the crate,
explain that each variant wraps a concrete store type
(ManyCarWithGarbageCollectableParityDb, ManyCarWithMemoryDB, ManyCarParityDb,
Memory, ReadOpsTrackingManyCarParityDb and the test-only Chain4U) and note any
important behavior (e.g., which variants are used for testing or track read
ops). Place the comment immediately above the pub enum DbImpl declaration and
keep it concise but informative for users and maintainers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7f4efb01-0274-4742-8203-4beb77353225
📒 Files selected for processing (6)
src/blocks/chain4u.rssrc/chain/store/chain_store.rssrc/chain/store/tipset_tracker.rssrc/db/db_impl.rssrc/libp2p/service.rssrc/libp2p_bitswap/store.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/blocks/chain4u.rs
|
Superseded by #7051 |
Summary of changes
(ChainStore, StateManager, etc. are updated in #7051 we could either review and merge this one first, or discard this one)
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes
Other information and links
Change checklist
Outside contributions
Summary by CodeRabbit
Refactor
Chores