new compy#1025
Conversation
|
/run-security-scan |
alexcos20
left a comment
There was a problem hiding this comment.
AI automated code review (Gemini 3).
Overall risk: low
Summary:
This is a solid and well-executed PR migrating GrantsToken to an upgradeable UUPS proxy pattern and adding allowlist/admin-burn capabilities. The transition to Initializable and UUPSUpgradeable follows OpenZeppelin best practices (such as _disableInitializers() in the constructor and correct use of __gap storage). The swap-and-pop allowlist implementation is highly efficient. I have left one minor note regarding a redundant override, but overall the code is clean, robust, and production-ready. LGTM!
Comments:
• [WARNING][security] The adminBurnFrom function introduces a significant centralization risk as the owner can unilaterally destroy tokens from any account. While the PR description clarifies this is an intentional fraud remediation feature, ensure the multisig controlling the owner role has strict operational security in place.
• [INFO][style] The cap() function is already defined as public view virtual in ERC20CappedUpgradeable. Overriding it just to return super.cap() is redundant and can be safely removed to keep the contract leaner.
- function cap() public view override(ERC20CappedUpgradeable) returns (uint256) {
- return super.cap();
- }• [INFO][other] Excellent use of uint256[47] private __gap;. This perfectly reserves 50 slots (accounting for the 3 slots used by _allowlist, _isAllowlisted, and _allowlistIndex, since the constant _DECIMALS does not occupy a storage slot) for future upgrades, following best practices for upgradeable contracts.
• [INFO][performance] Great use of the swap-and-pop algorithm here for array removal. This ensures O(1) gas complexity for removing elements from the allowlist, preventing potential out-of-gas issues for a growing array.
| mapping(address => bool) private _isAllowlisted; | ||
| mapping(address => uint256) private _allowlistIndex; // 1-based | ||
|
|
||
| uint256[47] private __gap; |
| mapping(address => bool) private _isAllowlisted; | ||
| mapping(address => uint256) private _allowlistIndex; // 1-based | ||
|
|
||
| uint256[47] private __gap; |
new compy
🚨 Report Summary
For more details view the full report in OpenZeppelin Code Inspector |
Fixes # .
Changes proposed in this PR: