Conversation
🦋 Changeset detectedLatest commit: c6a1241 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Summary of ChangesHello @dieguezguille, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a bug where the application's 'getting started' flow incorrectly relied on the presence of funds ( Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
WalkthroughThe changes modify the KYC onboarding flow to evaluate account deployment status instead of USD balance. The GettingStarted component's Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the onboarding flow to allow users to proceed with KYC verification once their account is deployed, rather than after they have added funds. This is accomplished by changing the GettingStarted component to depend on isDeployed instead of hasFunds. The changes are logical and correctly implemented in GettingStarted.tsx and Home.tsx. The removal of the unused usdBalance variable is also a good cleanup. I have one suggestion to improve the naming of the new prop for better code clarity and maintainability.
| /> | ||
| )} | ||
| <GettingStarted hasFunds={usdBalance > 0n} hasKYC={KYCStatus === "ok"} /> | ||
| <GettingStarted isDeployed={!!bytecode} hasKYC={KYCStatus === "ok"} /> |
There was a problem hiding this comment.
To improve clarity and component encapsulation, consider renaming the isDeployed prop to isAddFundsStepComplete. The GettingStarted component uses this prop to determine if the 'add-funds' step is complete. Using the name isDeployed leaks an implementation detail from the Home component (that deployment status determines the completion of the 'add-funds' step) into the GettingStarted component. A more abstract prop name would make GettingStarted more self-contained and its purpose clearer.
This change will require updating the prop definition and its usages within GettingStarted.tsx accordingly.
| <GettingStarted isDeployed={!!bytecode} hasKYC={KYCStatus === "ok"} /> | |
| <GettingStarted isAddFundsStepComplete={!!bytecode} hasKYC={KYCStatus === "ok"} /> |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| setSteps((previous) => | ||
| previous.map((step) => { | ||
| if (step.id === "add-funds" && hasFunds) return { ...step, completed: true }; | ||
| if (step.id === "add-funds" && isDeployed) return { ...step, completed: true }; |
There was a problem hiding this comment.
Bug: The "Add funds" step now completes when an account is deployed (!!bytecode), not when funds are added, allowing users to finish onboarding with a zero balance.
Severity: HIGH
Suggested Fix
Revert the completion condition for the "add-funds" step to check the user's balance (e.g., usdBalance > 0n). The new requirement to "allow kyc on account deploy" should be handled separately from the fund-adding step to ensure each step's completion criteria matches its stated purpose.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/components/home/GettingStarted.tsx#L67
Potential issue: The logic for completing the "add-funds" onboarding step was changed
from checking `usdBalance > 0n` to checking if the account has been deployed
(`!!bytecode`). This creates a mismatch with the UI, which still instructs the user to
"Add funds to account". A user's account can be deployed without any funds, which would
incorrectly mark this step as complete. Consequently, the `GettingStarted` component
would be hidden, and the user could finish the entire onboarding process without adding
the required funds for collateral.
Did we get this right? 👍 / 👎 to inform future reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #672 +/- ##
==========================================
+ Coverage 60.95% 61.05% +0.10%
==========================================
Files 168 168
Lines 5273 5272 -1
Branches 1491 1491
==========================================
+ Hits 3214 3219 +5
+ Misses 1898 1892 -6
Partials 161 161
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/home/Home.tsx`:
- Line 196: The isDeployed prop currently uses !!bytecode which treats the
string "0x" as truthy; update the guard to explicitly detect non-empty on-chain
bytecode by checking the useBytecode result (bytecode) is not
null/undefined/empty and not exactly "0x" (e.g. isDeployed = Boolean(bytecode &&
bytecode !== "0x")). Locate the usage in Home.tsx where GettingStarted is
rendered and replace the !!bytecode check with an explicit bytecode !== "0x"
(and null/undefined) check so EOAs are not marked as deployed.
| /> | ||
| )} | ||
| <GettingStarted hasFunds={usdBalance > 0n} hasKYC={KYCStatus === "ok"} /> | ||
| <GettingStarted isDeployed={!!bytecode} hasKYC={KYCStatus === "ok"} /> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
wagmi useBytecode hook return value empty code EOA
💡 Result:
This is expected behavior. useBytecode calls eth_getCode under the hood; JSON‑RPC returns "0x" (or empty code) for externally owned accounts (EOAs), so useBytecode will return no bytecode for EOAs. [1][2]
Quick check (pseudo‑code):
- if (!data || data === '0x') → address is EOA (no contract)
- else → contract bytecode present
References:
- wagmi useBytecode docs. [1]
- eth_getCode: returns "0x" for EOAs. [2]
- Discussion / ethers provider.getCode behavior (confirmation). [3]
🌐 Web query:
viem getBytecode function returns null or 0x for non-deployed accounts
💡 Result:
Short answer: That's expected.
Details:
- eth_getCode (what viem calls getCode / getBytecode) returns empty bytecode for EOAs or addresses with no deployed contract — commonly shown as "0x" (or viem returns undefined for getCode). (viem.sh)
- viem deprecated getBytecode in favor of getCode; use publicClient.getCode and check for undefined or '0x' to detect "no contract deployed". (newreleases.io)
If you want, paste the exact code you're calling and I’ll show the minimal check to distinguish deployed contracts from non-deployed accounts.
Citations:
- 1: https://viem.sh/docs/contract/getCode?utm_source=openai
- 2: https://newreleases.io/project/github/wevm/viem/release/viem%402.14.1?utm_source=openai
🏁 Script executed:
grep -n "useBytecode\|GettingStarted" src/components/home/Home.tsx | head -20Repository: exactly/exa
Length of output: 305
🏁 Script executed:
rg "useBytecode" src/ -A 3 -B 3Repository: exactly/exa
Length of output: 23625
🏁 Script executed:
fd "Home.tsx" src/ --type f | xargs cat -n | grep -A 10 -B 10 "isDeployed"Repository: exactly/exa
Length of output: 1057
Guard against useBytecode returning "0x" for accounts with no deployed contract.
When eth_getCode finds no contract code, it returns "0x" instead of null. This means !!bytecode is true for EOAs, incorrectly marking them as deployed. Check explicitly for non-empty code.
🔧 Proposed fix
- <GettingStarted isDeployed={!!bytecode} hasKYC={KYCStatus === "ok"} />
+ <GettingStarted isDeployed={Boolean(bytecode && bytecode !== "0x")} hasKYC={KYCStatus === "ok"} />📝 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.
| <GettingStarted isDeployed={!!bytecode} hasKYC={KYCStatus === "ok"} /> | |
| <GettingStarted isDeployed={Boolean(bytecode && bytecode !== "0x")} hasKYC={KYCStatus === "ok"} /> |
🤖 Prompt for AI Agents
In `@src/components/home/Home.tsx` at line 196, The isDeployed prop currently uses
!!bytecode which treats the string "0x" as truthy; update the guard to
explicitly detect non-empty on-chain bytecode by checking the useBytecode result
(bytecode) is not null/undefined/empty and not exactly "0x" (e.g. isDeployed =
Boolean(bytecode && bytecode !== "0x")). Locate the usage in Home.tsx where
GettingStarted is rendered and replace the !!bytecode check with an explicit
bytecode !== "0x" (and null/undefined) check so EOAs are not marked as deployed.
2 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.