fix: ignore key-not-found errors in testWitness #1164
Conversation
WalkthroughThe changes modify error handling in the Changes
Sequence Diagram(s)sequenceDiagram
participant T as testWitness
participant R as rawdb.ReadDiskStateRoot
T->>R: Request state root
alt Error occurs
R-->>T: Return error
alt Error is leveldb.ErrNotFound
T->>T: Ignore error, continue processing
else Other error
T->>T: Return error and halt processing
end
else Successful read
R-->>T: Return state root
end
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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)
eth/api.go (1)
373-374: Consider updating the FIXME commentThere's a FIXME comment above the testWitness usage that mentions occasional state root mismatches. Since you're modifying the error handling in testWitness, consider updating this comment to reflect that key-not-found errors are now being handled.
-// FIXME: testWitness will fail from time to time, the problem is caused by occasional state root mismatch -// after processing the block based on witness. We need to investigate the root cause and fix it. +// FIXME: testWitness will fail from time to time, the problem is caused by occasional state root mismatch +// after processing the block based on witness. Key-not-found errors are now ignored, but we still need +// to investigate the root cause of other mismatches and fix them.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
eth/api.go(3 hunks)params/version.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build-and-push
- GitHub Check: Analyze (go)
🔇 Additional comments (4)
params/version.go (1)
27-27: Appropriate version bump for bug fixThe patch version increment from 34 to 35 is consistent with semantic versioning for a bug fix change that doesn't alter the API.
eth/api.go (3)
47-47: Properly added required import for error handlingThe leveldb package import is correctly added to support the new error checking functionality.
388-389: Appropriate handling of key-not-found errorsThe modification to error handling now correctly allows execution to continue when a state root key is not found in the database (leveldb.ErrNotFound), while still properly handling other error types. This addresses the issue mentioned in the PR title.
413-414: Consistent error handling for post-state rootThe same error handling pattern is appropriately applied to the post-state root check, ensuring consistent behavior throughout the function. This correctly implements the PR's goal of ignoring key-not-found errors in testWitness.
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
Bug Fixes
Chores