Skip to content

Conversation

@arr00
Copy link
Contributor

@arr00 arr00 commented Nov 10, 2025

Summary by CodeRabbit

  • Refactor
    • Enhanced token operation workflows for mint, burn, and force transfer functions with additional authorization validation steps. Internal improvements maintain existing functionality with no changes to public interfaces.

@arr00 arr00 requested a review from a team as a code owner November 10, 2025 14:37
@netlify
Copy link

netlify bot commented Nov 10, 2025

Deploy Preview for confidential-tokens ready!

Name Link
🔨 Latest commit 0282c9c
🔍 Latest deploy log https://app.netlify.com/projects/confidential-tokens/deploys/69149e92d3843c0008551fe7
😎 Deploy Preview https://deploy-preview-242--confidential-tokens.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 10, 2025

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The ERC7984Rwa.sol contract now stores intermediate operation results in local variables before returning them, allowing FHE.allow() authorization calls to be invoked on mint, burn, and force transfer operations across all method variants without changing public signatures.

Changes

Cohort / File(s) Summary
FHE Authorization Pattern
contracts/token/ERC7984/extensions/ERC7984Rwa.sol
Modified mint, burn, and force transfer methods (including "with proof" variants) to capture operation results in local variables, invoke FHE.allow() on each result for authorization, then return the variable. Control flow unchanged; adds post-operation FHE authorization layer.

Sequence Diagram(s)

sequenceDiagram
    participant Caller
    participant ERC7984Rwa
    participant FHE

    Caller->>ERC7984Rwa: mint/burn/forceTransfer
    ERC7984Rwa->>ERC7984Rwa: _mint/_burn/_forceUpdate
    ERC7984Rwa->>ERC7984Rwa: Store result in local var
    Note over ERC7984Rwa: mintedAmount / burnedAmount / forceTransferAmount
    ERC7984Rwa->>FHE: allow(resultVar)
    FHE-->>ERC7984Rwa: Authorization recorded
    ERC7984Rwa-->>Caller: return resultVar
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Note: Highly repetitive pattern applied consistently across multiple method variants; single file modification with no signature changes; straightforward variable capture and FHE authorization logic makes this a straightforward review despite multiple affected methods.

Poem

🐰 A rabbit hops through FHE's gated garden,
Each mint, each burn, now seeks permission first,
Local vars dance, then authorization's granted—
No signatures changed, just trust deepened. 🔐

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'M-03: grant allowances to agent in ERC7984Rwa' directly and specifically describes the main change: granting allowances to an agent in the ERC7984Rwa contract by invoking FHE.allow() after mint, burn, and force transfer operations.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (1)
contracts/token/ERC7984/extensions/ERC7984Rwa.sol (1)

183-185: Allowance pattern correctly applied to force transfer variant.

The implementation properly grants allowances to the agent.

Note: The named return variable transferred (line 178) is declared but forceTransferAmount is used instead. While functionally correct, consider using the named return variable for consistency, or remove it from the signature.

Apply this diff for naming consistency:

