feat: disable Circom for client-side ZK framework#291
Conversation
- Default clientZkFramework to Noir instead of Circom - Remove assignPreferredZkFramework SDK calls that override user selections - Clean up client framework dropdown to only show Noir option - Update local proving messages to be framework-agnostic
📝 WalkthroughWalkthroughThe pull request updates Local Proving availability messaging and changes how ZK frameworks are selected during blueprint creation. The default Client ZK Framework is switched from Circom to Noir, and automatic framework inference logic is removed in favor of explicit user selection through the UI. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
⏰ 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)
🔇 Additional comments (5)
✏️ Tip: You can disable this entire section by setting 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 |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/[id]/SelectEmails.tsx (1)
478-481: Click handler missing framework check present in the disabled styling.The disabled state at lines 470-475 includes the
clientZkFrameworkcheck, but the click handler at line 479 only guards againstselectedEmail === null || !!isCreateProofLoading. IfclientZkFrameworkisNone, the card appears disabled (via CSS) but clicking it will still invokehandleStartProofGeneration(true).Proposed fix
onClick={() => { - if (selectedEmail === null || !!isCreateProofLoading) return; + if ( + selectedEmail === null || + !!isCreateProofLoading || + !blueprint?.props.clientZkFramework || + // `@ts-ignore` ZkFramework can be None + blueprint?.props.clientZkFramework === ZkFramework.None + ) return; handleStartProofGeneration(true); }}
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/app/[id]/AddInputs.tsxsrc/app/[id]/SelectEmails.tsxsrc/app/create/[id]/createBlueprintSteps/EmailDetails.tsxsrc/app/create/[id]/store.ts
⏰ 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: test
🔇 Additional comments (5)
src/app/[id]/AddInputs.tsx (1)
137-146: LGTM!The updated messaging is now framework-agnostic, correctly indicating that local proving is unavailable without referencing a specific framework. The condition logic properly checks
clientZkFrameworkrather thanserverZkFramework.src/app/create/[id]/createBlueprintSteps/EmailDetails.tsx (1)
258-262: LGTM!Good approach to disable Circom by commenting it out with a TODO rather than deleting. This preserves the option for easy re-enablement once client-side Circom proving is fixed, while achieving the immediate goal of blocking Circom selection.
src/app/create/[id]/store.ts (2)
66-67: LGTM!The default client framework is correctly updated to Noir while preserving Circom for server-side use. This aligns with the PR objective, and existing blueprints will retain their stored values through the Zustand persist middleware.
203-205: LGTM!Good documentation clarifying that framework selection is now user-driven via UI dropdowns. Removing the automatic
assignPreferredZkFramework()inference simplifies the flow and prevents unexpected overrides of user selections.src/app/[id]/SelectEmails.tsx (1)
502-511: LGTM on the framework check and messaging update.Correctly uses
clientZkFramework(notserverZkFramework) to determine local proving availability, and the message is now framework-agnostic, consistent with the changes inAddInputs.tsx.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Summary
Disable Circom as an option for client-side ZK framework, defaulting to Noir instead. Fixes the empty dropdown bug where the default value didn't match any available options.
Type of Change
Target Branch
dev- New feature or non-urgent fixstaging- Ready for QA validationmain- Hotfix for production issueChanges Made
clientZkFrameworkchanged fromCircomtoNoirin store initial stateassignPreferredZkFramework()SDK calls that could override user selectionsserverZkFrameworktoclientZkFrameworkBreaking Changes
None. Existing blueprints with
clientZkFramework: Circomretain their stored value. Download code still supports Circom for legacy blueprints.Testing
Manual testing completed:
Checklist
Screenshots/Recordings
Related Issues
Fixes REG-640
Additional Notes
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.