Update IERC7943, ERC20uRWA and ERC20Restricted#227
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe PR refactors the IERC7943 interface family and ERC20uRWA implementation to replace user-centric terminology with account-based naming, add boolean return values to mutating functions, and standardize function names for consistency across Fungible, NonFungible, and MultiToken variants. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
contracts/token/ERC20/extensions/ERC20uRWA.sol (1)
70-96: Consider renamingwasUserAllowedfor consistency.The implementation is correct and the boolean return value aligns with the interface. However, the local variable
wasUserAllowedon line 90 uses the old terminology while the rest of the code has migrated tocanTransact.♻️ Optional: Rename for consistency
- bool wasUserAllowed = canTransact(from); - if (!wasUserAllowed) _setRestriction(from, Restriction.ALLOWED); + bool couldTransact = canTransact(from); + if (!couldTransact) _setRestriction(from, Restriction.ALLOWED); _update(from, to, amount); // Explicit raw update to bypass all restrictions - if (!wasUserAllowed) _setRestriction(from, restriction); + if (!couldTransact) _setRestriction(from, restriction);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
contracts/interfaces/IERC7943.solcontracts/token/ERC20/extensions/ERC20uRWA.soltest/token/ERC20/extensions/ERC20uRWA.test.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: tests-foundry
- GitHub Check: tests
- GitHub Check: coverage
- GitHub Check: slither
🔇 Additional comments (13)
test/token/ERC20/extensions/ERC20uRWA.test.js (1)
28-34: LGTM! Interface support check correctly updated.The ERC165 interface support test now correctly references
canTransact(address)instead of the oldisUserAllowed(address), aligning with the account-centric API rename inIERC7943Fungible.contracts/token/ERC20/extensions/ERC20uRWA.sol (5)
15-17: LGTM! Documentation clarifies extension scope.The updated comment clearly communicates that derived contracts implementing mint/burn must include 7943-specific logic themselves.
20-24: LGTM! Clean wrapper for API migration.The
canTransactfunction properly delegates to the existingisUserAllowedimplementation, maintaining backwards compatibility while exposing the new EIP-7943 compliant API.
37-39: LGTM! Consistent use of new API.
canTransfernow usescanTransact(from)andcanTransact(to)consistently with the interface rename.
52-57: LGTM! Boolean return added per EIP-7943.The function now returns
trueon success as required by the updated interface specification.
104-108: LGTM! Comment reference updated.The internal comment now correctly references
canTransferterminology.contracts/interfaces/IERC7943.sol (7)
14-21: LGTM! Account-centric terminology applied consistently.The
Frozenevent andERC7943CannotTransacterror correctly useaccountparameter naming, aligning with the EIP-7943 terminology standardization.
23-33: LGTM! Error definitions are comprehensive.The new errors
ERC7943CannotTransferandERC7943InsufficientUnfrozenBalanceprovide clear, actionable information with appropriate parameters for debugging transfer failures.
40-49: LGTM! Boolean returns standardize mutating function signatures.Adding
returns(bool result)toforcedTransferandsetFrozenTokensfollows the common ERC pattern (like ERC20'stransfer) and enables better composability with external contracts.
51-61: LGTM!canTransactreplacesisUserAllowedappropriately.The rename from
isUserAllowedtocanTransactwithaccountparameter is clearer and more semantically accurate for the permission check purpose.
80-99: LGTM! NonFungible interface follows same patterns.The
IERC7943NonFungibleinterface consistently applies account-centric terminology and the error semantics are correctly adapted for NFT context (usingtokenIdinstead ofamountwhere appropriate).
149-171: LGTM! MultiToken interface properly handles both tokenId and amount.The
IERC7943MultiTokeninterface correctly includes bothtokenIdandamountparameters in events and errors, appropriately reflecting ERC-1155 semantics.
179-189: LGTM! MultiToken mutating functions return bool consistently.Both
forcedTransferandsetFrozenTokensreturnbool result, maintaining consistency with the Fungible and NonFungible interfaces.
Sync with latest EIP state
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.