fix: an attempt at making witness generation more stable #1166
Conversation
WalkthroughThis pull request updates the prefetcher initialization across multiple modules. In the blockchain and rollup pipeline, the second argument of the Changes
Sequence Diagram(s)sequenceDiagram
participant API as generateWitness
participant StateDB as StateDB
participant W as Witness
API->>StateDB: WithWitness(W)
API->>StateDB: Process block & validate state
sequenceDiagram
participant Caller as Chain/Pipeline Method
participant StateDB as StateDB
Caller->>StateDB: StartPrefetcher(namespace)
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
core/blockchain.go (1)
1839-1841: Update Prefetcher Tag in BuildAndWriteBlockThe call to
statedb.StartPrefetcher("l1sync")in theBuildAndWriteBlockfunction now explicitly uses the"l1sync"label instead of the label used in other contexts (e.g."chain"in the insertion path). This appears to be part of the effort to decouple witness generation from regular prefetching. Please verify that:
- The
"l1sync"identifier triggers the prefetcher logic appropriate for witness-related operations.- This change is consistent with the separate witness-handling mechanism (i.e. using the new
WithWitnessmethod in the state database) and does not lead to unintended cache or state inconsistencies.If this new configuration requires any documentation updates or downstream changes in how prefetcher results are consumed, please address those as well.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
core/blockchain.go(2 hunks)core/state/statedb.go(1 hunks)eth/api.go(1 hunks)rollup/pipeline/pipeline.go(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
core/state/statedb.go (2)
core/vm/interface.go (1)
StateDB(28-88)core/stateless/witness.go (1)
Witness(38-47)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build-and-push
- GitHub Check: test
- GitHub Check: Analyze (go)
🔇 Additional comments (4)
eth/api.go (1)
357-357: Good refactoring to separate witness association from prefetcher initialization.This change improves the design by making the witness association with the state DB more explicit through a dedicated method call, rather than coupling it with prefetcher initialization. This separation of concerns helps make witness generation more stable, which aligns with the PR objective.
rollup/pipeline/pipeline.go (1)
231-231: Properly updated to match the new StartPrefetcher signature.This change correctly removes the second argument (previously
nil) from the StartPrefetcher call to align with the updated method signature in the StateDB struct, where witness handling has been decoupled from prefetcher initialization.core/state/statedb.go (2)
163-166: Good addition of a dedicated method for witness association.Adding the
WithWitnessmethod provides a clear way to associate a witness with the state database. This improves code clarity and follows the principle of separation of concerns by decoupling witness handling from prefetcher initialization.
171-171: Appropriate signature update to remove witness handling from prefetcher initialization.Removing the witness parameter from the StartPrefetcher method is a good design change that simplifies the prefetcher's responsibilities. This change is consistent with the addition of the dedicated WithWitness method and contributes to making witness generation more stable.
1. Purpose or design rationale of this PR
...
2. PR title
Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:
3. Deployment tag versioning
Has the version in
params/version.gobeen updated?4. Breaking change label
Does this PR have the
breaking-changelabel?Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores