-
Notifications
You must be signed in to change notification settings - Fork 137
proof: improve re-org proof replacement, strengthen error handling, expand test coverage #1896
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
base: main
Are you sure you want to change the base?
proof: improve re-org proof replacement, strengthen error handling, expand test coverage #1896
Conversation
- Replace ReplaceProofInBlob with pure ReplaceProofInFiles that updates provided proof blobs and validates asset IDs. - Update re-org watcher and planter to fetch candidate proofs and explicitly import updates using their verifier context. Why this helps: - The replacement logic is now side-effect free and focused on transformation, making it easier to unit test. - Fetching and importing responsibilities are explicit at call sites, improving clarity and making state changes easier to reason about.
Summary of ChangesHello @ffranr, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the robustness and clarity of proof replacement mechanisms, particularly during blockchain re-organizations. By introducing a pure function for proof replacement and enforcing explicit error handling, it ensures that the system behaves more reliably and predictably when updating proofs, preventing critical errors from being overlooked. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request is a great improvement. It refactors the proof replacement logic into a pure function, which improves testability and separation of concerns. The error handling has been significantly strengthened by adding more context to error messages, which will make debugging much easier. The new unit tests are also a welcome addition, ensuring the new functionality is well-covered.
I've found one minor style issue that I've commented on. Otherwise, the changes look solid.
Pull Request Test Coverage Report for Build 19935475562Details
💛 - Coveralls |
jtobin
left a comment
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.
- The replacement logic is now side-effect free and focused on transformation, making it easier to unit test. - Fetching and importing responsibilities are explicit at call sites, improving clarity and making state changes easier to reason about.
😍
Ensure that proof decode errors and other issues are no longer ignored. Adds proper error propagation to make failures visible and easier to debug.
Add unit test coverage for `ReplaceProofInFiles`.
487aa3f to
c7eb0cc
Compare
This PR tightens the proof replacement path used during block re-org handling. It explicitly ensures that we no longer ignore errors that should be surfaced, giving more reliable behaviour during re-org updates.
Key changes
ReplaceProofInFilesfunction that updates supplied proof files and validates asset IDs.