-
Notifications
You must be signed in to change notification settings - Fork 23
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
fix(worker): switch trie backend only after migration is completed #141
Conversation
WalkthroughThe pull request introduces changes to two files: Changes
Sequence DiagramsequenceDiagram
participant Migrator
participant Worker
participant Header
participant StateDB
Migrator->>Migrator: Process safe block
Worker->>Header: Check timestamp
Header-->>Worker: Timestamp details
Worker->>StateDB: Determine migration status
StateDB-->>Worker: Migration state
Worker->>Worker: Configure Zktrie settings
The sequence diagram illustrates the high-level interaction between the migrator, worker, header, and state database during the migration and block processing workflow. Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
6e28ebc
to
c2dc5a2
Compare
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
migration/migrator.go (1)
115-115
: Consider handling migration errors more robustly.
If this transition fails, the logic merely logs it. Either retry the migration or propagate the error if partial failures require attention.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
migration/migrator.go
(1 hunks)miner/worker.go
(1 hunks)
🔇 Additional comments (4)
migration/migrator.go (3)
107-107
: Use of CurrentSafeBlock()
is appropriate.
This makes it clear that subsequent migration steps are based on the latest safe block.
109-109
: Validate boundary condition for skipping migration.
If safeBlock
is nil
or the migrated reference is at or beyond the safe block, the code continues to the next iteration. Ensure it’s intended to skip in these exact conditions (e.g., consider if you need an off-by-one comparison).
112-112
: Exiting on Kroma MPT activation is consistent.
This early return avoids performing additional migrations after MPT is enabled.
miner/worker.go (1)
750-751
: Toggling the chain config at runtime.
Recording Zktrie = false
directly in the chain config might affect downstream consumers of the config. Confirm there’s no concurrency risk from other goroutines or chain config usage patterns.
Would you like to implement a safer approach (e.g., a local-only override) if toggling global configs is not absolutely required?
No description provided.