-
Notifications
You must be signed in to change notification settings - Fork 92
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
refactor(rooch-da): restructure backend architecture for OpenDA #3140
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Refactored DAConfig and submission strategies to improve clarity and extensibility. Renamed `DAServerSubmitStrategy` to `DASubmitStrategy`, unified constants, and enhanced documentation. Added `background_submit_interval` at the top `DAConfig` level.
Move `background_submit_interval` assignment to server initialization. This simplifies `ServerBackends` by removing unused fields and centralizing configuration. No functional changes introduced.
Extract backend management logic to a shared `DABackends` struct. This removes duplication and simplifies backend initialization in both `mod.rs` and `server.rs`.
Update `DABackends::build` to accept an optional `DABackendConfig` and streamline DA backend setup. Renamed methods for better clarity, such as `create_background_submitter` to `run_background_submitter`.
Standardize naming by replacing "names" with "identifiers" for backend variables and updating method names to clarify purpose. This improves code readability and consistency across the deployment architecture.
Streamlined DA backend handling by removing the Nop backend and its associated constructs (`is_nop_backend`, `nop_backend` flags). Updated backend initialization and submission logic to handle configurations more effectively without defaulting to a no-op implementation.
Refactor batch submission to utilize `Arc<DABatch>` for improved memory efficiency and concurrent sharing. Adjusted method signatures and implementations to align with the new structure.
Reorganized OpenDA backend to enhance modularity and maintainability. Split `new_operator` into its own function, restructured module imports, and relocated `backend.rs` to `openda_backend.rs`. Improved error handling in segment submission logic.
Revamped the README to include a detailed architecture diagram and explanation for backend development. Clarifies the relationship between components like DAServerActor, OpenDABackendManager, and specific adapters.
Set a default submission strategy using a constant for clarity and consistency. Updated the fallback behavior in `adjust_submit_strategy` and `calculate_submit_threshold` methods to align with this default.
Refactored OpenDA backend by introducing an adapter-based architecture, consolidating repeated logic, and reorganizing code structure. Simplified configurations, removed deprecated files, and added backend prioritization for efficient batch submission.
popcnt1
requested review from
jolestar,
baichuan3 and
wow-sven
as code owners
December 30, 2024 20:44
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
jolestar
approved these changes
Dec 31, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 5 out of 14 changed files in this pull request and generated 1 comment.
Files not reviewed (9)
- crates/rooch-da/src/backend/openda/backend.rs: Evaluated as low risk
- crates/rooch-da/src/backend/openda/operator.rs: Evaluated as low risk
- crates/rooch-da/src/backend/openda/mod.rs: Evaluated as low risk
- crates/rooch-da/src/backend/openda/celestia.rs: Evaluated as low risk
- crates/rooch-da/src/backend/openda/manager.rs: Evaluated as low risk
- crates/rooch-da/src/backend/openda/opendal.rs: Evaluated as low risk
- crates/rooch-store/src/da_store/mod.rs: Evaluated as low risk
- crates/rooch-types/src/da/status.rs: Evaluated as low risk
- crates/rooch-da/src/backend/openda/avail.rs: Evaluated as low risk
Comments suppressed due to low confidence (3)
crates/rooch-da/src/backend/mod.rs:97
- The variable name
max_retires
is misspelled. It should bemax_retries
.
max_retires: None,
crates/rooch-da/src/backend/mod.rs:37
- The method
load_backends_from_configs
should handle the case wheregenesis_namespace
is empty or invalid.
genesis_namespace: String,
crates/rooch-config/src/da_config.rs:34
- Ensure all instances of DAServerSubmitStrategy are updated to DASubmitStrategy.
pub enum DASubmitStrategy {
Trim leading slashes from the namespace when updating storage root in config. Updated tests to validate behavior with and without an existing root path, ensuring consistent namespace handling.
Corrected `max_retires` to `max_retries` across related files and tests. Ensures consistency and prevents potential misconfigurations due to the typo.
Replaces linear priority lookup with a HashMap to optimize backend sorting. Introduces `UNKNOWN_BACKEND_PRIORITY` for unrecognized identifiers to handle edge cases efficiently.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Refactored OpenDA backend by introducing an adapter-based architecture, consolidating repeated logic, and reorganizing code structure. Simplified configurations, removed deprecated files, and added backend prioritization for efficient batch submission.