fix: Critical UI/UX failures, missing validation, and DX tooling gaps#38
fix: Critical UI/UX failures, missing validation, and DX tooling gaps#38kushbosamiya wants to merge 2 commits into
Conversation
### 1. Problem - The application suffered from severe network connection desyncs because `useNetworkGuard.ts` overwrote Wagmi's internal state mappings directly with React state bindings (`eventChain`) and invalidated `wallet_switchEthereumChain` RPC calls by injecting legacy `addEthereumChainParameter` fields. - Missing error boundaries caused the explorer page to crash entirely on failed API logic. - `createOracle.tsx` was bloated, lacking proper form validation, and included broken Tooltips implementations. - No developer pre-commit hook workflows existed to prevent malformed code pushing. ### 2. Current Behavior - **Reproduction:** If a user clicks to switch the chain on `ConnectBtn.tsx`, Wagmi transitions state but MetaMask never receives the popup, leading to a silent UI/Provider mismatch. - **Evidence:** Console logs reported `Chain Switched` internally, but no modal opened. Missing validation caused transaction reverts on-chain for the Oracle factory. ### 3. Proposed Solution - Rewrote `hooks/useNetworkGuard.ts` to strictly observe Wagmi's built-in `useSwitchChain` and `useAccount` states, dropping all manual EIP-1193 listener overrides. - Implemented `react-hook-form` coupled with `zod` schema validation for `createOracle.tsx`. - Refactored `createOracle.tsx` into modular Form components to reduce bundle bloat. - Introduced `Husky` and `lint-staged` pre-commit workflows. - Implemented robust generic `ErrorBoundary` wrappers. ### 4. Acceptance Criteria - [x] Network switching consistently triggers MetaMask modals. - [x] No behavioral regressions in Wagmi provider connectivity. - [x] All form inputs strictly validate before simulating the smart contract logic. - [x] Verified locally via `npm run build` and `tsc`.
📝 WalkthroughWalkthroughThis PR addresses critical issues from Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Navigation
participant ConnectBtn
participant wagmi as Wagmi Hooks
participant WalletProvider
participant EIP6963 as EIP-6963 Discovery
User->>Navigation: Open app
Navigation->>ConnectBtn: Render connect button
alt Wallet Connected
ConnectBtn->>wagmi: useAccount()
wagmi->>ConnectBtn: Return connected account
ConnectBtn->>ConnectBtn: Show connected state + network dropdown
User->>ConnectBtn: Click network dropdown
ConnectBtn->>wagmi: useNetworkGuard()
wagmi->>ConnectBtn: Return isWrongNetwork, targetChains
alt Wrong Network
User->>ConnectBtn: Select new chain
ConnectBtn->>wagmi: switchChainAsync(chainId)
wagmi->>WalletProvider: Request chain switch
WalletProvider->>User: Wallet prompts chain switch
end
else Wallet Disconnected
ConnectBtn->>ConnectBtn: Show "Connect Wallet" button
User->>ConnectBtn: Click connect button
ConnectBtn->>ConnectBtn: Open modal, dispatch EIP6963 request
EIP6963->>ConnectBtn: Return injected providers
ConnectBtn->>ConnectBtn: Display deduplicated connectors
User->>ConnectBtn: Select provider
ConnectBtn->>wagmi: useConnect().connectAsync(connector)
wagmi->>WalletProvider: Initialize connection
WalletProvider->>User: Wallet authorizes connection
ConnectBtn->>ConnectBtn: Close modal, show connected state
end
sequenceDiagram
participant User
participant ExplorerPage
participant ErrorBoundary
participant ExplorerContent
participant useOracles as useOracles Hook
participant API
User->>ExplorerPage: Navigate to /explorer
ExplorerPage->>ErrorBoundary: Wrap content
ErrorBoundary->>ExplorerContent: Render
ExplorerContent->>useOracles: Fetch oracles
alt Oracle Factory Deployed
useOracles->>API: Query oracle list
API->>useOracles: Return oracles
useOracles->>ExplorerContent: Return { oracles, loading, error: null }
ExplorerContent->>ExplorerContent: Filter active oracles by search
ExplorerContent->>User: Render oracle grid
else Oracle Factory Not Deployed
useOracles->>API: Query oracle list
API->>useOracles: Throw error "Oracle Factory not deployed"
ExplorerContent->>ErrorBoundary: Throw error upstream
ErrorBoundary->>ErrorBoundary: Catch error, render fallback
ErrorBoundary->>UnsupportedNetworkEmptyState: Render with supported chains
UnsupportedNetworkEmptyState->>User: Show switch-network buttons
end
sequenceDiagram
participant User
participant CreatePage
participant CreateOracleIntegrated
participant OracleMetadataForm
participant OracleParametersForm
participant useForm as react-hook-form
participant Zod as Zod Schema
participant Contract as Oracle Contract
User->>CreatePage: Navigate to /create
CreatePage->>CreateOracleIntegrated: Render form
CreateOracleIntegrated->>useForm: useForm<CreateOracleFormValues>()
CreateOracleIntegrated->>OracleMetadataForm: Render with register, errors
CreateOracleIntegrated->>OracleParametersForm: Render with register, errors, setValue
User->>OracleMetadataForm: Fill metadata fields
OracleMetadataForm->>useForm: Register field changes
User->>OracleParametersForm: Select token + fill parameters
OracleParametersForm->>useForm: Register field + setValue for token
User->>CreateOracleIntegrated: Submit form
CreateOracleIntegrated->>Zod: Validate via zodResolver
alt Validation Fails
Zod->>useForm: Return errors
useForm->>OracleMetadataForm: Update errors state
useForm->>OracleParametersForm: Update errors state
OracleMetadataForm->>User: Show inline error messages
OracleParametersForm->>User: Show inline error messages
else Validation Succeeds
CreateOracleIntegrated->>Contract: Submit transaction
Contract->>User: Show success/explorer link
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/createOracle.tsx (2)
127-145:⚠️ Potential issue | 🔴 Critical
ownerAddressis validated but never applied on-chain.
components/oracle/OracleMetadataForm.tsxtells users this field “will own and control the oracle contract” (Line 102), but this submission path never usesdata.ownerAddress; the only account forwarded on-chain here isaccount.address. Right now a user can enter a different owner, pass validation, and still deploy an oracle owned according to the factory’s default behavior instead of the typed address. Either remove this field, or add the explicit ownership transfer step after deployment if the contracts support it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/createOracle.tsx` around lines 127 - 145, The submitted ownerAddress (data.ownerAddress) is validated but never sent on-chain; after calling simulateContract for OracleFactory.createOracle you must apply ownership to the deployed oracle: capture the created oracle address from the createOracle result and then call simulateContract again against the new oracle contract (use the minted oracle's address and the oracle ABI) invoking transferOwnership (or the contract's ownership setter) with data.ownerAddress and account as signer; alternatively, if OracleFactory.createOracle accepts an owner param, include data.ownerAddress in constructorArgs before calling simulateContract so the factory sets the owner on creation.
156-178:⚠️ Potential issue | 🔴 CriticalDon't declare deployment success from a fixed 5-second timer.
This code marks the flow successful without confirming the transaction was mined, and
allOracles[allOracles.length - 1]can point at someone else’s oracle if the factory changes before the timeout fires. The stale timer also survives chain changes/navigation and can flip the UI back to success for an old submission. Please wait for the submitted transaction to confirm, then resolve the created oracle from that confirmed transaction before settingsubmitted.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/createOracle.tsx` around lines 156 - 178, Replace the fixed window.setTimeout flow with waiting for the actual transaction confirmation: when you submit the create-oracle transaction capture its tx hash, call provider.waitForTransaction or wagmi's waitForTransaction for that tx hash, then after it confirms call readContract(config, { address: factoryAddress, abi: OracleFactoryAbi, functionName: 'allOracles' }) to resolve the newly created oracle and call setOracleAddress(latestOracle.oracle), setSubmitted(true) and toast — instead of using the 5s timer. Also guard updates against stale navigation/chain changes by storing the currentTxHash or a mounted ref and checking it before calling setSubmitted/setOracleAddress so the handler no longer flips UI for an old submission.
🧹 Nitpick comments (7)
components/PillNav.tsx (3)
14-15: Dead code:logo,logoAltprops andlogoRefare defined but never used.The AI summary indicates logo-related artifacts were removed, but cleanup appears incomplete:
logoandlogoAltprops exist inPillNavPropsbut are never destructured or used in the component.logoRefis declared but never assigned to any JSX element, so it's alwaysnull.♻️ Proposed fix to remove dead code
export interface PillNavProps { - logo: string; - logoAlt?: string; items: PillNavItem[]; activeHref?: string; className?: string;const mobileMenuRef = useRef<HTMLDivElement | null>(null); const navItemsRef = useRef<HTMLDivElement | null>(null); - const logoRef = useRef<HTMLAnchorElement | HTMLElement | null>(null);Also applies to: 48-48
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/PillNav.tsx` around lines 14 - 15, Remove the unused logo artifacts: delete the logo and logoAlt properties from the PillNavProps type and remove any reference to them where props are destructured or documented, and remove the unused logoRef variable (and any related import/type) from the PillNav component; make sure to only keep props actually used by the component (e.g., items, value, onChange, etc.) and run a quick compile to ensure no remaining references to PillNavProps.logo, PillNavProps.logoAlt, or logoRef remain.
150-151: GSAP timelines and tweens are not cleaned up on unmount.The cleanup function only removes the resize listener, but
tlRefsandactiveTweenRefshold GSAP timeline/tween instances that should be killed on unmount to prevent memory leaks and potential errors if animations complete after the component unmounts.♻️ Proposed fix to add GSAP cleanup
- return () => window.removeEventListener("resize", onResize); + return () => { + window.removeEventListener("resize", onResize); + tlRefs.current.forEach((tl) => tl?.kill()); + activeTweenRefs.current.forEach((tw) => tw?.kill()); + }; }, [items, ease, initialLoadAnimation]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/PillNav.tsx` around lines 150 - 151, The effect cleanup currently only removes the resize listener but leaves GSAP instances alive; iterate over tlRefs.current and activeTweenRefs.current in the useEffect cleanup and call kill() (or kill() / killTweensOf / revert as appropriate for the GSAP object types) on each timeline/tween to stop and free them, then clear those arrays (e.g., set length = 0) before returning; keep the existing window.removeEventListener("resize", onResize) call so the cleanup removes the resize handler and also properly kills all GSAP timelines/tweens referenced by tlRefs and activeTweenRefs to prevent leaks and post-unmount callbacks.
127-148: Dead code path:logoRefanimation block will never execute.Since
logoRefis never assigned to a DOM element,logoRef.currentis alwaysnull. Theif (logo)check at line 131 will always fail, making this entire animation block unreachable.If the logo animation is intentionally removed, delete this block along with the
logoRefdeclaration. If it should work, the ref needs to be attached to a logo element in JSX.♻️ Proposed fix to remove dead animation code
if (initialLoadAnimation) { - const logo = logoRef.current; const navItems = navItemsRef.current; - if (logo) { - gsap.set(logo, { scale: 0 }); - gsap.to(logo, { - scale: 1, - duration: 0.6, - ease, - }); - } - if (navItems) { gsap.set(navItems, { width: 0, overflow: "hidden" });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/PillNav.tsx` around lines 127 - 148, The logoRef animation block is dead because logoRef.current is never assigned; either attach logoRef to the actual logo JSX element so logoRef.current is a DOM node (ensure the element uses ref={logoRef}) and keep the gsap animation in the initialLoadAnimation branch, or remove the unused logoRef declaration and the entire logo animation block (the if (logo) {...}) and related gsap code while keeping navItemsRef handling; update any imports/usages (logoRef, initialLoadAnimation) accordingly to avoid unused variables.components/ui/use-toast.ts (2)
113-123: Consider clearing timeouts when toasts are removed.When a toast is removed (e.g., due to
TOAST_LIMITslicing inADD_TOAST), its associated timeout intoastTimeoutsMap is not cleared. The timeout will still fire and dispatchREMOVE_TOASTfor a non-existent toast ID, which is unnecessary work.♻️ Proposed improvement: Clear timeout on removal
case "REMOVE_TOAST": if (action.toastId === undefined) { + toastTimeouts.forEach((timeout) => clearTimeout(timeout)); + toastTimeouts.clear(); return { ...state, toasts: [], }; } + const timeout = toastTimeouts.get(action.toastId); + if (timeout) { + clearTimeout(timeout); + toastTimeouts.delete(action.toastId); + } return { ...state, toasts: state.toasts.filter((t) => t.id !== action.toastId), };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/ui/use-toast.ts` around lines 113 - 123, The REMOVE_TOAST branch currently removes toast entries from state.toasts but never clears their scheduled timers in the toastTimeouts Map; update the reducer (case "REMOVE_TOAST") to look up action.toastId (or each removed toast id when ADD_TOAST slices due to TOAST_LIMIT), call clearTimeout on the stored timer, and delete the key from toastTimeouts before returning the new state, and likewise ensure ADD_TOAST's slicing logic clears timeouts for any toasts dropped by the limit; reference the REMOVE_TOAST case, the toastTimeouts Map, and the ADD_TOAST slicing logic to locate where to clearTimeout and Map.delete for removed toast ids.
18-23: Consider simplifying the ActionType definition.The current
ActionTypetype maps each action name to itself, adding unnecessary indirection. A direct string union type would be simpler and equally type-safe.♻️ Simpler alternative using string union type
-type ActionType = { - ADD_TOAST: "ADD_TOAST"; - UPDATE_TOAST: "UPDATE_TOAST"; - DISMISS_TOAST: "DISMISS_TOAST"; - REMOVE_TOAST: "REMOVE_TOAST"; -}; +type ActionType = "ADD_TOAST" | "UPDATE_TOAST" | "DISMISS_TOAST" | "REMOVE_TOAST";Then update the Action type to use the string literals directly:
type Action = | { - type: ActionType["ADD_TOAST"]; + type: "ADD_TOAST"; toast: ToasterToast; } | { - type: ActionType["UPDATE_TOAST"]; + type: "UPDATE_TOAST"; toast: Partial<ToasterToast>; } | { - type: ActionType["DISMISS_TOAST"]; + type: "DISMISS_TOAST"; toastId?: ToasterToast["id"]; } | { - type: ActionType["REMOVE_TOAST"]; + type: "REMOVE_TOAST"; toastId?: ToasterToast["id"]; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/ui/use-toast.ts` around lines 18 - 23, ActionType currently maps each action name to itself which is unnecessary; replace the ActionType object-style type with a direct string-union type (e.g., type ActionType = "ADD_TOAST" | "UPDATE_TOAST" | "DISMISS_TOAST" | "REMOVE_TOAST") and then update the Action type (the discriminated union that uses ActionType) to reference these string literals directly instead of ActionType.<Action> Ensure all usages of ActionType and the Action discriminant (e.g., in the reducer or dispatch calls) are updated to the new string-union type.testZod.ts (1)
9-68: Turn this into a real test instead of a duplicate schema script.This redefines
createOracleSchemainstead of importinglib/schemas/createOracleSchema.ts, and it already diverges from production validation by dropping the address checks. At repo root it also doesn't assert anything—it just logs. Please move this under the test runner and exercise the production schema directly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@testZod.ts` around lines 9 - 68, This file redefines createOracleSchema and only logs results; replace the local schema with an import of the production schema (import { createOracleSchema } from 'lib/schemas/createOracleSchema') and move the file into your test suite (e.g., tests/) so the test runner executes it; remove the local isPositiveInteger/isNonNegativeInteger redefinitions and any differing validators so you exercise the real logic, then use createOracleSchema.safeParse (or parse) in assertions (not console.log) to assert expected outcomes for valid and invalid inputs (check address validation, types for reward/halfLifeSeconds/quorumBps/depositLock/withdrawLock/alpha) and add specific expect/assert checks for error messages and success cases.components/oracle/OracleParametersForm.tsx (1)
55-55: Consider collapsing this grid to one column belowmd.
grid-cols-2at every breakpoint will squeeze these inputs on mini-app widths. Starting at one column and opting into two columns atmdkeeps the form much more usable on phones.♻️ Proposed fix
- <CardContent className='grid grid-cols-2 gap-4 pt-4'> + <CardContent className='grid grid-cols-1 gap-4 pt-4 md:grid-cols-2'>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/oracle/OracleParametersForm.tsx` at line 55, Update the CardContent grid so it collapses to a single column on small screens by changing the className in OracleParametersForm's CardContent from "grid grid-cols-2 gap-4 pt-4" to use a responsive utility such as "grid grid-cols-1 md:grid-cols-2 gap-4 pt-4" (or equivalent Tailwind responsive classes) so the form renders one column below the md breakpoint and two columns at md and above.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.npmrc:
- Line 2: Remove the disabling setting by deleting or changing the .npmrc entry
"audit=false" and add a CI step that runs "npm audit" (or an equivalent SCA
tool) so dependency vulnerabilities are scanned during builds; implement a CI
job/step named something like "dependency-scan" that runs npm audit --json (or
invokes Dependabot/dependency-review/SAST) and fails the build on high/critical
findings, and document the chosen SCA process in the repo so local installs and
CI both get scanned.
In `@app/`[oracleId]/InteractionClient.tsx:
- Around line 332-342: The effect that builds priceHistoryPoints should also
call updatePriceDisplays(...) so the summary cards (latestValue and
aggregatedValue) are initialized from fetched history; inside the useEffect
where you create history via buildPriceHistoryPoints(latestPriceHistoryData as
PriceHistoryResult), after setPriceHistoryPoints(history) invoke
updatePriceDisplays(history) (or the appropriate args that updatePriceDisplays
expects), and add updatePriceDisplays to the effect dependency array so the
chart and summary cards stay in sync on first load.
- Around line 590-610: The current conversion of submitValue uses parseFloat and
BigInt which can lose precision; replace that logic with ethers' parseUnits to
perform exact decimal-to-integer conversion using the existing PRICE_DECIMALS
constant. In the try block where setIsSubmitting(true) is called and before
writeContract({ address: oracleAddress!, abi: OracleAbi, functionName:
"submitValue", args: [valueAsInt] }), compute valueAsInt by calling
parseUnits(submitValue, PRICE_DECIMALS) (and convert to the expected
BigInt/string form if needed) and use that value in the args; keep the existing
console.log but update logged asInt to reflect the parseUnits result. Ensure
parseUnits is imported/available in this file.
- Around line 1465-1472: The render code that computes amount uses parseUnits
directly (parseUnits(depositAmount, weightTokenDecimals)) which can throw
InvalidDecimalNumberError on transient/invalid inputs and crash the UI; wrap the
parseUnits call in a try-catch (inside the current IIFE or a small helper like
safeParseUnits) and on error fall back to BigInt(0) (or another safe sentinel)
so amount computation never throws during render; keep the rest of logic that
computes allowance from tokenAllowanceData and needsApproval (amount >
allowance) unchanged so validation handlers can surface errors instead of
crashing the component.
- Around line 427-483: The checks in InteractionClient (the update block
handling lastSubmissionTimeData, rewardData, halfLifeSecondsData, quorumData,
operationLockingPeriodData, withdrawalLockingPeriodData, alphaData,
depositTimestampData, and lastOperationTimestampData) use truthiness and
therefore skip valid 0n/0 values; change each conditional to explicit
null/undefined checks (e.g., if (lastSubmissionTimeData !== null &&
lastSubmissionTimeData !== undefined)) so zero BigInt timestamps and numeric
zeros are processed and the UI updates (apply the same pattern for rewardData,
halfLifeSecondsData, quorumData, operationLockingPeriodData,
withdrawalLockingPeriodData, alphaData, depositTimestampData,
lastOperationTimestampData).
In `@app/explorer/page.tsx`:
- Around line 11-13: The ErrorBoundary currently always renders
UnsupportedNetworkEmptyState for any error; update behavior by either enhancing
ErrorBoundary to inspect the thrown error (e.g., check instance/type or message
for network/contract-specific indicators) and only render
<UnsupportedNetworkEmptyState /> when the error clearly matches a
network/unsupported-contract case, falling back to a generic error UI otherwise,
or remove the ErrorBoundary wrapper around <ExplorerContent /> and rely on
ExplorerContent's internal loading/API/unsupported-network state handling;
reference ErrorBoundary, UnsupportedNetworkEmptyState, and ExplorerContent when
making the change.
In `@app/globals.css`:
- Around line 2-3: Replace the fragile nested node_modules import with
Tailwind's public entrypoint: in globals.css remove the hard-coded "@import
\"../node_modules/@tailwindcss/postcss/node_modules/tailwindcss/index.css\";"
and import Tailwind via its public module name ("tailwindcss") instead, leaving
the "@import \"tw-animate-css\";" line intact; ensure the change is made where
globals.css declares those imports so PostCSS tooling resolves Tailwind
correctly.
In `@app/layout.tsx`:
- Around line 103-109: The KyaModal component is being mounted twice (once
inside ClientFooter and again in layout.tsx), causing duplicate dialogs and
duplicate Escape/focus-trap listeners; remove the redundant mount in layout.tsx
by deleting the standalone <KyaModal /> that appears after <ClientFooter />
while keeping the existing <KyaModal /> rendered by ClientFooter, making sure
KYAModalProvider still wraps Navigation, main, and ClientFooter so modal context
remains available.
In `@components/ClientFooter.tsx`:
- Around line 21-22: The KyaModal is double-mounted: remove the <KyaModal />
rendering from ClientFooter (keep Footer and its handleShareClick) so the modal
is only mounted once where it already exists inside the KYAModalProvider (the
instance in app/layout.tsx). Locate the KyaModal reference in ClientFooter.tsx
and delete that element (or move any modal-specific props/state to the single
modal instance), ensuring the app's single KyaModal stays inside the
KYAModalProvider so you don't get duplicate focus traps or Escape/backdrop
listeners.
In `@components/ConnectBtn.tsx`:
- Around line 216-231: The supported-network control is hidden on mobile because
the non-error branch's className includes "hidden ... sm:flex"; update the
non-error branch in ConnectBtn.tsx (the ternary that sets className when
isWrongNetwork is false) to remove the "hidden" so the button is visible on
small screens (e.g., change "hidden items-center gap-1.5 ... sm:flex" to
"items-center gap-1.5 ... flex sm:flex" or simply "items-center gap-1.5 ...
flex"), leaving the inner spans (the one using "hidden sm:inline" for
chain?.name / NETWORK_FALLBACK and the "text-xs sm:hidden" for the mobile "N")
unchanged so they continue to show the compact/mobile label while preserving the
larger label on larger screens.
In `@components/ExplorerContent.tsx`:
- Around line 17-24: The current useMemo that produces filteredOracles filters
out oracles with status === "inactive", causing degraded RPC/read failures to be
treated as no-data; update the logic in the useMemo that computes
filteredOracles (used in ExplorerContent) to include oracles whose status ===
"inactive" (or produce a separate list, e.g., degradedOracles) so they are
rendered with an explicit degraded/inactive UI state instead of being omitted;
adjust downstream rendering (where filteredOracles is consumed around lines
~67-100) to display inactive cards with error/inactive treatment or a dedicated
degraded section rather than letting them fall through to the empty-results
messages.
In `@components/oracle/OracleParametersForm.tsx`:
- Around line 56-78: The TokenSelector used in OracleParametersForm needs
form-safe semantics and accessibility: give the TokenSelector an explicit id
prop (e.g., id='weightTokenSelector'), update the Label's htmlFor to point to
that id instead of the hidden input, and pass an aria-describedby prop (pointing
to the validation/error element id) into TokenSelector so it renders
aria-describedby on the interactive element; update TokenSelector's
props/interface to accept and forward id and aria-describedby to its main
control and error node, and ensure all internal buttons in TokenSelector
(notably the modal close button) include type='button' to prevent accidental
form submission.
In `@components/ui/use-toast.ts`:
- Around line 172-180: The effect in useEffect is re-registering the same
listener on every state change because it depends on [state]; update the
dependency array to [] so the listener registration (listeners.push(setState))
runs only once on mount and the cleanup removes it on unmount; change the
dependency for the useEffect that references listeners and setState to an empty
array to prevent duplicate entries.
In `@components/UnsupportedNetworkEmptyState.tsx`:
- Around line 50-53: The button rendered for each chain (the element using
key={chain.id} and onClick={() => switchChain({ chainId: chain.id })}) lacks an
explicit type and will default to "submit" when inside a form; update that
button to include type="button" to prevent accidental form submissions while
keeping the existing onClick and classes intact.
In `@lib/schemas/createOracleSchema.ts`:
- Around line 13-18: The form collects ownerAddress but the value is never
passed to the contract constructor; update the constructorArgs array in
components/createOracle.tsx to include data.ownerAddress as the first element
(typed as `0x${string}`) so the Oracle constructor receives owner_ as its first
parameter per the ABI (see utils/abi/Oracle.ts:5); locate the constructorArgs
creation and insert data.ownerAddress as `0x${string}` before the existing
arguments.
In `@next.config.mjs`:
- Around line 14-19: The config currently disables TypeScript and ESLint
build-time checks (typescript.ignoreBuildErrors and eslint.ignoreDuringBuilds)
unconditionally; change it to only disable these in local dev fast-reload
scenarios and enable them in CI/production by gating on environment variables
(e.g., process.env.CI or NODE_ENV === 'production') so that in CI/production the
flags are false and Next.js build fails on type/lint errors while preserving
ignore behavior for local development.
In `@package.json`:
- Around line 12-14: The package.json currently uses eslint-config-next@15.x
which is incompatible with next@14.2.16; update the dependency
"eslint-config-next" to a 14.2.x release (e.g. "14.2.35") in package.json, then
run your package manager to regenerate the lockfile (npm/yarn/pnpm install) so
the installed versions match next's major/minor and prevent ESLint/Next
mismatches.
---
Outside diff comments:
In `@components/createOracle.tsx`:
- Around line 127-145: The submitted ownerAddress (data.ownerAddress) is
validated but never sent on-chain; after calling simulateContract for
OracleFactory.createOracle you must apply ownership to the deployed oracle:
capture the created oracle address from the createOracle result and then call
simulateContract again against the new oracle contract (use the minted oracle's
address and the oracle ABI) invoking transferOwnership (or the contract's
ownership setter) with data.ownerAddress and account as signer; alternatively,
if OracleFactory.createOracle accepts an owner param, include data.ownerAddress
in constructorArgs before calling simulateContract so the factory sets the owner
on creation.
- Around line 156-178: Replace the fixed window.setTimeout flow with waiting for
the actual transaction confirmation: when you submit the create-oracle
transaction capture its tx hash, call provider.waitForTransaction or wagmi's
waitForTransaction for that tx hash, then after it confirms call
readContract(config, { address: factoryAddress, abi: OracleFactoryAbi,
functionName: 'allOracles' }) to resolve the newly created oracle and call
setOracleAddress(latestOracle.oracle), setSubmitted(true) and toast — instead of
using the 5s timer. Also guard updates against stale navigation/chain changes by
storing the currentTxHash or a mounted ref and checking it before calling
setSubmitted/setOracleAddress so the handler no longer flips UI for an old
submission.
---
Nitpick comments:
In `@components/oracle/OracleParametersForm.tsx`:
- Line 55: Update the CardContent grid so it collapses to a single column on
small screens by changing the className in OracleParametersForm's CardContent
from "grid grid-cols-2 gap-4 pt-4" to use a responsive utility such as "grid
grid-cols-1 md:grid-cols-2 gap-4 pt-4" (or equivalent Tailwind responsive
classes) so the form renders one column below the md breakpoint and two columns
at md and above.
In `@components/PillNav.tsx`:
- Around line 14-15: Remove the unused logo artifacts: delete the logo and
logoAlt properties from the PillNavProps type and remove any reference to them
where props are destructured or documented, and remove the unused logoRef
variable (and any related import/type) from the PillNav component; make sure to
only keep props actually used by the component (e.g., items, value, onChange,
etc.) and run a quick compile to ensure no remaining references to
PillNavProps.logo, PillNavProps.logoAlt, or logoRef remain.
- Around line 150-151: The effect cleanup currently only removes the resize
listener but leaves GSAP instances alive; iterate over tlRefs.current and
activeTweenRefs.current in the useEffect cleanup and call kill() (or kill() /
killTweensOf / revert as appropriate for the GSAP object types) on each
timeline/tween to stop and free them, then clear those arrays (e.g., set length
= 0) before returning; keep the existing window.removeEventListener("resize",
onResize) call so the cleanup removes the resize handler and also properly kills
all GSAP timelines/tweens referenced by tlRefs and activeTweenRefs to prevent
leaks and post-unmount callbacks.
- Around line 127-148: The logoRef animation block is dead because
logoRef.current is never assigned; either attach logoRef to the actual logo JSX
element so logoRef.current is a DOM node (ensure the element uses ref={logoRef})
and keep the gsap animation in the initialLoadAnimation branch, or remove the
unused logoRef declaration and the entire logo animation block (the if (logo)
{...}) and related gsap code while keeping navItemsRef handling; update any
imports/usages (logoRef, initialLoadAnimation) accordingly to avoid unused
variables.
In `@components/ui/use-toast.ts`:
- Around line 113-123: The REMOVE_TOAST branch currently removes toast entries
from state.toasts but never clears their scheduled timers in the toastTimeouts
Map; update the reducer (case "REMOVE_TOAST") to look up action.toastId (or each
removed toast id when ADD_TOAST slices due to TOAST_LIMIT), call clearTimeout on
the stored timer, and delete the key from toastTimeouts before returning the new
state, and likewise ensure ADD_TOAST's slicing logic clears timeouts for any
toasts dropped by the limit; reference the REMOVE_TOAST case, the toastTimeouts
Map, and the ADD_TOAST slicing logic to locate where to clearTimeout and
Map.delete for removed toast ids.
- Around line 18-23: ActionType currently maps each action name to itself which
is unnecessary; replace the ActionType object-style type with a direct
string-union type (e.g., type ActionType = "ADD_TOAST" | "UPDATE_TOAST" |
"DISMISS_TOAST" | "REMOVE_TOAST") and then update the Action type (the
discriminated union that uses ActionType) to reference these string literals
directly instead of ActionType.<Action> Ensure all usages of ActionType and the
Action discriminant (e.g., in the reducer or dispatch calls) are updated to the
new string-union type.
In `@testZod.ts`:
- Around line 9-68: This file redefines createOracleSchema and only logs
results; replace the local schema with an import of the production schema
(import { createOracleSchema } from 'lib/schemas/createOracleSchema') and move
the file into your test suite (e.g., tests/) so the test runner executes it;
remove the local isPositiveInteger/isNonNegativeInteger redefinitions and any
differing validators so you exercise the real logic, then use
createOracleSchema.safeParse (or parse) in assertions (not console.log) to
assert expected outcomes for valid and invalid inputs (check address validation,
types for reward/halfLifeSeconds/quorumBps/depositLock/withdrawLock/alpha) and
add specific expect/assert checks for error messages and success cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3c0b1daa-9c1b-4fd4-a6e9-adab7c900cb5
⛔ Files ignored due to path filters (3)
package-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpublic/favicon.icois excluded by!**/*.ico
📒 Files selected for processing (37)
.eslintrc.json.husky/pre-commit.npmrcapp/[oracleId]/InteractionClient.tsxapp/[oracleId]/page.tsxapp/create/page.tsxapp/explorer/page.tsxapp/globals.cssapp/layout.tsxapp/page.tsxcomponents/ClientFooter.tsxcomponents/ConnectBtn.tsxcomponents/ErrorBoundary.tsxcomponents/ExplorerContent.tsxcomponents/Footer.tsxcomponents/KyaModal.tsxcomponents/Orb.tsxcomponents/PillNav.tsxcomponents/TokenSelector.tsxcomponents/UnsupportedNetworkEmptyState.tsxcomponents/createOracle.tsxcomponents/navigation.tsxcomponents/oracle/OracleMetadataForm.tsxcomponents/oracle/OracleParametersForm.tsxcomponents/ui/input.tsxcomponents/ui/textarea.tsxcomponents/ui/tooltip.tsxcomponents/ui/use-toast.tscontext/KYAModalContext.tsxhooks/useNetworkGuard.tslib/schemas/createOracleSchema.tsnext.config.mjspackage.jsonproviders/WalletProvider.tsxproviders/web3-provider.jstestZod.tsutils/config.ts
💤 Files with no reviewable changes (3)
- app/create/page.tsx
- app/page.tsx
- providers/web3-provider.js
| @@ -0,0 +1,8 @@ | |||
| fund=false | |||
| audit=false | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether compensating dependency security checks exist in CI/docs.
# Expected: at least one workflow or script runs npm audit (or equivalent SCA).
fd -HI 'package.json|pnpm-workspace.yaml|yarn.lock|package-lock.json|*.yml|*.yaml|README.md|SECURITY.md|CONTRIBUTING.md' . >/dev/null
echo "== Search workflows/scripts for dependency vulnerability scanning =="
rg -n -C 2 --iglob '**/*.yml' --iglob '**/*.yaml' --iglob '**/package.json' \
'npm audit|pnpm audit|yarn audit|dependency-review|osv-scanner|snyk|trivy|dependabot|codeql'
echo
echo "== Search docs for stated alternative security scanning process =="
rg -n -C 2 --iglob '**/README.md' --iglob '**/SECURITY.md' --iglob '**/CONTRIBUTING.md' \
'audit|vulnerab|dependency scan|SCA|security scan'Repository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 686
🏁 Script executed:
#!/bin/bash
# Find and search CI workflows and security documentation for vulnerability scanning
echo "=== Finding CI workflow files ==="
find . -path './node_modules' -prune -o -type f \( -name '*.yml' -o -name '*.yaml' \) -print | grep -E '(\.github|\.gitlab|\.circleci|\.travis)' | head -20
echo ""
echo "=== Finding security-related documentation ==="
find . -path './node_modules' -prune -o -type f \( -name 'SECURITY*' -o -name 'CONTRIBUTING*' \) -print
echo ""
echo "=== Searching for audit/vulnerability scanning in YAML files ==="
rg -t yaml 'audit|npm audit|dependency|vulnerab|scan' --max-count=5
echo ""
echo "=== Searching .npmrc content ==="
cat -n .npmrc 2>/dev/null || echo ".npmrc not found"
echo ""
echo "=== Searching for audit in JSON/JS files ==="
rg 'npm audit|npm ci|npm install' -t json -t js --max-count=5 package.json 2>/dev/null || echo "Search returned no results"Repository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 734
🏁 Script executed:
cat -n ./.github/workflows/nextjs.ymlRepository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 4163
Add compensating dependency vulnerability scanning to CI.
Setting audit=false disables npm's vulnerability checks during local installs and CI. No alternative security scan (npm audit, Dependabot, SAST, or dependency-review) exists in the workflow. Enable npm audit in CI or document and enforce an alternative SCA process.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.npmrc at line 2, Remove the disabling setting by deleting or changing the
.npmrc entry "audit=false" and add a CI step that runs "npm audit" (or an
equivalent SCA tool) so dependency vulnerabilities are scanned during builds;
implement a CI job/step named something like "dependency-scan" that runs npm
audit --json (or invokes Dependabot/dependency-review/SAST) and fails the build
on high/critical findings, and document the chosen SCA process in the repo so
local installs and CI both get scanned.
| useEffect(() => { | ||
| if (!latestPriceHistoryData) { | ||
| setPriceHistoryPoints([]) | ||
| return | ||
| setPriceHistoryPoints([]); | ||
| return; | ||
| } | ||
|
|
||
| const history = buildPriceHistoryPoints(latestPriceHistoryData as PriceHistoryResult) | ||
| setPriceHistoryPoints(history) | ||
| }, [latestPriceHistoryData, buildPriceHistoryPoints]) | ||
| const history = buildPriceHistoryPoints( | ||
| latestPriceHistoryData as PriceHistoryResult, | ||
| ); | ||
| setPriceHistoryPoints(history); | ||
| }, [latestPriceHistoryData, buildPriceHistoryPoints]); |
There was a problem hiding this comment.
Initialize the summary cards from fetched history.
This effect only updates priceHistoryPoints. On first load the chart renders, but latestValue and aggregatedValue stay — until the user manually clicks Read or submits a transaction. Reuse updatePriceDisplays(...) here so the chart and the summary cards stay in sync.
💡 Proposed fix
useEffect(() => {
if (!latestPriceHistoryData) {
- setPriceHistoryPoints([]);
+ updatePriceDisplays(undefined);
return;
}
- const history = buildPriceHistoryPoints(
- latestPriceHistoryData as PriceHistoryResult,
- );
- setPriceHistoryPoints(history);
- }, [latestPriceHistoryData, buildPriceHistoryPoints]);
+ updatePriceDisplays(latestPriceHistoryData as PriceHistoryResult);
+ }, [latestPriceHistoryData, updatePriceDisplays]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| if (!latestPriceHistoryData) { | |
| setPriceHistoryPoints([]) | |
| return | |
| setPriceHistoryPoints([]); | |
| return; | |
| } | |
| const history = buildPriceHistoryPoints(latestPriceHistoryData as PriceHistoryResult) | |
| setPriceHistoryPoints(history) | |
| }, [latestPriceHistoryData, buildPriceHistoryPoints]) | |
| const history = buildPriceHistoryPoints( | |
| latestPriceHistoryData as PriceHistoryResult, | |
| ); | |
| setPriceHistoryPoints(history); | |
| }, [latestPriceHistoryData, buildPriceHistoryPoints]); | |
| useEffect(() => { | |
| if (!latestPriceHistoryData) { | |
| updatePriceDisplays(undefined); | |
| return; | |
| } | |
| updatePriceDisplays(latestPriceHistoryData as PriceHistoryResult); | |
| }, [latestPriceHistoryData, updatePriceDisplays]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/`[oracleId]/InteractionClient.tsx around lines 332 - 342, The effect that
builds priceHistoryPoints should also call updatePriceDisplays(...) so the
summary cards (latestValue and aggregatedValue) are initialized from fetched
history; inside the useEffect where you create history via
buildPriceHistoryPoints(latestPriceHistoryData as PriceHistoryResult), after
setPriceHistoryPoints(history) invoke updatePriceDisplays(history) (or the
appropriate args that updatePriceDisplays expects), and add updatePriceDisplays
to the effect dependency array so the chart and summary cards stay in sync on
first load.
| if (lastSubmissionTimeData) { | ||
| const timestamp = Number(lastSubmissionTimeData as bigint) | ||
| const now = Math.floor(Date.now() / 1000) | ||
| const diff = now - timestamp | ||
| const timestamp = Number(lastSubmissionTimeData as bigint); | ||
| const now = Math.floor(Date.now() / 1000); | ||
| const diff = now - timestamp; | ||
| if (diff < 60) { | ||
| setLastUpdated(`${diff}s ago`) | ||
| setLastUpdated(`${diff}s ago`); | ||
| } else if (diff < 3600) { | ||
| setLastUpdated(`${Math.floor(diff / 60)}m ago`) | ||
| setLastUpdated(`${Math.floor(diff / 60)}m ago`); | ||
| } else { | ||
| setLastUpdated(`${Math.floor(diff / 3600)}h ago`) | ||
| setLastUpdated(`${Math.floor(diff / 3600)}h ago`); | ||
| } | ||
| } | ||
|
|
||
| // Update oracle configuration values | ||
| if (rewardData) { | ||
| const reward = Number(rewardData as bigint) | ||
| setRewardRate(`${(reward / 1000).toFixed(3)}%`) // Convert from basis points to percentage | ||
| const reward = Number(rewardData as bigint); | ||
| setRewardRate(`${(reward / 1000).toFixed(3)}%`); // Convert from basis points to percentage | ||
| } | ||
| if (halfLifeSecondsData) { | ||
| const halfLife = Number(halfLifeSecondsData as bigint) | ||
| setHalfLifeSeconds(`${halfLife}s`) | ||
| const halfLife = Number(halfLifeSecondsData as bigint); | ||
| setHalfLifeSeconds(`${halfLife}s`); | ||
| } | ||
| if (quorumData) { | ||
| const quorum = Number(quorumData as bigint) | ||
| setQuorumPercentage(`${(quorum / 100).toFixed(1)}%`) // Convert from basis points to percentage | ||
| const quorum = Number(quorumData as bigint); | ||
| setQuorumPercentage(`${(quorum / 100).toFixed(1)}%`); // Convert from basis points to percentage | ||
| } | ||
| if (operationLockingPeriodData) { | ||
| const operationLock = Number(operationLockingPeriodData as bigint) | ||
| setOperationLockPeriod(`${operationLock}s`) | ||
| const operationLock = Number(operationLockingPeriodData as bigint); | ||
| setOperationLockPeriod(`${operationLock}s`); | ||
| } | ||
| if (withdrawalLockingPeriodData) { | ||
| const withdrawalLock = Number(withdrawalLockingPeriodData as bigint) | ||
| setWithdrawalLockPeriod(`${withdrawalLock}s`) | ||
| const withdrawalLock = Number(withdrawalLockingPeriodData as bigint); | ||
| setWithdrawalLockPeriod(`${withdrawalLock}s`); | ||
| } | ||
| if (alphaData) { | ||
| const alpha = Number(alphaData as bigint) | ||
| setAlphaValue(alpha.toString()) | ||
| const alpha = Number(alphaData as bigint); | ||
| setAlphaValue(alpha.toString()); | ||
| } | ||
|
|
||
| // Update timestamp values | ||
| if (depositTimestampData) { | ||
| const timestamp = Number(depositTimestampData as bigint) | ||
| const timestamp = Number(depositTimestampData as bigint); | ||
| if (timestamp === 0) { | ||
| setDepositTimestamp("Never") | ||
| setDepositTimestamp("Never"); | ||
| } else { | ||
| const date = new Date(timestamp * 1000) | ||
| setDepositTimestamp(date.toLocaleString()) | ||
| const date = new Date(timestamp * 1000); | ||
| setDepositTimestamp(date.toLocaleString()); | ||
| } | ||
| } | ||
| if (lastOperationTimestampData) { | ||
| const timestamp = Number(lastOperationTimestampData as bigint) | ||
| const timestamp = Number(lastOperationTimestampData as bigint); | ||
| if (timestamp === 0) { | ||
| setLastOperationTimestamp("Never") | ||
| setLastOperationTimestamp("Never"); | ||
| } else { | ||
| const date = new Date(timestamp * 1000) | ||
| setLastOperationTimestamp(date.toLocaleString()) | ||
| const date = new Date(timestamp * 1000); | ||
| setLastOperationTimestamp(date.toLocaleString()); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "InteractionClient.tsx" | head -5Repository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 119
🏁 Script executed:
# Get file info and read the relevant section
cd app && git ls-files | grep -i interactionRepository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 113
🏁 Script executed:
# Search for InteractionClient file
fd "InteractionClient" --type fRepository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 117
🏁 Script executed:
wc -l app/[oracleId]/InteractionClient.tsxRepository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 122
🏁 Script executed:
sed -n '400,510p' app/[oracleId]/InteractionClient.tsx | cat -nRepository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 4548
🏁 Script executed:
sed -n '1,200p' app/[oracleId]/InteractionClient.tsx | cat -nRepository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 7936
🏁 Script executed:
# Search for useState declarations for the setters mentioned in the review
grep -n "setLastUpdated\|setRewardRate\|setAlphaValue\|setDepositTimestamp\|setLastOperationTimestamp" app/[oracleId]/InteractionClient.tsx | head -20Repository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 1088
🏁 Script executed:
# Check the read contract hooks to understand the data types
grep -n "useReadContract" app/[oracleId]/InteractionClient.tsx | grep -E "lastSubmissionTimeData|rewardData|alphaData|depositTimestampData|lastOperationTimestampData" | head -10Repository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 382
🏁 Script executed:
# Let's see the full read contract hook definitions for these specific values
sed -n '200,350p' app/[oracleId]/InteractionClient.tsx | cat -nRepository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 5916
Use explicit null/undefined checks instead of truthiness tests for BigInt values.
These conditions rely on truthiness, which fails for legitimate 0n values. Fresh oracles with lastSubmissionTimeData = 0n won't trigger the update, leaving Loading... stuck on screen. Similarly, alphaData = 0, depositTimestampData = 0n, and lastOperationTimestampData = 0n remain unprocessed, keeping stale UI state.
Note: Some data checks already use the correct pattern (lines 12, 22, 25), but the affected conditions here do not.
💡 Proposed fix
- if (lastSubmissionTimeData) {
+ if (lastSubmissionTimeData !== undefined && lastSubmissionTimeData !== null) {
const timestamp = Number(lastSubmissionTimeData as bigint);
const now = Math.floor(Date.now() / 1000);
const diff = now - timestamp;
...
}
- if (rewardData) {
+ if (rewardData !== undefined && rewardData !== null) {
const reward = Number(rewardData as bigint);
setRewardRate(`${(reward / 1000).toFixed(3)}%`);
}
- if (halfLifeSecondsData) {
+ if (halfLifeSecondsData !== undefined && halfLifeSecondsData !== null) {
const halfLife = Number(halfLifeSecondsData as bigint);
setHalfLifeSeconds(`${halfLife}s`);
}
- if (quorumData) {
+ if (quorumData !== undefined && quorumData !== null) {
const quorum = Number(quorumData as bigint);
setQuorumPercentage(`${(quorum / 100).toFixed(1)}%`);
}
- if (operationLockingPeriodData) {
+ if (operationLockingPeriodData !== undefined && operationLockingPeriodData !== null) {
const operationLock = Number(operationLockingPeriodData as bigint);
setOperationLockPeriod(`${operationLock}s`);
}
- if (withdrawalLockingPeriodData) {
+ if (withdrawalLockingPeriodData !== undefined && withdrawalLockingPeriodData !== null) {
const withdrawalLock = Number(withdrawalLockingPeriodData as bigint);
setWithdrawalLockPeriod(`${withdrawalLock}s`);
}
- if (alphaData) {
+ if (alphaData !== undefined && alphaData !== null) {
const alpha = Number(alphaData as bigint);
setAlphaValue(alpha.toString());
}
- if (depositTimestampData) {
+ if (depositTimestampData !== undefined && depositTimestampData !== null) {
const timestamp = Number(depositTimestampData as bigint);
...
}
- if (lastOperationTimestampData) {
+ if (
+ lastOperationTimestampData !== undefined &&
+ lastOperationTimestampData !== null
+ ) {
const timestamp = Number(lastOperationTimestampData as bigint);
...
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/`[oracleId]/InteractionClient.tsx around lines 427 - 483, The checks in
InteractionClient (the update block handling lastSubmissionTimeData, rewardData,
halfLifeSecondsData, quorumData, operationLockingPeriodData,
withdrawalLockingPeriodData, alphaData, depositTimestampData, and
lastOperationTimestampData) use truthiness and therefore skip valid 0n/0 values;
change each conditional to explicit null/undefined checks (e.g., if
(lastSubmissionTimeData !== null && lastSubmissionTimeData !== undefined)) so
zero BigInt timestamps and numeric zeros are processed and the UI updates (apply
the same pattern for rewardData, halfLifeSecondsData, quorumData,
operationLockingPeriodData, withdrawalLockingPeriodData, alphaData,
depositTimestampData, lastOperationTimestampData).
| try { | ||
| setIsSubmitting(true) | ||
| setIsSubmitting(true); | ||
| // Convert to int256 - the value should be a scaled integer | ||
| // For example, if submitting 2500, multiply by 1e18 to get proper precision | ||
| const valueAsFloat = parseFloat(submitValue) | ||
| const valueAsInt = BigInt(Math.floor(valueAsFloat * 1e18)) | ||
| console.log('Submitting value:', { | ||
| const valueAsFloat = parseFloat(submitValue); | ||
| const valueAsInt = BigInt(Math.floor(valueAsFloat * 1e18)); | ||
|
|
||
| console.log("Submitting value:", { | ||
| original: submitValue, | ||
| asFloat: valueAsFloat, | ||
| asInt: valueAsInt.toString(), | ||
| oracleAddress: oracleAddress, | ||
| userAddress: userAddress | ||
| }) | ||
| userAddress: userAddress, | ||
| }); | ||
|
|
||
| writeContract({ | ||
| address: oracleAddress!, | ||
| abi: OracleAbi, | ||
| functionName: 'submitValue', | ||
| functionName: "submitValue", | ||
| args: [valueAsInt], | ||
| }) | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "InteractionClient.tsx" -x cat -n {}Repository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 50393
🏁 Script executed:
cd app && find . -name "InteractionClient.tsx" -type fRepository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 115
🏁 Script executed:
rg "parseUnits" --type ts --type tsx -B 2 -A 2Repository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 108
🏁 Script executed:
rg "PRICE_DECIMALS" --type ts --type tsxRepository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 108
🏁 Script executed:
rg "parseFloat.*1e18\|parseFloat.*Math.floor" --type ts --type tsxRepository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 108
Replace floating-point conversion with parseUnits for price submission.
The current parseFloat(submitValue) * 1e18 approach can lose precision during floating-point arithmetic before the BigInt conversion, causing decimal inputs to be persisted as incorrect on-chain values. Use parseUnits(submitValue, PRICE_DECIMALS) instead, which performs exact decimal conversion and matches the pattern already used for token amounts throughout this file (deposit, approve, withdraw operations).
Suggested fix
- const valueAsFloat = parseFloat(submitValue);
- const valueAsInt = BigInt(Math.floor(valueAsFloat * 1e18));
+ const valueAsInt = parseUnits(submitValue, PRICE_DECIMALS);
console.log("Submitting value:", {
original: submitValue,
- asFloat: valueAsFloat,
asInt: valueAsInt.toString(),
oracleAddress: oracleAddress,
userAddress: userAddress,
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/`[oracleId]/InteractionClient.tsx around lines 590 - 610, The current
conversion of submitValue uses parseFloat and BigInt which can lose precision;
replace that logic with ethers' parseUnits to perform exact decimal-to-integer
conversion using the existing PRICE_DECIMALS constant. In the try block where
setIsSubmitting(true) is called and before writeContract({ address:
oracleAddress!, abi: OracleAbi, functionName: "submitValue", args: [valueAsInt]
}), compute valueAsInt by calling parseUnits(submitValue, PRICE_DECIMALS) (and
convert to the expected BigInt/string form if needed) and use that value in the
args; keep the existing console.log but update logged asInt to reflect the
parseUnits result. Ensure parseUnits is imported/available in this file.
| {(() => { | ||
| const amount = depositAmount ? parseUnits(depositAmount, weightTokenDecimals) : BigInt(0) | ||
| const allowance = tokenAllowanceData ? BigInt(tokenAllowanceData as bigint) : BigInt(0) | ||
| const needsApproval = amount > allowance | ||
| const amount = depositAmount | ||
| ? parseUnits(depositAmount, weightTokenDecimals) | ||
| : BigInt(0); | ||
| const allowance = tokenAllowanceData | ||
| ? BigInt(tokenAllowanceData as bigint) | ||
| : BigInt(0); | ||
| const needsApproval = amount > allowance; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and read the file
head -1 app/[oracleId]/InteractionClient.tsx | wc -c
wc -l app/[oracleId]/InteractionClient.tsxRepository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 125
🏁 Script executed:
# Read the specific lines and surrounding context
sed -n '1450,1490p' app/[oracleId]/InteractionClient.tsxRepository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 2023
🏁 Script executed:
# Search for depositAmount usage and where it comes from
rg 'depositAmount' app/[oracleId]/InteractionClient.tsx -B 5 -A 5 | head -100Repository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 3182
🏁 Script executed:
# Search for parseUnits imports and usage
rg 'parseUnits|import.*viem|from.*ethers' app/[oracleId]/InteractionClient.tsx -B 2 -A 2Repository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 941
🌐 Web query:
parseUnits viem library error handling invalid input
💡 Result:
In the viem library, the parseUnits utility function validates its value input (a string) using a regex test: /^(-?)([0-9]).?([0-9])$/.test(value). If the input does not match this pattern (optional negative sign, optional digits before decimal, optional decimal point, optional digits after), it throws an InvalidDecimalNumberError. Example invalid inputs that trigger this: - "abc" (non-digits) - "12.34.56" (multiple decimals) - "12." (decimal without digits after, but regex allows [0-9]* so empty is ok? Wait, test shows it matches) The regex allows empty integer or fraction parts. To handle this error, wrap in try-catch: import { parseUnits, type ParseUnitsErrorType } from 'viem' try { const units = parseUnits('invalid', 18) } catch (error) { const typedError = error as ParseUnitsErrorType if (typedError.name === 'InvalidDecimalNumberError') { console.log('Invalid input format:', typedError.details?.value) } } Other potential errors include SyntaxError from BigInt on very large numbers causing scientific notation loss, or exceeding uint256 max leading to IntegerOutOfRangeError in encoding contexts. Official docs: https://viem.sh/docs/utilities/parseUnits.html Source code confirms validation: https://github.com/wevm/viem/blob/main/src/utils/unit/parseUnits.ts
Citations:
- 1: https://viem.sh/docs/utilities/parseUnits.html
- 2: https://github.com/wevm/viem/blob/main/src/utils/unit/parseUnits.ts
- 3: https://viem.sh/docs/error-handling.html
Wrap parseUnits in a try-catch block to prevent render crashes on invalid input.
parseUnits throws InvalidDecimalNumberError when inputs like 1e3, ., or values with excess decimals fail its regex validation. Calling it directly in this render block causes immediate crashes on keystroke before handlers can show validation feedback.
Proposed fix
{(() => {
- const amount = depositAmount
- ? parseUnits(depositAmount, weightTokenDecimals)
- : BigInt(0);
+ let amount = BigInt(0);
+ let hasValidDepositAmount = false;
+ if (depositAmount) {
+ try {
+ amount = parseUnits(depositAmount, weightTokenDecimals);
+ hasValidDepositAmount = true;
+ } catch {
+ hasValidDepositAmount = false;
+ }
+ }
const allowance = tokenAllowanceData
? BigInt(tokenAllowanceData as bigint)
: BigInt(0);
- const needsApproval = amount > allowance;
+ const needsApproval = hasValidDepositAmount && amount > allowance;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {(() => { | |
| const amount = depositAmount ? parseUnits(depositAmount, weightTokenDecimals) : BigInt(0) | |
| const allowance = tokenAllowanceData ? BigInt(tokenAllowanceData as bigint) : BigInt(0) | |
| const needsApproval = amount > allowance | |
| const amount = depositAmount | |
| ? parseUnits(depositAmount, weightTokenDecimals) | |
| : BigInt(0); | |
| const allowance = tokenAllowanceData | |
| ? BigInt(tokenAllowanceData as bigint) | |
| : BigInt(0); | |
| const needsApproval = amount > allowance; | |
| {(() => { | |
| let amount = BigInt(0); | |
| let hasValidDepositAmount = false; | |
| if (depositAmount) { | |
| try { | |
| amount = parseUnits(depositAmount, weightTokenDecimals); | |
| hasValidDepositAmount = true; | |
| } catch { | |
| hasValidDepositAmount = false; | |
| } | |
| } | |
| const allowance = tokenAllowanceData | |
| ? BigInt(tokenAllowanceData as bigint) | |
| : BigInt(0); | |
| const needsApproval = hasValidDepositAmount && amount > allowance; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/`[oracleId]/InteractionClient.tsx around lines 1465 - 1472, The render
code that computes amount uses parseUnits directly (parseUnits(depositAmount,
weightTokenDecimals)) which can throw InvalidDecimalNumberError on
transient/invalid inputs and crash the UI; wrap the parseUnits call in a
try-catch (inside the current IIFE or a small helper like safeParseUnits) and on
error fall back to BigInt(0) (or another safe sentinel) so amount computation
never throws during render; keep the rest of logic that computes allowance from
tokenAllowanceData and needsApproval (amount > allowance) unchanged so
validation handlers can surface errors instead of crashing the component.
| React.useEffect(() => { | ||
| listeners.push(setState) | ||
| listeners.push(setState); | ||
| return () => { | ||
| const index = listeners.indexOf(setState) | ||
| const index = listeners.indexOf(setState); | ||
| if (index > -1) { | ||
| listeners.splice(index, 1) | ||
| listeners.splice(index, 1); | ||
| } | ||
| } | ||
| }, [state]) | ||
| }; | ||
| }, [state]); |
There was a problem hiding this comment.
Fix the useEffect dependency array to prevent duplicate listeners.
The dependency array [state] causes the effect to re-run on every state change, which will re-register the listener. If the component re-renders due to parent updates or prop changes, the same setState function could be added to the listeners array multiple times, causing duplicate notifications.
🔧 Proposed fix: Use empty dependency array
React.useEffect(() => {
listeners.push(setState);
return () => {
const index = listeners.indexOf(setState);
if (index > -1) {
listeners.splice(index, 1);
}
};
- }, [state]);
+ }, []);The listener registration should only happen once on mount and cleanup on unmount, not on every state change.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| React.useEffect(() => { | |
| listeners.push(setState) | |
| listeners.push(setState); | |
| return () => { | |
| const index = listeners.indexOf(setState) | |
| const index = listeners.indexOf(setState); | |
| if (index > -1) { | |
| listeners.splice(index, 1) | |
| listeners.splice(index, 1); | |
| } | |
| } | |
| }, [state]) | |
| }; | |
| }, [state]); | |
| React.useEffect(() => { | |
| listeners.push(setState); | |
| return () => { | |
| const index = listeners.indexOf(setState); | |
| if (index > -1) { | |
| listeners.splice(index, 1); | |
| } | |
| }; | |
| }, []); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/ui/use-toast.ts` around lines 172 - 180, The effect in useEffect
is re-registering the same listener on every state change because it depends on
[state]; update the dependency array to [] so the listener registration
(listeners.push(setState)) runs only once on mount and the cleanup removes it on
unmount; change the dependency for the useEffect that references listeners and
setState to an empty array to prevent duplicate entries.
| <button | ||
| key={chain.id} | ||
| onClick={() => switchChain({ chainId: chain.id })} | ||
| className='rounded-xl border border-primary/30 bg-background/60 px-5 py-2.5 text-sm font-medium text-slate-100 transition-colors hover:bg-accent hover:text-accent-foreground' |
There was a problem hiding this comment.
Set explicit button type to avoid accidental form submits.
These buttons default to type="submit". If this component is ever rendered inside a form, clicks can trigger unintended submits.
Suggested patch
{supportedChains.map((chain) => (
<button
+ type='button'
key={chain.id}
onClick={() => switchChain({ chainId: chain.id })}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <button | |
| key={chain.id} | |
| onClick={() => switchChain({ chainId: chain.id })} | |
| className='rounded-xl border border-primary/30 bg-background/60 px-5 py-2.5 text-sm font-medium text-slate-100 transition-colors hover:bg-accent hover:text-accent-foreground' | |
| <button | |
| type='button' | |
| key={chain.id} | |
| onClick={() => switchChain({ chainId: chain.id })} | |
| className='rounded-xl border border-primary/30 bg-background/60 px-5 py-2.5 text-sm font-medium text-slate-100 transition-colors hover:bg-accent hover:text-accent-foreground' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/UnsupportedNetworkEmptyState.tsx` around lines 50 - 53, The button
rendered for each chain (the element using key={chain.id} and onClick={() =>
switchChain({ chainId: chain.id })}) lacks an explicit type and will default to
"submit" when inside a form; update that button to include type="button" to
prevent accidental form submissions while keeping the existing onClick and
classes intact.
| ownerAddress: z | ||
| .string() | ||
| .min(1, 'Owner address is required') | ||
| .refine((value) => isAddress(value), { | ||
| message: 'Must be a valid 0x-prefixed Ethereum address', | ||
| }), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -i 'createOracle.tsx' . -x sh -c '
echo "== {} ==";
rg -n -C3 "ownerAddress|constructorArgs|createOracle\\(" "{}";
'Repository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 1122
🏁 Script executed:
sed -n '120,150p' ./components/createOracle.tsxRepository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 881
🏁 Script executed:
cat -n utils/abi/Oracle.ts | head -30Repository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 2547
Add ownerAddress as the first constructor argument.
The Oracle ABI expects owner_ as the first constructor parameter (utils/abi/Oracle.ts:5), but constructorArgs in components/createOracle.tsx is missing it entirely. The form now validates and collects ownerAddress, but it's never passed to the smart contract constructor. This means deployed oracles will have an incorrect or unintended owner, creating a critical security gap between user intent and contract state.
Insert data.ownerAddress as \0x${string}`as the first element of theconstructorArgs` array (line 128).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/schemas/createOracleSchema.ts` around lines 13 - 18, The form collects
ownerAddress but the value is never passed to the contract constructor; update
the constructorArgs array in components/createOracle.tsx to include
data.ownerAddress as the first element (typed as `0x${string}`) so the Oracle
constructor receives owner_ as its first parameter per the ABI (see
utils/abi/Oracle.ts:5); locate the constructorArgs creation and insert
data.ownerAddress as `0x${string}` before the existing arguments.
| typescript: { | ||
| ignoreBuildErrors: true, | ||
| }, | ||
| eslint: { | ||
| ignoreDuringBuilds: true, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether CI enforces lint/typecheck if Next build ignores them.
rg -n "ignoreBuildErrors|ignoreDuringBuilds" --iglob "next.config.*"
rg -n '"(build|lint|typecheck)"\s*:' package.json
rg -n "npm run (lint|typecheck)|pnpm (lint|typecheck)|yarn (lint|typecheck)" --iglob "*.yml" --iglob "*.yaml"Repository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 435
🏁 Script executed:
# Search for CI/CD configuration files
find . -type f \( -name ".gitlab-ci.yml" -o -name ".circleci" -o -name "Jenkinsfile" \) 2>/dev/null
fd -t f "\.github" -type dRepository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 251
🏁 Script executed:
# List GitHub Actions workflows if they exist
find .github/workflows -type f -name "*.yml" -o -name "*.yaml" 2>/dev/null | head -20Repository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 109
🏁 Script executed:
# Get a broader view of the repository structure and config files
git ls-files | grep -E "(^\.github|^\.gitlab|\.circleci|Jenkinsfile|\.travis\.yml)" | head -20Repository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 109
🏁 Script executed:
cat -n .github/workflows/nextjs.ymlRepository: StabilityNexus/OrbOracle-EVM-Frontend
Length of output: 4163
Re-enable build-time type/lint gates for production builds.
The CI workflow only runs next build without separate lint or typecheck steps. Since next.config.mjs ignores both TypeScript and ESLint errors, broken code can reach production undetected. Keep fast local loops if needed, but gate this by environment so CI still fails on type/lint errors.
Suggested patch
const nextConfig = {
+ const isCi = process.env.CI === 'true'
output: 'export',
distDir: 'out',
@@
- typescript: {
- ignoreBuildErrors: true,
- },
- eslint: {
- ignoreDuringBuilds: true,
- },
+ typescript: { ignoreBuildErrors: !isCi },
+ eslint: { ignoreDuringBuilds: !isCi },
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| typescript: { | |
| ignoreBuildErrors: true, | |
| }, | |
| eslint: { | |
| ignoreDuringBuilds: true, | |
| }, | |
| const isCi = process.env.CI === 'true' | |
| const nextConfig = { | |
| output: 'export', | |
| distDir: 'out', | |
| typescript: { ignoreBuildErrors: !isCi }, | |
| eslint: { ignoreDuringBuilds: !isCi }, | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@next.config.mjs` around lines 14 - 19, The config currently disables
TypeScript and ESLint build-time checks (typescript.ignoreBuildErrors and
eslint.ignoreDuringBuilds) unconditionally; change it to only disable these in
local dev fast-reload scenarios and enable them in CI/production by gating on
environment variables (e.g., process.env.CI or NODE_ENV === 'production') so
that in CI/production the flags are false and Next.js build fails on type/lint
errors while preserving ignore behavior for local development.
| "prepare": "husky", | ||
| "start": "next start", | ||
| "typecheck": "tsc --noEmit" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Using official Next.js documentation or package metadata: is eslint-config-next@15.5.14intended to be used withnext@14.2.16, or should eslint-config-nextstay on the same major/minor asnext?
💡 Result:
eslint-config-next should stay on the same major/minor version as next. eslint-config-next@15.5.14 is not intended to be used with next@14.2.16; instead, use eslint-config-next@14.2.x (e.g., the latest 14.2.35 tagged as 'next-14').
Citations:
- 1: https://www.npmjs.com/package/eslint-config-next?activeTab=versions
- 2: fix(nextjs): ensure
eslint-config-nextmatches Next.js 14 and 15 versions nrwl/nx#30259 - 3: https://nextjs.org/docs/pages/api-reference/create-next-app
- 4: https://www.npmjs.com/package/eslint-config-next
- 5: https://depfixer.com/compatibility/next-15-eslint-config-next-14
Update eslint-config-next to match the next version.
eslint-config-next@15.5.14 is not compatible with next@14.2.16. According to official Next.js package metadata, eslint-config-next must stay on the same major/minor version as next. Update to eslint-config-next@14.2.35 (or the latest 14.2.x version available).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 12 - 14, The package.json currently uses
eslint-config-next@15.x which is incompatible with next@14.2.16; update the
dependency "eslint-config-next" to a 14.2.x release (e.g. "14.2.35") in
package.json, then run your package manager to regenerate the lockfile
(npm/yarn/pnpm install) so the installed versions match next's major/minor and
prevent ESLint/Next mismatches.
Fix unsupported-network crashes, restore broken create flow UX, and add form/DX guardrails
Fixes #30
This PR addresses a set of frontend issues that were making the core flow brittle:
/explorercould crash on unsupported networks, the create form accepted clearly invalid input without surfacing errors, the KYA modal and field tooltips were broken, and the repo had no pre-commit guardrails to catch preventable issues early.What changed
/explorerso unsupported networks no longer hard-crash the page.createOracle.tsxvalidation withreact-hook-formand Zod instead of scattered manual checks.Why this matters
The previous behavior failed in a few important ways:
Implementation notes
/explorercreateOracle.tsxKYA modal
Escapeand backdrop click.Tooltips
DX tooling
.husky/pre-commit.lint-stagedto run formatting/lint checks before commits.Wallet dependency cleanup
Verification
npm run lintnpm run build/exploreron an unsupported network and confirmed graceful fallback behavior/createand confirmed inline validationOutcome
This keeps the app from failing at the edges of the main user journey, makes the create flow much harder to misuse, restores missing guidance in the UI, and adds a basic quality gate for future changes.