New Release#494
Conversation
* fix: activate deals versions for calibration * feat: add bad address error type * feat: clear bad address cache
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @ziscky, 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 focuses on enhancing the robustness and maintainability of the codebase by refining error handling mechanisms, centralizing common utility functions, and updating network version-specific logic. The changes aim to improve the clarity of error propagation and streamline code paths related to actor information retrieval and deal activation processing.
Highlights
- Improved Error Handling: Introduced a new error type, ErrBadAddress, and updated existing error returns to wrap this new error, allowing for more precise error identification and handling.
- Refactored Actor Name Retrieval: Extracted the logic for retrieving actor names from addresses into a new common utility function,
GetActorNameFromAddress, which also includes logic to clear the bad address cache upon encounteringErrBadAddress. - Updated Deal Activation Logic: Adjusted the version checks for parsing
ActivateDealsto correctly reflect changes for both Mainnet (NV21) and Calibration (NV20) networks, and streamlined the parsing flow. - Logging Level Adjustment: Changed a debug log to an error log when unable to retrieve actor code from the node, ensuring critical failures are more visible.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a new error, ErrBadAddress, to handle cases where an actor address cannot be found. It also refactors several parts of the code to use a new common utility function, GetActorNameFromAddress, which centralizes the logic for retrieving actor names and includes a mechanism to handle these bad addresses by clearing a cache. Additionally, there are updates to network version-specific logic for deal activations.
My review focuses on improving the consistency of error messages and suggesting a more efficient caching strategy for bad addresses. The changes are generally good, promoting better error handling and code reuse.
| // Check if this address is flagged as bad | ||
| if a.isBadAddress(add) { | ||
| return "", fmt.Errorf("address %s is flagged as bad", addStr) | ||
| return "", fmt.Errorf(" %w : %s is flagged as bad", ErrBadAddress, addStr) |
There was a problem hiding this comment.
The formatting of this error message is inconsistent with the one used in GetRobustAddress on line 193. The leading space and spaces around the colon are unconventional for wrapped errors. For consistency and better readability, please adopt the format used elsewhere.
Using a consistent format like "%w: address %s is flagged as bad" makes error messages easier to parse and handle programmatically.
| return "", fmt.Errorf(" %w : %s is flagged as bad", ErrBadAddress, addStr) | |
| return "", fmt.Errorf("%w: address %s is flagged as bad", ErrBadAddress, addStr) |
| if errors.Is(err, cache.ErrBadAddress) { | ||
| // the bad address may have been set due to a previous failed request to the node | ||
| // clear the cache to allow trying again when reprocessed | ||
| helper.GetActorsCache().ClearBadAddressCache() | ||
| } |
There was a problem hiding this comment.
Clearing the entire badAddress cache with ClearBadAddressCache() is a very broad action to take when a single address lookup fails. This could lead to performance issues by forcing re-verification of all addresses previously marked as bad, even if they are still invalid. A more precise approach would be to remove only the specific address that failed.
I suggest adding a RemoveBadAddress(address string) method to the IActorsCache interface and its implementation (ActorsCache). You can then use it here to target only the problematic address. This will make the cache invalidation more efficient.
| if errors.Is(err, cache.ErrBadAddress) { | |
| // the bad address may have been set due to a previous failed request to the node | |
| // clear the cache to allow trying again when reprocessed | |
| helper.GetActorsCache().ClearBadAddressCache() | |
| } | |
| if errors.Is(err, cache.ErrBadAddress) { | |
| // the bad address may have been set due to a previous failed request to the node | |
| // remove the address from the bad address cache to allow trying again when reprocessed | |
| helper.GetActorsCache().RemoveBadAddress(addr.String()) | |
| } |
2 participants
🔗 zboto Link