-
Notifications
You must be signed in to change notification settings - Fork 32
Add ERC7984Rwa extension
#160
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ Deploy Preview for confidential-tokens ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
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 Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds ERC7984Rwa: a confidential RWA extension (interface + abstract implementation) with admin/agent roles, pause/block controls, freezing, confidential mint/burn, and forced transfers. Refactors ERC7984 Restricted/Freezable internals, adds mocks, tests, interface helpers, and a docs changeset. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Admin as Admin
participant Agent as Agent
participant RWA as ERC7984Rwa
participant FHE as FHEVM
rect rgb(245,248,255)
note over Admin,RWA: Role setup
Admin->>RWA: addAgent(agent)
RWA-->>Admin: ok
end
rect rgb(245,255,245)
note over Agent,RWA: Pause control
Agent->>RWA: pause()
RWA-->>Agent: ok
Agent->>RWA: unpause()
RWA-->>Agent: ok
end
rect rgb(255,248,240)
note over Agent,RWA: Confidential mint (with proof)
Agent->>FHE: build encryptedAmount + proof
FHE-->>Agent: externalEuint64, proof
Agent->>RWA: confidentialMint(to, extAmt, proof)
RWA-->>Agent: euint64 minted
end
rect rgb(255,245,250)
note over Agent,RWA: Forced transfer (no proof)
Agent->>RWA: forceConfidentialTransferFrom(from,to,eAmt)
alt compliant and not paused
RWA-->>Agent: euint64 transferred
else error
RWA-->>Agent: revert
end
end
sequenceDiagram
autonumber
actor Agent as Agent
participant RWA as ERC7984Rwa
participant Restr as ERC7984Restricted
participant Freez as ERC7984Freezable
note over Agent,RWA: Transfer flow with restrictions and freezing
Agent->>RWA: confidentialTransfer(from,to,eAmt)
activate RWA
RWA->>Restr: _checkRestrictionFrom(from)
RWA->>Restr: _checkRestrictionTo(to)
RWA->>Freez: _getUpdateAmountIfNotExceedingFrozenFrom(from,eAmt)
Freez-->>RWA: eAmt' (possibly reduced)
RWA->>RWA: _preTransferCheck(...)
RWA->>RWA: super._update(from,to,eAmt')
RWA->>Freez: _refreshFrozenFrom(from,transferred)
RWA-->>Agent: euint64 transferred
deactivate RWA
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
contracts/mocks/token/ERC7984RwaMock.sol (2)
13-13: Remove unused private state to cut bytecode/gas
_frozenBalancesis declared but never read/written. Drop it to reduce bytecode size.- mapping(address account => euint64 encryptedAmount) private _frozenBalances;
26-29: Optional: return explicitly for readabilityBeing explicit can aid reviewers and static analyzers; no behavior change.
function createEncryptedAmount(uint64 amount) public returns (euint64 encryptedAmount) { FHE.allowThis(encryptedAmount = FHE.asEuint64(amount)); FHE.allow(encryptedAmount, msg.sender); + return encryptedAmount; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
contracts/mocks/token/ERC7984RwaMock.sol(1 hunks)contracts/token/ERC7984/extensions/ERC7984Rwa.sol(1 hunks)test/token/ERC7984/extensions/ERC7984Rwa.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- test/token/ERC7984/extensions/ERC7984Rwa.test.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-22T09:21:34.438Z
Learnt from: james-toussaint
PR: OpenZeppelin/openzeppelin-confidential-contracts#160
File: test/token/ERC7984/extensions/ERC7984Rwa.test.ts:474-479
Timestamp: 2025-09-22T09:21:34.438Z
Learning: For force transfers in ERC7984Freezable, the frozen balance should be reset to the new balance if the transfer amount exceeded the available balance. If the transfer amount was within the available balance, the frozen amount behavior needs clarification from the user.
Applied to files:
contracts/token/ERC7984/extensions/ERC7984Rwa.sol
📚 Learning: 2025-09-22T09:21:34.438Z
Learnt from: james-toussaint
PR: OpenZeppelin/openzeppelin-confidential-contracts#160
File: test/token/ERC7984/extensions/ERC7984Rwa.test.ts:474-479
Timestamp: 2025-09-22T09:21:34.438Z
Learning: In ERC7984Freezable force transfers, the frozen balance is reset to the new balance only when the transferred amount exceeds the available balance (balance - frozen). If the transferred amount is within the available balance, the frozen amount remains unchanged. This is implemented via FHE.select(FHE.gt(encryptedAmount, confidentialAvailable(account)), confidentialBalanceOf(account), frozen).
Applied to files:
contracts/token/ERC7984/extensions/ERC7984Rwa.sol
📚 Learning: 2025-09-16T10:32:50.913Z
Learnt from: arr00
PR: OpenZeppelin/openzeppelin-confidential-contracts#186
File: contracts/token/ERC7984/extensions/ERC7984Omnibus.sol:0-0
Timestamp: 2025-09-16T10:32:50.913Z
Learning: Custom errors are supported in require statements starting from Solidity 0.8.26 (via IR pipeline) and 0.8.27 (legacy pipeline). The syntax require(condition, CustomError(param)) is valid and compiles correctly in these versions.
Applied to files:
contracts/mocks/token/ERC7984RwaMock.sol
⏰ 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). (3)
- GitHub Check: boostsecurity - boostsecurityio/semgrep-pro
- GitHub Check: coverage
- GitHub Check: tests
🔇 Additional comments (4)
contracts/mocks/token/ERC7984RwaMock.sol (1)
31-37: Avoid$in public function names (tooling/friction risk)These helpers are public and the
$prefix can break tools and conventions. Prefer canonical names.- function $_setCompliantTransfer() public { + function setCompliantTransfer() public { compliantTransfer = true; } - function $_unsetCompliantTransfer() public { + function unsetCompliantTransfer() public { compliantTransfer = false; } - function $_mint(address to, uint64 amount) public returns (euint64 transferred) { + function mintForTest(address to, uint64 amount) public returns (euint64 transferred) { return _mint(to, FHE.asEuint64(amount)); }If tests rely on the old names, I can PR the updates to the spec files. Want me to push the rename across tests?
Also applies to: 39-41
contracts/token/ERC7984/extensions/ERC7984Rwa.sol (3)
246-251: Compile error:purereadsmsg.sigReading
msg.sigdisallowspure. Make itview.- function _isForceTransfer() private pure returns (bool) { + function _isForceTransfer() private view returns (bool) { return msg.sig == FORCE_CONFIDENTIAL_TRANSFER_FROM_SIG || msg.sig == FORCE_CONFIDENTIAL_TRANSFER_FROM_WITH_PROOF_SIG; }
115-118: Confirmed: compilers configured ≥ 0.8.27 — custom errors in require(...) are supportedHardhat config sets solidity = 0.8.29 and foundry.toml sets solc_version = 0.8.27; ERC7984Rwa.sol pragma is ^0.8.27. No change required.
177-194: Force transfers bypass pause — enforce pause check here
_forceConfidentialTransferFromcallssuper._update, skipping this contract’swhenNotPausedguard on_update. As written, agents can force-transfer while paused.Add an explicit pause check at function start to retain pause semantics:
function _forceConfidentialTransferFrom( address from, address to, euint64 encryptedAmount ) internal virtual returns (euint64 transferred) { + _requireNotPaused(); if (!FHE.isInitialized(encryptedAmount)) { return encryptedAmount; }Optionally document which checks are bypassed (from-side frozen/restriction, compliance) and which remain (pause, to-side restriction).
* Update RWA * fix lint * up * update comment
james-toussaint
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't approve but good to me 👍
Summary by Sourcery
Add ERC7984Rwa extension to support confidential Real World Assets with compliance enforcement, access control, pausing, freezing, and force transfer capabilities, along with interfaces, mock implementation, and extensive testing support
New Features:
Enhancements:
Tests:
Summary by CodeRabbit