Revert when wrapper is full

[arr00]

This PR fixes an issue where wrapping assumes that the ERC7984 _mint function is successful without any checks. This is not a safe assumption and can result in loss of funds if the _mint fails (would happen if the call results in overflow).

This issue is fixed by checking plaintext state, which allows the contract to infer an upper bound on the confidential total supply. If the confidential total supply may overflow, the wrap transaction is reverted.

Summary by CodeRabbit

• Bug Fixes

  • Added overflow protection for token wrapping operations when handling maximum representable amounts.
  • Improved internal minting logic to ensure precise amount conversions.

• Tests

  • Added test coverage validating maximum amount wrapping scenarios.
  • Added test cases verifying overflow rejection for out-of-bounds amounts.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[netlify]

✅ Deploy Preview for confidential-tokens ready!

Name	Link
🔨 Latest commit	49d3803
🔍 Latest deploy log	https://app.netlify.com/projects/confidential-tokens/deploys/693822efcfa3920008963ffc
😎 Deploy Preview	https://deploy-preview-268--confidential-tokens.netlify.app
📱 Preview on mobile	Toggle 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]

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

Refactored minting logic in ERC7984ERC20Wrapper by re-introducing public accessors (decimals, rate, underlying), adding totalSupply() and internal _mint() functions with overflow checks, and simplifying type conversions by removing FHE wrapping in mint paths. Added tests validating maximum representable amounts and overflow behavior.

Changes

Cohort / File(s)	Summary
ERC7984ERC20Wrapper contract refactoring contracts/token/ERC7984/extensions/ERC7984ERC20Wrapper.sol	Re-introduced public accessors for decimals(), rate(), and underlying(); added totalSupply() returning underlying balance divided by rate; introduced internal _mint() with pre-flight overflow checks; simplified minting by removing FHE.asEuint64() wrapping in wrap and onTransferReceived paths; adjusted type conversions to use SafeCast.toUint64() directly.
Minting overflow tests test/token/ERC7984/extensions/ERC7984Wrapper.test.ts	Added "max amount works" test validating maximum representable amount (2^64 - 1) / 10^6 wraps correctly; added "amount exceeding max fails" test verifying SafeCastOverflowedUintDowncast revert on overflow; both tests support callback and direct wrap paths.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Extra attention to minting logic changes and type conversion removal to ensure no rounding or overflow issues
• Verify totalSupply() calculation and _mint() overflow check logic
• Confirm test coverage adequately validates boundary conditions with maximum uint64 values

Suggested reviewers

• james-toussaint

Poem

🐰 Wrapped tokens hop with grace so bright,
Math precise, conversions tight,
FHE wrapping shed away,
Overflow guards save the day! ✨

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 'Revert when wrapper is full' accurately describes the main change: adding overflow checks that cause the wrapper to revert when maximum capacity is reached.
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.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

contracts/token/ERC7984/extensions/ERC7984ERC20Wrapper.sol (1)

161-164: Consider expanding the comment to clarify the invariant.

The overflow check works because both call sites (wrap and onTransferReceived) transfer the underlying tokens before calling this function. A more descriptive comment could help future maintainers understand this invariant.

 function _mint(address to, uint64 amount) internal virtual {
-    totalSupply(); // check that total supply doesn't overflow
+    // Revert if new total supply would overflow uint64.
+    // Requires: underlying tokens already transferred before this call.
+    totalSupply();
     _mint(to, FHE.asEuint64(amount));
 }

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4824aa4 and 70a1e5e.

📒 Files selected for processing (2)

• contracts/token/ERC7984/extensions/ERC7984ERC20Wrapper.sol (4 hunks)
• test/token/ERC7984/extensions/ERC7984Wrapper.test.ts (1 hunks)

🧰 Additional context used

🧠 Learnings (2)

📚 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/ERC7984ERC20Wrapper.sol

📚 Learning: 2025-09-15T14:43:25.644Z

Learnt from: arr00
Repo: OpenZeppelin/openzeppelin-confidential-contracts PR: 186
File: contracts/token/ERC7984/extensions/ERC7984Omnibus.sol:140-167
Timestamp: 2025-09-15T14:43:25.644Z
Learning: In ERC7984Omnibus callback functions like confidentialTransferFromAndCallOmnibus, the encrypted sender and recipient addresses are not passed to the callback recipient - only the standard transfer parameters (omnibusFrom, omnibusTo, amount, data) are passed. The ACL grants for the encrypted addresses are for omnibus event emission and future access, not for callback usage.

Applied to files:

• contracts/token/ERC7984/extensions/ERC7984ERC20Wrapper.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: Redirect rules - confidential-tokens
• GitHub Check: tests
• GitHub Check: coverage
• GitHub Check: slither

🔇 Additional comments (5)

contracts/token/ERC7984/extensions/ERC7984ERC20Wrapper.sol (3)

53-72: LGTM!

The updated flow correctly handles the minting after excess refund. The SafeCast.toUint64 ensures the wrapped amount fits within uint64 bounds, and the subsequent totalSupply() check in _mint validates the post-transfer state since tokens are already in the contract when this callback executes.

---

79-85: LGTM!

The ordering is correct: underlying tokens are transferred first (line 81), so when _mint calls totalSupply(), the balance already reflects the incoming tokens. This ensures the overflow check validates the post-mint state.

---

152-159: LGTM!

The implementation correctly uses SafeCast.toUint64 which provides the overflow protection mechanism. The NOTE appropriately documents that direct transfers can inflate this value.

test/token/ERC7984/extensions/ERC7984Wrapper.test.ts (2)

85-104: LGTM!

The max amount calculation is correct: (2^64 - 1) / 10^6 yields the maximum value that, when scaled to 18 decimals and divided by the rate, still fits in uint64. The test properly verifies both callback and direct wrap paths.

---

106-119: LGTM!

The test correctly validates that wrapping maxAmount + 1 triggers an overflow. The SafeCastOverflowedUintDowncast error is the expected revert from SafeCast.toUint64 when the value exceeds uint64.max.

[james-toussaint]

Same question than here https://github.com/OpenZeppelin/openzeppelin-confidential-contracts/pull/268/files#r2603361771. If kept I would (a) choose a different name (totalSupplyEstimation?) with same code or (b) call it totalUnderlyingSupply with return underlying().balanceOf(address(this)).

[arr00]

#268 (comment) used in tandem with maxTotalSupply to understand if the wrap request will be successful.

[arr00]

Renaming the function is an interesting idea. @Amxx what do you think? It may be worthwhile to ensure that people don't use it without looking into the additional notes around it.

[Amxx]

I don't have a strong attachment to the name.