refactor: re-organize client template#831
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughReplaces the monolithic WizardSDK with a router-based, context-driven wizard: adds react-router-dom, a WizardProvider context with SDK wiring, per-step route components and shared UI components, a new SDK config helper, and removes runtime env-var validation and the original WizardSDK file. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant App
participant Router
participant WizardProvider as WizardProvider
participant WizardRoutes as Routes
participant Step as Step Components
participant SDK as Enclave SDK
User->>App: Load
App->>Router: Initialize BrowserRouter
Router->>WizardProvider: Mount with SDK config
WizardProvider->>SDK: initialize (useEnclaveSDK)
WizardProvider-->>WizardRoutes: provide context (state, sdk)
WizardRoutes->>Step: render current step component
Step->>SDK: call/subscribe (requestE3 / activate / encrypt / publish)
SDK-->>WizardProvider: emit events (requested, committee, activated, outputs)
WizardProvider-->>Step: update context (advance step, set state)
Step-->>Router: navigate when step advances
sequenceDiagram
autonumber
participant EnterInputs
participant SDK
participant EncryptSubmit
participant WizardCtx as Wizard Context
EnterInputs->>SDK: encryptNumber(input1, pubKey)
EnterInputs->>SDK: encryptNumber(input2, pubKey)
EnterInputs->>SDK: publishEncryptedInputs()
EnterInputs->>WizardCtx: setCurrentStep(ENCRYPT_SUBMIT)
EncryptSubmit->>SDK: listen PLAINTEXT_OUTPUT_PUBLISHED
SDK-->>EncryptSubmit: plaintextOutput event
EncryptSubmit->>WizardCtx: setResult/decode, update e3State
EncryptSubmit->>WizardCtx: setCurrentStep(RESULTS)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (6)
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: 5
♻️ Duplicate comments (1)
templates/default/client/src/pages/steps/EncryptSubmit.tsx (1)
28-54: Same event unsubscription concern as ActivateE3Ensure listener cleanup is correct. If onEnclaveEvent doesn’t return an unwatch function and sdk.off isn’t implemented/symmetric, you’ll get duplicate callbacks and memory leaks.
- Prefer: const unwatch = onEnclaveEvent(...); return () => unwatch?.()
- Else: verify sdk.off removes the exact handler for PLAINTEXT_OUTPUT_PUBLISHED.
Use the script shared in the other comment to verify SDK API.
🧹 Nitpick comments (6)
templates/default/client/src/pages/steps/ActivateE3.tsx (2)
135-147: Disable button when E3 ID is missingHandler no-ops when id/publicKey is null, but the button only disables on isRequesting/isActivated/!publicKey. Also disable when e3State.id is null to avoid a confusing click.
- disabled={isRequesting || e3State.isActivated || !e3State.publicKey} + disabled={isRequesting || e3State.isActivated || !e3State.publicKey || e3State.id === null}
63-70: Remove stray console.log or gate with debug flagConsole noise in templates can confuse users; prefer a debug logger or remove.
- console.log('handleActivateE3') + // nooptemplates/default/client/src/pages/steps/EncryptSubmit.tsx (1)
32-46: Avoid setResult inside setE3State updater; add decode guardCalling setResult inside another state updater is safe but harder to reason about. Also, decodePlaintextOutput may throw on malformed data.
- const handlePlaintextOutput = (event: any) => { - const { e3Id, plaintextOutput } = event.data - setE3State((prev) => { - if (prev.id !== null && e3Id === prev.id) { - const decodedResult = decodePlaintextOutput(plaintextOutput) - setResult(decodedResult) - return { - ...prev, - plaintextOutput: plaintextOutput as string, - hasPlaintextOutput: true, - } - } - return prev - }) - } + const handlePlaintextOutput = (event: any) => { + const { e3Id, plaintextOutput } = event.data + // Update E3 flags first + setE3State((prev) => { + if (prev.id !== null && e3Id === prev.id) { + return { + ...prev, + plaintextOutput: plaintextOutput as string, + hasPlaintextOutput: true, + } + } + return prev + }) + // Decode separately with guard + try { + const decodedResult = decodePlaintextOutput(plaintextOutput) + setResult(decodedResult) + } catch (e) { + console.warn('Failed to decode plaintext output', e) + } + }templates/default/client/src/pages/components/StepIndicator.tsx (1)
20-46: Add basic ARIA semantics for accessibilityExpose the sequence as a list of steps and mark the current step to improve screen reader navigation.
- return ( - <div className='mb-8 flex justify-center'> - <div className='flex items-center space-x-2'> + return ( + <div className='mb-8 flex justify-center' role="group" aria-label="Wizard steps"> + <div className='flex items-center space-x-2' role="list"> {steps.map(({ step, icon: IconComponent }, index) => { const isActive = currentStep >= step const isCompleted = currentStep > step return ( - <div key={step} className='flex items-center'> + <div + key={step} + className='flex items-center' + role="listitem" + aria-current={isActive && !isCompleted ? 'step' : undefined} + > <div className={`flex h-10 w-10 items-center justify-center rounded-full border-2 transition-all duration-200 ${ isActive ? 'border-enclave-400 bg-enclave-100 text-enclave-600' : 'border-slate-300 bg-slate-100 text-slate-400' }`} > <IconComponent size={24} className={isActive ? 'text-enclave-500' : 'text-slate-400'} /> </div>templates/default/client/src/pages/steps/EnterInputs.tsx (1)
51-56: Clarify encryption failure vs null SDK in error message.The error message "Failed to encrypt inputs" (line 55) does not distinguish between a null SDK (
sdk.sdkbeing undefined) and an actual encryption failure. This makes debugging harder.Consider checking for null SDK explicitly before attempting encryption:
+ if (!sdk.sdk) { + throw new Error('SDK not initialized') + } + // Encrypt both inputs - const encryptedInput1 = await sdk.sdk?.encryptNumber(num1, publicKeyBytes) - const encryptedInput2 = await sdk.sdk?.encryptNumber(num2, publicKeyBytes) + const encryptedInput1 = await sdk.sdk.encryptNumber(num1, publicKeyBytes) + const encryptedInput2 = await sdk.sdk.encryptNumber(num2, publicKeyBytes) if (!encryptedInput1 || !encryptedInput2) { throw new Error('Failed to encrypt inputs') }templates/default/client/src/pages/steps/RequestComputation.tsx (1)
121-121: Extract hardcoded ETH value to constant.The value
BigInt('1000000000000000')(0.001 ETH) is hardcoded in the component. Extract this to a named constant for better maintainability and to make the value configurable.Apply this diff:
+const E3_REQUEST_VALUE_WEI = BigInt('1000000000000000') // 0.001 ETH + const RequestComputation: React.FC = () => { // ... component code ... const hash = await requestE3({ filter: contracts.filterRegistry, threshold, startWindow, duration, e3Program: contracts.e3Program, e3ProgramParams, computeProviderParams, - value: BigInt('1000000000000000'), // 0.001 ETH + value: E3_REQUEST_VALUE_WEI, })
📜 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 (17)
templates/default/client/package.json(1 hunks)templates/default/client/src/App.tsx(1 hunks)templates/default/client/src/context/WizardContext.tsx(1 hunks)templates/default/client/src/pages/WizardRoutes.tsx(1 hunks)templates/default/client/src/pages/WizardSDK.tsx(0 hunks)templates/default/client/src/pages/components/EnvironmentError.tsx(1 hunks)templates/default/client/src/pages/components/ErrorDisplay.tsx(1 hunks)templates/default/client/src/pages/components/SDKErrorDisplay.tsx(1 hunks)templates/default/client/src/pages/components/StepIndicator.tsx(1 hunks)templates/default/client/src/pages/steps/ActivateE3.tsx(1 hunks)templates/default/client/src/pages/steps/ConnectWallet.tsx(1 hunks)templates/default/client/src/pages/steps/EncryptSubmit.tsx(1 hunks)templates/default/client/src/pages/steps/EnterInputs.tsx(1 hunks)templates/default/client/src/pages/steps/RequestComputation.tsx(1 hunks)templates/default/client/src/pages/steps/Results.tsx(1 hunks)templates/default/client/src/utils/env-config.ts(1 hunks)templates/default/client/src/utils/sdk-config.ts(1 hunks)
💤 Files with no reviewable changes (1)
- templates/default/client/src/pages/WizardSDK.tsx
🧰 Additional context used
🧬 Code graph analysis (10)
templates/default/client/src/App.tsx (1)
templates/default/client/src/context/WizardContext.tsx (1)
WizardProvider(82-175)
templates/default/client/src/pages/components/ErrorDisplay.tsx (1)
templates/default/client/src/utils/error-formatting.ts (1)
formatContractError(10-51)
templates/default/client/src/pages/steps/RequestComputation.tsx (3)
templates/default/client/src/context/WizardContext.tsx (1)
useWizard(64-70)templates/default/client/src/utils/env-config.ts (1)
getContractAddresses(27-34)packages/enclave-sdk/src/enclave-sdk.ts (1)
onEnclaveEvent(281-302)
templates/default/client/src/pages/steps/EnterInputs.tsx (1)
templates/default/client/src/context/WizardContext.tsx (1)
useWizard(64-70)
templates/default/client/src/pages/WizardRoutes.tsx (2)
templates/default/client/src/context/WizardContext.tsx (1)
useWizard(64-70)templates/default/client/src/utils/env-config.ts (1)
MISSING_ENV_VARS(15-22)
templates/default/client/src/utils/sdk-config.ts (2)
templates/default/client/src/utils/env-config.ts (1)
getContractAddresses(27-34)packages/enclave-sdk/src/index.ts (1)
FheProtocol(42-42)
templates/default/client/src/pages/steps/EncryptSubmit.tsx (2)
templates/default/client/src/context/WizardContext.tsx (1)
useWizard(64-70)packages/enclave-sdk/src/enclave-sdk.ts (1)
onEnclaveEvent(281-302)
templates/default/client/src/pages/steps/Results.tsx (1)
templates/default/client/src/context/WizardContext.tsx (1)
useWizard(64-70)
templates/default/client/src/context/WizardContext.tsx (2)
examples/CRISP/client/libs/wasm/pkg/crisp_worker.js (2)
result(32-36)sdk(18-30)templates/default/client/src/utils/sdk-config.ts (1)
getEnclaveSDKConfig(13-23)
templates/default/client/src/pages/steps/ActivateE3.tsx (2)
templates/default/client/src/context/WizardContext.tsx (1)
useWizard(64-70)packages/enclave-sdk/src/enclave-sdk.ts (1)
onEnclaveEvent(281-302)
🪛 Biome (2.1.2)
templates/default/client/src/pages/WizardRoutes.tsx
[error] 71-71: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
⏰ 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). (8)
- GitHub Check: test_net
- GitHub Check: build_e3_support_dev
- GitHub Check: build_sdk
- GitHub Check: test_contracts
- GitHub Check: rust_unit
- GitHub Check: build_e3_support_risc0
- GitHub Check: integration_prebuild
- GitHub Check: build_enclave_cli
🔇 Additional comments (17)
templates/default/client/package.json (1)
26-26: LGTM!The addition of
react-router-domsupports the new router-based wizard flow architecture.templates/default/client/src/pages/components/EnvironmentError.tsx (1)
8-61: LGTM!The UI refactor improves visual consistency while preserving the component's public interface and functionality.
templates/default/client/src/pages/components/SDKErrorDisplay.tsx (1)
9-26: LGTM!The component provides a clean, focused display for SDK errors with appropriate styling.
templates/default/client/src/pages/components/ErrorDisplay.tsx (1)
16-37: LGTM!The component provides good UX with progressive disclosure of technical details and proper integration with the error formatting utility.
templates/default/client/src/App.tsx (1)
8-19: LGTM!The refactor successfully introduces a router-based architecture with proper provider nesting. The changes align well with the PR's objective to modularize the wizard flow.
templates/default/client/src/utils/sdk-config.ts (1)
13-23: LGTM!The helper function provides a clean abstraction for SDK configuration, promoting maintainability by centralizing the config logic.
templates/default/client/src/pages/steps/ConnectWallet.tsx (1)
12-49: LGTM!The component is well-documented and provides a clear, user-friendly introduction to the Enclave wizard flow.
templates/default/client/src/pages/WizardRoutes.tsx (2)
31-47: LGTM!The step configuration structure is clean and maintainable, providing a clear mapping between wizard steps, routes, and components.
73-93: LGTM!The routing structure effectively enforces linear wizard navigation by redirecting to the current step's path, ensuring users follow the intended flow.
templates/default/client/src/pages/steps/Results.tsx (1)
25-89: LGTMClean, contextual summary of outputs and reset path. No correctness issues spotted.
Optional: since RESULTS is reached only after hasPlaintextOutput, result should typically be non-null. If RESULT can be null, confirm intended UX for "Computing..." on this step.
templates/default/client/src/pages/steps/ActivateE3.tsx (1)
31-55: Event listener cleanup is correct
The SDK’soffmethod invokeseventListener.offwith the same callback you pass toonEnclaveEvent, so youruseEffectcleanup properly unsubscribes and won’t leak.templates/default/client/src/pages/steps/EnterInputs.tsx (2)
27-73: LGTM: Encryption and publish flow is well-structured.The input validation (line 30), state transitions (line 35), and error handling (lines 69-72) are properly implemented. The sequential encryption and publishing of both inputs follows a clear flow.
59-67: Latest transaction hash capture is intentional
The firstpublishInputreturn value isn’t consumed elsewhere and only the most recent hash is displayed to users.templates/default/client/src/pages/steps/RequestComputation.tsx (2)
87-133: LGTM: Request flow properly handles state reset and error cases.The function correctly resets E3 state (lines 93-103), builds parameters using SDK utilities (lines 105-110), and handles both success and error paths with appropriate user feedback.
59-67: Verify SDK event ordering guarantees. The logic inhandleCommitteePublished(RequestComputation.tsx:59–67) assumesCOMMITTEE_PUBLISHEDalways followsE3_REQUESTED; without explicit buffering or ordering in the SDK, this could drop updates if events arrive out of order. Confirm the SDK’s behavior or implement handling for out-of-order events.templates/default/client/src/context/WizardContext.tsx (2)
82-172: LGTM: Clean context-based architecture with proper memoization.The WizardProvider is well-structured with:
- Properly memoized SDK config (line 86) to prevent re-initializations
- Comprehensive state management covering all wizard steps
- Well-defined handlers with
useCallbackfor stable references (lines 115-138)- Correct dependency arrays in the context value memoization (lines 160-171)
This provides a solid foundation for the modular wizard flow.
106-113: Add reset logic on SDK re-initialization or confirm current behaviorNo existing effect handles
sdk.isInitializedflipping false, so the wizard won’t reset if the SDK de-initializes mid-flow. Either add auseEffectto callhandleReset()when!sdk.isInitialized, or confirm that preserving progress on SDK re-init is intentional.
* refactor: re-organize client template * refactor: move hook before returns
2 participants
Closes #737
Summary by CodeRabbit
New Features
Refactor
Style
Chores