feat(frontend): rank economic tokens above test tokens in Transfer picker (#136) #155

Merged
PlasticDigits merged 2 commits from feat/gl-136-economic-token-rank into main 2026-08-31 12:21:17 +00:00
PlasticDigits commented 2026-08-31 05:18:41 +00:00 (Migrated from gitlab.com)

Summary

  • Transfer token picker now ranks economic tokens first and known noneconomic faucet tokens last (display/default-selection only; test tokens stay selectable).
  • Classification is a closed denylist of canonical ids (Terra CW20/denom, EVM address, SPL mint) shared with Settings → Faucet via faucetTokens.ts — never display symbol (spoof-resistant).
  • TransferForm auto-selects the first ranked token when empty/invalid (defaultTransferTokenId); an explicit still-valid test-token choice is kept.
  • Documents INV-FE-TOKEN-RANK-1 and third-party skill skills/agent-frontend-token-rank.md.

Closes #136

Test plan

  • cd packages/frontend && npm run test:unit (686 tests passed locally)
  • Ranking units: npm run test:run -- src/utils/tokenEconomicRank.test.ts src/services/transfer/buildTransferTokens.test.ts src/components/transfer/SubComponents.test.tsx
  • Open Transfer, open Amount token combobox: LUNC/CL8Y/… contiguous at top, testa/testb/tdec (or local TKNA/B/C/KDEC) contiguous at bottom
  • Default token is economic when any exist; selecting testa from the bottom keeps testa after amount input / re-render
  • Reload twice and switch BSC ↔ opBNB ↔ MegaETH ↔ Terra ↔ Solana: ranking holds on mixed routes
  • Settings → Faucet still lists testa/testb/tdec in existing order (this MR does not redesign faucet)
  • Playwright e2e/token-selection.spec.ts when local e2e infra is up (workers: 5 already in playwright.config.ts)
## Summary - Transfer token picker now ranks **economic tokens first** and **known noneconomic faucet tokens last** (display/default-selection only; test tokens stay selectable). - Classification is a **closed denylist of canonical ids** (Terra CW20/denom, EVM address, SPL mint) shared with Settings → Faucet via `faucetTokens.ts` — never display `symbol` (spoof-resistant). - `TransferForm` auto-selects the first ranked token when empty/invalid (`defaultTransferTokenId`); an explicit still-valid test-token choice is kept. - Documents **INV-FE-TOKEN-RANK-1** and third-party skill [`skills/agent-frontend-token-rank.md`](../skills/agent-frontend-token-rank.md). Closes #136 ## Test plan - [ ] `cd packages/frontend && npm run test:unit` (686 tests passed locally) - [ ] Ranking units: `npm run test:run -- src/utils/tokenEconomicRank.test.ts src/services/transfer/buildTransferTokens.test.ts src/components/transfer/SubComponents.test.tsx` - [ ] Open Transfer, open Amount token combobox: LUNC/CL8Y/… contiguous at top, testa/testb/tdec (or local TKNA/B/C/KDEC) contiguous at bottom - [ ] Default token is economic when any exist; selecting testa from the bottom keeps testa after amount input / re-render - [ ] Reload twice and switch BSC ↔ opBNB ↔ MegaETH ↔ Terra ↔ Solana: ranking holds on mixed routes - [ ] Settings → Faucet still lists testa/testb/tdec in existing order (this MR does not redesign faucet) - [ ] Playwright `e2e/token-selection.spec.ts` when local e2e infra is up (`workers: 5` already in playwright.config.ts)
PlasticDigits commented 2026-08-31 09:13:32 +00:00 (Migrated from gitlab.com)

RECOMMEND: ACCEPT

Reviewed against #136 (acceptance criteria, attack table, verification checklist). This is a frontend display/default-selection change only. It does not alter mappings, fees, hash encoding, or which tokens are bridgeable.

Security

No BLOCK issues. Ranking cannot cause a wrong-token submit if id / tokenId / evmTokenAddress stay bound to the row.

Vector Result
Spoofed symbol() (CL8Y on testa, testa on CL8Y) Ranked by canonical id; covered in tokenEconomicRank.test.ts and TokenSelect click tests
XSS in symbol Rendered as text (data-tokenid is the submit id); component test covers <img> / <script>
Identity swap after sort rankTransferTokens slices then sorts; does not mutate identity fields
Closed denylist bypass via “looks like test” Unknown ids default economic (documented residual: a new faucet mint omitted from faucetTokens.ts appears at the top)
Auto-select yanking a user off testa defaultTransferTokenId keeps a still-valid explicit id
Query-param / localStorage token force Not introduced
Oracle / tokenomic / SC / DB N/A (UI order only)
Prompt injection None

#136 acceptance criteria

Criterion Status
Economic options above test options MET — compareTransferTokenRank
testa/testb/tdec + local TKNA/B/C/KDEC bottom MET — shared faucetTokens.ts + SPL mint set
LUNC/USTC/CL8Y and non-denylisted ids top MET
Test tokens remain selectable with correct id MET
Default is first economic; test-only routes default to first test MET
Explicit test selection kept if still valid MET
EVM mapping-load races neutralized MET — sort after every builder path
Settings → Faucet order unchanged MET — catalog extracted, panel order not redesigned
Unit tests: mixed / only-econ / only-test / unknown / spoof MET
Listbox order test MET — unit + Playwright spec
INV documented MET — INV-FE-TOKEN-RANK-1

Gaps (non-blocking)

  • Playwright e2e/token-selection.spec.ts is written (economic-before-test + keep testa) but not run in the MR checklist. Run it before merge when local e2e infra is up.
  • Manual mixed-route matrix (BSC/opBNB/MegaETH/Terra/Solana) still unchecked — ranking is deterministic, so this is confirmation, not a design hole.
  • Optional muted “Test” suffix was out of scope; some users may still miss that a bottom-row testa is a faucet token. Acceptable per issue.

Testing

Unit coverage on the rank helper, buildTransferTokens paths, TokenSelect data-tokenid, and catalog-sync is thorough. Happy path (economic default) and bad path (spoofed label still submits test id) are covered. No additional DeFi attack tests are required for this MR.

Code is readable, DRY (one catalog for Faucet + rank), and matches the issue’s recommended design. Safe to merge after the existing Playwright spec is green in CI or a local run.

RECOMMEND: ACCEPT Reviewed against #136 (acceptance criteria, attack table, verification checklist). This is a frontend display/default-selection change only. It does not alter mappings, fees, hash encoding, or which tokens are bridgeable. ## Security No BLOCK issues. Ranking cannot cause a wrong-token submit if `id` / `tokenId` / `evmTokenAddress` stay bound to the row. | Vector | Result | |---|---| | Spoofed `symbol()` (`CL8Y` on testa, `testa` on CL8Y) | Ranked by canonical id; covered in `tokenEconomicRank.test.ts` and TokenSelect click tests | | XSS in symbol | Rendered as text (`data-tokenid` is the submit id); component test covers `<img>` / `<script>` | | Identity swap after sort | `rankTransferTokens` slices then sorts; does not mutate identity fields | | Closed denylist bypass via “looks like test” | Unknown ids default **economic** (documented residual: a new faucet mint omitted from `faucetTokens.ts` appears at the top) | | Auto-select yanking a user off testa | `defaultTransferTokenId` keeps a still-valid explicit id | | Query-param / localStorage token force | Not introduced | | Oracle / tokenomic / SC / DB | N/A (UI order only) | | Prompt injection | None | ## #136 acceptance criteria | Criterion | Status | |---|---| | Economic options above test options | MET — `compareTransferTokenRank` | | testa/testb/tdec + local TKNA/B/C/KDEC bottom | MET — shared `faucetTokens.ts` + SPL mint set | | LUNC/USTC/CL8Y and non-denylisted ids top | MET | | Test tokens remain selectable with correct id | MET | | Default is first economic; test-only routes default to first test | MET | | Explicit test selection kept if still valid | MET | | EVM mapping-load races neutralized | MET — sort after every builder path | | Settings → Faucet order unchanged | MET — catalog extracted, panel order not redesigned | | Unit tests: mixed / only-econ / only-test / unknown / spoof | MET | | Listbox order test | MET — unit + Playwright spec | | INV documented | MET — INV-FE-TOKEN-RANK-1 | ## Gaps (non-blocking) - Playwright `e2e/token-selection.spec.ts` is written (economic-before-test + keep testa) but **not run** in the MR checklist. Run it before merge when local e2e infra is up. - Manual mixed-route matrix (BSC/opBNB/MegaETH/Terra/Solana) still unchecked — ranking is deterministic, so this is confirmation, not a design hole. - Optional muted “Test” suffix was out of scope; some users may still miss that a bottom-row testa is a faucet token. Acceptable per issue. ## Testing Unit coverage on the rank helper, `buildTransferTokens` paths, TokenSelect `data-tokenid`, and catalog-sync is thorough. Happy path (economic default) and bad path (spoofed label still submits test id) are covered. No additional DeFi attack tests are required for this MR. Code is readable, DRY (one catalog for Faucet + rank), and matches the issue’s recommended design. Safe to merge after the existing Playwright spec is green in CI or a local run.
PlasticDigits commented 2026-08-31 09:16:00 +00:00 (Migrated from gitlab.com)

Follow-up (second pass): still RECOMMEND: ACCEPT. Non-blocking test nits:

  1. e2e/token-selection.spec.ts keep-selection locator — options.filter({ has: page.locator('[data-tokenid="…"]') }) matches a descendant with that attribute. data-tokenid is on the <li role="option"> itself, so the MAINNET_TESTA/B/TDEC branches likely never match and always fall through to “click the last option”. Prefer [role="option"][data-tokenid="…"] scoped to the token listbox.

  2. Local e2e ranking can pass vacuously — the spec imports isNoneconomicBridgeToken from Vite-built faucetTokens.ts. Playwright’s Node process does not populate import.meta.env the way Vite does, so local TKNA/B/C/KDEC ids may drop out of the denylist in the runner while the page still ranks them. An inverted local ranking would not fail CI. Classify e2e ids from process.env / .env.e2e.local (plus hardcoded mainnet CW20s).

  3. TransferForm default/race is covered on defaultTransferTokenId only. A single TransferForm test (empty → first economic; keep testa across array identity change) would match the #136 test plan.

Unit/component coverage remains the real ranking proof. Happy to merge with these as follow-ups.

Follow-up (second pass): still **RECOMMEND: ACCEPT**. Non-blocking test nits: 1. **`e2e/token-selection.spec.ts` keep-selection locator** — `options.filter({ has: page.locator('[data-tokenid="…"]') })` matches a *descendant* with that attribute. `data-tokenid` is on the `<li role="option">` itself, so the MAINNET_TESTA/B/TDEC branches likely never match and always fall through to “click the last option”. Prefer `[role="option"][data-tokenid="…"]` scoped to the token listbox. 2. **Local e2e ranking can pass vacuously** — the spec imports `isNoneconomicBridgeToken` from Vite-built `faucetTokens.ts`. Playwright’s Node process does not populate `import.meta.env` the way Vite does, so local TKNA/B/C/KDEC ids may drop out of the denylist in the runner while the page still ranks them. An inverted local ranking would not fail CI. Classify e2e ids from `process.env` / `.env.e2e.local` (plus hardcoded mainnet CW20s). 3. **TransferForm default/race** is covered on `defaultTransferTokenId` only. A single `TransferForm` test (empty → first economic; keep testa across array identity change) would match the #136 test plan. Unit/component coverage remains the real ranking proof. Happy to merge with these as follow-ups.
PlasticDigits commented 2026-08-31 12:20:13 +00:00 (Migrated from gitlab.com)

added 7 commits

  • 2a20cd3e...a5a7620c - 6 commits from branch main
  • d86a844d - chore: merge main into feat/gl-136-economic-token-rank (GL-136 + clickwrap/writer docs)

Compare with previous version

added 7 commits <ul><li>2a20cd3e...a5a7620c - 6 commits from branch <code>main</code></li><li>d86a844d - chore: merge main into feat/gl-136-economic-token-rank (GL-136 + clickwrap/writer docs)</li></ul> [Compare with previous version](/PlasticDigits/cl8y-bridge-monorepo/-/merge_requests/15/diffs?diff_id=1994058106&start_sha=2a20cd3e4f40482a6692ac7bf7b3f36c034e5a30)
PlasticDigits commented 2026-08-31 12:21:17 +00:00 (Migrated from gitlab.com)

mentioned in commit 50b593d47c

mentioned in commit 50b593d47c4d47605671c47cc2fadccc392055d1
PlasticDigits (Migrated from gitlab.com) merged commit 50b593d47c into main 2026-08-31 12:21:17 +00:00
PlasticDigits commented 2026-08-31 12:39:27 +00:00 (Migrated from gitlab.com)

mentioned in issue #136

mentioned in issue #136
Sign in to join this conversation.
No description provided.