Skip to content

feat(balances): migrate account balances to the v2 indexer endpoint#935

Open
aristidesstaffieri wants to merge 7 commits into
mainfrom
feat/balances-v2-migration
Open

feat(balances): migrate account balances to the v2 indexer endpoint#935
aristidesstaffieri wants to merge 7 commits into
mainfrom
feat/balances-v2-migration

Conversation

@aristidesstaffieri

@aristidesstaffieri aristidesstaffieri commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

What

Migrates account balance fetching to the freighter-backend-v2 indexer endpoint (POST /accounts/balances), behind a new use_balances_v2 remote-config flag. This ports the browser extension's feat/balances-v2-migration work, adapted to mobile's zustand/axios architecture.

  • fetchBalancesV2 POSTs to freighter-backend-v2 /accounts/balances and is used on PUBLIC/TESTNET when the flag is on; Futurenet stays on v1.
  • mapAccountBalancesV2 normalizes the snake_case v2 wire shape into the app's BalanceMap:
    • native → "XLM"
    • classic/SAC → "CODE:ISSUER" (SAC kept classic-shaped so display doesn't double-scale the pre-formatted amount)
    • SEP-41 → Soroban shape with raw i128 amount + decimals
    • liquidity pool → "<poolId>:lp"
  • addBlockaidScanResults stamps benign defaults and merges mainnet bulk scan verdicts client-side via scanBulkTokens, matching the payload the v1 backend builds server-side. Scan failures never break balances.
  • The v2 path needs no contract_ids param: the wallet-backend returns all indexed holdings (trustlines, SACs, SEP-41s, pool shares) automatically.
  • The flag defaults to off (v1) since the wallet-backend indexer isn't deployed yet; Amplitude flips it on to roll out v2 without a release, and back off to roll back.

Why

The v1 account-balances endpoint is being replaced by the wallet-backend indexer, which serves balances from indexed data instead of querying Horizon/RPC per request. Gating the switch behind remote config lets us roll it out (and roll it back) without shipping a new app version.

Release blockers

This PR can merge, but the v2 path must not be enabled in production until:

  1. The use_balances_v2 feature flag is turned on in Amplitude for the rollout (the in-app default is off, so v1 stays in use until then).
  2. A wallet backend is deployed for every network — the indexer currently isn't deployed for all networks the app supports; as of 2026-07-15, POST /accounts/balances returns 500 on both prod and staging for all networks.
  3. Backend v2 accepts Futurenet requests — until then, Futurenet is hard-coded to the v1 path regardless of the flag.

Known limitations

  • Futurenet always uses the v1 endpoint (see release blockers above).
  • The v2 response carries no Blockaid data yet, so token scan results are merged client-side; this is an extra scanBulkTokens round trip on mainnet.

Checklist

PR structure

  • This PR does not mix refactoring changes with feature changes (break it down into smaller PRs if not).
  • This PR has reasonably narrow scope (break it down into smaller PRs if not).
  • This PR includes relevant before and after screenshots/videos highlighting these changes.
  • I took the time to review my own PR.

Testing

  • These changes have been tested and confirmed to work as intended on Android.
  • These changes have been tested and confirmed to work as intended on iOS.
  • These changes have been tested and confirmed to work as intended on small iOS screens.
  • These changes have been tested and confirmed to work as intended on small Android screens.
  • I have tried to break these changes while extensively testing them.
  • This PR adds tests for the new functionality or fixes.

Release

  • This is not a breaking change.
  • This PR updates existing JSDocs when applicable.
  • This PR adds JSDocs to new functionalities.
  • I've checked with the product team if we should add metrics to these changes.
  • I've shared relevant before and after screenshots/videos highlighting these changes with the design team and they've approved the changes.

  Adds a v2 balances path behind the new use_balances_v2 remote-config flag
  (defaults to on; Amplitude can flip it off to roll back to v1 without a
  release). Ports the browser extension's feat/balances-v2-migration work,
  adapted to mobile's zustand/axios architecture.

  - fetchBalancesV2 POSTs to freighter-backend-v2 /accounts/balances and
    routes there on PUBLIC/TESTNET when the flag is on; Futurenet stays on v1
  - mapAccountBalancesV2 normalizes the snake_case v2 wire shape into the
    app's BalanceMap: native -> "XLM", classic/SAC -> "CODE:ISSUER" (SAC kept
    classic-shaped so display doesn't double-scale the pre-formatted amount),
    SEP-41 -> Soroban shape with raw i128 + decimals, LP -> "<poolId>:lp"
  - addBlockaidScanResults stamps benign defaults and merges mainnet bulk
    scan verdicts client-side via scanBulkTokens, matching the v1 backend's
    server-side payload; scan failures never break balances
  - v2 needs no contract_ids: the wallet-backend returns all indexed
    holdings (trustlines, SACs, SEP-41s, pool shares) automatically
@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

iOS Simulator preview build is ready: https://github.com/stellar/freighter-mobile/releases/tag/untagged-0aa981c5ed3b43c66440 (SDF collaborators only — install instructions in the release description)

aristidesstaffieri and others added 2 commits July 15, 2026 10:58
…ployed

  The v2 /accounts/balances endpoint currently returns 500 on prod and
  staging for every network, so defaulting the flag to on broke balance
  fetching (and the send/swap e2e suites) wherever remote config falls
  back to code defaults. Default to the v1 path and let Amplitude flip
  the flag on once the wallet-backend indexer is live.
@aristidesstaffieri
aristidesstaffieri requested a review from a team July 15, 2026 17:40
@aristidesstaffieri
aristidesstaffieri marked this pull request as ready for review July 15, 2026 17:40
Copilot AI review requested due to automatic review settings July 15, 2026 17:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Migrates balance fetching to the indexed v2 backend behind a remotely controlled rollout flag.

Changes:

  • Adds v2 balance routing and response normalization.
  • Adds client-side Blockaid scanning for v2 balances.
  • Adds remote-config integration and unit coverage.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/services/backend.ts Routes supported networks to v2.
src/helpers/mapAccountBalancesV2.ts Maps v2 balance wire types.
src/helpers/addBlockaidScanResults.ts Adds Blockaid verdicts.
src/ducks/remoteConfig.ts Defines the rollout flag.
src/ducks/balances.ts Passes the runtime flag.
__tests__/services/backend.test.ts Tests endpoint routing.
__tests__/helpers/mapAccountBalancesV2.test.ts Tests balance mapping.
__tests__/helpers/addBlockaidScanResults.test.ts Tests scan merging.
__tests__/ducks/remoteConfig.test.ts Tests flag behavior.
__tests__/ducks/balances.test.ts Updates store expectations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/services/backend.ts
Comment thread src/helpers/addBlockaidScanResults.ts Outdated
Comment thread src/helpers/mapAccountBalancesV2.ts Outdated
  The v2 backend contract returns one entry per requested address —
  unfunded accounts arrive as a matching entry with is_funded: false,
  never as an omission. Mapping an absent entry to "unfunded" masked
  malformed/partial responses and could replace a funded user's balances
  with the unfunded UI. Throw instead, so the store's error path keeps
  the existing balances, and tighten mapAccountBalancesV2 to require an
  account entry.
@piyalbasu

Copy link
Copy Markdown
Contributor

Pre-merge review — balances v2 migration

Reviewed against a whole-system checklist (invariants, flag state machine, replaced-behavior parity, error paths, test fidelity) and cross-checked against the sibling extension PR (freighter#2906). This is a faithful, well-tested port — and on two design calls it's the safer of the two platforms (flag defaults OFF, and missing-account is rejected rather than rendered as unfunded — nice). The blockers below are mostly shared with the extension (both clients copied the same wire assumption, so checking the two clients against each other looks clean — the bug is against the backend).

Verdict: safe to merge; must not enable the flag until #1 and #2 are resolved.


1. SEP-41 token balances are double-scaled by 1e7 — CRITICAL (shared with extension)

CRITICAL — merge blocker for enabling the flag. Every SEP-41 token balance would render 10,000,000× too small, and spendable/send math is wrong the same way.

TL;DR: The mapper treats the SEP-41 balance as a raw unscaled integer and re-scales it by decimals at display time. But wallet-backend already returns that amount as a human-readable decimal (it divides by 1e7 before serializing). So it gets scaled twice — a balance of 500 shows as 0.00005. The test fixtures encode the same wrong assumption, so they stay green.

Steps to reproduce:

  1. Hold any SEP-41 token on an account.
  2. Enable use_balances_v2 and load balances on pubnet/testnet.
  3. Observe the displayed balance is off by a factor of 1e7 vs. the v1 path.

Detailed explanation (for agents)

Root cause: wallet-backend's GraphQL resolver serializes the SEP-41 balance with amount.String128, which divides the i128 by One = 10_000_000 and returns a 7-decimal string (go-stellar-sdk .../amount/main.go String128rat.Quo(rat, bigOne); rat.FloatString(7)). See wallet-backend/internal/serve/graphql/resolvers/account_balances_utils.go parseSEP41BalancebalanceStr := amount.String128(i128Parts). freighter-backend-v2 passes it straight through (internal/services/account_balances_mapping.go, case *wbtypes.SEP41Balance; its "raw i128" comment describes spendability, not the serialized value).

The client mapper treats it as raw + decimals:

Also note amount.String128 is decimals-blind (always ÷1e7), so for a token whose true precision ≠ 7 the value is wrong even before the client touches it — a backend concern worth raising.

Caveat / confirm before acting: freighter-backend-v2's own comment says "raw i128", contradicting its resolver. The static evidence (String128 ÷1e7) is unambiguous, but please capture one real v2 response for a SEP-41 holder and add it as a recorded golden fixture before flipping the flag — it settles the contradiction and would have caught this.

Suggested fixes:

  1. If the wire value is confirmed pre-formatted: map SEP-41 like classic (pass the decimal string through, drop the re-scale).
  2. Root cause (preferred): make the contract unambiguous (backend emits a genuinely raw value, or client trusts the pre-formatted one) and add the recorded-response contract test on both platforms.

2. Flag ON against a failing v2 = no balances, no fallback — HIGH (shared with extension)

HIGH — not a merge blocker (default-OFF protects you today), but a rollout landmine.

TL;DR: Defaulting the flag OFF is the right call and keeps users safe on v1 until the backend is ready — good. But there's no fallback: the moment the flag is flipped ON while the endpoint is still returning 500 (per the PR body, it 500s on prod + staging today), every pubnet/testnet user gets an error state with no balances, recoverable only by flipping the flag back and waiting for the next config poll / restart.


Detailed explanation (for agents)

Root cause: fetchBalancesV2 rejection propagates straight to the duck catch (src/ducks/balances.ts L359), which sets error and leaves stale/empty balances; the working v1 path is never attempted. Default-OFF verified end-to-end: remoteConfig.ts L78/L103 + call-time read at balances.ts L318.

Suggested fix: fall back to v1 on v2 5xx/network error (response shapes are identical by design, so it's a small change), or stage the rollout on a small segment. Decide this symmetrically with the extension — a fallback on only one platform is itself a divergence. If fail-closed is deliberate, note it in the PR body.


3. User-added custom tokens can silently disappear on the v2 path — HIGH (shared with extension)

HIGH — not a merge blocker; needs an explicit decision before rollout.

TL;DR: The comment says indexed contract tokens "come back automatically", but v2 only returns SEP-41 balances the indexer has already ingested for that holder. A token the user explicitly added (v1 queried it directly) vanishes from the list, with no error, if the indexer hasn't ingested that contract/holder pair yet.


Detailed explanation (for agents)

Root cause: the v2 path sends no contract_ids (backend.ts L256-257); custom tokens persisted in CUSTOM_TOKEN_LIST / resolved by retrieveCustomTokens are no longer force-queried. wallet-backend only returns a SEP-41 balance for a contract it has a row for.

Suggested fix: verify indexer coverage for the add-token flow before rollout, or diff customTokensContractsIds against the v2 result and log/alert on gaps (cheap observability for exactly this regression).


4. Client-side bulk scan runs uncached on every 30s poll and blocks balance render — MEDIUM

MEDIUM — mobile-specific; the right caching chokepoint already exists in the repo.

TL;DR: addBlockaidScanResults calls scanBulkTokens raw, before balances are set — so on mainnet with the flag on, every 30s poll adds a second sequential HTTP round-trip (delaying balance display) and fires a Blockaid analytics event every 30s per active user. The repo already has a disk-backed, TTL'd cache (ensureTokenScans) for the same API that this bypasses.


Detailed explanation (for agents)

Root cause: addBlockaidScanResults.ts L58 scanBulkTokens(...) is awaited inside fetchBalancesV2 before the duck's set({ balances }). ducks/blockaidTokenScans.ts ensureTokenScans is a partial-hit-aware TTL cache over the same endpoint.

Suggested fixes:

  1. Route the scan through ensureTokenScans (kills the event spam and most requests).
  2. And/or stamp benign defaults, set balances, then merge scan verdicts asynchronously (as the store already does for prices) so raw balances never wait on the scan.

5. Freshly-funded account can render as "unfunded" under indexer lag — MEDIUM

MEDIUM — not client-fixable; flag it as a known/accepted rollout risk.

TL;DR: v2 is_funded comes from the indexer, not Horizon. The friendbot flow funds then immediately refetches; on v2 the refetch may still show unfunded (zero balances) until ingestion catches up — same window after a first deposit on mainnet. State the indexer's lag SLO when enabling the flag.


6. The flag-ON duck wiring and the 500 path are untested — MEDIUM

MEDIUM — test gap on the one new load-bearing line.

TL;DR: Tests only ever assert useV2: false. Nothing sets the flag ON and asserts fetchBalances receives useV2: true — the exact line this PR exists to add. There's also no explicit v2-rejection (500) test through fetchBalances, notable given the endpoint currently 500s.


Detailed explanation (for agents)

The load-bearing line with no red-on-break coverage: balances.ts L318. Add: (a) a test with useRemoteConfigStore use_balances_v2: true asserting useV2: true reaches fetchBalances; (b) a v2-500 test asserting it rejects and locks in whatever #2's fallback decision is; (c) the unfunded path through fetchBalances (currently covered only in the mapper test).


7. SAC balances would be 1e7-inflated — MEDIUM (latent; shared with extension)

MEDIUM — latent, not currently reachable. Mirror image of #1.

TL;DR: SAC amounts arrive raw (DB passthrough, not ÷1e7), but the mapper treats them as pre-formatted decimals, so they'd display 1e7× too large. Unreachable today because SAC balances key on a C… holder and the v2 handler only accepts G… — but the mapper and tests pin the wrong behavior for when that changes.


Detailed explanation (for agents)

Root cause: wallet-backend/internal/data/sac_balances.go stores Balance as a raw i128 string and buildSACBalanceFromDB passes it through unformatted (unlike native/trustline). The client mapSac L182-196 treats it as pre-formatted classic. Resolve alongside #1 with a live fixture, or captureException on unexpected SAC arrival.


LOW / NIT — additional findings
  • Stale-response race widens under v2 — no abort/sequence guard in fetchAccountBalances; v2's second sequential request (the bulk scan, Add Typography and style shorthands #4) lengthens the last-write-wins window on account/network switch. scanBulkTokens accepts an AbortSignal this helper never passes. Follow-up ticket rather than a blocker.
  • Comment inaccuracyaddBlockaidScanResults.ts L35 says "v1 leaves them [LP entries] unstamped too" — false; v1's addScannedStatus stamps a benign blockaidData on every entry including LP. Skipping LP in the scan request is fine (an improvement); just fix the comment so the parity claim is true.
  • Minimal benign stamp — the helper stamps only { result_type: "Benign" } vs v1's full default object (malicious_score, attack_types, chain, …). Only result_type is consumed today, so fine — one sentence in the doc noting the deliberate slimming would prevent a future surprise.
  • Native available semantics change — v2 available = balance − minimum_balance − selling_liabilities vs v1's total − sellingLiabilities. Mobile recomputes native spendable from total in calculateSpendableAmount, so no consumer breaks (this is exactly why the extension has a max-send bug here and mobile doesn't) — but it is an observable BalanceMap value difference between paths; worth one line in the mapper doc.
  • mapClassic edge — with backend-optional code/issuer omitted, produces key ":" with credit_alphanum4; a if (!b.code) return null skip would be cheaper than a malformed entry if it ever occurs.
  • DRY — the { data: T } v2 envelope is inlined a 3rd time (fetchTokenPrices/fetchCollectibles/fetchBalancesV2) — a shared V2Envelope<T> would be the DRY move (optional).

Verified solid (coverage)
  • Secrets: v2 body carries only public keys ({ addresses: [publicKey] }); apiFactory redacts Authorization at any depth; error logging is status/body only; no key/mnemonic/seed material in requests, logs, or analytics.
  • Endpoint path: freighterBackendV2 base URL already carries /api/v1, so /accounts/balances composes to the same absolute route the extension uses — MATCH.
  • Flag default + read path: OFF in both dev and prod initial states, store not persisted, variant-gated updates, call-time getState() read; all 5 balance entry points route through the single fetchAccountBalances chokepoint. Unloaded/failed remote config ⇒ v1.
  • Missing-account handling is the CORRECT contractbackend.ts L287 rejects a 200 that omits the requested account as malformed, rather than rendering it as unfunded. (The extension does the opposite and caches it — flagged on that PR.)
  • v1 path untouched (v1 branch byte-identical to main; useV2 required param); Futurenet pinned to v1; routing matches the backend's PUBLIC/TESTNET-only validation.
  • Key formats (XLM, CODE:ISSUER, SYMBOL:CONTRACT, <poolId>:lp) match v1 conventions; dropped fields have no BalanceMap consumers.
  • Tests exercise real mapper/blockaid code (only HTTP client + scanBulkTokens mocked); no node-crypto misuse in tests (RN uses react-native-crypto); no new sync crypto on the JS thread.

🤖 Generated with Claude Code (Fable) — cross-platform pre-merge review, verified against wallet-backend + freighter-backend-v2 source.

@aristidesstaffieri

Copy link
Copy Markdown
Contributor Author
  1. SEP-41 token balances are double-scaled by 1e7 — CRITICAL (shared with extension)
    CRITICAL — merge blocker for enabling the flag. Every SEP-41 token balance would render 10,000,000× too small, and spendable/send math is wrong the same way.

This is incorrect, wallet backend already scales the value during ingestion.

  1. Live ingestion formats: internal/indexer/processors/sac_balances.go:144 — amount.String128(entries[0].Val.MustI128()) extracts the i128 from the SAC contract-data entry and formats it.
  2. String128 divides by 1e7: SDK go-stellar-sdk/amount/main.go:134-143 — rat.Quo(rat, bigOne) with bigOne = 10000000 (line 25/29), returns FloatString(7). Doc comment: "converts a signed 128-bit integer into a
  3. string, boldly assuming 7-decimal precision."
  4. Checkpoint/backfill path formats identically: internal/services/checkpoint.go:751 — same amount.String128.
  5. DB stores the formatted decimal (numeric column); resolver buildSACBalanceFromDB (account_balances_utils.go:60-71) passes it through — passthrough of a formatted value.
  6. Not a recent change: String128 has been there since PR 470 ("Remove redis dependency and store balances in postgres"), i.e. since the postgres-backed balances existed.
  7. SEP-41 raw is deliberate, and explicitly contrasted: internal/services/sep41/events.go:242-243 — "Unlike amount.String128, this preserves the raw integer (no 10^7 divisor), which is what SEP-41 events
  8. carry."

@aristidesstaffieri

Copy link
Copy Markdown
Contributor Author
  1. Flag ON against a failing v2 = no balances, no fallback — HIGH (shared with extension)

Same concern as on the extension - so is the suggestion that in the case of v2 failures that we try v1? I don't think we intend to rely on v1 for much longer and will aim to deprecate it so my gut reaction is that we should treat v2 as a critical dependency and ensure reliability there.

@aristidesstaffieri

Copy link
Copy Markdown
Contributor Author
  1. User-added custom tokens can silently disappear on the v2 path — HIGH (shared with extension)

This is incorrect, by design v2 relies on the wallet backend to index contract tokens.

  addBlockaidScanResults called scanBulkTokens raw on every 30s balance
  poll, adding a sequential HTTP round-trip before balances render and
  firing a BLOCKAID_BULK_TOKEN_SCAN analytics event per poll on mainnet.

  Use the blockaidTokenScans duck's scanBulkWithCache instead (disk-backed,
  30-min TTL, per-token) — the same chokepoint the swap flow uses, with a
  compatible CODE-ISSUER key format so both surfaces share cache entries.
  Warm polls become a local read with no network call and no analytics
  event; real scans drop to one per token per 30 minutes.

  Tests now mock the raw scan API + storage underneath the real duck so
  the cache path is exercised, with a new case pinning that fresh cache
  entries produce zero scanBulkTokens calls.
@aristidesstaffieri

Copy link
Copy Markdown
Contributor Author
  1. Client-side bulk scan runs uncached on every 30s poll and blocks balance render — MEDIUM
    MEDIUM — mobile-specific; the right caching chokepoint already exists in the repo.

TL;DR: addBlockaidScanResults calls scanBulkTokens raw, before balances are set — so on mainnet with the flag on, every 30s poll adds a second sequential HTTP round-trip (delaying balance display) and fires a Blockaid analytics event every 30s per active user. The repo already has a disk-backed, TTL'd cache (ensureTokenScans) for the same API that this bypasses.

This is fixed by 27f88e7

@aristidesstaffieri

Copy link
Copy Markdown
Contributor Author
  1. Freshly-funded account can render as "unfunded" under indexer lag — MEDIUM
    MEDIUM — not client-fixable; flag it as a known/accepted rollout risk.

TL;DR: v2 is_funded comes from the indexer, not Horizon. The friendbot flow funds then immediately refetches; on v2 the refetch may still show unfunded (zero balances) until ingestion catches up — same window after a first deposit on mainnet. State the indexer's lag SLO when enabling the flag.

Im not sure what we can do about this, the clients rely on the indexer as the source of truth here and the SLO is that it is caught up to the tip at all times.

  Review feedback flagged that the duck tests only ever asserted
  useV2: false — a hardcoded value at the fetchBalances call site would
  have passed the suite. Add a test that flips use_balances_v2 on after
  render and asserts the duck passes useV2: true, pinning the
  read-at-call-time behavior. Reset the flag in beforeEach so it can't
  leak between tests.

  Also pin that a v2 server error propagates without silently falling
  back to v1, which would mask indexer outages behind v1 data.
@aristidesstaffieri

Copy link
Copy Markdown
Contributor Author
  1. The flag-ON duck wiring and the 500 path are untested — MEDIUM
    MEDIUM — test gap on the one new load-bearing line.

TL;DR: Tests only ever assert useV2: false. Nothing sets the flag ON and asserts fetchBalances receives useV2: true — the exact line this PR exists to add. There's also no explicit v2-rejection (500) test through fetchBalances, notable given the endpoint currently 500s.

this is fixed by 5846c4a

@aristidesstaffieri

Copy link
Copy Markdown
Contributor Author
  1. SAC balances would be 1e7-inflated — MEDIUM (latent; shared with extension)
    MEDIUM — latent, not currently reachable. Mirror image of Default React Native project #1.

TL;DR: SAC amounts arrive raw (DB passthrough, not ÷1e7), but the mapper treats them as pre-formatted decimals, so they'd display 1e7× too large. Unreachable today because SAC balances key on a C… holder and the v2 handler only accepts G… — but the mapper and tests pin the wrong behavior for when that changes.

This is incorrect for the same reason as issue 1 is

  freighter-backend-v2#138 renamed `balance` to `total` and added
  server-derived `key` and `token` fields to every balance entry, so the
  mapper no longer derives asset identity client-side.

  - mapAccountBalancesV2.ts: rename `balance` → `total` in the wire types;
    add `V2Token`/`V2TokenIssuer` and `key`/`token` to the base, narrowed
    per variant (trustline type verbatim on CLASSIC, no `type` on SEP-41,
    no token on LP entries)
  - mapper: pass server `key`/`token` through verbatim and drop
    `classicAssetType`; the native entry alone is still re-keyed from the
    server's "native" to "XLM", the app convention
  - tests: fixtures updated to the new wire shape; the code-length type
    derivation test is now a key/token pass-through assertion; backend
    routing fixture carries `key`/`token`/`total`
@piyalbasu

piyalbasu commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Native XLM max-send is overstated for accounts with open sell offers

MEDIUM — not a merge blocker. Pre-existing logic (not introduced by this PR), but worth fixing here since the sibling extension PR (freighter#2906) already handles this correctly, and this PR is touching the balance-mapping plumbing.

TL;DR: When computing spendable XLM, the app subtracts the base reserve and the fee but not the XLM locked by the account's open sell offers (selling liabilities). So for an account with active sell offers, "send max" offers more XLM than is actually spendable, and the transaction fails at submit.

Steps to reproduce:

  1. Use an account with an open offer selling XLM (non-zero selling liabilities).
  2. Open Send XLM and tap "max" (or send an amount near the max).
  3. Submitting fails with tx_insufficient_balance / op_underfunded — the offered amount exceeds truly spendable XLM.

Detailed explanation (for agents)

Root cause: the native branch of calculateSpendableAmount recomputes the reserve locally and never subtracts sellingLiabilities (nor does it read the mapped minimumBalance):

And the v2 mapper passes the bare base reserve through, without folding liabilities in:

How the extension does it (the pattern to mirror): it keys off minimumBalance, not available, and normalizes minimumBalance at the v2 mapper to fold in selling liabilities, then the consumer subtracts that field:

  • v2 mapper folds: minimumBalance = minimum_balance + selling_liabilities (@shared/api/helpers/mapAccountBalancesV2.ts mapNative).
  • consumer: available = total − minimumBalance − fee (extension/src/popup/helpers/soroban.ts getAvailableBalance).

Why minimumBalance and not the server available: available is defined inconsistently across paths — v2 native available = total − minimum_balance − selling_liabilities (reserve subtracted), but the v1 backend's native available = total − selling_liabilities (reserve not subtracted; freighter-backend horizon-rpc.ts:49 even notes it "should also subtract the minimumBalance"). Since calculateSpendableAmount is shared by both the v1 and v2 balance paths and v1 is not being changed, switching the native branch to available − fee would regress v1 (users could spend into their base reserve). By contrast, both paths already agree that native minimumBalance = reserve + sellingLiabilities** (v1 folds it too — freighter-backend transformers.ts:138/horizon-rpc.ts:141 .plus(sellingLiabilities)), so keying off minimumBalance` is correct on both without touching v1.

Suggested fix (mirror the extension, v1-safe):

  1. Fold selling liabilities into the mapped native minimumBalance (parity with the extension mapper):
    minimumBalance: new BigNumber(b.minimum_balance).plus(b.selling_liabilities),
  2. In calculateSpendableAmount's native branch, use the mapped field instead of recomputing the reserve:
    const spendableAmount = totalBalance.minus(balance.minimumBalance).minus(fee);
    This works on both v1 and v2 (both supply minimumBalance = reserve + sellingLiabilities) and, as a bonus over the local recompute, picks up sponsorship netting that the server already reflects in the reserve. Add a test with selling_liabilities > 0 asserting the reduced max-send.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants