feat(wallet): surface public balance + first-run fixes (Send label, post-shield refresh) - #97
feat(wallet): surface public balance + first-run fixes (Send label, post-shield refresh)#97hitakshiA wants to merge 3 commits into
Conversation
The eERC SDK's register() ignores the decryptionKey passed to the EERC constructor and derives its own from a deterministic wallet signature (generateDecryptionKey), binding the on-chain public key and the ElGamal-encrypted balance to THAT key. Our read path constructed a fresh EERC with account.eercDecryptionKey and never called generateDecryptionKey, so calculateTotalBalance decrypted with the wrong key, failed its eGCT verify, and every shielded balance rendered $0.00 even though the deposit was on-chain (activity showed it correctly). Have createEerc install the SDK's own key up front. The signature is local (self-custody key) and deterministic, so it is stable across sessions and matches whatever key registration used. Verified end-to-end on Fuji: a fresh wallet that deposited 0.04 USDC now decrypts to 40000 (was 0).
A new wallet is airdropped test USDC to its PUBLIC balance by the faucet on creation, but Home only shows the PRIVATE balance ($0 until shielded), so the airdrop was invisible: a new user saw $0.00 + a "Receive money to get going" empty state and reasonably assumed the wallet was empty. Surface it explicitly: when the public (shieldable) balance is non-zero, show a banner with the USDC mark, "You've been airdropped X USDC", and a one-tap "Make it private" CTA into the shield flow. It self-clears once shielded (public baseUnits -> 0) and is dismissible (persisted in localStorage). The USDC logo is an inlined SVG so it renders under the PWA's strict CSP with no external request.
📝 WalkthroughWalkthroughThe wallet now initializes eERC decryption keys, presents public USDC with shield navigation, labels the send action, and refreshes balances repeatedly after shield or unshield operations. ChangesWallet features
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant useWallet
participant Home
participant ShieldRoute
useWallet->>Home: provide publicBalance and chain status
Home->>ShieldRoute: navigate to /shield?mode=shield
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const shieldableUnits = BigInt(publicBalance?.baseUnits ?? "0"); | ||
| const showAirdrop = !chainUnavailable && !airdropDismissed && shieldableUnits > 0n; |
There was a problem hiding this comment.
When publicBalance.baseUnits is not an integer string, BigInt(...) throws during render and Home fails to load. A transient malformed balance value from the wallet store or RPC parsing path should hide the banner or treat the amount as zero, not crash the main wallet screen.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/wallet/src/screens/Home.tsx
Line: 79-80
Comment:
**Public Balance Parse Crash**
When `publicBalance.baseUnits` is not an integer string, `BigInt(...)` throws during render and Home fails to load. A transient malformed balance value from the wallet store or RPC parsing path should hide the banner or treat the amount as zero, not crash the main wallet screen.
How can I resolve this? If you propose a fix, please make it concise.
| // decrypt with our own `account.eercDecryptionKey`, calculateTotalBalance()'s | ||
| // eGCT verify fails and every private balance reads $0. So let the SDK derive | ||
| // and install its own key (and matching publicKey) up front — the signature is | ||
| // local (self-custody key) and deterministic, so it's stable across sessions |
There was a problem hiding this comment.
createEerc now runs generateDecryptionKey() for every caller, including paths that only need to construct the SDK for balance or setup work. If the wallet cannot sign during a background refresh or read-only flow, EERC creation fails and those callers lose all EERC functionality instead of only skipping private-key-dependent work.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/wallet/src/lib/eerc.ts
Line: 142
Comment:
**Constructor Triggers Key Signing**
`createEerc` now runs `generateDecryptionKey()` for every caller, including paths that only need to construct the SDK for balance or setup work. If the wallet cannot sign during a background refresh or read-only flow, EERC creation fails and those callers lose all EERC functionality instead of only skipping private-key-dependent work.
How can I resolve this? If you propose a fix, please make it concise.![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
🧹 Nitpick comments (1)
apps/wallet/src/screens/Home.tsx (1)
28-31: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPotential precision loss converting large base-unit strings via
Number.
Number(baseUnits)loses precision once the value exceedsNumber.MAX_SAFE_INTEGER. Fine for small faucet drops today, but this helper has no bound and could misrender if reused for larger balances.💡 Precision-safe alternative using BigInt arithmetic
function formatUsdc(baseUnits: string): string { - const n = Number(baseUnits) / 10 ** USDC_DECIMALS; - return n.toLocaleString(undefined, { minimumFractionDigits: 2, maximumFractionDigits: 2 }); + const divisor = 10n ** BigInt(USDC_DECIMALS); + const value = BigInt(baseUnits); + const whole = value / divisor; + const frac = (value % divisor).toString().padStart(USDC_DECIMALS, "0").slice(0, 2); + return `${whole.toLocaleString()}.${frac}`; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/wallet/src/screens/Home.tsx` around lines 28 - 31, Update formatUsdc to avoid converting baseUnits through Number, using BigInt-based integer and fractional arithmetic to preserve precision for values beyond Number.MAX_SAFE_INTEGER while still rendering exactly two decimal places with locale-aware formatting.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/wallet/src/screens/Home.tsx`:
- Line 26: Update AIRDROP_DISMISS_KEY usage in the Home screen to include the
active wallet address, ensuring dismissal state is isolated per wallet rather
than shared globally. Use the existing wallet-address symbol and preserve the
current dismissal behavior for that wallet.
---
Nitpick comments:
In `@apps/wallet/src/screens/Home.tsx`:
- Around line 28-31: Update formatUsdc to avoid converting baseUnits through
Number, using BigInt-based integer and fractional arithmetic to preserve
precision for values beyond Number.MAX_SAFE_INTEGER while still rendering
exactly two decimal places with locale-aware formatting.
🪄 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 Plus
Run ID: a1a017ff-589d-4f89-a3d0-5bfc8e2cb5cf
📒 Files selected for processing (4)
apps/wallet/src/lib/eerc.shield.test.tsapps/wallet/src/lib/eerc.tsapps/wallet/src/screens/Home.tsxapps/wallet/src/ui/UsdcMark.tsx
| import { COPY } from "../lib/copy"; | ||
| import { ActivityItem } from "../ui/ActivityItem"; | ||
|
|
||
| const AIRDROP_DISMISS_KEY = "benzo.airdrop.dismissed.v1"; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "AIRDROP_DISMISS_KEY|benzo\.airdrop\.dismissed" -C3
rg -n "localStorage\.(removeItem|clear)" apps/wallet/srcRepository: Miny-Labs/benzo-wallet
Length of output: 160
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant file and nearby logic
sed -n '1,180p' apps/wallet/src/screens/Home.tsx
# Search for wallet reset / switch / logout flows and any localStorage cleanup
rg -n "localStorage\.(removeItem|clear)|AIRDROP_DISMISS_KEY|benzo\.airdrop\.dismissed|reset wallet|switch wallet|logout|disconnect" apps/wallet/src -C 3
# Find wallet-related state/identifiers that could scope the dismissal
rg -n "publicBalance\.address|session\.address|walletAddress|activeWallet|selectedWallet|accountAddress|address" apps/wallet/src -C 2Repository: Miny-Labs/benzo-wallet
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the wallet/session teardown flows and their localStorage cleanup.
rg -n "clearSiweAddress|clearLocalHistory|clearLocalInvites|clearPasskey|localStorage\.clear\(|removeItem\(" apps/wallet/src/lib apps/wallet/src/screens -C 2
# Inspect the wallet teardown implementation around the cleanup.
sed -n '430,485p' apps/wallet/src/lib/localWallet.ts
sed -n '1,220p' apps/wallet/src/lib/api.tsRepository: Miny-Labs/benzo-wallet
Length of output: 17809
Scope the airdrop dismiss key to the active wallet. A dismissed banner will stay hidden across wallet resets/new wallets because this localStorage key is global and isn’t cleared in wallet teardown; tie it to the wallet address or clear it on delete.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/wallet/src/screens/Home.tsx` at line 26, Update AIRDROP_DISMISS_KEY
usage in the Home screen to include the active wallet address, ensuring
dismissal state is isolated per wallet rather than shared globally. Use the
existing wallet-address symbol and preserve the current dismissal behavior for
that wallet.
…hield Three first-run fixes from live feedback: - Public balance was never shown as a balance anywhere (Home only shows the PRIVATE balance), and the first airdrop banner was cramped and ugly. Replace it with a clean, tappable Public balance card: USDC mark + "$X.XX USDC" on one line + context, that leads straight into shielding. Shows whenever the public balance is non-zero; "airdropped" framing only on a brand-new wallet. - Send was an unlabelled center FAB, so users couldn't find it. Label it "Send" like every other tab. - After shielding, Home showed $0 until the 15s tick or a manual reload, because the single post-op refresh read the stale pre-shield balance off the load-balanced Fuji RPC. Poll the balance a few times after the op so the new amount lands within a couple seconds.
hitakshiA
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/wallet/src/screens/Home.tsx (2)
64-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStale/duplicate comment block left behind.
Lines 64-68 are an old comment referencing a "dismiss" mechanism that no longer exists (the airdrop-dismiss
useState/localStorage logic was removed per the summary), and lines 69-72 re-explain essentially the same rationale in the new wording. This leaves two overlapping comment blocks, one of which is now inaccurate and will mislead future readers about a feature that doesn't exist.♻️ Proposed cleanup
- // A fresh wallet is airdropped test USDC to its PUBLIC balance by the faucet on - // creation, but Home only shows the PRIVATE balance ($0 until shielded), so the - // airdrop is invisible and a new user thinks the wallet is empty. Surface it - // explicitly with a one-tap path into the shield flow. It self-clears once the - // public balance is shielded (baseUnits -> 0); the dismiss is for early hiding. // The faucet airdrops test USDC to the PUBLIC balance on wallet creation, but // the hero shows the PRIVATE balance ($0 until shielded), so the public funds // were invisible and a new user thought the wallet was empty. Surface the // public balance as a clean, tappable row that leads straight into shielding.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/wallet/src/screens/Home.tsx` around lines 64 - 72, Remove the stale first comment block above the public-balance row, including the inaccurate reference to dismiss behavior, and retain only the current comment describing the public USDC balance and shielding flow.
89-119: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCard's accessible name is verbose; consider a concise
aria-label.The
motion.buttonhas noaria-label, so its accessible name is the concatenation of all inner text ("Public balance $X USDC ... Tap to make it private ... Make private"). A shortaria-label(e.g.Make $X USDC private) would give screen-reader users a cleaner announcement.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/wallet/src/screens/Home.tsx` around lines 89 - 119, Add a concise aria-label to the motion.button rendering the public balance card, using the formatted public balance amount and USDC so screen readers announce the action clearly, such as making that amount private. Keep the existing visible card text and behavior unchanged.apps/wallet/src/screens/Shield.tsx (1)
131-133: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPolling timers aren't cancelled on unmount, and out-of-order responses can overwrite fresher state.
Four
setTimeouts are scheduled but never cleared if the component unmounts (e.g., user backs out of Shield before all four fire), and there is no sequencing/AbortController to guard against a later-fired-but-earlier-resolvingrefreshBalance()call clobbering a fresher value from an earlier timer. SincerefreshBalancepresumably reads from a global wallet store rather than local component state, an unmount here is unlikely to trigger a "setState on unmounted component" warning, but the lack of cancellation is still worth guarding, especially if the user re-enters Shield and fires again, stacking further timers.♻️ Proposed cleanup with cancellable timers
- void refresh(); - for (const delay of [1500, 3500, 6500, 10_000]) { - setTimeout(() => void refreshBalance(), delay); - } + void refresh(); + const timers = [1500, 3500, 6500, 10_000].map((delay) => + setTimeout(() => void refreshBalance(), delay) + ); + // Clear any still-pending polls if this component unmounts. + pendingTimersRef.current.push(...timers);And clear
pendingTimersRef.currenton unmount via auseEffectcleanup.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/wallet/src/screens/Shield.tsx` around lines 131 - 133, Update the Shield component’s delayed refresh flow around refreshBalance to track all setTimeout handles in a ref, clear them during useEffect cleanup on unmount, and reset the collection before scheduling a new polling sequence. Add sequencing or cancellation protection so stale, out-of-order refreshBalance responses cannot overwrite fresher wallet state.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/wallet/src/screens/Home.tsx`:
- Around line 73-77: Update the freshAirdrop calculation in Home so it also
requires the history loading flag to be false before showing airdrop framing.
Preserve the existing public-balance, chain-availability, and empty-history
conditions.
---
Nitpick comments:
In `@apps/wallet/src/screens/Home.tsx`:
- Around line 64-72: Remove the stale first comment block above the
public-balance row, including the inaccurate reference to dismiss behavior, and
retain only the current comment describing the public USDC balance and shielding
flow.
- Around line 89-119: Add a concise aria-label to the motion.button rendering
the public balance card, using the formatted public balance amount and USDC so
screen readers announce the action clearly, such as making that amount private.
Keep the existing visible card text and behavior unchanged.
In `@apps/wallet/src/screens/Shield.tsx`:
- Around line 131-133: Update the Shield component’s delayed refresh flow around
refreshBalance to track all setTimeout handles in a ref, clear them during
useEffect cleanup on unmount, and reset the collection before scheduling a new
polling sequence. Add sequencing or cancellation protection so stale,
out-of-order refreshBalance responses cannot overwrite fresher wallet state.
🪄 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 Plus
Run ID: 90beb123-1f45-482a-998f-d42ae063d434
📒 Files selected for processing (3)
apps/wallet/src/App.tsxapps/wallet/src/screens/Home.tsxapps/wallet/src/screens/Shield.tsx
| const shieldableUnits = BigInt(publicBalance?.baseUnits ?? "0"); | ||
| const showPublicBalance = !chainUnavailable && shieldableUnits > 0n; | ||
| // "Airdropped" framing only fits a brand-new wallet; once there's activity the | ||
| // same public balance is just funds waiting to be made private. | ||
| const freshAirdrop = showPublicBalance && history.length === 0; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
freshAirdrop can briefly show incorrect "airdropped" copy before history loads.
freshAirdrop is derived from history.length === 0 without checking the loading flag. If publicBalance resolves before history does, a returning user with existing activity will briefly see "Airdropped to try Benzo — make it private to spend" until history populates and the copy flips. This is a self-recovering flicker, but it's an easy one-line guard.
🐛 Proposed fix
- const freshAirdrop = showPublicBalance && history.length === 0;
+ const freshAirdrop = showPublicBalance && !loading && history.length === 0;📝 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 shieldableUnits = BigInt(publicBalance?.baseUnits ?? "0"); | |
| const showPublicBalance = !chainUnavailable && shieldableUnits > 0n; | |
| // "Airdropped" framing only fits a brand-new wallet; once there's activity the | |
| // same public balance is just funds waiting to be made private. | |
| const freshAirdrop = showPublicBalance && history.length === 0; | |
| const shieldableUnits = BigInt(publicBalance?.baseUnits ?? "0"); | |
| const showPublicBalance = !chainUnavailable && shieldableUnits > 0n; | |
| // "Airdropped" framing only fits a brand-new wallet; once there's activity the | |
| // same public balance is just funds waiting to be made private. | |
| const freshAirdrop = showPublicBalance && !loading && history.length === 0; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/wallet/src/screens/Home.tsx` around lines 73 - 77, Update the
freshAirdrop calculation in Home so it also requires the history loading flag to
be false before showing airdrop framing. Preserve the existing public-balance,
chain-availability, and empty-history conditions.
1 participant
First-run UX fixes, driven by live feedback on a fresh wallet.
1. Public balance was invisible
A new wallet is airdropped test USDC to its public balance, but Home only shows the private balance ($0 until shielded) — so a new user saw $0.00 and thought the wallet was empty. Now Home shows a clean, tappable Public balance card (USDC mark +
$X.XX USDCon one line + context) that leads straight into shielding. It shows whenever the public balance is non-zero and self-clears once shielded; "airdropped" framing only appears on a brand-new wallet. (Replaces the first, cramped banner iteration.)2. Send was undiscoverable
The center nav FAB was an unlabelled glyph. Now it's labelled Send like every other tab.
3. Balance didn't update after shielding
After a shield, Home showed $0 until the 15s refresh tick or a manual reload — the single post-op refresh read the stale pre-shield balance off Fuji's load-balanced RPC. Now the balance is polled a few times after the op so it lands within a couple seconds.
Verified
Live on wallet.benzo.space with a fresh wallet: public balance card renders ($3.00 USDC), Send is labelled, and the private balance shows immediately after shielding without a reload. USDC logo is an inlined SVG (strict CSP). Typecheck, lint, 217 tests, build all pass.
Summary by CodeRabbit
New Features
Bug Fixes
Greptile Summary
This PR improves the first-run wallet flow for public funds and shielding. The main changes are:
Confidence Score: 5/5
This looks safe to merge.
Important Files Changed
Reviews (2): Last reviewed commit: "fix(wallet): show public balance cleanly..." | Re-trigger Greptile