Conversation
🛡️ Immunefi PR ReviewsWe noticed that your project isn't set up for automatic code reviews. If you'd like this PR reviewed by the Immunefi team, you can request it manually using the link below: Once submitted, we'll take care of assigning a reviewer and follow up here. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef3da8a278
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| address ownerAddress, | ||
| uint256 ownerRoles | ||
| ) HCAEquivalence(hcaFactory) MetadataMixin(metadata) { | ||
| LABEL_STORE = labelStore; |
There was a problem hiding this comment.
Enforce a contract label store in constructor
register() now relies entirely on LABEL_STORE.setLabel(label) for label-size validation, but the constructor stores labelStore without checking that it is a deployed contract. If this is misconfigured to address(0) or an EOA, the external call succeeds as a no-op and empty/oversized labels stop reverting (a regression from the previous in-contract NameCoder.assertLabelSize check), so registration invariants silently weaken in those deployments.
Useful? React with 👍 / 👎.
|
|
||
| assertEq(labelStore.getLabel(LibLabel.withVersion(labelId, version)), "", "before"); | ||
|
|
||
| vm.expectEmit(); |
There was a problem hiding this comment.
Replace expectEmit with log inspection in new event tests
The new event assertion uses vm.expectEmit, but the repository guideline in /workspace/contracts-v2/AGENTS.md explicitly requires vm.recordLogs with explicit log checks for event testing. Keeping expectEmit here risks under-specified assertions (e.g., emitter/topic matching pitfalls) and violates the project’s mandated testing pattern for event correctness.
Useful? React with 👍 / 👎.
ILabelStoreevent Label(bytes32 labelHash, string label)LabelStoresetLabel(label)andgetLabel(anyId) returns (label)PermissionedRegistry,UserRegistry, and related constructorssetParent()— decided against