Migrate @fhevm/solidity to 0.8.0
#202
Conversation
✅ Deploy Preview for confidential-tokens ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideMigrate the project to FHEVM v0.8.0 by updating core dependencies (core-contracts, hardhat-plugin, solidity compiler, and relayer SDK), refactoring all finalize* gateway callbacks to consume and decode cleartexts in signature verification calls, and removing the now-obsolete decryption oracle setup. Sequence diagram for new finalizeSwap interaction with FHEVM v0.8.0sequenceDiagram
participant User
participant SwapConfidentialToERC20
participant FHE
User->>SwapConfidentialToERC20: call finalizeSwap(requestID, cleartexts, signatures)
SwapConfidentialToERC20->>FHE: checkSignatures(requestID, cleartexts, signatures)
FHE-->>SwapConfidentialToERC20: verification result
SwapConfidentialToERC20->>SwapConfidentialToERC20: decode amount from cleartexts
SwapConfidentialToERC20->>User: transfer ERC20 tokens
Sequence diagram for new finalizeDiscloseEncryptedAmount interaction with FHEVM v0.8.0sequenceDiagram
participant User
participant ERC7984
participant FHE
User->>ERC7984: call finalizeDiscloseEncryptedAmount(requestId, cleartexts, signatures)
ERC7984->>FHE: checkSignatures(requestId, cleartexts, signatures)
FHE-->>ERC7984: verification result
ERC7984->>ERC7984: decode amount from cleartexts
ERC7984->>User: emit AmountDisclosed
Sequence diagram for new finalizeUnwrap interaction with FHEVM v0.8.0sequenceDiagram
participant User
participant ERC7984ERC20Wrapper
participant FHE
User->>ERC7984ERC20Wrapper: call finalizeUnwrap(requestID, cleartexts, signatures)
ERC7984ERC20Wrapper->>FHE: checkSignatures(requestID, cleartexts, signatures)
FHE-->>ERC7984ERC20Wrapper: verification result
ERC7984ERC20Wrapper->>ERC7984ERC20Wrapper: decode amount from cleartexts
ERC7984ERC20Wrapper->>User: transfer ERC20 tokens
Class diagram for updated gateway callback signaturesclassDiagram
class ERC7984 {
finalizeDiscloseEncryptedAmount(requestId: uint256, cleartexts: bytes, signatures: bytes)
}
class SwapConfidentialToERC20 {
finalizeSwap(requestID: uint256, cleartexts: bytes, signatures: bytes)
}
class ERC7984ERC20Wrapper {
finalizeUnwrap(requestID: uint256, cleartexts: bytes, signatures: bytes)
}
Class diagram for VestingWalletCliffExecutorConfidential after decryption oracle removalclassDiagram
class VestingWalletCliffExecutorConfidential {
initialize(executor)
// FHE.setCoprocessor(ZamaConfig.getSepoliaConfig())
// FHE.setDecryptionOracle(ZamaConfig.getSepoliaOracleAddress()) [REMOVED]
}
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
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 WalkthroughSeveral contract callbacks were migrated to accept packed Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant Contract as ERC7984 / Wrapper / SwapMock
participant FHE as FHE Library
participant Recipient
Caller->>Contract: finalize*(requestId, cleartexts, signatures)
Contract->>FHE: checkSignatures(requestId, cleartexts, signatures)
FHE-->>Contract: ok / revert
Note over Contract: amount = abi.decode(cleartexts, (uint64))
Contract->>Contract: resolve recipient from _receivers and delete mapping
alt amount != 0
Contract->>Recipient: transfer tokens (amount or amount * rate)
end
Contract-->>Caller: emit events / finish
sequenceDiagram
autonumber
participant Caller
participant Factory as VestingWalletConfidentialFactoryMock
participant FHE as FHE Library
Caller->>Factory: initialize(...)
Note over Factory: setCoprocessor(...) only\n(FHE.setDecryptionOracle call removed)
Factory-->>Caller: initialized
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
RegisGraptin
force-pushed
the
chore-migrate-fhevm-0.8.0
branch
from
a910e01 to
1968dbc
Compare
RegisGraptin
force-pushed
the
chore-migrate-fhevm-0.8.0
branch
from
1968dbc to
fb3cd45
Compare
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
contracts/package.json (1)
35-35: Peer dependency alignment confirmed: pinned0.8.0matches the root devDependency and no version skew detected. Optional: switch to^0.8.0to accept future patch releases.contracts/token/ERC7984/ERC7984.sol (1)
228-230: Add a minimal sanity check to avoid OOB on empty handles.Guard against unexpected empty handle arrays from the gateway to prevent an index underflow.
Apply within this block:
- euint64 requestHandle = euint64.wrap(FHE.loadRequestedHandles(requestId)[0]); + bytes32[] memory handles = FHE.loadRequestedHandles(requestId); + if (handles.length == 0) revert ERC7984InvalidGatewayRequest(requestId); + euint64 requestHandle = euint64.wrap(handles[0]);contracts/token/ERC7984/extensions/ERC7984ERC20Wrapper.sol (1)
137-138: Skip zero-amount transfers to maximize ERC‑20 compatibilitySome nonstandard tokens revert on zero transfers. Guarding the transfer avoids edge-case failures without behavior change.
- SafeERC20.safeTransfer(underlying(), to, amount * rate()); + uint256 payout = uint256(amount) * rate(); + if (payout != 0) { + SafeERC20.safeTransfer(underlying(), to, payout); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
contracts/mocks/docs/SwapERC7984ToERC20.sol(1 hunks)contracts/mocks/finance/VestingWalletConfidentialFactoryMock.sol(0 hunks)contracts/package.json(1 hunks)contracts/token/ERC7984/ERC7984.sol(1 hunks)contracts/token/ERC7984/extensions/ERC7984ERC20Wrapper.sol(1 hunks)package.json(2 hunks)
💤 Files with no reviewable changes (1)
- contracts/mocks/finance/VestingWalletConfidentialFactoryMock.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). (2)
- GitHub Check: tests
- GitHub Check: slither
🔇 Additional comments (6)
contracts/token/ERC7984/ERC7984.sol (2)
221-227: Finalize callback migrated to cleartexts/signatures — good ordering and decode.Signature check occurs before decode; decoding to uint64 matches the single-handle request in discloseEncryptedAmount. LGTM.
221-227: All finalize callers/tests use the updated(uint256, bytes, bytes)signature; no stale usage found.contracts/token/ERC7984/extensions/ERC7984ERC20Wrapper.sol (2)
130-138: Migration to cleartexts + signature verification looks correctCallback shape, signature check, receiver lookup, and deletion-before-transfer all look good. Transfer scales by rate() as expected.
130-133: Verify gateway callback payloads in FHEVM v0.8
In contracts/token/ERC7984/extensions/ERC7984ERC20Wrapper.sol (finalizeUnwrap, lines 130–133), confirm that:
cleartextsis ABI-encoded for a singleuint64(soabi.decode(cleartexts,(uint64))is valid).signaturesis delivered as onebytesblob (notbytes[]), matchingFHE.checkSignatures(uint256,bytes,bytes).
Any discrepancy in encoding or signature shape will break decryption or verification—ensure your local FHE interface matches the upstream v0.8.0 definitions.contracts/mocks/docs/SwapERC7984ToERC20.sol (2)
37-47: Callback update matches the new pattern; post‑verify transfer is safeSignature check first, receiver lookup + delete, then conditional transfer is sound.
37-39: No outdated callback signatures remain
AllfinalizeSwapimplementations now usebytes memory cleartextsandbytes memory signatures, matching theFHE.checkSignatures(uint256, bytes, bytes)signature in FHE.sol.
package.json
Outdated
| "@fhevm/core-contracts": "0.8.0", | ||
| "@fhevm/hardhat-plugin": "0.1.0-0", | ||
| "@fhevm/solidity": "0.8.0", | ||
| "@nomicfoundation/hardhat-chai-matchers": "^2.0.6", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
FHE stack bumped (core-contracts 0.8.0, hardhat-plugin 0.1.0-0, solidity 0.8.0, relayer-sdk ^0.2.0).
Looks consistent with contract changes. Please verify Hardhat config still loads the updated plugin and tasks, and that Node 20+ works end-to-end.
- If the plugin interface changed, ensure hardhat.config.{ts,js} imports "@fhevm/hardhat-plugin" and any config keys were migrated.
- Consider pinning the prerelease plugin to an exact version in CI to avoid surprise breaks.
Run:
Also applies to: 66-66
🏁 Script executed:
#!/bin/bash
rg -nP '@fhevm/hardhat-plugin|fhevm' -C2 hardhat.config.* scripts || true
node -v
npm ci
npm run compileLength of output: 2867
🏁 Script executed:
#!/bin/bash
# Show lines around the docs/config import in Hardhat config
sed -n '30,50p' hardhat.config.ts
# Verify existence of docs/config file
find . -type f | grep 'docs/config' || echo 'No docs/config file found'Length of output: 278
Hardhat configuration broken by missing docs/config import
- The
docgensetting in hardhat.config.ts usesrequire('./docs/config'), but no such file exists, causing build failures. Add the missingdocs/configmodule or update the import path. - Ensure
import '@fhevm/hardhat-plugin'and any plugin-specific config keys in hardhat.config.{ts,js} reflect the updated interface. - Pin
@fhevm/hardhat-pluginto the exact prerelease version in CI to avoid surprise breaks.
🤖 Prompt for AI Agents
In package.json around lines 47 to 50, the project references
@fhevm/hardhat-plugin but the Hardhat config currently requires a missing
./docs/config module which breaks builds; add the missing docs/config module
(exporting the config object expected by hardhat.config.ts) or update
hardhat.config.ts to import the correct path, update hardhat.config.{ts,js} to
include import '@fhevm/hardhat-plugin' and adjust any plugin-specific config
keys to match the plugin's current interface, and pin @fhevm/hardhat-plugin to
the exact prerelease version in package.json and CI (use the full version
string) so installs are deterministic.
|
Notice some issue in the test relating to a missing import form |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/helpers/accounts.ts (1)
19-20: Good move to resolve ACL at runtime; add a guard (and optional fallback) to avoid brittle failures.
- Protect against undefined/zero address and optionally allow an env override for CI.
- Also consider importing the ACL artifact JSON instead of reading from fs (path stability).
Please verify the exact API/field name on the upgraded plugin (that
fhevm.getRelayerMetadata()exists and returnsACLAddress)—the docs show thefhevmmodule but don’t explicitly list this method. (docs.zama.ai)Apply within this hunk:
- const aclAddress = (await fhevm.getRelayerMetadata()).ACLAddress; - const aclContract = await hre.ethers.getContractAt(acl_abi, aclAddress); + const { ACLAddress } = await fhevm.getRelayerMetadata(); + const aclAddress = process.env.ACL_ADDRESS ?? ACLAddress; + if (!ethers.isAddress(aclAddress) || aclAddress === ethers.ZeroAddress) { + throw new Error( + "ACL address unavailable. Ensure fhevm.getRelayerMetadata() is supported by @fhevm/hardhat-plugin 0.1.0-0 and/or set ACL_ADDRESS." + ); + } + const aclContract = await hre.ethers.getContractAt(acl_abi, aclAddress);Outside this hunk (optional), prefer importing the artifact over fs:
// at top-level (requires resolveJsonModule in tsconfig) import aclArtifact from '@fhevm/core-contracts/artifacts/contracts/ACL.sol/ACL.json'; const acl_abi = (aclArtifact as any).abi;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/helpers/accounts.ts(2 hunks)
🔇 Additional comments (1)
test/helpers/accounts.ts (1)
5-5: Correct usage: importingfhevmfrom Hardhat is supported—LGTM.
The plugin exposesfhevmas a named export from "hardhat". (docs.zama.ai)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
test/token/ERC7984/extensions/ERC7984Freezable.test.ts (5)
45-53: Potentially redundant double call to createEncryptedAmount in no-proof pathYou do both a stateful call and a staticCall. If createEncryptedAmount doesn't require a stateful pre-call, drop the first tx to speed up tests.
If safe:
- } else { - await token.connect(freezer).createEncryptedAmount(amount); - params.push(await token.connect(freezer).createEncryptedAmount.staticCall(amount)); - } + } else { + params.push(await token.connect(freezer).createEncryptedAmount.staticCall(amount)); + }
200-207: Parse the exact ConfidentialTransfer event (avoid address-only filter)Filter by the event topic to avoid matching an unrelated log from the same contract.
- const transferEvent = (await tx - .wait() - .then(receipt => receipt!.logs.filter((log: any) => log.address === token.target)[0])) as EventLog; + const receipt = await tx.wait(); + const topic = ethers.id('ConfidentialTransfer(address,address,bytes32)'); + const transferEvent = receipt!.logs.find( + (log: any) => log.address === token.target && log.topics[0] === topic, + ) as EventLog;
226-230: Prefer anyone.address in withArgs for determinismPassing the Signer object works with some matchers, but using the address is unambiguous across matcher versions.
- .withArgs(encryptedInput.handles[0], anyone); + .withArgs(encryptedInput.handles[0], anyone.address);
27-31: Cache token address within each test to avoid repeated await token.getAddress()Minor perf/readability win.
Example:
const tokenAddress = await token.getAddress(); // ... reuse tokenAddress in createEncryptedInput/userDecryptEuint callsAlso applies to: 45-49, 67-69, 71-73, 141-143, 156-160, 212-214
21-22: Add ACLAddress validity check and consolidate retrieval logic
- Validate
ACLAddressfromgetRelayerMetadatabefore connecting:- const acl = IACL__factory.connect((await fhevm.getRelayerMetadata()).ACLAddress, ethers.provider); + const { ACLAddress } = await fhevm.getRelayerMetadata(); + if (!ethers.isAddress(ACLAddress)) { + throw new Error(`Invalid ACL address from relayer metadata: ${ACLAddress}`); + } + const acl = IACL__factory.connect(ACLAddress, ethers.provider);
- Extract this pattern into a shared test helper to avoid duplication across suites.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
test/helpers/accounts.ts(2 hunks)test/token/ERC7984/extensions/ERC7984Freezable.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- test/helpers/accounts.ts
|
Thanks for kicking off the effort here @RegisGraptin ! |
james-toussaint
left a comment
james-toussaint
left a comment
There was a problem hiding this comment.
Thank you @RegisGraptin 👍
3 participants
Following #201
Upgrade the FHEVM to the v0.8.0
Apply the migration on the Gateway callback to have the expected signature
Summary by Sourcery
Migrate FHEVM integration to version 0.8.0 and adapt contracts to the updated cleartexts-based API.
Enhancements:
Build:
Chores:
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores
Tests