New Release#483
Conversation
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 prepares for a new release by addressing two minor issues. It corrects a type definition in the verified registry actor parameters and enhances the verification registry's parsing logic to correctly handle provider addresses that might be represented as floating-point numbers when unmarshalled from JSON.
Highlights
- Actor Parameter Correction: Corrected the expected return type for V15 in the removedVerifiedClientDataCapReturn map within actors/v2/verifiedRegistry/params.go from RemoveDataCapParams to RemoveDataCapReturn.
- Provider Address Parsing Improvement: Enhanced the parserUniversalReceiverHook in tools/verifreg/parse.go to correctly handle provider addresses that are unmarshalled as float64 from JSON, converting them to uint64 for consolidation. This change also streamlines the handling of numeric ID addresses.
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 contains two fixes. One is a typo fix in verifiedRegistry/params.go where a Params type was used instead of a Return type, which is correct. The other change in tools/verifreg/parse.go addresses an issue with parsing provider addresses from JSON. While the change correctly identifies that numeric types from JSON are unmarshalled as float64, the current implementation has a critical flaw that can lead to data corruption for large address IDs due to precision loss in float64. I've added a detailed comment on how to address this.
| case float64: | ||
| addr, err = common.ConsolidateIDAddress(uint64(provider), eg.helper, eg.logger, eg.config) |
There was a problem hiding this comment.
While this change correctly handles numeric provider IDs from JSON as float64, it introduces a significant risk of data corruption due to precision loss.
A float64 can only represent integers up to 2^53 precisely. Actor IDs are uint64 and can be much larger. When a large integer ID is unmarshalled from JSON into an interface{}, it becomes a float64 and loses precision. Casting this back to uint64 will result in an incorrect ID.
Example of failure:
An ID like 9007199254740993 (which is 2^53 + 1) will be read as 9007199254740992 from JSON into a float64, silently corrupting the data.
Recommendation:
To fix this robustly, you should avoid float64 conversion altogether. The recommended approach is to use json.Number during JSON decoding. This can be enabled by using json.Decoder.UseNumber() in ParseFRC46TransactionMetadata.
Then, you can handle json.Number in this switch and safely convert it to uint64:
import (
"strconv"
"encoding/json"
)
// ... in switch
case json.Number:
id, err := strconv.ParseUint(string(provider), 10, 64)
if err != nil {
return "", "", nil, fmt.Errorf("could not parse provider ID from json.Number: %w", err)
}
addr, err = common.ConsolidateIDAddress(id, eg.helper, eg.logger, eg.config)This is a critical issue that should be addressed to prevent data corruption.
🔗 zboto Link