Validate hex addresses #801
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
|
""" WalkthroughThe validation logic for Ethereum contract addresses in the configuration was updated. A new function, Changes
Sequence Diagram(s)sequenceDiagram
participant ConfigValidator
participant HexAddressValidator
participant EthereumCommon
ConfigValidator->>HexAddressValidator: validateHexAddress(address, fieldName, set)
HexAddressValidator->>EthereumCommon: IsHexAddress(address)
EthereumCommon-->>HexAddressValidator: returns true/false
HexAddressValidator->>EthereumCommon: HexToAddress(address)
EthereumCommon-->>HexAddressValidator: returns Address
HexAddressValidator->>ConfigValidator: Add error to set if invalid
""" Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (1.64.8)Error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2 📜 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 (4)
✨ 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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
fbac
force-pushed
the
05-16-validate_chain_addresses
branch
from
11a7665 to
c9cc25f
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🔭 Outside diff range comments (3)
pkg/config/validation.go (3)
98-102:⚠️ Potential issueError return value is not checked
The error return value from
validateHexAddressis not being checked here, which is also flagged by the linter. Either handle the error appropriately or modify the function to not return an error if it's not intended to be used.- validateHexAddress( + _ = validateHexAddress( options.Contracts.SettlementChain.RateRegistryAddress, "contracts.settlement-chain.rate-registry-address", missingSet, )🧰 Tools
🪛 GitHub Check: Lint-Go
[failure] 98-98:
Error return value is not checked (errcheck)🪛 GitHub Actions: Lint
[error] 98-98: Error return value is not checked (errcheck)
131-135:⚠️ Potential issueError return value is not checked
The error return value from
validateHexAddressis not being checked, as flagged by the linter. Either handle the error or explicitly discard it.- validateHexAddress( + _ = validateHexAddress( options.Contracts.AppChain.IdentityUpdateBroadcasterAddress, "contracts.app-chain.identity-update-broadcaster-address", missingSet, )🧰 Tools
🪛 GitHub Check: Lint-Go
[failure] 131-131:
Error return value is not checked (errcheck)
158-162:⚠️ Potential issueError return value is not checked
Similar to previous instances, the error return value from
validateHexAddressis not being checked here.- validateHexAddress( + _ = validateHexAddress( options.Contracts.SettlementChain.NodeRegistryAddress, "contracts.settlement-chain.node-registry-address", missingSet, )
🛑 Comments failed to post (2)
pkg/config/validation.go (2)
126-130:
⚠️ Potential issueError return value is not checked
The error return value from
validateHexAddressis not being checked, as indicated by the static analysis tool. Either handle the error or explicitly ignore it.- validateHexAddress( + _ = validateHexAddress( options.Contracts.AppChain.GroupMessageBroadcasterAddress, "contracts.app-chain.group-message-broadcaster-address", missingSet, )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements._ = validateHexAddress( options.Contracts.AppChain.GroupMessageBroadcasterAddress, "contracts.app-chain.group-message-broadcaster-address", missingSet, )🧰 Tools
🪛 GitHub Check: Lint-Go
[failure] 126-126:
Error return value is not checked (errcheck)🤖 Prompt for AI Agents
In pkg/config/validation.go around lines 126 to 130, the call to validateHexAddress returns an error that is currently not checked. Modify the code to capture the error return value and handle it appropriately, such as returning the error from the current function or logging it. If the error can be safely ignored, explicitly ignore it using the blank identifier. This ensures proper error handling and satisfies the static analysis tool.
221-229: 🛠️ Refactor suggestion
Consider removing the error return if it's not being used
The
validateHexAddressfunction returns an error, but none of the callers check this return value. Since all error conditions are already handled by adding to the providedsetmap parameter, consider simplifying the function signature by removing the error return.-func validateHexAddress(address string, fieldName string, set map[string]struct{}) error { +func validateHexAddress(address string, fieldName string, set map[string]struct{}) { if address == "" { set[fmt.Sprintf("--%s is required", fieldName)] = struct{}{} } if !common.IsHexAddress(address) || common.HexToAddress(address) == (common.Address{}) { set[fmt.Sprintf("--%s is invalid", fieldName)] = struct{}{} } - return nil }Alternatively, if you want to retain the error return for future use:
func validateHexAddress(address string, fieldName string, set map[string]struct{}) error { if address == "" { set[fmt.Sprintf("--%s is required", fieldName)] = struct{}{} + return fmt.Errorf("address is empty") } if !common.IsHexAddress(address) || common.HexToAddress(address) == (common.Address{}) { set[fmt.Sprintf("--%s is invalid", fieldName)] = struct{}{} + return fmt.Errorf("address is invalid") } return nil }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.func validateHexAddress(address string, fieldName string, set map[string]struct{}) { if address == "" { set[fmt.Sprintf("--%s is required", fieldName)] = struct{}{} } if !common.IsHexAddress(address) || common.HexToAddress(address) == (common.Address{}) { set[fmt.Sprintf("--%s is invalid", fieldName)] = struct{}{} } }func validateHexAddress(address string, fieldName string, set map[string]struct{}) error { if address == "" { set[fmt.Sprintf("--%s is required", fieldName)] = struct{}{} return fmt.Errorf("address is empty") } if !common.IsHexAddress(address) || common.HexToAddress(address) == (common.Address{}) { set[fmt.Sprintf("--%s is invalid", fieldName)] = struct{}{} return fmt.Errorf("address is invalid") } return nil }🤖 Prompt for AI Agents
In pkg/config/validation.go around lines 221 to 229, the function validateHexAddress returns an error but never actually returns any error value, and callers do not check this return. Remove the error return type from the function signature and eliminate the return statement to simplify the function, since all error conditions are handled by updating the provided set map.
Add hex address validation for blockchain contract addresses in config validationIntroduces a new
This validation is now applied to rate registry, group message broadcaster, identity update broadcaster, and node registry contract addresses in both app chain and settlement chain configurations. 📍Where to StartStart with the new Macroscope summarized c9cc25f. |
Add Ethereum address validation to ensure valid hex and non-zero contract addresses in configuration validation
Introduces a new
validateHexAddress()function in validation.go that performs hex address validation for contract addresses. The function validates that addresses are non-empty, valid hex format, and not zero addresses using thego-ethereum/commonpackage. This validation is applied toRateRegistryAddress,GroupMessageBroadcasterAddress,IdentityUpdateBroadcasterAddress, andNodeRegistryAddressconfiguration fields.📍Where to Start
Start with the new
validateHexAddress()function in validation.go which contains the core validation logic for Ethereum addresses.Macroscope summarized 11a7665.
Summary by CodeRabbit
Bug Fixes
Chores