fix(wallets): saving chain addresses no longer deletes engine tokens - #1408
Conversation
profile.tokens is written wholesale, so whatever the payload contains becomes the user's entire on-chain token list. The external-wallet screens (link MetaMask, import watch-only address) send CHAIN entries only, and getGroupedChainTokens filtered the carry-forward base to chain entries too, so the broadcast replaced the list with chain addresses alone and silently deleted every Hive-Engine token the user had selected. Nothing outside Ecency writes profile.tokens, so that loss is unrecoverable: the user has to re-pick every token, if they notice. The reach is unlucky too, since the "link MetaMask" prompt targets accounts with no chain tokens yet. Existing entries the caller does not manage are now carried forward. Which entries those are is inferred from the payload: one carrying non-chain entries comes from the token picker, which owns the whole list and where omission means deselect, so nothing is preserved; a chain-only payload keeps the rest. The degenerate case is safe — a picker submitting before its list loaded sends chain-only and preserves rather than deletes. Extracted the assembly into buildTokensPayload so it is unit-testable; the regression test fails without the carry-forward and the deselect test fails if it is applied unconditionally.
|
Warning Review limit reached
Next review available in: 20 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (7)
📒 Files selected for processing (3)
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 |
Fixes #1406.
Problem
profile.tokensis written wholesale, so whatever the payload contains becomes the user's entire on-chain token list. The two external-wallet screens send CHAIN entries only:setup-external-metamask.tsx:166(link MetaMask)setup-external-import.tsx:96(import watch-only address)and
getGroupedChainTokensfiltered the carry-forward base to chain entries as well, so the broadcast replaced the list with chain addresses alone — silently deleting every Hive-Engine token the user had selected.Reproduced against the real code before the fix:
profile.namesurvived, so the profile deep-merge was working correctly — it was thetokensarray specifically being replaced.Nothing outside Ecency writes
profile.tokens, so the loss is unrecoverable: the user must re-pick every token, if they notice. The reach is unlucky too —profile-wallet-external-banner.tsxpromotes the "link MetaMask" CTA specifically to accounts with no chain tokens yet, which is the population most likely to have a curated engine list.Worth noting this produced the same user-visible symptom as the metadata-stripping node bug (#1393), by a completely different mechanism, and needs no misbehaving node.
Change
Entries the caller does not manage are now carried forward. Which those are is inferred from the payload:
The degenerate case is safe: a picker submitting before its token list has loaded sends chain-only and therefore preserves, which is the non-destructive outcome.
Inferring rather than adding an option is deliberate — the apps resolve this package through the committed
dist, so a new public option could not be used by a call site until the next release, whereas this fixes both screens with no app-side change.The assembly moved into an exported
buildTokensPayloadso it is unit-testable without rendering the hook.Verification
packages/walletssuite: 146 passed (5 new).tsc --noEmitclean forpackages/walletsandapps/web.Note for release
This is package-internal, and the apps consume
@ecency/walletsthrough its committeddist. The fix does not reach ecency.com until the dist is rebuilt, which the release flow owns — I have not added any label or dist rebuild to this PR.Audit context
From a full sweep of both repos (46 write-adjacent sites across
apps/web,packages/*,apps/self-hostedand vision-mobile), these two call sites were the only unprotectedposting_json_metadatawrites; everything else routes throughuseAccountUpdateand is correctly protected. Separately,apps/web/src/api/operations.ts:846-863serializesJSON.stringify({ profile })and would drop non-profileroot keys, but nothing importsapi/operations— dead code, worth deleting in its own change.