feat: upgrade to hardhat v3 and configure repo#677
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughRepository-wide migration to Hardhat v3 and Ignition: replaces hardhat-deploy scripts/fixtures with Ignition modules and deploy-and-save scripts, centralizes runtime deployments in Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev
participant Script as deployEnclave.ts
participant Ignition
participant Enclave
participant Registry as CiphernodeRegistry
participant Filter as NaiveRegistryFilter
opt withMocks
participant Mocks
end
Dev->>Script: run()
Script->>Ignition: deploy Enclave(params, owner, maxDuration, registry)
Ignition-->>Enclave: deployed address
Script->>Ignition: deploy CiphernodeRegistry(owner, enclave)
Ignition-->>Registry: deployed address
Script->>Ignition: deploy NaiveRegistryFilter(owner, registry)
Ignition-->>Filter: deployed address
Script->>Enclave: setCiphernodeRegistry(if needed)
alt withMocks
Script->>Ignition: deploy mock contracts
Ignition-->>Mocks: mock addresses
Script->>Enclave: setDecryptionVerifier / enableE3Program
end
Script-->>Dev: persist addresses to deployed_contracts.json
sequenceDiagram
autonumber
actor User
participant Client
participant NaiveRegistryFilter
participant CiphernodeRegistry
User->>Client: publishCommittee(e3Id, nodes, publicKey)
Client->>NaiveRegistryFilter: publishCommittee(e3Id, nodes, publicKey)
Note right of NaiveRegistryFilter: compute publicKey hash\nstore committee data
NaiveRegistryFilter->>CiphernodeRegistry: publishCommittee(e3Id, abi.encode(nodes), publicKey)
CiphernodeRegistry-->>NaiveRegistryFilter: tx ack
NaiveRegistryFilter-->>Client: tx receipt
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (4 passed, 1 inconclusive)❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
Poem
✨ Finishing touches
🧪 Generate unit tests
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 |
ctrlc03
force-pushed
the
feat/hardhat-v3
branch
from
b7192f1 to
1c3f993
Compare
ctrlc03
force-pushed
the
feat/hardhat-v3
branch
from
1c3f993 to
6ae6c3e
Compare
|
❌ License Header Check Failed Some files are missing the required SPDX license header. Please add the following header to the beginning of all You can run Or run |
ctrlc03
force-pushed
the
feat/hardhat-v3
branch
from
c883403 to
15219f5
Compare
|
❌ License Header Check Failed Some files are missing the required SPDX license header. Please add the following header to the beginning of all You can run Or run |
ctrlc03
force-pushed
the
feat/hardhat-v3
branch
from
15219f5 to
c59df3d
Compare
|
❌ License Header Check Failed Some files are missing the required SPDX license header. Please add the following header to the beginning of all You can run Or run |
ctrlc03
force-pushed
the
feat/hardhat-v3
branch
from
c59df3d to
8ba1b4a
Compare
ctrlc03
force-pushed
the
feat/hardhat-v3
branch
from
5e549d3 to
ff10f89
Compare
ctrlc03
force-pushed
the
feat/hardhat-v3
branch
from
ff10f89 to
d1b1049
Compare
ctrlc03
force-pushed
the
feat/hardhat-v3
branch
from
d1b1049 to
1e60070
Compare
ctrlc03
force-pushed
the
feat/hardhat-v3
branch
from
3e8f7ed to
4cc5089
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 49
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
packages/enclave-contracts/test/CiphernodeRegistry/NaiveRegistryFilter.spec.ts (6)
139-146: No-op assertionexpect(await registry.requestCommittee(...)) doesn’t assert anything. Either assert non-reversion or events.
-expect( - await registry.requestCommittee( - request.e3Id, - request.filter, - request.threshold, - ), -); +await expect( + registry.requestCommittee( + request.e3Id, + request.filter, + request.threshold, + ), +).to.not.be.reverted;
161-167: Repeat: assert something meaningfulSame issue as above.
-expect( - await registry.requestCommittee( - request.e3Id, - request.filter, - request.threshold, - ), -); +await expect( + registry.requestCommittee( + request.e3Id, + request.filter, + request.threshold, + ), +).to.not.be.reverted;
178-185: Repeat: assert something meaningfulSame fix.
-expect( - await registry.requestCommittee( - request.e3Id, - request.filter, - request.threshold, - ), -); +await expect( + registry.requestCommittee( + request.e3Id, + request.filter, + request.threshold, + ), +).to.not.be.reverted;
195-201: Repeat: assert something meaningfulSame fix.
-expect( - await registry.requestCommittee( - request.e3Id, - request.filter, - request.threshold, - ), -); +await expect( + registry.requestCommittee( + request.e3Id, + request.filter, + request.threshold, + ), +).to.not.be.reverted;
212-218: Repeat: assert something meaningfulSame fix.
-expect( - await registry.requestCommittee( - request.e3Id, - request.filter, - request.threshold, - ), -); +await expect( + registry.requestCommittee( + request.e3Id, + request.filter, + request.threshold, + ), +).to.not.be.reverted;
256-261: No-op assertion on publishCommitteeThe expect(...) doesn’t assert; just await the tx or assert events.
-expect( - await filter.publishCommittee( - request.e3Id, - [AddressOne, AddressTwo], - AddressThree, - ), -); +await filter.publishCommittee( + request.e3Id, + [AddressOne, AddressTwo], + AddressThree, +);
🧹 Nitpick comments (56)
packages/enclave-sdk/src/contract-client.ts (1)
95-122: LGTM on the formatting; consider a tiny robustness tweak.
- No functional changes detected in the simulate path; looks good.
- Nit: prefer nullish coalescing for value to avoid boolean coercion semantics.
Apply this minimal diff:
- value: value || BigInt(0), + value: value ?? 0n,templates/default/server/index.ts (2)
301-312: Duplicate route registration for /sessionsThe handler is registered twice; keep one to avoid accidental middleware/order surprises.
app.post("/", handleWebhookRequest); app.get("/sessions", handleGetSessions); // This allows us to test interaction between server and program // TEST_MODE=1 pnpm dev:server if (process.env.TEST_MODE) { app.get("/test", handleTestInteraction); } -app.get("/sessions", handleGetSessions);
141-163: Potential leak: currentlyActivating Map never clearedAfter resolving, delete the entry to prevent unbounded growth across runs.
if (!e3Sessions.has(sessionKey)) { const sdk = await createPrivateSDK(); console.log("📡 Fetching E3 data from contract..."); const e3 = await sdk.getE3(e3Id); console.log("✅ Received E3 data from contract."); e3Sessions.set(sessionKey, { e3Id, e3ProgramParams: e3.e3ProgramParams, expiration, inputs: [], isProcessing: false, isCompleted: false, }); def.resolve(); + currentlyActivating.delete(e3Id); }packages/enclave-contracts/contracts/Enclave.sol (1)
170-178: Minor: avoid double map lookupUse the cached variable for clarity and tiny gas/readability wins.
- require( - decryptionVerifiers[encryptionSchemeId] != - IDecryptionVerifier(address(0)), - InvalidEncryptionScheme(encryptionSchemeId) - ); + require( + decryptionVerifier != IDecryptionVerifier(address(0)), + InvalidEncryptionScheme(encryptionSchemeId) + );tests/integration/base.sh (1)
3-3: Add pipefail for safer bash erroringPrevents hidden failures in piped commands.
-set -eu # Exit immediately if a command exits with a non-zero status +set -euo pipefail # Exit on error, unset vars, and fail on pipeline errorstests/integration/persist.sh (1)
3-3: Harden shell optionsMatch other scripts with pipefail for robustness.
-set -eu # Exit immediately if a command exits with a non-zero status +set -euo pipefail # Exit on error, unset vars, and fail on pipeline errorspackages/enclave-sdk/package.json (1)
23-23: Prefer workspace-aware prebuild to avoid brittle cd pathsUse pnpm’s workspace flags so the script works regardless of where it’s invoked.
Apply:
- "prebuild": "cd ../enclave-contracts && pnpm build && cd ../../crates/wasm && pnpm build", + "prebuild": "pnpm -w --filter @enclave-e3/contracts build && pnpm -w --filter @enclave-e3/wasm build",Please confirm @enclave-e3/contracts’ build runs Hardhat compile (artifacts + types) so SDK consumers get everything they need.
.github/workflows/ci.yml (2)
351-351: Node 22: ensure toolchain support + unify setup-node versionsHardhat v3 and Ignition should support Node 22, but please double-check locally/CI. Also, other jobs still use actions/setup-node@v3; consider standardizing on @v4 across the workflow for consistency.
535-547: Fail CI when SDK artifacts are missingUploading SDK artifacts with if-no-files-found: warn can hide regressions (e.g., missing types/artifacts after the v3 migration).
Apply:
- name: "Upload SDK artifacts" uses: actions/upload-artifact@v4 with: name: sdk-artifacts path: | packages/enclave-sdk/dist packages/enclave-contracts/dist packages/enclave-contracts/artifacts packages/enclave-contracts/cache packages/enclave-contracts/types crates/wasm/dist - retention-days: 1 - if-no-files-found: warn + retention-days: 1 + if-no-files-found: errorpackages/enclave-contracts/ignition/modules/mockCiphernodeRegistryEmptyKey.ts (1)
7-7: Drop any-cast; rely on Ignition typesThe module is already correctly typed by buildModule; the any cast and eslint disable are unnecessary.
Apply:
-/* eslint-disable @typescript-eslint/no-explicit-any */ import { buildModule } from "@nomicfoundation/hardhat-ignition/modules"; export default buildModule("MockCiphernodeRegistryEmptyKey", (m) => { const mockCiphernodeRegistryEmptyKey = m.contract( "MockCiphernodeRegistryEmptyKey", ); return { mockCiphernodeRegistryEmptyKey }; -}) as any; +});Also applies to: 10-16
tests/integration/fns.sh (1)
55-57: Prefer graceful termination before SIGKILLTry TERM first and fall back to KILL to avoid leaving temp files/locks.
Apply:
- pkill -9 -f "target/debug/enclave" || true - pkill -9 -f "hardhat" || true + pkill -TERM -f "target/debug/enclave" || true + pkill -TERM -f "hardhat" || true + sleep 1 + pkill -KILL -f "target/debug/enclave" || true + pkill -KILL -f "hardhat" || true- echo "Killing enclave" - pkill -9 -f "target/debug/enclave" || true - pkill -9 -f "enclave start" || true - pkill -9 -f "hardhat" || true + echo "Killing enclave" + pkill -TERM -f "target/debug/enclave" || true + pkill -TERM -f "enclave start" || true + pkill -TERM -f "hardhat" || true + sleep 1 + pkill -KILL -f "target/debug/enclave" || true + pkill -KILL -f "enclave start" || true + pkill -KILL -f "hardhat" || trueAlso applies to: 176-181
packages/enclave-contracts/tsconfig.json (2)
5-6: Confirm bundler resolution works with Hardhat v3/ts-node"module: ESNext" + "moduleResolution: bundler" can trip ts-node/Hardhat plugins. If you hit import resolution issues, switch to NodeNext.
- "module": "ESNext", - "moduleResolution": "bundler", + "module": "NodeNext", + "moduleResolution": "NodeNext",
11-12: Avoid emitting declarations for tests/scripts/tasksWith declaration emit enabled and tests/scripts in "include", you'll fill dist/ with d.ts for non-publishable files. Prefer a dedicated build tsconfig.
Option A (keep current tsconfig for IDE, add build config):
// packages/enclave-contracts/tsconfig.build.json { "extends": "./tsconfig.json", "compilerOptions": { "noEmit": false, "emitDeclarationOnly": true, "outDir": "dist" }, "include": ["src/**/*.ts", "types/**/*.ts"], "exclude": ["test", "tests", "scripts", "tasks", "ignition/modules"] }Run builds with: tsc -p tsconfig.build.json.
Also applies to: 14-21
templates/default/tsconfig.json (2)
17-18: Redundant test globsBoth test/** and tests/** are included. Keep one (prefer tests/**).
- "test/**/*.ts", - "tests/**/*.ts", + "tests/**/*.ts",
5-6: Sanity-check bundler resolution for template consumersIf downstream users run Hardhat/ts-node directly, bundler resolution can be brittle. If issues arise, recommend NodeNext for broader compatibility.
- "module": "ESNext", - "moduleResolution": "bundler", + "module": "NodeNext", + "moduleResolution": "NodeNext",tests/integration/enclave.config.yaml (1)
5-7: De-duplicate sources of truth for addressesConsider generating this from packages/enclave-contracts/deployed_contracts.json (or validating against it in CI) to avoid drift.
I can add a small script to sync/validate if you’d like.
examples/CRISP/client/.env.example (1)
4-5: Satisfy dotenv-linter: order keys and quote valuesSort keys and quote 0x values.
-VITE_ENCLAVE_API=http://127.0.0.1:4000 -VITE_TWITTER_SERVERLESS_API= -VITE_WALLETCONNECT_PROJECT_ID= -VITE_E3_PROGRAM_ADDRESS=0x4ed7c70F96B99c776995fB64377f0d4aB3B0e1C1 # Default E3 program address from anvil -VITE_SEMAPHORE_ADDRESS=0x9A9f2CCfdE556A7E9Ff0848998Aa4a0CFD8863AE +VITE_E3_PROGRAM_ADDRESS="0x4ed7c70F96B99c776995fB64377f0d4aB3B0e1C1" # Default E3 program address from anvil +VITE_ENCLAVE_API=http://127.0.0.1:4000 +VITE_SEMAPHORE_ADDRESS="0x9A9f2CCfdE556A7E9Ff0848998Aa4a0CFD8863AE" +VITE_TWITTER_SERVERLESS_API= +VITE_WALLETCONNECT_PROJECT_ID=crates/Dockerfile (1)
35-35: Add fail-fast check for deployed_contracts.json in crates/Dockerfilecrates/entrypoint/build.rs references ../../packages/enclave-contracts/deployed_contracts.json (confirmed). Add a quick runtime check so Docker builds fail early if the file is missing/empty:
COPY --from=evm-builder /build/packages/enclave-contracts/artifacts ../packages/enclave-contracts/artifacts COPY --from=evm-builder /build/packages/enclave-contracts/deployed_contracts.json ../packages/enclave-contracts/deployed_contracts.json +RUN test -s ../packages/enclave-contracts/deployed_contracts.json || (echo "deployed_contracts.json missing or empty" >&2; exit 1)examples/CRISP/server/.env.example (1)
14-16: Fix dotenv lint warnings (quotes/order) and keep addresses unquotedRemove quotes to satisfy QuoteCharacter; optionally order keys before ENCLAVE_ADDRESS per linter.
Apply this diff:
-ENCLAVE_ADDRESS="0xe7f1725E7734CE288F8367e1Bb143E90bb3F0512" -CIPHERNODE_REGISTRY_ADDRESS="0xCf7Ed3AccA5a467e9e704C703E8D87F634fB0Fc9" -NAIVE_REGISTRY_FILTER_ADDRESS="0xDc64a140Aa3E981100a9becA4E685f962f0cF6C9" -E3_PROGRAM_ADDRESS="0x4ed7c70F96B99c776995fB64377f0d4aB3B0e1C1" # CRISPProgram Contract Address +E3_PROGRAM_ADDRESS=0x4ed7c70F96B99c776995fB64377f0d4aB3B0e1C1 # CRISPProgram Contract Address +CIPHERNODE_REGISTRY_ADDRESS=0xCf7Ed3AccA5a467e9e704C703E8D87F634fB0Fc9 +NAIVE_REGISTRY_FILTER_ADDRESS=0xDc64a140Aa3E981100a9becA4E685f962f0cF6C9 +ENCLAVE_ADDRESS=0xe7f1725E7734CE288F8367e1Bb143E90bb3F0512packages/enclave-contracts/ignition/modules/naiveRegistryFilter.ts (2)
7-7: Drop unnecessary eslint disableThe explicit-any disable is only needed because of the cast below; removing both improves lint hygiene.
Apply this diff:
-/* eslint-disable @typescript-eslint/no-explicit-any */
10-20: Prefer typed module export; avoidas anyIgnition’s buildModule returns a properly typed definition; the cast is unnecessary. Keep the export typed and remove the cast.
Apply this diff:
-export default buildModule("NaiveRegistryFilter", (m) => { +export default buildModule("NaiveRegistryFilter", (m) => { @@ - return { naiveRegistryFilter }; -}) as any; + return { naiveRegistryFilter }; +});If TS complains, confirm @nomicfoundation/hardhat-ignition types are v3 and that tsconfig excludes ESM interop issues.
templates/default/scripts/dev_ciphernodes.sh (2)
9-13: Make cleanup resilient under set -epkill returns non‑zero when no match; with set -e that can abort cleanup mid‑way. Guard with || true.
- pkill -9 -f "enclave start" + pkill -9 -f "enclave start" || true sleep 2 - pkill enclave + pkill enclave || true
42-44: Quote addresses to avoid word-splitting/globbingNot critical, but safer in bash.
-pnpm hardhat ciphernode:add --ciphernode-address $CN1 --network localhost -pnpm hardhat ciphernode:add --ciphernode-address $CN2 --network localhost -pnpm hardhat ciphernode:add --ciphernode-address $CN3 --network localhost +pnpm hardhat ciphernode:add --ciphernode-address "$CN1" --network localhost +pnpm hardhat ciphernode:add --ciphernode-address "$CN2" --network localhost +pnpm hardhat ciphernode:add --ciphernode-address "$CN3" --network localhostexamples/CRISP/tests/crisp.contracts.test.ts (2)
201-203: Minor: unused variablepublicClient is assigned but not used.
- const publicClient = await viemHardhat.getPublicClient(); + await viemHardhat.getPublicClient(); // ensure client is instanced; value unused
90-104: Leaked dev private key in comments can trip GitleaksEven commented, the well-known Hardhat key triggers scanners. Either redact or add an allowlist rule scoped to localhost/dev paths.
I can propose a .gitleaks.toml snippet to allowlist the Hardhat default key only under examples/ and templates/.
Also applies to: 101-101
examples/CRISP/package.json (1)
32-60: Verify plugin mix and remove stale deps
- You depend on both hardhat-toolbox and hardhat-toolbox-viem plus hardhat-ethers; ensure you aren’t loading conflicting plugins in hardhat.config.ts. If CRISP is Viem-only, you likely don’t need hardhat-ethers via toolbox.
- hardhat-deploy remains but the PR states it’s removed; drop if unused.
"devDependencies": { "@enclave-e3/config": "^0.0.10-test", "@nomicfoundation/hardhat-ethers": "^3.0.0", "@nomicfoundation/hardhat-toolbox": "^5.0.0", "@nomicfoundation/hardhat-ignition": "^3.0.0", "@nomicfoundation/hardhat-toolbox-viem": "^5.0.0", @@ - "hardhat-deploy": "^0.12.4",If you keep both toolboxes, ensure only one is imported in hardhat.config.ts to avoid duplicate plugin side-effects.
packages/enclave-contracts/ignition/modules/enclave.ts (2)
18-26: Constructor arg shape: avoid wrappingparamsin an arrayIf the Enclave constructor expects
bytes(as implied by upstream encoding), passparamsdirectly, not[params].- const enclave = m.contract( - "Enclave", - [owner, registry, maxDuration, [params]], + const enclave = m.contract( + "Enclave", + [owner, registry, maxDuration, params], { libraries: { PoseidonT3: poseidonT3, }, }, );
28-29: Dropas anyIgnition’s
buildModulereturns a typed module; avoidas anyto keep type-safety.packages/enclave-contracts/scripts/deployAndSave/mockDecryptionVerifier.ts (1)
22-23: Persist chainId and harden network reads
- Prefer storing
chainIdalongsidenameto avoid collisions and improve portability.- Make
blockNumbernon-null.- const chain = (await signer.provider?.getNetwork())?.name ?? "localhost"; + const net = await signer.provider!.getNetwork(); + const chain = net.name ?? "localhost"; + const chainId = Number(net.chainId); @@ - const blockNumber = await signer.provider?.getBlockNumber(); + const blockNumber = await signer.provider!.getBlockNumber(); @@ storeDeploymentArgs( { - blockNumber, + blockNumber, address: decryptionVerifierAddress, + chainId, }, "MockDecryptionVerifier", chain, );Also applies to: 32-41
packages/enclave-contracts/scripts/deployAndSave/mockProgram.ts (1)
26-29: Also store chainId for portabilityMirroring other scripts, persist
chainIdin the stored args.- const chain = (await signer.provider?.getNetwork())?.name ?? "localhost"; + const net = await signer.provider!.getNetwork(); + const chain = net.name ?? "localhost"; + const chainId = Number(net.chainId); @@ storeDeploymentArgs( { constructorArgs: { mockInputValidator }, - blockNumber, + blockNumber, address: e3ProgramAddress, + chainId, }, "MockE3Program", chain, );Also applies to: 55-63
packages/enclave-contracts/scripts/deployAndSave/mockInputValidator.ts (1)
27-37: Capture chainId and avoid nullable readsAdd
chainIdand makeblockNumbernon-null.- const chain = (await signer.provider?.getNetwork())?.name ?? "localhost"; - const blockNumber = await signer.provider?.getBlockNumber(); + const net = await signer.provider!.getNetwork(); + const chain = net.name ?? "localhost"; + const chainId = Number(net.chainId); + const blockNumber = await signer.provider!.getBlockNumber(); @@ storeDeploymentArgs( { blockNumber, address: inputValidatorAddress, + chainId, }, "MockInputValidator", chain, );templates/default/scripts/deploy-local.ts (1)
34-35: Exit with non-zero code on failureEnsure CI fails if deployment fails.
-main().catch(console.error); +main().catch((err) => { + console.error(err); + process.exit(1); +});crates/entrypoint/build.rs (1)
65-65: Use the computed absolute path in rerun-if-changed to avoid path drift.The hard-coded relative path can desync from
deployments_path.- println!("cargo:rerun-if-changed=../../packages/enclave-contracts/deployed_contracts.json"); + println!("cargo:rerun-if-changed={}", deployments_path.display());packages/enclave-contracts/scripts/deployEnclave.ts (1)
78-109: Centralize encryptionSchemeId derivation.Hard-coding
"fhe.rs:BFV"risks divergence. Expose a shared helper/constant (e.g., in the SDK) and reuse here.templates/default/tests/integration.spec.ts (2)
262-265: Decode the plaintext as a full BigInt, not a single byte.Parsing only
parsed[0]is fragile. Use viem’sfromHexto compare the full value.- const parsed = hexToUint8Array(plaintextEvent.data.plaintextOutput); - - expect(BigInt(parsed[0])).toBe(num1 + num2); + import { fromHex } from "viem"; + const out = fromHex(plaintextEvent.data.plaintextOutput, { to: "bigint" }); + expect(out).toBe(num1 + num2);
35-39: Deduplicate helpers: use viem’s hexToBytes instead of custom hex parser.You already import
hexToBytes; the localhexToUint8Arrayis redundant.-function hexToUint8Array(hexString: string) { - const hex = hexString.startsWith("0x") ? hexString.slice(2) : hexString; - const m = hex.match(/.{2}/g)?.map((byte) => parseInt(byte, 16)) ?? []; - return new Uint8Array(m); -} +// Use viem.hexToBytes where neededpackages/enclave-contracts/hardhat.config.ts (1)
117-123: Ganache accounts config differs from others; intentional?Elsewhere you resolve accounts from
PRIVATE_KEY/MNEMONICwithconfigVariable. Here you hardcodemnemoniconly. If that’s intentional for local dev, ignore; otherwise align with the shared logic.templates/default/deployed_contracts.json (1)
1-67: Template looks coherent; consider embedding constructor args everywhere you can.You already include constructorArgs for major contracts; adding them for ImageID/InputValidator/MyProgram (if applicable) improves reproducibility and later verification. Low-priority.
packages/enclave-contracts/scripts/deployAndSave/naiveRegistryFilter.ts (2)
66-68: Minor: property shorthandTidy up constructorArgs.
- ciphernodeRegistryAddress: ciphernodeRegistryAddress, + ciphernodeRegistryAddress,
40-44: Optional: verify on-chain code before reconnectingBefore connecting to a cached address, consider validating code is present (not "0x") to avoid stale/corrupt entries.
I can provide a small helper to fetch and assert bytecode if you want.
packages/enclave-contracts/scripts/deployAndSave/enclave.ts (1)
59-68: Optional: type-safety on params/maxDurationIf Enclave expects bytes/uint values, prefer stricter types (e.g., Hex string branded type or bigint) to prevent accidental non-hex strings reaching ignition.
I can align the types to the module’s expected parameter shapes if you share the EnclaveModule signature.
packages/enclave-contracts/scripts/deployMocks.ts (1)
20-24: Fix JSDoc: remove nonexistent parameter.The function takes no args; the
@param enclaveAddressline is misleading./** * Deploys the mock contracts and returns the addresses. - * @param enclaveAddress - The address of the enclave contract. * @returns The addresses of the mock contracts. */packages/enclave-contracts/scripts/deployAndSave/ciphernodeRegistryOwnable.ts (1)
40-47: Optional: validate addresses before reuse.Consider checking code at
preDeployedArgs.addressto avoid connecting to a dead address after a chain reset.Happy to add a lightweight
getCode(address)check and warn/skip if it's0x.templates/default/deploy/default.ts (1)
42-52: Minor cleanup: avoid repeated awaits of addresses.Cache addresses to reduce RPC calls and improve readability.
- storeDeploymentArgs({ - address: await e3Program.getAddress(), - constructorArgs: { - enclave: await enclave.getAddress(), - verifier: await verifier.getAddress(), - programId, - inputValidator: await inputValidator.getAddress(), - }, - }, "MyProgram", chain); + const [enclaveAddr, verifierAddr, inputValidatorAddr, e3ProgramAddr] = + await Promise.all([ + enclave.getAddress(), + verifier.getAddress(), + inputValidator.getAddress(), + e3Program.getAddress(), + ]); + + storeDeploymentArgs({ + address: e3ProgramAddr, + constructorArgs: { + enclave: enclaveAddr, + verifier: verifierAddr, + programId, + inputValidator: inputValidatorAddr, + }, + }, "MyProgram", chain);packages/enclave-contracts/scripts/utils.ts (2)
9-9: Consider making the deployment file path configurable.The hardcoded path to
deployed_contracts.jsonat the root may not be flexible enough for different environments or configurations.Consider using an environment variable with a default fallback:
-const deploymentsFile = path.join("deployed_contracts.json"); +const deploymentsFile = path.join( + process.env.DEPLOYMENTS_FILE || "deployed_contracts.json" +);
106-111: Consider atomic file operations for concurrent safety.The deployment file operations are not atomic, which could lead to data corruption if multiple processes try to write simultaneously.
Consider implementing file locking or using atomic write operations (e.g., write to a temp file and rename) to prevent race conditions during concurrent deployments. Libraries like
write-file-atomiccould help ensure data integrity.packages/enclave-contracts/test/CiphernodeRegistry/CiphernodeRegistryOwnable.spec.ts (1)
22-23: Consider moving top-level await into test setup.Using top-level await for network connection could cause issues in certain test environments or when running tests in parallel.
Consider moving the network connection into the setup function or a before hook:
-const { ethers, networkHelpers, ignition } = await network.connect(); -const { loadFixture } = networkHelpers; +let ethers: any, networkHelpers: any, ignition: any, loadFixture: any; + +before(async function () { + const connection = await network.connect(); + ethers = connection.ethers; + networkHelpers = connection.networkHelpers; + ignition = connection.ignition; + loadFixture = networkHelpers.loadFixture; +});packages/enclave-contracts/tasks/ciphernode.ts (1)
20-33: Consider adding success/failure feedback for deployments.The tasks don't provide clear feedback about whether the deployment was reused or newly created.
Consider enhancing the logging to indicate deployment status:
const { ciphernodeRegistry } = await deployAndSaveCiphernodeRegistryOwnable({ hre }); + console.log(`Using CiphernodeRegistry at: ${await ciphernodeRegistry.getAddress()}`); const tx = await ciphernodeRegistry.addCiphernode(ciphernodeAddress); await tx.wait(); console.log(`Ciphernode ${ciphernodeAddress} registered`);examples/CRISP/hardhat.config.ts (1)
87-95: API key environment variables not validatedThe API keys are passed as empty strings when environment variables are not set, which might cause issues with block explorer verification.
Consider adding validation or warnings for missing API keys:
- arbitrum: getChainConfig("arbitrum-mainnet", process.env.ARBISCAN_API_KEY || ""), + arbitrum: getChainConfig("arbitrum-mainnet", process.env.ARBISCAN_API_KEY || (() => { + console.warn("Warning: ARBISCAN_API_KEY not set"); + return ""; + })()),packages/enclave-contracts/tasks/enclave.ts (1)
320-324: Consider validating file existence before readingThe file read operations don't check if the file exists before attempting to read it, which could cause unhelpful error messages.
if (dataFile) { + if (!fs.existsSync(dataFile)) { + throw new Error(`File not found: ${dataFile}`); + } const file = fs.readFileSync(dataFile); dataToSend = file.toString(); }packages/enclave-contracts/test/Enclave.spec.ts (2)
29-55: Consider extracting test constants to a shared module.These test constants (encryption scheme IDs, E3 program parameters, data/proof values) could be extracted to a shared test utilities module to promote reusability across test files and reduce duplication.
Consider creating a test utilities module:
// test/utils/constants.ts export const TEST_CONSTANTS = { THIRTY_DAYS_IN_SECONDS: 60 * 60 * 24 * 30, ENCRYPTION_SCHEME_ID: "0x2c2a814a0495f913a3a312fc4771e37552bc14f8a2d4075a08122d356f0849c6", // ... other constants }; export const TEST_E3_PARAMS = { polynomial_degree: ethers.toBigInt(2048), plaintext_modulus: ethers.toBigInt(1032193), moduli: [ethers.toBigInt("18014398492704769")] };
194-196: Redundantfromparameter in connect call.When using
connect()with a signer, thefromparameter in the transaction options is redundant as the signer's address is already used.- await expect( - enclave - .connect(notTheOwner) - .setMaxDuration(1, { from: await notTheOwner.getAddress() }), - ) + await expect( + enclave + .connect(notTheOwner) + .setMaxDuration(1), + )templates/default/hardhat.config.ts (4)
20-23: Consider validating environment variables.While using
configVariableis good practice, consider adding validation or fallback values for critical configuration to prevent runtime failures.const mnemonic = configVariable("MNEMONIC"); const privateKey = configVariable("PRIVATE_KEY"); const infuraApiKey = configVariable("INFURA_API_KEY"); // Add validation if (!mnemonic && !privateKey) { console.warn("Warning: Neither MNEMONIC nor PRIVATE_KEY is set. Some network operations may fail."); }
95-100: Hardcoded salt in create2 configuration.Using a zero salt for CREATE2 deployments removes the benefit of deterministic addresses across chains. Consider using a more meaningful salt or making it configurable.
ignition: { strategyConfig: { create2: { salt: process.env.CREATE2_SALT || ethers.id("enclave-v3-migration"), } } },
137-148: Consider version management for imported contracts.The
npmFilesToBuildlist hardcodes specific contract paths. Consider documenting the versioning strategy for these dependencies to ensure compatibility.Would you like me to create a configuration file that tracks contract dependency versions and their compatibility matrix?
51-62: Accounts union is valid — prefer using ConfigurationVariable[] instead of the single-element tupleHardhat accepts either an array of configuration variables (private keys) or an HD-wallet object; the current union is acceptable, but change the type from [ConfigurationVariable] to ConfigurationVariable[] to allow multiple keys and better match Hardhat's accounts shape. (hardhat.org)
Location: templates/default/hardhat.config.ts (lines 51–62)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/enclave-contracts/test/fixtures/pubkey.binis excluded by!**/*.binpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (94)
.github/workflows/ci.yml(3 hunks)crates/Dockerfile(1 hunks)crates/entrypoint/build.rs(3 hunks)examples/CRISP/client/.env.example(1 hunks)examples/CRISP/enclave.config.yaml(1 hunks)examples/CRISP/hardhat.config.ts(3 hunks)examples/CRISP/package.json(1 hunks)examples/CRISP/server/.env.example(1 hunks)examples/CRISP/tests/CRISPProgram.t.sol(0 hunks)examples/CRISP/tests/crisp.contracts.test.ts(1 hunks)examples/CRISP/tsconfig.json(1 hunks)package.json(1 hunks)packages/enclave-contracts/.gitignore(1 hunks)packages/enclave-contracts/artifacts/contracts/interfaces/ICiphernodeRegistry.sol/ICiphernodeRegistry.json(2 hunks)packages/enclave-contracts/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.json(2 hunks)packages/enclave-contracts/artifacts/contracts/registry/NaiveRegistryFilter.sol/NaiveRegistryFilter.json(2 hunks)packages/enclave-contracts/contracts/Enclave.sol(2 hunks)packages/enclave-contracts/contracts/interfaces/IEnclave.sol(1 hunks)packages/enclave-contracts/contracts/registry/NaiveRegistryFilter.sol(1 hunks)packages/enclave-contracts/contracts/test/MockE3Program.sol(0 hunks)packages/enclave-contracts/deploy/enclave.ts(0 hunks)packages/enclave-contracts/deploy/mocks.ts(0 hunks)packages/enclave-contracts/deployed_contracts.json(1 hunks)packages/enclave-contracts/deployments/sepolia/.chainId(0 hunks)packages/enclave-contracts/deployments/sepolia/MockComputeProvider.json(0 hunks)packages/enclave-contracts/deployments/sepolia/MockDecryptionVerifier.json(0 hunks)packages/enclave-contracts/deployments/sepolia/MockE3Program.json(0 hunks)packages/enclave-contracts/deployments/sepolia/MockInputValidator.json(0 hunks)packages/enclave-contracts/deployments/sepolia/NaiveRegistryFilter.json(0 hunks)packages/enclave-contracts/deployments/sepolia/solcInputs/f32a0aa4ee06fa210859a817cc9dafc5.json(0 hunks)packages/enclave-contracts/hardhat.config.cts(0 hunks)packages/enclave-contracts/hardhat.config.ts(1 hunks)packages/enclave-contracts/ignition/modules/ciphernodeRegistry.ts(1 hunks)packages/enclave-contracts/ignition/modules/enclave.ts(1 hunks)packages/enclave-contracts/ignition/modules/mockCiphernodeRegistry.ts(1 hunks)packages/enclave-contracts/ignition/modules/mockCiphernodeRegistryEmptyKey.ts(1 hunks)packages/enclave-contracts/ignition/modules/mockComputeProvider.ts(1 hunks)packages/enclave-contracts/ignition/modules/mockDecryptionVerifier.ts(1 hunks)packages/enclave-contracts/ignition/modules/mockE3Program.ts(1 hunks)packages/enclave-contracts/ignition/modules/mockInputValidator.ts(1 hunks)packages/enclave-contracts/ignition/modules/naiveRegistryFilter.ts(1 hunks)packages/enclave-contracts/ignition/modules/poseidonT3.ts(1 hunks)packages/enclave-contracts/package.json(2 hunks)packages/enclave-contracts/scripts/deployAndSave/ciphernodeRegistryOwnable.ts(1 hunks)packages/enclave-contracts/scripts/deployAndSave/enclave.ts(1 hunks)packages/enclave-contracts/scripts/deployAndSave/mockComputeProvider.ts(1 hunks)packages/enclave-contracts/scripts/deployAndSave/mockDecryptionVerifier.ts(1 hunks)packages/enclave-contracts/scripts/deployAndSave/mockInputValidator.ts(1 hunks)packages/enclave-contracts/scripts/deployAndSave/mockProgram.ts(1 hunks)packages/enclave-contracts/scripts/deployAndSave/naiveRegistryFilter.ts(1 hunks)packages/enclave-contracts/scripts/deployEnclave.ts(1 hunks)packages/enclave-contracts/scripts/deployMocks.ts(1 hunks)packages/enclave-contracts/scripts/run.ts(1 hunks)packages/enclave-contracts/scripts/utils.ts(1 hunks)packages/enclave-contracts/tasks/accounts.ts(0 hunks)packages/enclave-contracts/tasks/ciphernode.ts(1 hunks)packages/enclave-contracts/tasks/enclave.ts(1 hunks)packages/enclave-contracts/tasks/utils.ts(1 hunks)packages/enclave-contracts/test/CiphernodeRegistry/CiphernodeRegistryOwnable.spec.ts(5 hunks)packages/enclave-contracts/test/CiphernodeRegistry/NaiveRegistryFilter.spec.ts(4 hunks)packages/enclave-contracts/test/Enclave.spec.ts(20 hunks)packages/enclave-contracts/test/fixtures/CiphernodeRegistryOwnable.fixture.ts(0 hunks)packages/enclave-contracts/test/fixtures/Enclave.fixture.ts(0 hunks)packages/enclave-contracts/test/fixtures/MockCiphernodeRegistry.fixture.ts(0 hunks)packages/enclave-contracts/test/fixtures/MockComputeProvider.fixture.ts(0 hunks)packages/enclave-contracts/test/fixtures/MockDecryptionVerifier.fixture.ts(0 hunks)packages/enclave-contracts/test/fixtures/MockE3Program.fixture.ts(0 hunks)packages/enclave-contracts/test/fixtures/MockInputValidator.fixture.ts(0 hunks)packages/enclave-contracts/test/fixtures/NaiveRegistryFilter.fixture.ts(0 hunks)packages/enclave-contracts/test/fixtures/PoseidonT3.fixture.ts(0 hunks)packages/enclave-contracts/tsconfig.json(1 hunks)packages/enclave-contracts/tsup.config.mjs(0 hunks)packages/enclave-sdk/package.json(1 hunks)packages/enclave-sdk/src/contract-client.ts(1 hunks)packages/enclave-sdk/tsup.config.js(2 hunks)templates/default/.gitignore(1 hunks)templates/default/contracts/Mocks/MockRISC0Verifier.sol(1 hunks)templates/default/contracts/MyProgram.sol(1 hunks)templates/default/deploy/default.ts(1 hunks)templates/default/deploy/enclave.ts(0 hunks)templates/default/deployed_contracts.json(1 hunks)templates/default/enclave.config.yaml(1 hunks)templates/default/hardhat.config.ts(1 hunks)templates/default/package.json(2 hunks)templates/default/scripts/deploy-local.ts(1 hunks)templates/default/scripts/dev_ciphernodes.sh(2 hunks)templates/default/scripts/test_integration.sh(1 hunks)templates/default/server/index.ts(1 hunks)templates/default/tests/integration.spec.ts(4 hunks)templates/default/tsconfig.json(1 hunks)tests/integration/base.sh(1 hunks)tests/integration/enclave.config.yaml(1 hunks)tests/integration/fns.sh(3 hunks)tests/integration/persist.sh(1 hunks)
💤 Files with no reviewable changes (24)
- packages/enclave-contracts/test/fixtures/NaiveRegistryFilter.fixture.ts
- packages/enclave-contracts/deployments/sepolia/MockE3Program.json
- packages/enclave-contracts/deployments/sepolia/MockComputeProvider.json
- packages/enclave-contracts/test/fixtures/CiphernodeRegistryOwnable.fixture.ts
- packages/enclave-contracts/test/fixtures/MockInputValidator.fixture.ts
- packages/enclave-contracts/tsup.config.mjs
- packages/enclave-contracts/test/fixtures/MockE3Program.fixture.ts
- templates/default/deploy/enclave.ts
- packages/enclave-contracts/hardhat.config.cts
- packages/enclave-contracts/test/fixtures/PoseidonT3.fixture.ts
- packages/enclave-contracts/deployments/sepolia/MockInputValidator.json
- packages/enclave-contracts/deploy/enclave.ts
- packages/enclave-contracts/deploy/mocks.ts
- packages/enclave-contracts/deployments/sepolia/.chainId
- packages/enclave-contracts/tasks/accounts.ts
- packages/enclave-contracts/test/fixtures/MockDecryptionVerifier.fixture.ts
- packages/enclave-contracts/deployments/sepolia/NaiveRegistryFilter.json
- packages/enclave-contracts/test/fixtures/MockComputeProvider.fixture.ts
- packages/enclave-contracts/test/fixtures/Enclave.fixture.ts
- packages/enclave-contracts/contracts/test/MockE3Program.sol
- packages/enclave-contracts/test/fixtures/MockCiphernodeRegistry.fixture.ts
- examples/CRISP/tests/CRISPProgram.t.sol
- packages/enclave-contracts/deployments/sepolia/MockDecryptionVerifier.json
- packages/enclave-contracts/deployments/sepolia/solcInputs/f32a0aa4ee06fa210859a817cc9dafc5.json
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2024-11-25T09:47:48.863Z
Learnt from: ryardley
PR: gnosisguild/enclave#184
File: packages/ciphernode/net/tests/entrypoint.sh:4-8
Timestamp: 2024-11-25T09:47:48.863Z
Learning: When reviewing test scripts like `packages/ciphernode/net/tests/entrypoint.sh`, avoid suggesting additional error handling and cleanup for `iptables` commands, as it may not be necessary.
Applied to files:
templates/default/scripts/dev_ciphernodes.sh
📚 Learning: 2024-09-26T04:12:09.345Z
Learnt from: ryardley
PR: gnosisguild/enclave#107
File: tests/basic_integration/test.sh:103-114
Timestamp: 2024-09-26T04:12:09.345Z
Learning: In `tests/basic_integration/test.sh`, the user prefers not to refactor the ciphernode addition section to reduce duplication.
Applied to files:
templates/default/scripts/dev_ciphernodes.shtests/integration/base.sh
📚 Learning: 2024-10-08T19:45:18.209Z
Learnt from: ryardley
PR: gnosisguild/enclave#107
File: tests/basic_integration/test.sh:81-83
Timestamp: 2024-10-08T19:45:18.209Z
Learning: In `tests/basic_integration/test.sh`, it's acceptable to wait indefinitely for the EVM node to start without a timeout, as it's unlikely to fail here.
Applied to files:
templates/default/tests/integration.spec.tstests/integration/base.sh
📚 Learning: 2024-11-05T06:56:49.157Z
Learnt from: ryardley
PR: gnosisguild/enclave#173
File: packages/ciphernode/enclave_node/src/aggregator.rs:0-0
Timestamp: 2024-11-05T06:56:49.157Z
Learning: `RegistryFilterSol` does not have a reader and does not require a repository reader or deploy block when calling `RegistryFilterSol::attach` in `packages/ciphernode/enclave_node/src/aggregator.rs`.
Applied to files:
packages/enclave-contracts/contracts/registry/NaiveRegistryFilter.sol
🧬 Code graph analysis (20)
packages/enclave-contracts/ignition/modules/enclave.ts (1)
crates/events/src/enclave_event/compute_request/trbfv.rs (1)
params(59-63)
packages/enclave-contracts/scripts/deployAndSave/mockInputValidator.ts (1)
packages/enclave-contracts/scripts/utils.ts (1)
storeDeploymentArgs(34-62)
packages/enclave-contracts/scripts/deployAndSave/naiveRegistryFilter.ts (1)
packages/enclave-contracts/scripts/utils.ts (2)
readDeploymentArgs(70-78)storeDeploymentArgs(34-62)
packages/enclave-contracts/tasks/ciphernode.ts (1)
packages/enclave-contracts/scripts/deployAndSave/ciphernodeRegistryOwnable.ts (1)
deployAndSaveCiphernodeRegistryOwnable(29-86)
packages/enclave-contracts/hardhat.config.ts (3)
packages/enclave-contracts/tasks/ciphernode.ts (3)
ciphernodeAdd(11-33)ciphernodeRemove(35-68)ciphernodeSiblings(70-102)packages/enclave-contracts/tasks/enclave.ts (7)
requestCommittee(13-184)publishPlaintext(406-474)publishCiphertext(336-404)publishInput(289-334)activateE3(258-287)publishCommittee(211-256)enableE3(186-209)packages/enclave-contracts/tasks/utils.ts (1)
cleanDeploymentsTask(11-26)
packages/enclave-contracts/scripts/deployAndSave/enclave.ts (1)
packages/enclave-contracts/scripts/utils.ts (2)
readDeploymentArgs(70-78)storeDeploymentArgs(34-62)
templates/default/deploy/default.ts (1)
packages/enclave-contracts/scripts/utils.ts (2)
readDeploymentArgs(70-78)storeDeploymentArgs(34-62)
packages/enclave-contracts/scripts/deployAndSave/mockProgram.ts (1)
packages/enclave-contracts/scripts/utils.ts (2)
readDeploymentArgs(70-78)storeDeploymentArgs(34-62)
packages/enclave-contracts/tasks/utils.ts (1)
packages/enclave-contracts/scripts/utils.ts (1)
cleanDeployments(101-111)
packages/enclave-contracts/scripts/deployAndSave/mockDecryptionVerifier.ts (1)
packages/enclave-contracts/scripts/utils.ts (1)
storeDeploymentArgs(34-62)
packages/enclave-contracts/scripts/deployMocks.ts (4)
packages/enclave-contracts/scripts/deployAndSave/mockComputeProvider.ts (1)
deployAndSaveMockComputeProvider(15-48)packages/enclave-contracts/scripts/deployAndSave/mockDecryptionVerifier.ts (1)
deployAndSaveMockDecryptionVerifier(15-49)packages/enclave-contracts/scripts/deployAndSave/mockInputValidator.ts (1)
deployAndSaveMockInputValidator(15-45)packages/enclave-contracts/scripts/deployAndSave/mockProgram.ts (1)
deployAndSaveMockProgram(20-71)
templates/default/hardhat.config.ts (2)
packages/enclave-contracts/tasks/ciphernode.ts (1)
ciphernodeAdd(11-33)packages/enclave-contracts/tasks/utils.ts (1)
cleanDeploymentsTask(11-26)
templates/default/tests/integration.spec.ts (1)
packages/enclave-sdk/src/utils.ts (5)
DEFAULT_E3_CONFIG(86-92)calculateStartWindow(141-146)encodeBfvParams(98-123)encodeComputeProviderParams(128-136)DEFAULT_COMPUTE_PROVIDER_PARAMS(79-83)
packages/enclave-contracts/scripts/run.ts (1)
packages/enclave-contracts/scripts/deployEnclave.ts (1)
deployEnclave(16-109)
crates/entrypoint/build.rs (1)
crates/config/src/contract.rs (1)
deploy_block(28-34)
packages/enclave-contracts/scripts/deployEnclave.ts (4)
packages/enclave-contracts/scripts/deployAndSave/enclave.ts (1)
deployAndSaveEnclave(28-88)packages/enclave-contracts/scripts/deployAndSave/ciphernodeRegistryOwnable.ts (1)
deployAndSaveCiphernodeRegistryOwnable(29-86)packages/enclave-contracts/scripts/deployAndSave/naiveRegistryFilter.ts (1)
deployAndSaveNaiveRegistryFilter(21-82)packages/enclave-contracts/scripts/deployMocks.ts (1)
deployMocks(25-60)
packages/enclave-contracts/scripts/deployAndSave/mockComputeProvider.ts (1)
packages/enclave-contracts/scripts/utils.ts (1)
storeDeploymentArgs(34-62)
templates/default/scripts/deploy-local.ts (2)
packages/enclave-contracts/scripts/deployEnclave.ts (1)
deployEnclave(16-109)templates/default/deploy/default.ts (1)
deployTemplate(11-59)
packages/enclave-contracts/tasks/enclave.ts (3)
packages/enclave-contracts/scripts/deployAndSave/enclave.ts (1)
deployAndSaveEnclave(28-88)packages/enclave-contracts/scripts/utils.ts (1)
readDeploymentArgs(70-78)packages/enclave-contracts/scripts/deployAndSave/naiveRegistryFilter.ts (1)
deployAndSaveNaiveRegistryFilter(21-82)
packages/enclave-contracts/scripts/deployAndSave/ciphernodeRegistryOwnable.ts (1)
packages/enclave-contracts/scripts/utils.ts (2)
readDeploymentArgs(70-78)storeDeploymentArgs(34-62)
🪛 dotenv-linter (3.3.0)
examples/CRISP/client/.env.example
[warning] 4-4: [UnorderedKey] The VITE_E3_PROGRAM_ADDRESS key should go before the VITE_ENCLAVE_API key
(UnorderedKey)
[warning] 4-4: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 5-5: [UnorderedKey] The VITE_SEMAPHORE_ADDRESS key should go before the VITE_TWITTER_SERVERLESS_API key
(UnorderedKey)
examples/CRISP/server/.env.example
[warning] 14-14: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 14-14: [UnorderedKey] The CIPHERNODE_REGISTRY_ADDRESS key should go before the ENCLAVE_ADDRESS key
(UnorderedKey)
[warning] 15-15: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 16-16: [UnorderedKey] The E3_PROGRAM_ADDRESS key should go before the ENCLAVE_ADDRESS key
(UnorderedKey)
🪛 Gitleaks (8.27.2)
examples/CRISP/tests/crisp.contracts.test.ts
[high] 101-101: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 Shellcheck (0.10.0)
tests/integration/fns.sh
[warning] 24-24: ENCLAVE_CONTRACT appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 25-25: REGISTRY_CONTRACT appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 26-26: REGISTRY_FILTER_CONTRACT appears unused. Verify use (or export if used externally).
(SC2034)
[warning] 27-27: INPUT_VALIDATOR_CONTRACT appears unused. Verify use (or export if used externally).
(SC2034)
⏰ 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). (1)
- GitHub Check: crisp_e2e
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (8)
packages/enclave-contracts/tsconfig.json (1)
21-21: Keep custom ambient declarations included (repeat from prior review)Excluding
"types"can hide local.d.tsaugmentations (e.g., custom matchers/util globals). Unless intentionally unused, remove it fromexclude(or explicitly includetypes/**/*.ts).- "exclude": ["node_modules", "dist", "types"] + "exclude": ["node_modules", "dist"]crates/entrypoint/build.rs (2)
39-57: Address the duplicate key collision issue from previous review.The code still generates PHF map keys using only
contract_name, which will cause collisions if multiple networks are added to the JSON manifest in the future. While currently only Sepolia exists, this creates technical debt.Apply this diff to prefix keys with network name:
- if let Some(networks) = json.as_object() { + if let Some(networks) = json.as_object() { - if let Some(sepolia_data) = networks.get("sepolia") { + for (network_name, network_data) in networks { - if let Some(contracts) = sepolia_data.as_object() { + if let Some(contracts) = network_data.as_object() { for (contract_name, contract_data) in contracts { // Extract address and block number from the contract data if let (Some(address), Some(deploy_block)) = ( contract_data["address"].as_str(), contract_data["blockNumber"].as_u64(), ) { contract_info.push_str(&format!( - " \"{}\" => ContractInfo {{\n address: \"{}\",\n deploy_block: {},\n }},\n", - contract_name, address, deploy_block + " \"{}:{}\" => ContractInfo {{\n address: \"{}\",\n deploy_block: {},\n }},\n", + network_name, contract_name, address, deploy_block )); } } } } - }
45-53: Add error handling for malformed JSON entries.The code silently skips contracts with missing
addressorblockNumberfields, making debugging difficult if the JSON is malformed.Apply this diff to fail fast with clear error messages:
if let (Some(address), Some(deploy_block)) = ( contract_data["address"].as_str(), contract_data["blockNumber"].as_u64(), ) { contract_info.push_str(&format!( " \"{}\" => ContractInfo {{\n address: \"{}\",\n deploy_block: {},\n }},\n", contract_name, address, deploy_block )); + } else { + panic!( + "deployed_contracts.json: missing address or blockNumber for '{}'", + contract_name + ); }templates/default/deploy/default.ts (1)
15-17: Chain resolution may be undefined - fallback to localhost.As noted in learnings,
hre.globalOptions.networkcan be undefined in some contexts. The current implementation doesn't handle this case, which could lead to undefined behavior when accessing deployment arguments.Apply this fix to ensure robust chain resolution:
- const chain = hre.globalOptions.network; + const chain = hre.globalOptions.network ?? "localhost";packages/enclave-contracts/tasks/ciphernode.ts (4)
15-19: Remove invalid ZeroAddress default for required ciphernode parameter.Using
ZeroAddressas the default value will cause the task to fail when executed without arguments, as registering the zero address is typically invalid.Remove the default value to make the parameter required:
.addOption({ name: "ciphernodeAddress", description: "address of ciphernode to register", - defaultValue: ZeroAddress, })
39-48: Fix inconsistent default values for remove task parameters.The
siblingsparameter expects a comma-separated list but defaults to an empty string, whileciphernodeAddressdefaults toZeroAddresswhich is invalid.Update both parameters to be required:
.addOption({ name: "ciphernodeAddress", description: "address of ciphernode to remove", - defaultValue: ZeroAddress, }) .addOption({ name: "siblings", description: "comma separated siblings from tree proof", - defaultValue: "", })
57-57: Add robust error handling for siblings parsing.The current implementation will throw a cryptic error if the siblings input is malformed or contains invalid values.
Add validation and error handling:
- const siblingsArray = siblings.split(",").map((s: string) => BigInt(s)); + const siblingsArray = siblings + .split(",") + .filter(s => s.trim() !== "") + .map((s: string) => { + const trimmed = s.trim(); + try { + return BigInt(trimmed); + } catch (error) { + throw new Error(`Invalid sibling value: "${trimmed}" - must be a valid integer`); + } + });
74-84: Fix incorrect default value for ciphernodeAddresses parameter.Using
ZeroAddressas default for a comma-separated list parameter is semantically incorrect and will cause parsing errors.Make both parameters required:
.addOption({ name: "ciphernodeAddress", description: "address of ciphernode to get siblings for", - defaultValue: ZeroAddress, }) .addOption({ name: "ciphernodeAddresses", description: "comma separated addresses of ciphernodes in the order they were added to the registry", - defaultValue: ZeroAddress, })
🧹 Nitpick comments (10)
packages/enclave-contracts/tsconfig.json (6)
5-6: Prefer NodeNext modules/resolution for Hardhat/Node toolingHardhat, scripts, tasks, and Ignition modules run in Node.
module: "ESNext"withmoduleResolution: "bundler"can break Node/ts-node/tsx resolution and package "exports" condition handling. RecommendNodeNextfor both.- "module": "ESNext", - "moduleResolution": "bundler", + "module": "NodeNext", + "moduleResolution": "NodeNext",
3-4: Align lib and target to avoid typing-runtime mismatchesYou’re targeting ES2022 but typing against ES2023 APIs. Either bump target or drop lib back to ES2022. If CI/engines use Node ≥18/20 and you rely on ES2023 APIs, bump target.
Option A (bump target):
- "lib": ["es2023"], - "target": "ES2022", + "lib": ["es2023"], + "target": "ES2023",Option B (conservative libs):
- "lib": ["es2023"], + "lib": ["es2022"],
10-12: Avoid emitting declarations for tests/scripts/tasks; split configs or disable emitCurrent config will emit .d.ts/.js for tests, tasks, ignition, and hardhat.config into
dist. Typically we type-check these without emit and let Hardhat/tsx transpile at runtime. Suggest disabling emit here, and (if needed) create a separate build tsconfig for any publishable library code.Minimal change (no emit):
- "outDir": "dist", - "declaration": true, - "declarationMap": true + "noEmit": trueOr split configs:
- tsconfig.json (tools):
"noEmit": true,module/moduleResolution: "NodeNext".- tsconfig.build.json (library/artifacts packaging): emits with
outDir,declaration, excludes tests/scripts/tasks/hardhat config.
2-13: Enable JSON imports if code imports deployment manifestsYou mention reading/writing
deployed_contracts.json. If imported in TS, addresolveJsonModule."compilerOptions": { + "resolveJsonModule": true,
2-13: Pin ambient types for cleaner tool/test DXTo avoid pulling every
@types/*and to get globals for Hardhat/Mocha/Chai, explicitly settypes. Adjust if using Vitest instead of Mocha."compilerOptions": { + "types": ["node", "mocha", "chai", "hardhat"],
14-20: Optional: isolate tools from tests in includesGrouping tools (
hardhat.config.ts,scripts/**/*.ts,tasks/**/*.ts,ignition/modules/**/*.ts) and tests under one tsconfig can make editor/tsserver slower. Consider a dedicatedtsconfig.tools.json(referenced by IDE) and keep tests in the main tsconfig (or vice versa).packages/enclave-contracts/tasks/enclave.ts (3)
11-11: Consider caching deployment imports and resultsThe deployment functions are dynamically imported and executed in every task action. This could lead to unnecessary file reads and potential performance issues if the same tasks are run multiple times in a session.
Consider moving the imports to the module level or implementing a caching mechanism to avoid repeated deployments when the configuration hasn't changed:
// At module level let deploymentCache: Record<string, any> = {}; // In task actions if (!deploymentCache.enclave) { const { deployAndSaveEnclave } = await import( "../scripts/deployAndSave/enclave" ); deploymentCache.enclave = await deployAndSaveEnclave({ hre }); } const { enclave } = deploymentCache.enclave;Also applies to: 86-88, 195-197, 235-237, 280-282, 320-322, 379-381, 449-451
327-331: Add validation for empty file contentThe tasks read files but don't validate if the content is empty or if the file exists, which could lead to confusing behavior.
Add validation for file content:
if (dataFile) { + if (!fs.existsSync(dataFile)) { + throw new Error(`Data file not found: ${dataFile}`); + } const file = fs.readFileSync(dataFile); + if (file.length === 0) { + throw new Error(`Data file is empty: ${dataFile}`); + } dataToSend = file.toString(); }Also applies to: 387-390, 455-460, 462-467
175-175: Consider making ETH value configurableThe ETH value sent with the
enclave.requesttransaction is hardcoded to 1 ETH. This might not be appropriate for all networks or use cases.Consider adding an option for the ETH value:
+ .addOption({ + name: "value", + description: "ETH value to send with the request (in wei)", + defaultValue: "1000000000000000000", + type: ArgumentType.STRING, + }) .setAction(async () => ({ default: async ( { // ... existing parameters + value, }, hre, ) => { // ... existing code const tx = await enclave.request( { // ... existing parameters }, - { value: "1000000000000000000" }, + { value }, );templates/default/hardhat.config.ts (1)
70-74: Block explorer configuration may be incompleteThe block explorer configuration only includes
apiUrlbut other networks might require additional configuration likeurlfor the explorer web interface.Consider adding the explorer URL for completeness:
blockExplorers: { etherscan: { apiUrl, + url: getBlockExplorerUrl(chain), }, },And add a helper function:
function getBlockExplorerUrl(chain: keyof typeof chainIds): string { const explorers: Record<string, string> = { "arbitrum-mainnet": "https://arbiscan.io", "avalanche": "https://snowtrace.io", "bsc": "https://bscscan.com", // ... other networks }; return explorers[chain] || `https://${chain}.etherscan.io`; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
crates/entrypoint/build.rs(3 hunks)examples/CRISP/tsconfig.json(1 hunks)packages/enclave-contracts/.gitignore(1 hunks)packages/enclave-contracts/artifacts/contracts/interfaces/ICiphernodeRegistry.sol/ICiphernodeRegistry.json(2 hunks)packages/enclave-contracts/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.json(2 hunks)packages/enclave-contracts/artifacts/contracts/registry/NaiveRegistryFilter.sol/NaiveRegistryFilter.json(2 hunks)packages/enclave-contracts/contracts/Enclave.sol(1 hunks)packages/enclave-contracts/deployed_contracts.json(1 hunks)packages/enclave-contracts/hardhat.config.ts(1 hunks)packages/enclave-contracts/ignition/modules/mockE3Program.ts(1 hunks)packages/enclave-contracts/package.json(2 hunks)packages/enclave-contracts/scripts/deployAndSave/ciphernodeRegistryOwnable.ts(1 hunks)packages/enclave-contracts/scripts/deployAndSave/enclave.ts(1 hunks)packages/enclave-contracts/scripts/deployAndSave/mockComputeProvider.ts(1 hunks)packages/enclave-contracts/scripts/deployAndSave/mockDecryptionVerifier.ts(1 hunks)packages/enclave-contracts/scripts/deployAndSave/mockInputValidator.ts(1 hunks)packages/enclave-contracts/scripts/deployAndSave/mockProgram.ts(1 hunks)packages/enclave-contracts/scripts/deployAndSave/naiveRegistryFilter.ts(1 hunks)packages/enclave-contracts/scripts/run.ts(1 hunks)packages/enclave-contracts/scripts/utils.ts(1 hunks)packages/enclave-contracts/tasks/ciphernode.ts(1 hunks)packages/enclave-contracts/tasks/enclave.ts(1 hunks)packages/enclave-contracts/tsconfig.json(1 hunks)templates/default/deploy/default.ts(1 hunks)templates/default/hardhat.config.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (15)
- packages/enclave-contracts/scripts/deployAndSave/mockProgram.ts
- packages/enclave-contracts/scripts/deployAndSave/mockInputValidator.ts
- packages/enclave-contracts/.gitignore
- packages/enclave-contracts/scripts/utils.ts
- packages/enclave-contracts/artifacts/contracts/interfaces/ICiphernodeRegistry.sol/ICiphernodeRegistry.json
- packages/enclave-contracts/scripts/run.ts
- packages/enclave-contracts/scripts/deployAndSave/mockDecryptionVerifier.ts
- packages/enclave-contracts/scripts/deployAndSave/ciphernodeRegistryOwnable.ts
- packages/enclave-contracts/ignition/modules/mockE3Program.ts
- packages/enclave-contracts/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.json
- packages/enclave-contracts/hardhat.config.ts
- packages/enclave-contracts/scripts/deployAndSave/mockComputeProvider.ts
- examples/CRISP/tsconfig.json
- packages/enclave-contracts/scripts/deployAndSave/enclave.ts
- packages/enclave-contracts/scripts/deployAndSave/naiveRegistryFilter.ts
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-09-11T13:09:03.764Z
Learnt from: ctrlc03
PR: gnosisguild/enclave#677
File: packages/enclave-contracts/scripts/deployAndSave/naiveRegistryFilter.ts:30-31
Timestamp: 2025-09-11T13:09:03.764Z
Learning: In Hardhat v3 deployment scripts, use `(await signer.provider?.getNetwork())?.name ?? "localhost"` instead of `hre.globalOptions.network` to reliably get the network name, as `hre.globalOptions.network` can be undefined in some contexts.
Applied to files:
templates/default/deploy/default.ts
📚 Learning: 2025-09-11T13:09:03.764Z
Learnt from: ctrlc03
PR: gnosisguild/enclave#677
File: packages/enclave-contracts/scripts/deployAndSave/naiveRegistryFilter.ts:30-31
Timestamp: 2025-09-11T13:09:03.764Z
Learning: In Hardhat v3, hre.network.name is not available anymore. Use hre.globalOptions.network to get the network name instead.
Applied to files:
templates/default/deploy/default.ts
📚 Learning: 2025-09-11T13:02:56.305Z
Learnt from: ctrlc03
PR: gnosisguild/enclave#677
File: packages/enclave-contracts/scripts/deployAndSave/mockComputeProvider.ts:6-6
Timestamp: 2025-09-11T13:02:56.305Z
Learning: In Hardhat v3, the import path "hardhat/types/hre" may be valid for importing HardhatRuntimeEnvironment, unlike in Hardhat v2 where the standard path was "hardhat/types".
Applied to files:
packages/enclave-contracts/package.json
📚 Learning: 2025-09-11T13:21:30.996Z
Learnt from: ctrlc03
PR: gnosisguild/enclave#677
File: packages/enclave-contracts/tasks/utils.ts:7-8
Timestamp: 2025-09-11T13:21:30.996Z
Learning: In Hardhat v3, the task API syntax has changed significantly from v2. The new syntax uses:
- `.addOption({ name, description, defaultValue, type })` instead of `.addOptionalParam()`
- `.setAction(async () => ({ default: (args, hre) => { ... } }))` instead of direct `.setAction((args, hre) => { ... })`
- `.build()` is required to finalize task definitions
- `ArgumentType.STRING` is used for option types instead of `types.string`
Applied to files:
templates/default/hardhat.config.tspackages/enclave-contracts/tasks/enclave.tspackages/enclave-contracts/tasks/ciphernode.ts
🧬 Code graph analysis (5)
crates/entrypoint/build.rs (1)
crates/config/src/contract.rs (1)
deploy_block(28-34)
templates/default/deploy/default.ts (1)
packages/enclave-contracts/scripts/utils.ts (2)
readDeploymentArgs(74-88)storeDeploymentArgs(34-66)
templates/default/hardhat.config.ts (2)
packages/enclave-contracts/tasks/ciphernode.ts (1)
ciphernodeAdd(11-33)packages/enclave-contracts/tasks/utils.ts (1)
cleanDeploymentsTask(11-26)
packages/enclave-contracts/tasks/enclave.ts (3)
packages/enclave-contracts/scripts/deployAndSave/enclave.ts (1)
deployAndSaveEnclave(28-91)packages/enclave-contracts/scripts/utils.ts (1)
readDeploymentArgs(74-88)packages/enclave-contracts/scripts/deployAndSave/naiveRegistryFilter.ts (1)
deployAndSaveNaiveRegistryFilter(21-87)
packages/enclave-contracts/tasks/ciphernode.ts (1)
packages/enclave-contracts/scripts/deployAndSave/ciphernodeRegistryOwnable.ts (1)
deployAndSaveCiphernodeRegistryOwnable(29-91)
🔇 Additional comments (12)
packages/enclave-contracts/artifacts/contracts/registry/NaiveRegistryFilter.sol/NaiveRegistryFilter.json (3)
302-303: Recompiled bytecode — re-check verification and determinism.
- Bytecode changed; update Etherscan/Blockscout verification artifacts/paths.
- Confirm a single solc version and identical optimizer/metadata settings across all builds; scan found multiple hardhat.config.* "solidity"/"optimizer" blocks and the build-info solc-version query returned no results — verify/unify configs and regenerate deterministic build.
2-2: Verify external tooling supports Hardhat v3 artifacts ("hh3-artifact-1")rg returned only artifact JSON hits (no ts/js/sh files in this repo appear to hard-code the old artifact format). Matches: packages/enclave-contracts/artifacts/contracts/interfaces/ICiphernodeRegistry.sol/ICiphernodeRegistry.json (lines 219–222), packages/enclave-contracts/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.json (lines 704–707), packages/enclave-contracts/artifacts/contracts/registry/NaiveRegistryFilter.sol/NaiveRegistryFilter.json (lines 305–308). Verify external CI/deploy/verify/packaging scripts that consume artifacts accept "_format": "hh3-artifact-1" and the hh3 layout (buildInfoId / inputSourceName / immutableReferences / deployedLinkReferences).
305-309: Verify tooling that depends on artifact source paths; prefer buildInfoId
- Artifacts include buildInfoId and both sourceName (repo-relative) and inputSourceName (prefixed with "project/…"). Example: packages/enclave-contracts/artifacts/contracts/interfaces/ICiphernodeRegistry.sol/ICiphernodeRegistry.json shows inputSourceName="project/contracts/interfaces/ICiphernodeRegistry.sol" and a buildInfoId.
- Quick scan under packages found no JS/TS references to artifacts.readArtifactSync, artifact.sourceName, or readArtifactSync; if any tooling relies on artifact paths, update it to use buildInfoId instead.
packages/enclave-contracts/contracts/Enclave.sol (1)
173-173: Whitespace-only change - no functional impact.This is a cosmetic formatting change that adds a blank line for better code readability. The change aligns with the broader code reformatting efforts in this PR's migration to Hardhat v3.
packages/enclave-contracts/deployed_contracts.json (1)
1-30: LGTM! Centralized deployment manifest provides clear structure.The JSON structure is well-organized with proper nesting (network -> contract -> metadata). The contract entries include all necessary deployment information (address, blockNumber, and constructorArgs), and the constructor arguments properly reference each other (e.g., CiphernodeRegistry references Enclave address, NaiveRegistryFilter references CiphernodeRegistry address).
crates/entrypoint/build.rs (3)
25-25: LGTM! Path correctly updated to use centralized manifest.The change from a deployments directory structure to a single
deployed_contracts.jsonfile aligns with the PR objective to consolidate deployment metadata.
35-37: LGTM! JSON parsing approach is appropriate.The switch from directory traversal to JSON parsing is clean and matches the new centralized manifest approach.
65-65: LGTM! Build dependency correctly updated.The rerun trigger properly points to the new centralized manifest file instead of the old deployments directory.
templates/default/deploy/default.ts (1)
11-63: LGTM! Successful migration to Hardhat v3 with proper deployment flow.The deployment implementation correctly:
- Awaits all deployments before contract interactions
- Uses the new ESM export pattern
- Properly stores deployment arguments for reuse
- Handles chain-specific deployments
packages/enclave-contracts/package.json (1)
6-6: LGTM! Correct migration to ES modules.Adding
"type": "module"aligns with Hardhat v3's modern ESM-first approach and is consistent with the PR's migration goals.templates/default/hardhat.config.ts (2)
54-62: LGTM! Clean implementation of account configurationThe account configuration logic properly handles both private key and mnemonic scenarios with appropriate fallbacks.
130-141: LGTM! Well-organized Solidity configurationThe
npmFilesToBuildapproach provides clear visibility of external dependencies and the contracts being compiled. The optimizer settings are appropriate for production deployments.
2 participants
fix #663
More details:
utils:clean-deployments --chain $CHAIN)Summary by CodeRabbit