Rewards v3: buy a tier by locking FUSE or paying the annual fee - #2523
MusabShakeel576 wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| depositStore.resetDepositFlow(); | ||
|
|
||
| if (route === 'lock') { | ||
| depositStore.setSavingsFundIntent('savings'); | ||
| depositStore.setDepositFromSolid(false); | ||
| selectSavingsFundToken('WFUSE'); | ||
| return; | ||
| } | ||
|
|
||
| router.push(path.DEPOSIT); | ||
| }; |
There was a problem hiding this comment.
Bug: The 'lock' route "Top up" button on the upgrade screen silently fails because the deposit modal it tries to open is not mounted on the current route.
Severity: HIGH
Suggested Fix
The handleTopUp function for the 'lock' route should navigate to a screen where the deposit modal provider is mounted, similar to how the 'cash' route uses router.push(path.DEPOSIT). This will ensure the deposit flow is correctly initiated and displayed to the user.
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: components/Rewards/NewRewards/UpgradeTier/UpgradeTierScreen.tsx#L164-L176
Potential issue: The `handleTopUp` function for the 'lock' route on the
`UpgradeTierScreen` calls `selectSavingsFundToken('WFUSE')`, which sets state in a
deposit store and attempts to trigger a modal via `setModal()`. However, the
`DepositModalProvider` responsible for rendering this modal is not mounted on the
`/rewards/upgrade` route. Consequently, when a user with insufficient FUSE clicks the
"Top up" button, no navigation or UI change occurs, making the button appear
non-functional. This contrasts with the 'cash' route, which correctly navigates the user
to the deposit flow.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Checked this one and it isn't valid — the premise that DepositModalProvider is route-scoped is wrong.
It is mounted by DeferredModalProviders (components/DeferredModalProviders.tsx:49), which the root layout renders at app/_layout.tsx:447 as a sibling of <Stack>, not inside a route. There is no route gating on it, so it is mounted on /rewards/upgrade exactly as it is on /rewards. The comment above it says as much: the providers are deferred to after first paint, and "since modals use Zustand stores, they remain fully functional once mounted."
The chain the button fires also completes without navigating:
selectSavingsFundToken('WFUSE')→useSavingsFundFlow.selectToken(hooks/useSavingsFundFlow.ts:103)- WFUSE has a single network, so it skips the chain list and calls
selectNetwork(...)(:113) - which prepares the deposit session and calls
setModal(DEPOSIT_MODAL.OPEN_SAVINGS_FUND_ADDRESS)(:102)
DepositModalProvider is subscribed to that store, so the address sheet opens over whatever screen is on top. That is deliberate: the lock route tops up in FUSE through the savings funding flow, and keeping the user on /rewards/upgrade means they return to the upgrade they were in the middle of when the sheet closes. The cash route pushes to path.DEPOSIT because USDC tops up through the full deposit flow, which is a screen rather than a sheet — the difference is the two currencies' funding paths, not one of them being broken.
No change made.
Generated by Claude Code
| }; | ||
|
|
||
| /** A date as the membership sheet writes it: "Sep 14, 2026". */ | ||
| export const formatMembershipDay = (iso: string | null | undefined): string => { | ||
| if (!iso) return ''; | ||
|
|
||
| const date = new Date(iso); | ||
| if (Number.isNaN(date.getTime())) return ''; | ||
|
|
||
| return `${MONTHS[date.getMonth()]} ${date.getDate()}, ${date.getFullYear()}`; | ||
| }; |
There was a problem hiding this comment.
Bug: Date formatting functions use local timezone methods on UTC dates, causing dates to display incorrectly (often one day early) for many users.
Severity: HIGH
Suggested Fix
Update the formatMembershipDate and formatMembershipDay functions to use UTC-specific methods (getUTCDate(), getUTCMonth(), getUTCFullYear()) instead of their local timezone counterparts to ensure date components are extracted from the correct timezone.
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: lib/tierUpgrade.ts#L169-L186
Potential issue: The `formatMembershipDate` and `formatMembershipDay` functions parse
UTC ISO date strings from the backend but then use local timezone methods (`getDate()`,
`getMonth()`) for formatting. This causes an off-by-one-day error for users in timezones
west of UTC. For instance, a membership expiring on `2027-09-10T00:00:00Z` would be
displayed as September 9, 2027, to a user in a UTC-5 timezone. This affects the display
of critical dates like membership expiry, subscription renewals, and lock periods.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Not valid as written, and the suggested fix would introduce the bug it describes.
The finding calls these values "UTC ISO date strings". They are ISO instants, not calendar dates. currentPeriodEnd, nextChargeAt, graceEndsAt and subscribedAt are Dates written by the billing sweep at the moment it runs; lockedSince and nextUnlockAt are derived from block.timestamp. None of them is a date-only value that happens to be serialised at midnight — 2027-09-10T00:00:00Z only appears in the example because it was invented for the report.
For an instant, local formatting is the correct rendering. A membership that ends at 2027-09-10T02:00:00Z really does end on 9 September for a user in UTC-5 — that is the day on their own calendar when their benefits stop. Switching to getUTCDate()/getUTCMonth() would print 10 September to that user, which is the day after the one their clock will show when it happens. That is the off-by-one, and it would be ours rather than theirs.
So formatMembershipDate and formatMembershipDay stay on the local accessors. The MONTHS table above them is there for a different, real problem — toLocaleDateString gives "Sept" on recent ICU and the three engines we ship on don't agree — which is why the functions read the parts by hand rather than delegating.
There is a genuine timezone defect next door, in the copy the backend composes rather than in this file: the past-due push formatted graceEndsAt with toLocaleDateString on the server, so it named the deadline in the service's timezone while this screen names it in the user's, and the two could disagree by a day about the same instant. Fixed on the backend by making the push say how long is left ("within 7 days") instead of printing a date it has no way to get right for every recipient: Solid-Money/solid-backend@5a00e15d.
No change here.
Generated by Claude Code
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. Summary: Reviewed 243 changed files (16,063 insertions, 3,005 deletions) across tier membership, card management, rewards upgrade flows, and supporting infrastructure. No bugs, security issues, debug leftovers, or development artifacts were found in the introduced code. 🤖 Generated with Claude Code |
The app half of rewards v3. Points no longer buy a tier, so the upgrade path stops being a nudge toward earning more and becomes a purchase: a price, a term, a choice of how to pay, and a signature. That is why it is a screen and not the old 470px sheet. Committing FUSE for a year is not reversible by asking nicely, and the term is the part a user is most likely to skim — so the review step is its own screen where the duration is one of four lines rather than one row among a price, a balance and a toggle. The route switch renders the routes that exist rather than a fixed pair: Prime is sold both ways and shows two segments, Ultra is FUSE-only and shows one. A greyed-out "Cash" segment would advertise a way to buy Ultra that does not exist. Which CTA the user gets — "Top up" or "Review upgrade" — comes from one predicate, so the button and the screen behind it cannot disagree. Two precision bugs were caught by the tests rather than by users. `amount * 1e18` is past Number.MAX_SAFE_INTEGER above ~9 FUSE and rounds DOWN, which is the one direction the share maths must never go: the contract floors `shares * rate`, so a share too few leaves the position a wei under the threshold and buys nothing. Both conversions now go through the decimal string and round up. Dates are spelled out instead of localised for a related reason — recent ICU writes September as "Sept" and Hermes, JSC and V8 do not ship the same ICU, so the same membership would be dated differently on iOS, Android and web. Locked FUSE gets its own tile on Earn rather than joining the portfolio total. It cannot be withdrawn until its date, and a headline the withdraw flow then refuses is worse than a second line. It says it is still earning, because it is: what is locked is the vault share, not the asset. Also fixes a defect the master merge introduced: master narrowed `getBuyFuseTierTargets` to price its rungs from the backend's own block, while qa's `skipTheLine` still called it with one argument — so the preview fallback, which exists precisely because that block is absent, silently produced no rungs and hid the upgrade card. The second argument is optional, so it compiled; two tests caught it. The benefit glyphs move out of TierBenefitsGrid into a shared module. They are one set in the design — the same cashback diamond and yield-boost bolt at 50px on the grid and 33px on a tier card — and a second copy is how the two drift. 1268 tests pass, 28 of them new. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XGzkz4QFpEZVg6GpNH311x
A deep link naming a tier the user already holds priced the upgrade at nothing and handed the review screen zero FUSE to lock, where the hook refused it with "Enter an amount to lock" — a dead end reached by an ordinary link. It now says they already hold it. `toFixed` gives up and returns exponential notation at 1e21, which `BigInt` cannot parse, so a large amount crashed the share conversion rather than being refused. No tier is priced anywhere near that; this is a guard, not a case to support, and the test says so. The route list was rebuilt every render, so the effect that settles on a route re-ran every render. It only ever set state when the selection was genuinely stale, so nothing looped — it was simply work for nothing. Also corrects a comment that claimed a multicall the hook does not make. Five parallel reads are the right shape here; describing them as something else is how the next person "fixes" it into something slower. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XGzkz4QFpEZVg6GpNH311x
856ace0 to
6392b17
Compare
MusabShakeel576
changed the title
2 participants
Summary
Rewards v3 changes how a tier is held. Points no longer upgrade anyone; a tier is bought, by one of two routes:
Ultra is deliberately lock-only: its annual price ships as
0, and the screens read that as "not purchasable for cash" rather than showing a $0 offer.This branch was rebased onto
qaso the diff is only the v3 work: 2 commits, 30 files.Screens
/rewards/upgrade(UpgradeTierScreen) — hero card for the tier being bought, a route switch showing only the routes actually available, what the user holds against what the tier costs, and the benefits it unlocks./rewards/upgrade-review(UpgradeTierReviewScreen) — the confirmation step, with the amount, the term, and what is signed.TierMembershipSheet— the membership itself: how it is held, when it renews or unlocks, and cancel/resume for a subscription.LockedFuseTileon Earn — locked FUSE shown as its own position, apart from spendable balance, with its APY and unlock date.Contracts and hooks
hooks/useTierMembership.ts— membership state, chain state, and the four writes (lock, subscribe, cancel, resume).lib/abis/SolidTierLock.ts,lib/abis/SolidSubscriptionModule.ts— the two contracts from Add Solid rewards v3 tier upgrade contracts boring-vault#8.lib/tierUpgrade.ts— the arithmetic and formatting, kept out of the components so it can be tested directly.Notable details
toFixed(18)string rather thanamount * 1e18, which silently rounds down pastNumber.MAX_SAFE_INTEGER; the same conversion returns0nrather than a garbage bigint for values that reach exponential notation.Number.toLocaleDateString: recent ICU says "Sept" where the design says "Sep", and Hermes, JSC and V8 do not ship the same ICU.skipTheLine.tspasses an explicit fallback rung list togetBuyFuseTierTargets, which master narrowed to require one.Testing
lib/__tests__/tierUpgrade.test.ts— 29 tests over the conversions, affordability, route availability and formatting. Full suite: 105 files, 1278 tests, passing.tsc --noEmitreports the same 9 pre-existing errors asqadoes on its own; eslint is clean over the changed files.🤖 Generated with Claude Code
https://claude.ai/code/session_01XGzkz4QFpEZVg6GpNH311x