-    ) public virtual onlyAgent returns (euint64 transferred) {
+    ) public virtual onlyAgent returns (euint64) {
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b2c6388 and b8b296c.

📒 Files selected for processing (1)
  • contracts/token/ERC7984/extensions/ERC7984Rwa.sol (6 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: james-toussaint
Repo: OpenZeppelin/openzeppelin-confidential-contracts PR: 160
File: test/token/ERC7984/extensions/ERC7984Rwa.test.ts:474-479
Timestamp: 2025-09-22T09:21:34.470Z
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).
Learnt from: james-toussaint
Repo: OpenZeppelin/openzeppelin-confidential-contracts PR: 160
File: test/token/ERC7984/extensions/ERC7984Rwa.test.ts:474-479
Timestamp: 2025-09-22T09:21:34.470Z
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.
📚 Learning: 2025-09-22T09:21:34.470Z
Learnt from: james-toussaint
Repo: OpenZeppelin/openzeppelin-confidential-contracts PR: 160
File: test/token/ERC7984/extensions/ERC7984Rwa.test.ts:474-479
Timestamp: 2025-09-22T09:21:34.470Z
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-22T09:21:34.470Z
Learnt from: james-toussaint
Repo: OpenZeppelin/openzeppelin-confidential-contracts PR: 160
File: test/token/ERC7984/extensions/ERC7984Rwa.test.ts:474-479
Timestamp: 2025-09-22T09:21:34.470Z
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
⏰ 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: boostsecurity - boostsecurityio/semgrep-pro
  • GitHub Check: slither
  • GitHub Check: tests
  • GitHub Check: coverage
🔇 Additional comments (5)
contracts/token/ERC7984/extensions/ERC7984Rwa.sol (5)

123-125: LGTM: Allowance correctly granted to agent for mint result.

The change properly stores the minted amount and grants the agent permission to access the encrypted result before returning. This allows the agent to monitor or verify the operation outcome.


134-136: Consistent implementation across mint variants.

The allowance pattern is correctly applied to the overloaded variant, maintaining consistency with the "with proof" version.


145-147: Burn operation properly grants allowances.

The implementation correctly follows the established pattern for burn operations with proof.


156-158: Consistent allowance handling in burn variant.

The pattern is correctly applied to the overloaded burn function.


168-170: Force transfer correctly grants allowances.

The allowance pattern is properly applied to force transfer operations, enabling agents to monitor enforcement actions.

@arr00 arr00 merged commit b900820 into master Nov 12, 2025
15 checks passed
@arr00 arr00 deleted the fix/give-agent-allowance branch November 12, 2025 16:40
arr00 added a commit that referenced this pull request Nov 12, 2025
* M-03: grant allowances to agent in `ERC7984Rwa`

* up
@coderabbitai coderabbitai bot mentioned this pull request Nov 28, 2025
arr00 added a commit that referenced this pull request Dec 1, 2025
* Start release candidate

* Release v0.3.0 (rc) (#221)

* Release v0.3.0 (rc)

* Update changelog

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: James Toussaint <33313130+james-toussaint@users.noreply.github.com>

* Update `checkOnTransferReceived` doc (#235)

* Versioned Docs (#236)

* generate versioned docs

* publish docs even on pre-release

* N-04: remove unused import in `ERC7984Rwa` (#243)

* N-01: reset user instead of allowing user in `unblockUser` (#244)

* N-05: Named mapping var in `ERC7984ObserverAccess` (#251)

* N-05: Named mapping var in `ERC7984ObserverAccess`

* Update contracts/token/ERC7984/extensions/ERC7984ObserverAccess.sol

Co-authored-by: James Toussaint <33313130+james-toussaint@users.noreply.github.com>

---------

Co-authored-by: James Toussaint <33313130+james-toussaint@users.noreply.github.com>

* N-08: constant names are screaming camel case (#247)

* N-08: constant names are screaming camel case

* fix lint

* N-02: reorder allowances omnibus (#250)

* Support ERC-165 interface detection on ERC-7984 (#246)

* Support ERC-165 interface detection on ERC-7984

* update link format

* Add ERC7984 impl changeset

* Update changeset

---------

Co-authored-by: Arr00 <13561405+arr00@users.noreply.github.com>

* M-03: grant allowances to agent in `ERC7984Rwa` (#242)

* M-03: grant allowances to agent in `ERC7984Rwa`

* up

* N-12: update docs in `ERC7984Restricted` (#245)

* Upgrade to use fhevm contracts v0.9.0 (#248)

* chore: fhevm-v9

* chore: port all tests for fhevm v9

* Merge pull request #1 from OpenZeppelin/chore/update-disclose-flow

update disclose flow

* Update wrapper contract (#2)

* Update wrapper contract

* fix tests

* fix mock

* update docs

* add changeset

* request id unnecessary

* Update contracts/token/ERC7984/extensions/ERC7984ERC20Wrapper.sol

Co-authored-by: James Toussaint <33313130+james-toussaint@users.noreply.github.com>

* remove unused params

* Update test/token/ERC7984/ERC7984.test.ts

Co-authored-by: James Toussaint <33313130+james-toussaint@users.noreply.github.com>

* `cts` -> `handles`

* `cleartext` -> `cleartextAmount`

* Update test/token/ERC7984/extensions/ERC7984Wrapper.test.ts

Co-authored-by: James Toussaint <33313130+james-toussaint@users.noreply.github.com>

* nit

---------

Co-authored-by: 0xalexbel <alexandre.belhoste@zama.ai>
Co-authored-by: James Toussaint <33313130+james-toussaint@users.noreply.github.com>

* N-[9,11]: fix `ERC7984Rwa` docs (#249)

* M-11: fix `ERC7984Rwa` docs

* add docs

* Update contracts/token/ERC7984/extensions/ERC7984Rwa.sol

* L-05: Grant allowances in `confidentialAvailable` (#252)

* L-05: Grant allowances in `confidentialAvailable`

* fix doc

* L-01: `tryDecrease` return initialized value if delta is initialized (#241)

* L-01: `tryDecrease` return initialized value if delta is initialized

* add comment

* Add changeset

* Upgrade to use fhevm contracts v0.9.1 (#254)

* Upgrade to use fhevm contracts v0.9.1

* bump sub package as well

* Update `ERC7984Rwa` docs (#255)

* Exit pre-release (#258)

* Release v0.3.0 (#253)

* Release v0.3.0

* Update changelog (#259)

* Update changelog

* Update CHANGELOG.md

Co-authored-by: James Toussaint <33313130+james-toussaint@users.noreply.github.com>

---------

Co-authored-by: James Toussaint <33313130+james-toussaint@users.noreply.github.com>

* remove duplicate entry

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Arr00 <13561405+arr00@users.noreply.github.com>
Co-authored-by: James Toussaint <33313130+james-toussaint@users.noreply.github.com>

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: James Toussaint <33313130+james-toussaint@users.noreply.github.com>
Co-authored-by: Arr00 <13561405+arr00@users.noreply.github.com>
Co-authored-by: 0xalexbel <alexandre.belhoste@zama.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants