Add additional functions to IERC7984ERC20Wrapper#323
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 ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughRefactors the unwrap mechanism in ERC7984 wrapper contracts and BatcherConfidential, replacing the stored unwrapAmount (euint64) with unwrapRequestId (bytes32), updating method signatures, return types, and related getter functions throughout the contracts and tests. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@contracts/interfaces/IERC7984ERC20Wrapper.sol`:
- Around line 33-44: The contract exposes finalizeUnwrap(unwrapRequestId,
unwrapAmountCleartext, decryptionProof) but lacks a way to validate or resolve
an opaque unwrapRequestId into the decryptable handle/amount; add a companion
API such as resolveUnwrapRequest(bytes32 unwrapRequestId) external view returns
(uint64 unwrapAmountCleartext, bytes decryptableHandle) or
validateUnwrapRequest(bytes32 unwrapRequestId, uint64 unwrapAmountCleartext,
bytes calldata proof) external view returns (bool) so callers (e.g.,
BatcherConfidential.dispatchBatchCallback()) can either resolve the id to the
original cleartext/handle or verify (unwrapRequestId, cleartext, proof) before
retry/finalize; implement the function signature in the interface and ensure
downstream code uses it to check matching amounts/handles prior to calling
finalizeUnwrap.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f76e0c4b-9620-48af-b8db-6677a951ad4d
📒 Files selected for processing (5)
contracts/finance/BatcherConfidential.solcontracts/interfaces/IERC7984ERC20Wrapper.solcontracts/token/ERC7984/extensions/ERC7984ERC20Wrapper.soltest/finance/BatcherConfidential.test.tstest/helpers/interface.ts
* remove incorrect docs on `unwrap` function * add finalize unwrap and rate to wrapper interface * update batcher to use unwrap request ids instead of unwrap amounts * add `unwrapAmount` to wrapper interface for unwrap request introspection * additional docs * call getter for `unwrapAmount` in `finalizeUnwrap` * update events and errors appropriately * remove `ERC7984ERC20Wrapper` import * fix import order * Move events to interface from `ERC7984ERC20Wrapper` * add note on request id for `unwrap` * change mapping to be of `bytes32` identifiers * add changeset
* Start release candidate * Fix local docs generation (#320) * remove properties symlink * add properties.js * lint fix * Remove backwards compatibility from Batcher (#321) * Remove backwards compatibility from Batcher * Apply suggestion from @arr00 * Validate `fromToken` and `toToken` on construction (#322) * Validate `fromToken` on construction * use `ERC165Checker` * check toToken as well * directly validate to and from token * Add additional functions to `IERC7984ERC20Wrapper` (#323) * remove incorrect docs on `unwrap` function * add finalize unwrap and rate to wrapper interface * update batcher to use unwrap request ids instead of unwrap amounts * add `unwrapAmount` to wrapper interface for unwrap request introspection * additional docs * call getter for `unwrapAmount` in `finalizeUnwrap` * update events and errors appropriately * remove `ERC7984ERC20Wrapper` import * fix import order * Move events to interface from `ERC7984ERC20Wrapper` * add note on request id for `unwrap` * change mapping to be of `bytes32` identifiers * add changeset * Allow claiming on behalf of another account in batcher (#324) * Allow claiming on behalf of another account in batcher * revert on claim if users not part of the batch * add error on quit too * Add additional warnings to the batcher (#325) * Return a bytes32 unwrap id from the `_unwrap` function in `BatcherConfidential` (#326) * Return a bytes32 unwrap id from the `_unwrap` function in `BatcherConfidential` * modify warning * Document possible rounding down of user deposits (#327) * Document possible rounding down of user deposits * add contract level comment as well * up * Release v0.4.0 (rc) (#318) * Release v0.4.0 (rc) * update changelog * update changelog * Apply suggestion from @james-toussaint Co-authored-by: James Toussaint <33313130+james-toussaint@users.noreply.github.com> --------- 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> * Update node version to 24.x in CI (#328) * Exit release candidate * Release v0.4.0 (#335) * Release v0.4.0 * update changelog * remove changelog duplicate --------- 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: github-actions <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>
This PR adds
finalizeUnwrap,rate, andunwrapAmountto the wrapper interface. We keep it unopinionated by moving away from directly using a euint64 of the unwrap amount to a bytes32unwrapRequestId. In our implementation the unwrapRequestId is still simply the unwrap amount.This interface is enough now to solely use it to interact with a wrapper contract.
Summary by CodeRabbit
Release Notes
New Features
Updates