Conversation
Contributor
|
PR Preview build is ready: https://github.com/stellar/freighter/releases/tag/untagged-dc3fe20d58a5d621dbc3 |
CassioMG
force-pushed
the
earn-positions
branch
from
August 26, 2026 18:20
7c7769e to
0e40167
Compare
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e Blockaid rescan Whole-branch review findings C1, I3, I4, I7 -- all seams between per-task work that no single task's review could see: - C1: the view computed AccountPositions' hasError from resolvedData.hasPositionsError alone, but a failed account fetch discards resolvedData entirely, so a fetch failure rendered as "No positions yet" instead of the error state. - I3: getBlendPools and getBlendEarnOptions were awaited ahead of the mainnet scanned-balances refetch, delaying Blockaid's isSuspicious/isMalicious labels on every earn-supported Home load. Both now start unawaited alongside positionsRequest (or as soon as their own gating condition is known) and land via the same mutate-then-spread-dispatch pattern collectibles and positions use, so neither sits in front of the rescan. - I4: the 30s positions refresh kept the last good positions but also flagged hasPositionsError, so a single flaky tick flashed rows to an error banner and back. Now only surfaced when there is no prior data left to show. - I7: positionsRequest was unawaited across several `await`s before its own try/catch, so a rejection in that window fired Sentry's unhandledrejection handler on top of the deliberate captureException. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…position Whole-branch review findings C2, I1, I2: - C2: toDepositRow subtracted lifetime interestEarned from the current totalTokens to derive principal. A fully-exited position (total_tokens back to 0 with interest history intact) or any over-withdrawal drives that subtraction negative -- three figures on "Your position" that no longer reconcile. When the derivation goes out of range the row's principal reading no longer applies, so it now reports tokens: "0" and usd: null instead of a negative amount. - I2: the focused-asset fallback (`supply.find(...) || supply[0]`) couldn't tell "no asset was named" from "named an asset the position doesn't hold" -- and EarnAmount always names one now, so every mismatch fell back to a different asset's figures under this asset's headers. A genuine mismatch now resolves to an empty scope, and PoolDetailsSheet computes hasPosition from the same resolution rule (hasResolvableSupply) so it never shows a tab with nothing in it. - I1: the row formatted its USD figures with formatAmount/roundUsdValue while the sheet used formatCompactUsd, which goes compact at $1,000 -- a $1,500 position read differently in each place. Both now go through one new formatAccountUsd, added next to formatCompactUsd with the scope each is for spelled out in its docstring. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Whole-branch review findings I5, T7-c: - I5: the supply-APY-plus-emissions formula -- the branch's one deliberate exception to null-is-not-zero -- was copy-pasted in positionRows.ts and positionSummary.ts (byte-identical), inlined again in earnProjection.ts, and pre-existed a third time in useGetEarnTokensData.tsx. Four copies means a future change to the null-emissions behaviour silently misses three of them. Extracted to formatPoolStats.ts, next to formatRate, and imported at all four call sites; useGetEarnTokensData.tsx (a colleague's in-flight PR) only had its local definition swapped for the import, nothing else touched. - T7-c: earnProjection.test.ts's "returns null when no pool has a priced rate" test passed an empty pools array, so it only exercised the reduce's initial value, never the pool.supplyApy === null skip branch it claimed to cover. Renamed to describe what it actually tests, and added a genuine null-rate case (mixed with a priced pool, so the skip branch is actually exercised). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ed balances
The mainnet rescan block built a throwaway scannedPayload local
(balances + isScanAppended: true) and dispatched that, without ever
writing those fields back onto payload itself. That was safe only
because nothing dispatched from payload again afterward -- but the I3
fix moved the earnOptions landing to run after this block precisely so
it would stop blocking the rescan, and that landing dispatches
{ ...payload }. Since the reducer fully replaces state per dispatch
(helpers/request.ts), that silently reverted balances and
isScanAppended right back to their pre-scan values.
Affects every mainnet account with zero Blend positions (no error,
empty list) -- the only population for which earnOptionsRequest is
non-null. AccountAssets renders Blockaid's isSuspicious/isMalicious
flags from these balances, so a malicious-token warning would appear
when the scan lands and then disappear moments later, until the next
30s tick restores it.
Fixed by mutating balances/isScanAppended onto payload itself before
dispatching, the same pattern collectibles/positions/pools/earnOptions
already use elsewhere in this function -- so any dispatch added after
this block, now or later, carries the scanned data forward instead of
reverting it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follows the convention a22cfd8 established across the Earn flow. The constant is the same "--" string, so nothing renders differently; this keeps the placeholder in one place so a future change to it cannot miss a call site. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
roundUsdValue floors by multiplying the value by 100 as a JS float, so binary float error turns an exact 1.15 into 114.99999999999999 and floors it to 1.14 -- 6.56% of exact-cent values in the $0.01-$10,000.00 range render a penny low. formatAccountUsd is now the single formatter for every position figure on the Positions tab, so this reached the pool card total, My position's total and gain, and the sheet's balance, deposits and earnings. Truncate via BigNumber#decimalPlaces(2, ROUND_DOWN) instead, which truncates the decimal string exactly rather than through float multiplication. roundUsdValue itself is untouched -- it is used app-wide and out of scope for this fix.
totalUsd came from position.netUsd, documented as "supplied minus borrowed", while the list beneath the header sums supplied assets only. An account supplying $1,000 USDC and borrowing $400 in the same pool (creatable in Blend's own UI, though Freighter cannot create one) would show a $600.00 header above a list reading $1,000.00 -- two figures shown together that don't reconcile. A borrow-heavy position could even render a negative header over an empty-looking list. Take the total from suppliedUsd, falling back to netUsd only when suppliedUsd itself is unpriced, so the header is the same quantity as the rows it heads. Every existing fixture set borrowedUsd: 0, which is why this went uncaught -- add a borrow-present case.
Wrapper passed no remoteConfig preloaded state, so this suite inherited the real initialState default for earn_deposit -- currently true, but about to flip to false upstream. Every sibling flag-dependent test already sets the flag explicitly; this was the one outlier that would have broken silently on the next rebase.
The two role="tab" buttons sat in a container with no role="tablist" to place them in, so a screen reader announced "tab" with no set context.
pool was a required prop the component never read -- its destructure deliberately omitted it to satisfy noUnusedLocals, per the comment this removes. Remove the prop, its type, and the call site argument.
Radix's Sheet here is controlled by open={Boolean(selectedPosition)}.
When a 30-second refresh tick drops the selected pool from positions,
the sheet closes declaratively -- but Radix's onOpenChange only fires
on user interaction, so selectedPoolId and sheetMode were never
cleared. If the pool reappeared on a later tick, selectedPosition went
truthy again and the sheet reopened by itself, nested modal restored,
with no tap from the account.
Clear both in an effect the moment the selection stops resolving to a
live position. Also pins the existing "opens on Your position" sheet
test to the tapped asset's own balance, guarding the class of bug this
branch already fixed once for the pool sheet.
About 20 comments across this branch's shipped files pointed at this run's internal planning -- "Task N", "(I1)", "(I2)", "(I3)", "(I5)", "(R2)", "(R3)", "(C1)", "(C2)", "(T7-c)". Those artifacts are git-ignored and mean nothing to anyone reading the repo later, including whoever this branch merges into. Rewrote each to say what it means without the pointer, keeping the substance. Left untouched every hit that predates this branch (RFC #2883's (D5)/(D6)/(D8) markers in popup/metrics, and an unrelated CAP-71 "(V2)" test name that isn't a plan reference at all).
`loginToTestAccount` navigates before registering stubs, so the popup's one-shot `fetchFeatureFlags` had already fired and rejected by the time stubBlendEarn registered its vardata route — a route registered after the fact never intercepts a request already sent. The stub was therefore never effective; the flag's old `true` default was silently doing the work, and upstream flipping it to `false` exposed that. Reload once the routes are in place so the popup remounts and refetches with the stub live, mirroring what stubExperimentVariants already does for the maintenance flags. Scoped to the Earn stubs rather than changing the shared login helper, which every suite depends on. This also unblocks earnDeposit.test.ts, which fails 7/7 on feat/earn-deposit alone for the same reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CassioMG
force-pushed
the
earn-positions
branch
from
August 27, 2026 00:42
0e40167 to
6249f9d
Compare
Figma is reachable again (node 9808-30558 for the populated pool card, 9808-30015 for the empty state). Fill in the PositionsPoolCard row's background/radius/padding/type-scale, correct the empty state's type scale and the "you could earn up to" card (it turns out to be small inline text, not a hero figure), and settle the green question raised in both this file and PoolDetailsSheet's: node 9808-30558's Success/Text/Primary (#4cc38a) is green-11 in this dark theme, so PositionRow__apy moves from green-09 to green-11 and both "unresolved counter-example" comments are replaced with that citation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Figma node 9956-72252 is reachable again. Fill in the Blend chip and "About pool" pill (background, border, radius, padding, and the PoolIcon size they each need, scoped locally rather than resized globally), correct the totals block's type scale (the total figure is 24px regular, not medium, with the app's usual -4% tracking), color the gain green per node 9808-30558's resolved Success/Text/Primary, and give each supplied-asset row the card treatment the node draws around it via a BalanceRow-scoped override rather than touching PositionRow or BalanceRow themselves. The node nests the pool name under the Blend chip rather than as a full-width line below the whole identity row the way this component's markup renders it; fixing that needs a JSX restructure that's out of scope for a pixels-only pass, so it's left as a documented gap -- see the task report. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Figma node 9965-62123 ("Your position" tab) is reachable again;
node 9808-34747 (Overview, maintained separately) renders the same
header/tabs chrome and corroborates the values below, so this fixes
that chrome too without touching anything Overview-specific.
Fix the header title (14px medium, not 16px semibold), the "by Blend"
tagline (12px regular, previously wrongly inheriting the title's
color along with its own wrong size), the close button's background,
and the PoolIcon size. Redraw the tab strip as the much-less-rounded
segmented control the node actually shows (6px radius, not a full
pill) with the correct brand colors on the active tab. Strip the card
background off the balance/APY header row entirely -- the node draws
it as plain text -- and correct both figures' type scale. Add the
missing divider between the earnings row and the stat rows below it,
scoped to this file rather than the shared StatRow component. Settle
the same green question AccountPositions/styles.scss's commit does:
node 9808-30558's Success/Text/Primary is green-11, so both existing
green-11 uses here are confirmed correct and their stale "unresolved
counter-example" comment is replaced with that citation.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
TL;DR
Adds a Positions tab to Home, between Tokens and Collectibles, listing the Blend pools the account has supplied to. Tapping a pool opens a My position sheet with the pool's balance, its interest earned, and the tokens supplied to it; tapping one of those tokens opens the pool sheet on a new Your position tab, and About pool opens the same sheet showing market data alone. Accounts with no positions get an empty state projecting what they could earn.
Every route into the deposit flow — the floating + Deposit pill, Start Earning, and the sheet's Deposit button — is gated behind the same
earn_depositflag as Home's Earn tile, so they all appear and disappear together. With the flag off the tab, the pool cards and both sheet tabs still work: the flag gates depositing, not viewing.Positions load alongside balances and collectibles on Home, refresh on the same 30-second cadence, and are cached per network and account.
Before this leaves draft
git revert 726cc9ea— temporary debug logging of raw Blend payloads, deliberately kept so the responses could be inspected while building. (The SHA has moved twice through rebases; this is the current one.)blend-positions-wip.mov
Implementation details (for agents)
Data. A
getBlendPositionsclient forPOST /api/v1/accounts/positionsplus camelCase domain types. Positions cache in the existingcacheslice keyed[network][publicKey], read through auseGetPositionshook modelled onuseGetCollectibles— with one deliberate divergence: it re-throws rather than swallowing failures, because the tab must distinguish "could not load" from "you have none". A position is money.Orchestration.
useGetAccountDatafetches positions unawaited alongside collectibles and joins the existing 30s balances interval.useGetPositions.fetchDatatakes a per-calluseCacheoverride so the initial load uses the 3-minute cache while the 30s refresh and account switches bypass it — without that lever the "30-second refresh" would have genuinely hit the network on roughly one tick in six.Presentation.
positionRows.tsis the single place the pool-shaped payload is flattened into token rows.poolPositionSummary.tsowns the pool-level figures;positionSummary.tsowns the supply-level ones. Two functions with distinct jobs rather than one with a mode flag.Sheet.
PoolDetailsSheetgainedposition,focusedAssetId,onDeposit,defaultTab,onTabChangeandoverviewOnly— all optional. GatinghasPositionon!overviewOnlyhides the tab strip and forces the Overview pane with no third code path.Prefill. The deposit prefill travels as a query flag rather than pre-dispatched state, because the Earn view runs
resetSubmission()on mount and that reducer returns the whole slice toinitialState.Number handling. A nullable USD/APY value is
nullwhen unavailable and0when genuinely zero; nulls renderNO_FIAT_VALUEand are never coalesced. On-chain amounts stay full-precision integer strings scaled with BigNumber.Two correctness fixes worth calling out:
formatAccountUsdused to truncate viaMath.floor(parseFloat(v) * 100) / 100. Binary float makes1.15 * 100 = 114.99999999999999, so 6.56% of exact-cent values rendered one cent low — $1.15 showed as $1.14 — across every position figure. Now usesBigNumber.ROUND_DOWN; verified 0 of 1,000,000 wrong. The app-wideroundUsdValuestill behaves this way and is used by Send, Swap and the balance list — deliberately left alone here as a much larger blast radius, but it wants its own look.The pool total now comes from
suppliedUsd, notnetUsd.netUsdis supplied minus borrowed, while the interest total and the list beneath the header both come from supply rows — so with a borrow present the header read $600 above a list totalling $1,000. Borrows are not surfaced anywhere in this UI.Analytics.
source(home|position_row|positions_empty|positions_pill) ridesducks/earn.tsand is stamped on the deposit outcomes; the pool sheet's own events get a separatePoolDetailsSourceunion, because About pool opens the sheet but can never start a deposit. Note theposition_rowfunnel is genuinely shorter — it skips the token picker, soearn.token_selectednever fires for it. Documented rather than papered over with a synthesised event.earn.pool_details_openedandearn.pool_details_tab_selectedpreviously emittedhome | position_row | positions_empty; they now emitearn_amount | position_row | about_pool. Onlyposition_rowis common to both.Verification. 246 suites / 1963 tests, 0 failures ·
earnPositions.test.ts4/4 ·earnDeposit.test.ts7/7 ·build:extensionclean.The last commit before the design pass fixes an e2e bug that predates this branch:
loginToTestAccountnavigates before registering stubs, so the popup's feature-flag fetch had already resolved to the default and theearn_depositvardata stub never applied. It was never effective — the oldtruedefault was covering for it. This also unblocksearnDeposit.test.ts, which fails 7/7 onfeat/earn-depositalone.Follow-ups / out of scope:
roundUsdValue's float truncation, app-wide (see above).getBlendSuppliedTokens—EarnAmountnow reads its pool position from the cache but still issues a second positions request.AssetIcon.fetchBalances), andclearIntervaldoesn't cancel an in-flight tick. Both pre-existing on the balances path; positions now ride the same timer.feat/earn-deposit's own node9808-34747, Overview's "Net APY" reads green-10 where ours reads green-11 — left untouched as its owner's call.Note for @aristides: the design pass changed the sheet's shared header (pool name
md/semi-bold →sm/medium, "by Blend"sm→xs), which alters your deposit-flow sheet too. Your own node corroborates those values, so it corrects the header rather than diverging from it — but it will show up in your diff.🤖 Generated with Claude Code