feat: harden orb frontend create and oracle interaction flows#31
Open
arunabha003 wants to merge 13 commits into
Open
feat: harden orb frontend create and oracle interaction flows#31arunabha003 wants to merge 13 commits into
arunabha003 wants to merge 13 commits into
Conversation
📝 WalkthroughWalkthroughCentralizes contract error formatting; adds token-decimal-aware numeric parsing and validation flags; consolidates UI disabling via an overall in-flight flag; adds chain explorer URL support; improves oracle creation form validation and supported-chain awareness; integrates wallet/chain status into Explorer and renders a Toaster in layout. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/`[oracleId]/InteractionClient.tsx:
- Around line 1151-1170: The icon-only explorer link and copy button currently
rely on title text and lack an explicit accessible name and button type; update
the anchor that renders the ExternalLink and the copy <button> (the element with
onClick={() => navigator.clipboard.writeText(oracle.address)}) to include
descriptive aria-label attributes (e.g., aria-label={`Open oracle
${oracle.address} in explorer`} and aria-label={`Copy oracle ${oracle.address}
address`}) and set type="button" on the copy <button>; apply the same changes to
the repeated block around the elements referenced at the later occurrence (lines
1183-1205) so both explorer and copy controls are discoverable by assistive
tech.
- Around line 414-417: The derived booleans currently treat unresolved RPC reads
as zero; change them to represent an explicit loading/unknown state by checking
for the presence of the raw read values before comparing. Specifically, in the
expressions that compute hasEnoughWalletBalance, hasEnoughUnlockedBalance,
allowanceRaw and needsApproval, first ensure depositAmountRaw, withdrawAmountRaw
and the read values (userTokenBalanceRaw, unlockedTokensRaw, tokenAllowanceData)
are non-null/defined (or use their associated isLoading flags) and only then
perform BigInt comparisons; otherwise set the derived flags to null/undefined
(or a separate "loading" state) so the UI can wait for the RPC results before
showing insufficiency. Apply the same guarding pattern wherever similar logic
appears (the other ranges referenced) to avoid flashing "not enough" while reads
are pending.
- Around line 408-409: Replace the fragile Number.parseFloat/Math.floor flow by
parsing the trimmed submitValue with parseUnits using PRICE_DECIMALS (same
pattern as parseTokenAmount) to produce a validated bigint; update the logic
that defines hasValidSubmitValue to check that parseUnits(submitValue.trim(),
PRICE_DECIMALS) succeeds and yields a non-negative bigint, store that bigint
(e.g., submitValueUnits) and use it directly when encoding or calling the
contract instead of re-encoding or converting via floating-point math; apply the
same change to the other occurrence that currently uses submitValueNumber (the
analogous code around the other validation/encoding site).
In `@components/createOracle.tsx`:
- Around line 79-81: The form stores errors in submissionError via
setSubmissionError but never renders it; update the Create Oracle component's
render near the submit CTA to display an inline error banner when
submissionError is non-null (e.g., conditionally render a <Banner/Alert> or
similar component next to the submit button using submissionError as its
message), and ensure the catch blocks that call setSubmissionError keep the
formatted message; also clear submissionError on successful submit or when the
user edits relevant fields (use setSubmissionError(null) in the success path and
on relevant onChange handlers or in setTouchedFields logic).
- Around line 106-118: The form collects and validates owner but never passes it
to the deployment call: update either the UI or the constructorArgs to avoid the
UX trap; specifically either remove/hide the owner input and related validation
(the owner state and OwnerInput usage) or include owner in the arguments passed
to createOracle by adjusting constructorArgs to match createOracle's expected
parameters if the contract actually accepts owner, and update any call sites of
createOracle accordingly; locate the owner state/validation and the
constructorArgs array in createOracle.tsx and ensure owner is either eliminated
from the form/validation and any labels updated to indicate it is ignored, or
added to constructorArgs and synchronized with createOracle invocation so the
supplied owner is actually used.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 778b07cc-ea59-43f8-acb2-6548ca0e6e6d
⛔ Files ignored due to path filters (1)
app/icon.svgis excluded by!**/*.svg
📒 Files selected for processing (6)
app/[oracleId]/InteractionClient.tsxapp/explorer/page.tsxapp/layout.tsxcomponents/createOracle.tsxutils/addresses.tsutils/formatContractError.ts
There was a problem hiding this comment.
🧹 Nitpick comments (3)
components/createOracle.tsx (3)
290-314: Consider cleanup for thesetTimeoutto avoid state updates on unmounted component.If the user navigates away before the 5-second timeout completes, the callback will attempt to update state on an unmounted component. While this won't break functionality, it's a minor code smell.
♻️ Suggested fix using a ref
+ const isMountedRef = useRef(true) + + useEffect(() => { + return () => { isMountedRef.current = false } + }, []) // Inside createOracle: setTimeout(async () => { + if (!isMountedRef.current) return try { // ... existing code } catch (err) { + if (!isMountedRef.current) return // ... existing code } }, 5000)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/createOracle.tsx` around lines 290 - 314, The setTimeout callback that calls setOracleAddress, setSubmissionError, setSubmitted and toast can run after the component unmounts; store the timeout id (from setTimeout) in a ref (e.g., timerRef) when scheduling it and clear it in a cleanup effect (useEffect return) or componentWillUnmount to avoid state updates on an unmounted component; alternatively track mounted state in a ref (isMounted) and check it before calling setOracleAddress/setSubmissionError/setSubmitted/toast so the callback no-ops if the component is unmounted.
171-171: Remove unusedownerfromvalidationErrorsdependency array.The
validationErrorsmemo includesownerin its dependency array, butowneris no longer used in the validation logic (it was removed when the field became read-only). This creates unnecessary re-computations whenownerchanges.♻️ Suggested fix
- }, [alpha, depositLock, description, halfLifeSeconds, name, owner, quorumBps, reward, weightToken, withdrawLock]) + }, [alpha, depositLock, description, halfLifeSeconds, name, quorumBps, reward, weightToken, withdrawLock])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/createOracle.tsx` at line 171, The useMemo for validationErrors still lists owner in its dependency array even though owner is no longer referenced in the validation logic; remove owner from the dependency array of validationErrors (the useMemo that computes validationErrors) so the memo only depends on actual fields (alpha, depositLock, description, halfLifeSeconds, name, quorumBps, reward, weightToken, withdrawLock) and avoid unnecessary recomputations when owner changes.
287-302: Race condition in oracle lookup—consider waiting for transaction confirmation before reading state.The polling approach is fragile because it assumes the last oracle in
allOracleswas just created by this user. If another user creates an oracle between the 5-second delay and your read, you'll get the wrong address.Since
createOraclereturns the oracle address directly, the most robust fix is to wait for transaction confirmation usingwaitForTransactionReceiptand then parse theOracleCreatedevent (which contains the oracle address and is indexed by creator). Alternatively, querycreatorOracleList(userAddress)to deterministically find your oracle after the transaction settles.The current comment already flags this as a limitation, but for production use, implement proper receipt handling rather than polling.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/createOracle.tsx` around lines 287 - 302, Replace the fragile timeout/polling lookup of allOracles with deterministic receipt/event handling: after calling createOracle, wait for the transaction receipt via waitForTransactionReceipt (or the provider's wait method), parse the OracleCreated event from the receipt to extract the oracle address (event name OracleCreated, indexed creator) and call setOracleAddress with that value; as a fallback you can query creatorOracleList(userAddress) after the receipt to find the created oracle instead of reading allOracles. Ensure you locate the logic around createOracle, the timeout block that reads allOracles, and replace it with the receipt parsing or creatorOracleList query and then setOracleAddress.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@components/createOracle.tsx`:
- Around line 290-314: The setTimeout callback that calls setOracleAddress,
setSubmissionError, setSubmitted and toast can run after the component unmounts;
store the timeout id (from setTimeout) in a ref (e.g., timerRef) when scheduling
it and clear it in a cleanup effect (useEffect return) or componentWillUnmount
to avoid state updates on an unmounted component; alternatively track mounted
state in a ref (isMounted) and check it before calling
setOracleAddress/setSubmissionError/setSubmitted/toast so the callback no-ops if
the component is unmounted.
- Line 171: The useMemo for validationErrors still lists owner in its dependency
array even though owner is no longer referenced in the validation logic; remove
owner from the dependency array of validationErrors (the useMemo that computes
validationErrors) so the memo only depends on actual fields (alpha, depositLock,
description, halfLifeSeconds, name, quorumBps, reward, weightToken,
withdrawLock) and avoid unnecessary recomputations when owner changes.
- Around line 287-302: Replace the fragile timeout/polling lookup of allOracles
with deterministic receipt/event handling: after calling createOracle, wait for
the transaction receipt via waitForTransactionReceipt (or the provider's wait
method), parse the OracleCreated event from the receipt to extract the oracle
address (event name OracleCreated, indexed creator) and call setOracleAddress
with that value; as a fallback you can query creatorOracleList(userAddress)
after the receipt to find the created oracle instead of reading allOracles.
Ensure you locate the logic around createOracle, the timeout block that reads
allOracles, and replace it with the receipt parsing or creatorOracleList query
and then setOracleAddress.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f284f3df-f7eb-4d64-afa0-bde9d483f72a
📒 Files selected for processing (2)
app/[oracleId]/InteractionClient.tsxcomponents/createOracle.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addressed Issues:
Validation
npm run buildScreenshots/Recordings:
4.Inline Oracle Parameter Validation
Additional Notes:
AI Usage Disclosure:
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact. AI slop is strongly discouraged and may lead to banning and blocking. Do not spam our repos with AI slop.
Check one of the checkboxes below:
I have used the following AI models and tools: TODO
Checklist
Summary by CodeRabbit
New Features
Improvements