chore: generate solidity verifiers for noir circuits [skip-line-limit]#1295
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds end-to-end Noir verifier generation and deployment: a CLI generator for Solidity verifiers, discovery/deploy utilities that link ZKTranscriptLib, integration into enclave deployment flow, package/script/config updates, and updated deployment metadata. Changes
Sequence Diagram(s)sequenceDiagram
actor Dev as Developer
participant CLI as generate-verifiers.ts
participant FS as Filesystem (circuits/bin)
participant Nargo as nargo
participant BB as Barretenberg (bb)
participant Out as contracts/verifier
Dev->>CLI: run with --group/--circuit
CLI->>FS: discover circuits & metadata
alt compile enabled
CLI->>Nargo: compile circuits
Nargo-->>CLI: compiled artifacts
end
CLI->>BB: generate VKs & Solidity verifiers
BB-->>CLI: verifier Solidity files
CLI->>Out: normalize names/headers and write contracts
CLI-->>Dev: summary of generated verifiers
sequenceDiagram
participant Deploy as deployEnclave.ts
participant VerifierMgr as deployAndSaveAllVerifiers()
participant Discovery as discoverVerifierContracts()
participant LibraryDeployer as deployZKTranscriptLib()
participant Chain as Blockchain
participant Records as deployed_contracts.json
Deploy->>VerifierMgr: request verifier deployments
VerifierMgr->>Discovery: list verifier contracts
Discovery-->>VerifierMgr: contract names
loop for each missing verifier
VerifierMgr->>LibraryDeployer: ensure ZKTranscriptLib deployed
LibraryDeployer->>Chain: deploy library tx
Chain-->>Records: store library address/block
VerifierMgr->>Chain: deploy verifier with linked library
Chain-->>Records: store verifier address/block
end
VerifierMgr-->>Deploy: mapping of verifier name→address
Deploy->>Deploy: log "Circuit Verifiers" section
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/enclave-contracts/hardhat.config.ts (1)
176-189: 🛠️ Refactor suggestion | 🟠 MajorReducing optimizer runs globally affects all contracts, not just verifiers.
Lowering from 800 to 100 runs trades higher runtime gas costs for smaller bytecode — a reasonable tradeoff for large auto-generated verifier contracts that may exceed the 24KB contract size limit. However, this change penalizes all other contracts in the project (e.g., Enclave, BondingRegistry) that benefit from higher optimization runs.
Hardhat supports per-file compiler overrides. Instead of a global reduction, configure overrides at the
soliditylevel to target only the verifier contracts:Suggested approach
solidity: { compilers: [ { version: "0.8.28", settings: { optimizer: { enabled: true, - runs: 100, + runs: 800, }, metadata: { bytecodeHash: "none", }, }, }, ], + overrides: { + "contracts/verifier/MyVerifier.sol": { + settings: { + optimizer: { + enabled: true, + runs: 100, + }, + }, + }, + }, },Replace
"contracts/verifier/MyVerifier.sol"with the specific verifier contract file paths.
🤖 Fix all issues with AI agents
In `@packages/enclave-contracts/deployed_contracts.json`:
- Around line 110-115: Remove the stale top-level "undefined" entry from
deployed_contracts.json and update all deployment scripts (including
verifiers.ts and any script that reads/writes deployed_contracts.json) to stop
using hre.globalOptions.network; instead derive the network via the
signer/provider, e.g. use (await signer.provider?.getNetwork())?.name ??
"localhost" when computing the network key used to store contracts like
PoseidonT3, so future runs write the correct network name rather than
"undefined".
In `@packages/enclave-contracts/ignition/modules/dkgPkVerifier.ts`:
- Around line 8-12: This Ignition module currently defines DkgPkVerifier via
buildModule/DkgPkVerifier and returns { dkgPkVerifier } but is unused and casts
to any; either delete this orphaned module file or implement proper library
linking and typing: import and register the ZKTranscriptLib using
m.library("ZKTranscriptLib", ...) and ensure m.contract("DkgPkVerifier") is
linked to that library just like the scripts/deployAndSave/verifiers.ts flow,
then replace the unsafe "as any" with a concrete module return type matching the
module's contract shape; remove dead exports if you choose deletion.
In `@packages/enclave-contracts/scripts/deployAndSave/verifiers.ts`:
- Around line 59-68: The code uses hre.globalOptions.network to set the chain
name (assigned to variable chain) which can be undefined; update the logic that
sets chain to derive the network name from the signer/provider instead (e.g.,
use the signer or ethers provider getNetwork result and fall back to
'localhost') so readDeploymentArgs and deployment writes use a reliable network
key; change the assignment that creates chain (where hre.globalOptions.network
is read) to use (await signer.provider?.getNetwork())?.name ?? 'localhost' or
equivalent provider.getNetwork() call before calling
readDeploymentArgs(contractName, chain).
In `@scripts/generate-verifiers.ts`:
- Around line 333-351: The CLI parsing loop that handles flags like '--circuit',
'--group', and '--oracle-hash' does not validate the next argument, so args[++i]
may be undefined or another flag; update the for-loop logic to first peek at
args[i+1], ensure it exists and does not start with '-' before consuming it, and
only then assign to options.circuits (push), options.groups (split into
CircuitGroup[]), or options.oracleHash; if the value is missing or invalid,
print a clear error (or call showHelp) and exit with non-zero status to avoid
propagating undefined values into downstream functions like discoverCircuits or
the bb write_vk call.
🧹 Nitpick comments (9)
packages/enclave-contracts/scripts/deployAndSave/verifiers.ts (3)
16-26:discoverVerifierContractsonly scans top-level files — non-recursive.
readdirSyncwithout{ recursive: true }will miss verifier contracts in subdirectories undercontracts/verifier/. If the directory structure is guaranteed to be flat, this is fine, but worth noting for future-proofing.
32-49:linkReferencesaccessed viaas anycast.The
artifacttype from Hardhat may not includelinkReferencesin its type definition, but it is part of the standard Solidity compiler output (and Hardhat artifacts do include it). Theas anycast works but silently bypasses type safety. Consider using a type assertion to a more specific interface or checking if the Hardhat v3 artifact type now includeslinkReferences.
88-104: Library deployed using short name — could be ambiguous.Line 90 resolves the library factory using just
libName(e.g.,"ZKTranscriptLib"), not the fully-qualified name (source:libName). If two different source files define libraries with the same name, this would be ambiguous. Currently this is unlikely but is a latent fragility.packages/enclave-contracts/scripts/deployEnclave.ts (1)
251-252: Minor indentation mismatch in the deployment log.
verifierLinesuses 4-space indentation (${name}: ${addr}), but the surrounding template literal context already starts each line at an 4-space indent. The result is that verifier entries will be left-aligned relative to the other entries (e.g.,MockFeeToken: ...at column 4 vs verifier names also at column 4), while theCircuit Verifiers:label sits at the same level. This is cosmetically fine but slightly different from what may be intended — if you want sub-items indented under the label, bumpverifierLinesto 6 or 8 spaces.scripts/generate-verifiers.ts (5)
344-345: No validation of--groupvalues against known circuit groups.Passing an invalid group name (e.g.,
--group foo) silently produces zero results. Consider validating againstALL_GROUPS:Proposed validation
} else if (arg === '--group') { - options.groups = args[++i]?.split(',') as CircuitGroup[] + const val = args[++i] + const groups = val?.split(',') ?? [] + const invalid = groups.filter((g) => !ALL_GROUPS.includes(g as CircuitGroup)) + if (invalid.length > 0) { + console.error(`Error: unknown group(s): ${invalid.join(', ')}. Valid: ${ALL_GROUPS.join(', ')}`) + process.exit(1) + } + options.groups = groups as CircuitGroup[]
207-211: License-header regex is greedy — may consume more than intended.The pattern
(\/\/[^\n]*\n)*after the SPDX match will eat every consecutive//-comment line, not just the license block. Ifbbever emits additional//comments (e.g., a version stamp or// Auto-generated) between the license andpragma solidity, those lines will be silently removed. Consider anchoring more tightly, for example by stopping at a blank line or atpragma:- solidity = solidity.replace(/\/\/\s*SPDX-License-Identifier:[^\n]*\n(\/\/[^\n]*\n)*/, LICENSE_HEADER) + solidity = solidity.replace(/\/\/\s*SPDX-License-Identifier:[^\n]*\n(\/\/[^\n]*\n)*?(?=\n|pragma)/, LICENSE_HEADER)Low risk with current
bboutput, but worth hardening.
381-381:require.main === moduleis a CJS idiom in an ESM-style file.This works because
tsxtranspiles to CJS, but it's fragile if the project ever switches to native ESM (e.g., Node's--experimental-strip-types). A more portable guard would be to just callmain()unconditionally since this file is only invoked as a script via thegenerate:verifiersnpm script.
254-282: Inconsistent cleanup of barevkfile.When
vkalready exists (line 262–264), the file is copied to<pkg>.vkbut the originalvkis left in place. Whenvkis freshly generated (line 272–274), the original is removed after copying. If this asymmetry is intentional (preserving a user-suppliedvk), a brief comment would help; otherwise, consider unifying the behavior.
238-240:execSyncwithstdio: 'pipe'swallows stderr on failure.When
nargo compile(orbb) fails, the thrown error's.messageonly contains the generic "Command failed" text. The actual diagnostic output from the tool is inerror.stderr, which is never surfaced to the user. Consider including stderr in the error message:Proposed improvement
- execSync('nargo compile', { cwd: circuit.path, stdio: 'pipe' }) + try { + execSync('nargo compile', { cwd: circuit.path, stdio: 'pipe' }) + } catch (err: any) { + const stderr = err.stderr?.toString().trim() + throw new Error(`nargo compile failed${stderr ? `:\n${stderr}` : ''}`) + }The same applies to
bb write_vk(line 269) andbb write_solidity_verifier(line 194).
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/enclave-contracts/scripts/deployAndSave/verifiers.ts`:
- Around line 89-91: The deployment uses the short name via
ethers.getContractFactory(libName) which can cause ambiguity for identically
named libraries; change the factory lookup to use the fully qualified name (fqn)
instead of libName (i.e., call ethers.getContractFactory(fqn) when creating
libFactory) and update the console/debug message if desired to reference fqn;
ensure the rest of the code that references libFactory and deployed library
address (variables like libFactory and libName) continues to work with the
FQN-based factory.
- Around line 16-26: The verifier discovery uses process.cwd() which breaks in
monorepos; update discoverVerifierContracts to build verifierDir using __dirname
(or path.resolve(__dirname, "..", "..", "contracts/verifier") as appropriate)
instead of process.cwd(), so the path is resolved relative to the script file;
keep the rest of the logic (fs.existsSync, readdirSync, filter map) unchanged
and ensure verifierDir variable is the only path change.
In `@scripts/generate-verifiers.ts`:
- Around line 267-269: Replace string-interpolated execSync invocations with
execFileSync and pass command + arguments as an array to avoid shell
interpolation: import execFileSync (alongside execSync) and change the call that
currently invokes `execSync(`bb write_vk -b "${jsonFile}" -o
"${targetDir}"${oracleHashFlag}`, { stdio: 'pipe' })` to call
`execFileSync('bb', [ 'write_vk', '-b', jsonFile, '-o', targetDir, /* include
'--oracle_hash' and this.options.oracleHash only if present */ ], { stdio:
'pipe' })`; similarly convert the `bb write_solidity_verifier` and `nargo
compile` calls to use execFileSync with explicit argument arrays so no
user-controlled values (like this.options.oracleHash) are passed into a shell
string.
🧹 Nitpick comments (3)
scripts/generate-verifiers.ts (1)
193-194:execSynccalls swallow stderr — failures will be opaque.All
execSynccalls usestdio: 'pipe', and on failure the caught error'smessagerarely includes the process's stderr. Consider capturing stderr and including it in the thrown error for debuggability.Illustrative approach
- execSync(`bb write_solidity_verifier -k "${vkPath}" -o "${rawSolPath}"`, { stdio: 'pipe' }) + try { + execSync(`bb write_solidity_verifier -k "${vkPath}" -o "${rawSolPath}"`, { stdio: 'pipe' }) + } catch (err: any) { + throw new Error(`bb write_solidity_verifier failed: ${err.stderr?.toString() ?? err.message}`) + }Same applies to lines 240 and 269.
packages/enclave-contracts/scripts/deployAndSave/verifiers.ts (2)
32-48: Library detection logic looks correct for standard Solidity artifacts.The unlinked-library placeholder regex and
linkReferencesextraction are aligned with the standard Hardhat artifact format.Minor note:
(artifact as any).linkReferences—linkReferencesis part of the HardhatArtifacttype, so theas anycast may be unnecessary.
142-149: Redundant network connection —hre.network.connect()called here and again inside eachdeployAndSaveVerifiercall.
deployAndSaveAllVerifiersconnects to the network (lines 146-148) solely for logging the chain name, then eachdeployAndSaveVerifiercall reconnects independently. Consider passing theethersinstance andchainas parameters todeployAndSaveVerifierto avoid repeated connections, or at minimum note this is intentional for standalone usage.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/enclave-contracts/scripts/deployAndSave/verifiers.ts`:
- Around line 86-90: The library FQN currently includes an incorrect "project/"
prefix which prevents Hardhat v3 from matching compiled artifact linkReferences;
update the construction of libraryFQN in verifiers.ts to remove the "project/"
prefix so it becomes `contracts/verifier/${contractName}.sol:ZKTranscriptLib`,
and ensure the libraries object still maps that corrected libraryFQN to
zkTranscriptLibAddress (references: libraryFQN, contractName,
zkTranscriptLibAddress, libraries).
In `@scripts/README.md`:
- Around line 306-318: Update the fenced code block that starts with ``` and
contains "🔮 Generating Solidity verifiers from Noir circuits..." by adding a
language identifier (e.g., change the opening fence from ``` to ```text or
```console) so the block is marked as plain terminal output and satisfies the
MD040 lint rule.
🧹 Nitpick comments (1)
packages/enclave-contracts/scripts/deployAndSave/verifiers.ts (1)
32-59:deployZKTranscriptLibrelies on an externally-passedchainwhiledeployAndSaveVerifierderives it internally — consider making this consistent.
deployZKTranscriptLibtrusts the caller to pass the correctchainstring, whiledeployAndSaveVerifier(line 77) derives it from the provider. IfdeployZKTranscriptLibis called standalone with an incorrect chain value, the deployment record will be stored under the wrong network key.Consider either having
deployZKTranscriptLibderive the chain internally (likedeployAndSaveVerifierdoes), or havingdeployAndSaveVerifieralso acceptchainas a parameter for consistency.
cedoor
left a comment
cedoor
left a comment
There was a problem hiding this comment.
LGTM
*We won't need to generate verifiers for wrappers probably.
3 participants
Implements automated generation and deployment of Solidity verifier contracts from Noir circuits.
Usage:
Summary by CodeRabbit
New Features
Chores