feat: add contract verification on CRISP#885
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
|
❌ 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 |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughAdds contract verification scripts for CRISP and enclave-contracts packages, updates Hardhat configuration to support verification workflows via the hardhat-verify plugin, expands deployment artifact metadata for Sepolia, and refactors artifact lookup to use compiled artifacts instead of source files. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant VerifyScript as verify.ts
participant Hardhat
participant VerifyFn as verifyContracts()
participant Explorer as Block Explorer
User->>VerifyScript: Run verify script
VerifyScript->>Hardhat: Determine network from signer
Hardhat-->>VerifyScript: Network name (or "localhost")
VerifyScript->>VerifyFn: verifyContracts(chainName)
loop For each contract
VerifyFn->>VerifyFn: Search artifacts for contract
alt External package
VerifyFn->>VerifyFn: Skip (log)
else Local contract
VerifyFn->>VerifyFn: Extract sourceName & contractName
VerifyFn->>Explorer: Submit for verification
Explorer-->>VerifyFn: Verification status
end
end
VerifyFn-->>VerifyScript: Complete
VerifyScript-->>User: Done
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes The changes span multiple heterogeneous categories (new scripts, configuration updates, artifact-based lookup refactoring, and deployment data expansion) across 7 files, requiring contextual review of both the new verification flow and the updated lookup mechanism, though individual changes within each file are relatively self-contained. Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ 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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
examples/CRISP/hardhat.config.ts (1)
123-130: Consider validating the API key presence.The configuration uses an empty string fallback for
ETHERSCAN_API_KEY, which is consistent with other API keys in this config. However, verification will fail if the key is not provided. Consider adding a check in the verify script (deploy/verify.ts) to ensure the API key is set before attempting verification.For example, in
deploy/verify.ts:if (!process.env.ETHERSCAN_API_KEY) { console.warn("⚠️ ETHERSCAN_API_KEY not set. Verification will fail."); process.exit(1); }examples/CRISP/deploy/verify.ts (2)
11-17: Consider adding network validation.The network detection logic is sound, but verification is typically run on public networks (not localhost). Consider adding a check to warn if attempting to verify on localhost or ensure the ETHERSCAN_API_KEY is set before proceeding.
Example validation:
const chain = (await signer.provider?.getNetwork())?.name ?? "localhost"; if (chain === "localhost" || chain === "hardhat") { console.warn("⚠️ Verification is not supported on local networks"); process.exit(0); } if (!process.env.ETHERSCAN_API_KEY) { console.error("❌ ETHERSCAN_API_KEY environment variable is required"); process.exit(1); }
19-21: Consider adding explicit exit code on error.The error handler logs the error but doesn't explicitly exit with a non-zero code. This could cause CI/CD pipelines to incorrectly report success.
main().catch((error) => { console.error(error); + process.exit(1); });examples/CRISP/package.json (1)
49-49: Update @nomicfoundation/hardhat-verify to the latest stable version.The latest stable version of @nomicfoundation/hardhat-verify is 3.0.3, released Oct 8, 2025. The PR currently specifies ^3.0.1. Consider updating to 3.0.3 to ensure you have the latest features and bug fixes.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
examples/CRISP/deploy/verify.ts(1 hunks)examples/CRISP/deployed_contracts.json(1 hunks)examples/CRISP/hardhat.config.ts(3 hunks)examples/CRISP/package.json(1 hunks)packages/enclave-contracts/deployed_contracts.json(2 hunks)packages/enclave-contracts/scripts/index.ts(1 hunks)packages/enclave-contracts/scripts/verify.ts(2 hunks)
🔇 Additional comments (13)
examples/CRISP/hardhat.config.ts (2)
16-16: LGTM! Hardhat verify plugin imported correctly.The import follows the standard pattern for Hardhat plugins and aligns with the package dependency.
72-72: LGTM! Plugin registered correctly.The hardhat-verify plugin is properly added to the plugins array.
examples/CRISP/package.json (2)
29-29: LGTM! Playwright report script added.The report script provides a convenient way to view Playwright test results.
30-30: LGTM! Verification script added correctly.The verify script properly invokes the new verification workflow via
deploy/verify.ts.packages/enclave-contracts/deployed_contracts.json (1)
4-52: LGTM! Deployment artifacts updated consistently.The deployment data has been updated with new block numbers and addresses. The cross-references are consistent—for example,
MockE3Program.constructorArgs.mockInputValidatorcorrectly matches the updatedMockInputValidator.address. The sequential block numbers (9473394-9473398) indicate a fresh deployment sequence.packages/enclave-contracts/scripts/verify.ts (4)
18-21: LGTM! Artifact-based lookup is the correct approach.The parameter change from
contractsDirtoartifactsDircorrectly reflects the new artifact-based lookup strategy. Using the standardartifactsdirectory aligns with Hardhat conventions.Note: This is a breaking API change if external code calls
findContractPathdirectly.
32-62: Artifact-based lookup implementation looks solid.The refactored logic correctly:
- Searches for artifact JSON files by contract name
- Parses artifact metadata (sourceName, contractName)
- Skips external packages (node_modules, @-prefixed)
- Returns properly formatted qualified names
The error handling for JSON parsing is appropriate.
40-48: External package detection is well-implemented.The logic correctly identifies and skips external contracts by checking for:
- Scoped packages (
@)- Local npm references (
./@)- Node modules paths (
node_modules)This prevents unnecessary verification attempts on third-party contracts.
51-57: LGTM! Path formatting is correct.The logic properly formats the fully qualified contract name by:
- Removing the leading
./from relative paths- Constructing the format
path:contractNameas expected by Hardhat verifypackages/enclave-contracts/scripts/index.ts (1)
17-17: LGTM! Verify module properly exported.The re-export makes the verification functionality available through the public API, enabling usage in the CRISP example and other consumers.
examples/CRISP/deploy/verify.ts (1)
7-9: LGTM! Imports are correct.The script properly imports the necessary modules from Hardhat and the enclave-contracts package.
examples/CRISP/deployed_contracts.json (2)
11-83: LGTM! Deployment data is internally consistent.The deployment artifacts have been added with proper structure. All address cross-references are consistent:
CiphernodeRegistryOwnable.constructorArgs.enclaveAddressmatchesEnclave.addressNaiveRegistryFilter.constructorArgs.ciphernodeRegistryAddressmatchesCiphernodeRegistryOwnable.addressMockE3Program.constructorArgs.mockInputValidatormatchesMockInputValidator.addressCRISPProgram.constructorArgscorrectly references all dependent contract addresses
1-85: Comprehensive deployment data added successfully.The file adds complete deployment information for CRISP contracts on Sepolia, including:
- Core Enclave infrastructure
- Mock contracts for testing
- CRISP-specific contracts (CRISPInputValidatorFactory, HonkVerifier, CRISPProgram)
The JSON structure is valid and follows the established pattern from the enclave-contracts package.
ctrlc03
force-pushed
the
feat/verify-crisp-contracts
branch
from
2f831bd to
200e78b
Compare
2 participants
add verification for CRISP contracts
Summary by CodeRabbit
New Features
Updates