Show factory-sourced pair contracts in swap signing modal (#449) #991

Merged
PlasticDigits merged 3 commits from fix/449-swap-signing-pair-contract into main 2026-07-01 00:38:52 +00:00
PlasticDigits commented 2026-06-30 22:18:21 +00:00 (Migrated from gitlab.com)

Summary

Addresses #449 (SEC-I01 H07 / SEC-I02): the swap pre-sign summary now shows factory-sourced pair contract address(es) (truncated, copy + explorer link) so users can verify hop targets before signing.

  • Direct swaps: one Pair contract row with the LCD factory-resolved pair address (same source as submit).
  • Multihop: one Pair contracts row listing each hop pair address in order.
  • Trade market mirror on /trade Market tab.
  • Extracted shared resolver resolveSwapRoutePairAddresses.ts (reused by pause/blacklist probe on Swap).
  • No extra factory validation step: per code review on #449, submit already targets LCD factory pairs or the fixed router — not indexer-supplied pair addresses. Documented in MR checklist as SKIP.

Docs/skills updated: docs/frontend.md, docs/security-model.md, skills/AGENTS_FRONTEND_SWAP_SIGNING_CONFIRMATION.md.

Acceptance checklist

Criterion Verification Result
Signing modal displays pair contract address (truncated + explorer/copy) for direct swaps cd frontend-dapp && npm run test:run -- src/components/swap/__tests__/SwapPreSubmitSummary.test.tsx — renders factory-sourced pair contract address PASS
Signing modal shows hop pair addresses for multihop routes Same test file — renders hop pair contract addresses for multihop routes PASS
Factory pair registry validation rejects unregistered indexer pair addresses before submit Code review + existing submit path (directPair from LCD getAllPairsPaginated, multihop via ROUTER_CONTRACT_ADDRESS); no indexer pair addr in submit payload SKIP — not applicable; chain-sourced submit already
Frontend test asserts signing modal renders pair address field SwapPreSubmitSummary.test.tsx + resolveSwapRoutePairAddresses.test.ts; integration: SwapPage.test.tsx pre-sign test still passes PASS

Third-party verification

make test-frontend
# or targeted:
cd frontend-dapp && npm run test:run -- \
  src/components/swap/__tests__/SwapPreSubmitSummary.test.tsx \
  src/utils/__tests__/resolveSwapRoutePairAddresses.test.ts \
  src/pages/SwapPage.test.tsx \
  src/components/trade/__tests__/TradeMarketOrderPanel.submitSnapshot.test.tsx

Manual (LocalTerra + make dev):

  1. Open /swap, pick a CW20 pair, enter amount → confirm swap-pre-submit-summary shows Pair contract with truncated bech32 + copy/explorer.
  2. For a multihop route (indexer ≥2 hops), confirm Pair contracts row lists hop addresses (swap-confirm-hop-pair-0, …).
  3. On /trade/:pairAddr Market tab, confirm trade-market-pre-submit-summary shows the same pair contract row.

Blockers

None.

## Summary Addresses [#449](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/issues/449) (SEC-I01 H07 / SEC-I02): the swap pre-sign summary now shows **factory-sourced pair contract address(es)** (truncated, copy + explorer link) so users can verify hop targets before signing. - **Direct swaps:** one **Pair contract** row with the LCD factory-resolved pair address (same source as submit). - **Multihop:** one **Pair contracts** row listing each hop pair address in order. - **Trade market** mirror on `/trade` Market tab. - Extracted shared resolver [`resolveSwapRoutePairAddresses.ts`](frontend-dapp/src/utils/resolveSwapRoutePairAddresses.ts) (reused by pause/blacklist probe on Swap). - **No extra factory validation step:** per code review on #449, submit already targets LCD factory pairs or the fixed router — not indexer-supplied pair addresses. Documented in MR checklist as SKIP. Docs/skills updated: [`docs/frontend.md`](docs/frontend.md), [`docs/security-model.md`](docs/security-model.md), [`skills/AGENTS_FRONTEND_SWAP_SIGNING_CONFIRMATION.md`](skills/AGENTS_FRONTEND_SWAP_SIGNING_CONFIRMATION.md). ## Acceptance checklist | Criterion | Verification | Result | |-----------|--------------|--------| | Signing modal displays pair contract address (truncated + explorer/copy) for direct swaps | `cd frontend-dapp && npm run test:run -- src/components/swap/__tests__/SwapPreSubmitSummary.test.tsx` — `renders factory-sourced pair contract address` | **PASS** | | Signing modal shows hop pair addresses for multihop routes | Same test file — `renders hop pair contract addresses for multihop routes` | **PASS** | | Factory pair registry validation rejects unregistered indexer pair addresses before submit | Code review + existing submit path (`directPair` from LCD `getAllPairsPaginated`, multihop via `ROUTER_CONTRACT_ADDRESS`); no indexer pair addr in submit payload | **SKIP** — not applicable; chain-sourced submit already | | Frontend test asserts signing modal renders pair address field | `SwapPreSubmitSummary.test.tsx` + `resolveSwapRoutePairAddresses.test.ts`; integration: `SwapPage.test.tsx` pre-sign test still passes | **PASS** | ## Third-party verification ```bash make test-frontend # or targeted: cd frontend-dapp && npm run test:run -- \ src/components/swap/__tests__/SwapPreSubmitSummary.test.tsx \ src/utils/__tests__/resolveSwapRoutePairAddresses.test.ts \ src/pages/SwapPage.test.tsx \ src/components/trade/__tests__/TradeMarketOrderPanel.submitSnapshot.test.tsx ``` Manual (LocalTerra + `make dev`): 1. Open `/swap`, pick a CW20 pair, enter amount → confirm `swap-pre-submit-summary` shows **Pair contract** with truncated bech32 + copy/explorer. 2. For a multihop route (indexer ≥2 hops), confirm **Pair contracts** row lists hop addresses (`swap-confirm-hop-pair-0`, …). 3. On `/trade/:pairAddr` Market tab, confirm `trade-market-pre-submit-summary` shows the same pair contract row. ## Blockers None.
PlasticDigits commented 2026-06-30 22:21:04 +00:00 (Migrated from gitlab.com)

Severity: Medium

Issue: Pre-sign Pair contract(s) row can disagree with the on-chain submit path on /swap.

Impact: #449 / SEC-I02 surfaces factory pair address(es) for user verification before signing. This prop is fed from swapBlacklistProbe.pairAddresses, which resolves ops via route ?? nativeRouteInfo?.operations ?? simQuery.data?.indexerOperations (unchanged line 630). Submit instead prioritizes simData.indexerOperations when swapOpsRequireRouter(idxOps) (≥2 hops) at lines 753–772.

For CW20↔CW20 the indexer hybrid quote runs first; a direct factory pair can exist while quote/submit use a 2-hop router path. BFS route is shortest-path (often 1-hop direct), so the modal can show a single direct Pair contract while the wallet signs execute_swap_operations through different hop pair(s).

Attack path (no compromised indexer):

  1. Tokens A/C have a direct factory pair; multihop A→B→C also exists.
  2. Indexer returns 2-hop router_operations (default CW20 path).
  3. swapBlacklistProbe resolves pair addresses from 1-hop route.
  4. User confirms the misleading row and signs the 2-hop router tx.

Contrast: TradeMarketOrderPanel passes simQuery.data?.indexerOperations directly to the resolver — Swap wiring inherits the stale routeOps precedence.

Suggestion: Derive display ops with the same precedence as submit (prefer indexerOperations when router execution applies).

**Severity: Medium** **Issue:** Pre-sign **Pair contract(s)** row can disagree with the on-chain submit path on `/swap`. **Impact:** `#449` / SEC-I02 surfaces factory pair address(es) for user verification before signing. This prop is fed from `swapBlacklistProbe.pairAddresses`, which resolves ops via `route ?? nativeRouteInfo?.operations ?? simQuery.data?.indexerOperations` (unchanged line 630). Submit instead prioritizes `simData.indexerOperations` when `swapOpsRequireRouter(idxOps)` (≥2 hops) at lines 753–772. For CW20↔CW20 the indexer hybrid quote runs first; a direct factory pair can exist while quote/submit use a 2-hop router path. BFS `route` is shortest-path (often 1-hop direct), so the modal can show a single direct **Pair contract** while the wallet signs `execute_swap_operations` through different hop pair(s). **Attack path (no compromised indexer):** 1. Tokens A/C have a direct factory pair; multihop A→B→C also exists. 2. Indexer returns 2-hop `router_operations` (default CW20 path). 3. `swapBlacklistProbe` resolves pair addresses from 1-hop `route`. 4. User confirms the misleading row and signs the 2-hop router tx. **Contrast:** `TradeMarketOrderPanel` passes `simQuery.data?.indexerOperations` directly to the resolver — Swap wiring inherits the stale `routeOps` precedence. **Suggestion:** Derive display ops with the same precedence as submit (prefer `indexerOperations` when router execution applies).
PlasticDigits commented 2026-06-30 22:21:14 +00:00 (Migrated from gitlab.com)

Security review — MR !991

Commit reviewed: 2585a1936223829fedc4cf7b447795d9cc63e121

Scope: Pre-sign factory pair contract display for swap/trade (SwapPreSubmitSummary, resolveSwapRoutePairAddresses, SwapPage + TradeMarketOrderPanel wiring), docs/skills updates.

Outcome: FINDINGS: 1 medium+

Findings

Sev Location Summary
Medium frontend-dapp/src/pages/SwapPage.tsx (inline on pairContractAddresses wiring) /swap pre-sign Pair contract(s) can show BFS route pair addresses while submit executes indexer router_operations (≥2 hops). Undermines #449 verification; no compromised indexer required when direct + multihop paths coexist.

Not reported (reviewed, below threshold)

  • XSS / open redirect: AddressRow + getExplorerAddressUrl bech32 validation; React text rendering for truncated labels.
  • Indexer-supplied pair addresses in submit: unchanged — still LCD factory / fixed router.
  • resolveSwapRoutePairAddresses from-token/to-token fallback: edge-case UX only; secondary to the routeOps precedence issue above.
  • Trade market path: uses indexerOperations directly (aligned with submit).

Prior MR security-review threads: none found.

Label block:security applied.

## Security review — MR !991 **Commit reviewed:** `2585a1936223829fedc4cf7b447795d9cc63e121` **Scope:** Pre-sign factory pair contract display for swap/trade (`SwapPreSubmitSummary`, `resolveSwapRoutePairAddresses`, SwapPage + TradeMarketOrderPanel wiring), docs/skills updates. **Outcome:** `FINDINGS: 1` medium+ ### Findings | Sev | Location | Summary | |-----|----------|---------| | Medium | `frontend-dapp/src/pages/SwapPage.tsx` (inline on `pairContractAddresses` wiring) | `/swap` pre-sign **Pair contract(s)** can show BFS `route` pair addresses while submit executes indexer `router_operations` (≥2 hops). Undermines #449 verification; no compromised indexer required when direct + multihop paths coexist. | ### Not reported (reviewed, below threshold) - XSS / open redirect: `AddressRow` + `getExplorerAddressUrl` bech32 validation; React text rendering for truncated labels. - Indexer-supplied pair addresses in submit: unchanged — still LCD factory / fixed router. - `resolveSwapRoutePairAddresses` from-token/to-token fallback: edge-case UX only; secondary to the `routeOps` precedence issue above. - Trade market path: uses `indexerOperations` directly (aligned with submit). Prior MR security-review threads: none found. Label `block:security` applied.
PlasticDigits commented 2026-07-01 00:12:39 +00:00 (Migrated from gitlab.com)

Security fixes (633628a4)

Finding Resolution
Medium — /swap pre-sign Pair contract(s) could show BFS route pair addresses while submit executes indexer router_operations (≥2 hops) Added deriveSwapSubmitRouteOps() in swapRouteDisplay.ts with the same precedence as swapMutation (native wrap → indexer router ops when swapOpsRequireRouter → client BFS). SwapPage swapBlacklistProbe now uses simData.indexerOperations via that helper for pair resolution, pause, and blacklist probes. Unit test covers direct BFS vs indexer multihop precedence.

All prior low+ findings addressed. Targeted tests pass (swapRouteDisplay.test.ts, SwapPreSubmitSummary.test.tsx, resolveSwapRoutePairAddresses.test.ts, SwapPage.test.tsx, TradeMarketOrderPanel.submitSnapshot.test.tsx).

## Security fixes (`633628a4`) | Finding | Resolution | |---------|------------| | **Medium** — `/swap` pre-sign **Pair contract(s)** could show BFS `route` pair addresses while submit executes indexer `router_operations` (≥2 hops) | Added `deriveSwapSubmitRouteOps()` in `swapRouteDisplay.ts` with the same precedence as `swapMutation` (native wrap → indexer router ops when `swapOpsRequireRouter` → client BFS). `SwapPage` `swapBlacklistProbe` now uses `simData.indexerOperations` via that helper for pair resolution, pause, and blacklist probes. Unit test covers direct BFS vs indexer multihop precedence. | All prior **low+** findings addressed. Targeted tests pass (`swapRouteDisplay.test.ts`, `SwapPreSubmitSummary.test.tsx`, `resolveSwapRoutePairAddresses.test.ts`, `SwapPage.test.tsx`, `TradeMarketOrderPanel.submitSnapshot.test.tsx`).
PlasticDigits commented 2026-07-01 00:13:15 +00:00 (Migrated from gitlab.com)

added 1 commit

  • 633628a4 - Align swap pre-sign pair addresses with submit route ops (#449).

Compare with previous version

added 1 commit <ul><li>633628a4 - Align swap pre-sign pair addresses with submit route ops (#449).</li></ul> [Compare with previous version](/PlasticDigits/cl8y-dex-terraclassic/-/merge_requests/273/diffs?diff_id=1879514556&start_sha=2585a1936223829fedc4cf7b447795d9cc63e121)
PlasticDigits commented 2026-07-01 00:15:19 +00:00 (Migrated from gitlab.com)

Security review — MR !991

Commit reviewed: 633628a491aec140d811b9cf0862c3cd32f76eb2

Scope: Pre-sign factory pair contract display (SwapPreSubmitSummary, resolveSwapRoutePairAddresses, deriveSwapSubmitRouteOps), SwapPage + TradeMarketOrderPanel wiring, docs/skills updates.

Outcome: FINDINGS: 0 medium+ — NONE

Security review: no medium+ findings on this diff.

Prior thread disposition

Prior finding Status
Medium — /swap pre-sign Pair contract(s) could disagree with submit when indexer multihop beat BFS direct Remediated in 633628a4 via deriveSwapSubmitRouteOps() (same precedence as swapMutation); swapBlacklistProbe now uses simData.indexerOperations through that helper. Discussion resolved.

Reviewed (below threshold)

  • XSS / open redirect: AddressRow renders factory bech32 via React text nodes; getExplorerAddressUrl validates with isValidTerraBech32Address before building explorer links.
  • Indexer-supplied pair addresses in submit: unchanged — submit still targets LCD factory pairs or the fixed router contract, not indexer pair addresses.
  • resolveSwapRoutePairAddresses from/to fallback: only when hop ops fail to match factory list; edge-case UX, not an attacker-controlled spoof path on the remediated wiring.
  • Trade market: indexerOperations + selectedPair resolver aligns with market submit (swapOpsRequireRouter → router, else direct on selected pair).

No new inline security threads on this revision.

## Security review — MR !991 **Commit reviewed:** `633628a491aec140d811b9cf0862c3cd32f76eb2` **Scope:** Pre-sign factory pair contract display (`SwapPreSubmitSummary`, `resolveSwapRoutePairAddresses`, `deriveSwapSubmitRouteOps`), SwapPage + TradeMarketOrderPanel wiring, docs/skills updates. **Outcome:** `FINDINGS: 0` medium+ — **NONE** Security review: no medium+ findings on this diff. ### Prior thread disposition | Prior finding | Status | |---------------|--------| | Medium — `/swap` pre-sign **Pair contract(s)** could disagree with submit when indexer multihop beat BFS direct | **Remediated** in `633628a4` via `deriveSwapSubmitRouteOps()` (same precedence as `swapMutation`); `swapBlacklistProbe` now uses `simData.indexerOperations` through that helper. Discussion resolved. | ### Reviewed (below threshold) - **XSS / open redirect:** `AddressRow` renders factory bech32 via React text nodes; `getExplorerAddressUrl` validates with `isValidTerraBech32Address` before building explorer links. - **Indexer-supplied pair addresses in submit:** unchanged — submit still targets LCD factory pairs or the fixed router contract, not indexer pair addresses. - **`resolveSwapRoutePairAddresses` from/to fallback:** only when hop ops fail to match factory list; edge-case UX, not an attacker-controlled spoof path on the remediated wiring. - **Trade market:** `indexerOperations` + `selectedPair` resolver aligns with market submit (`swapOpsRequireRouter` → router, else direct on selected pair). No new inline security threads on this revision.
PlasticDigits commented 2026-07-01 00:15:24 +00:00 (Migrated from gitlab.com)

resolved all threads

resolved all threads
PlasticDigits commented 2026-07-01 00:33:35 +00:00 (Migrated from gitlab.com)

added 12 commits

  • 633628a4...9b98249d - 11 commits from branch main
  • 4dad91d8 - Merge branch 'main' into fix/449-swap-signing-pair-contract

Compare with previous version

added 12 commits <ul><li>633628a4...9b98249d - 11 commits from branch <code>main</code></li><li>4dad91d8 - Merge branch &#39;main&#39; into fix/449-swap-signing-pair-contract</li></ul> [Compare with previous version](/PlasticDigits/cl8y-dex-terraclassic/-/merge_requests/273/diffs?diff_id=1879535064&start_sha=633628a491aec140d811b9cf0862c3cd32f76eb2)
PlasticDigits commented 2026-07-01 00:33:36 +00:00 (Migrated from gitlab.com)

Merged main into fix/449-swap-signing-pair-contract and resolved the single conflict in skills/AGENTS_FRONTEND_SWAP_SIGNING_CONFIRMATION.md.

Resolution: kept this branch's #449 pair-contract row guidance (factory-sourced hop targets) and main's #461 limit-order pre-sign field notes (SEC-I05).

Targeted frontend tests (SwapPreSubmitSummary, resolveSwapRoutePairAddresses, SwapPage, TradeMarketOrderPanel.submitSnapshot) — 42/42 pass.

Merged `main` into `fix/449-swap-signing-pair-contract` and resolved the single conflict in `skills/AGENTS_FRONTEND_SWAP_SIGNING_CONFIRMATION.md`. **Resolution:** kept this branch's #449 pair-contract row guidance (factory-sourced hop targets) and `main`'s #461 limit-order pre-sign field notes (SEC-I05). Targeted frontend tests (SwapPreSubmitSummary, resolveSwapRoutePairAddresses, SwapPage, TradeMarketOrderPanel.submitSnapshot) — 42/42 pass.
PlasticDigits commented 2026-07-01 00:35:57 +00:00 (Migrated from gitlab.com)

Security review — MR !991

Commit reviewed: 4dad91d80d80e5a609e91124efb4903f021706d5

Scope: Pre-sign factory pair contract display (SwapPreSubmitSummary, resolveSwapRoutePairAddresses, deriveSwapSubmitRouteOps), SwapPage + TradeMarketOrderPanel wiring, docs/skills updates. Post-merge re-review includes 633628a4 route-op alignment fix and main merge conflict resolution in skills/AGENTS_FRONTEND_SWAP_SIGNING_CONFIRMATION.md.

Outcome: FINDINGS: 0 medium+ — NONE

Security review: no medium+ findings on this diff.

Prior thread disposition

Prior finding Status
Medium — /swap pre-sign Pair contract(s) could show BFS route pair addresses while submit executes indexer router_operations (≥2 hops) Remediated in 633628a4 via deriveSwapSubmitRouteOps() (same precedence as swapMutation); swapBlacklistProbe uses simData.indexerOperations through that helper. Verified still present after 4dad91d8 merge.

Reviewed (below threshold)

  • XSS / open redirect: AddressRow renders factory bech32 via React text nodes; getExplorerAddressUrl validates with isValidTerraBech32Address before building explorer links (terraExplorer.ts).
  • Indexer-supplied pair addresses in submit: unchanged — submit still targets LCD factory pairs or the fixed router contract, not indexer pair addresses.
  • resolveSwapRoutePairAddresses from/to fallback: only when hop ops fail to match the LCD factory list; edge-case UX on a trusted chain source, not an attacker-controlled spoof path on the remediated wiring.
  • Trade market: no client BFS route; indexerOperations + selectedPair resolver aligns with market submit (swapOpsRequireRouter → router, else direct on selectedPair).
  • Pause/blacklist probes: reuse the same swapBlacklistProbe.pairAddresses as the pre-sign row — no new bypass introduced.

No new inline security threads on this revision.

## Security review — MR !991 **Commit reviewed:** `4dad91d80d80e5a609e91124efb4903f021706d5` **Scope:** Pre-sign factory pair contract display (`SwapPreSubmitSummary`, `resolveSwapRoutePairAddresses`, `deriveSwapSubmitRouteOps`), SwapPage + TradeMarketOrderPanel wiring, docs/skills updates. Post-merge re-review includes `633628a4` route-op alignment fix and `main` merge conflict resolution in `skills/AGENTS_FRONTEND_SWAP_SIGNING_CONFIRMATION.md`. **Outcome:** `FINDINGS: 0` medium+ — **NONE** Security review: no medium+ findings on this diff. ### Prior thread disposition | Prior finding | Status | |---------------|--------| | Medium — `/swap` pre-sign **Pair contract(s)** could show BFS `route` pair addresses while submit executes indexer `router_operations` (≥2 hops) | **Remediated** in `633628a4` via `deriveSwapSubmitRouteOps()` (same precedence as `swapMutation`); `swapBlacklistProbe` uses `simData.indexerOperations` through that helper. Verified still present after `4dad91d8` merge. | ### Reviewed (below threshold) - **XSS / open redirect:** `AddressRow` renders factory bech32 via React text nodes; `getExplorerAddressUrl` validates with `isValidTerraBech32Address` before building explorer links (`terraExplorer.ts`). - **Indexer-supplied pair addresses in submit:** unchanged — submit still targets LCD factory pairs or the fixed router contract, not indexer pair addresses. - **`resolveSwapRoutePairAddresses` from/to fallback:** only when hop ops fail to match the LCD factory list; edge-case UX on a trusted chain source, not an attacker-controlled spoof path on the remediated wiring. - **Trade market:** no client BFS route; `indexerOperations` + `selectedPair` resolver aligns with market submit (`swapOpsRequireRouter` → router, else direct on `selectedPair`). - **Pause/blacklist probes:** reuse the same `swapBlacklistProbe.pairAddresses` as the pre-sign row — no new bypass introduced. No new inline security threads on this revision.
PlasticDigits (Migrated from gitlab.com) merged commit 37b005bd6a into main 2026-07-01 00:38:52 +00:00
PlasticDigits commented 2026-07-01 00:39:41 +00:00 (Migrated from gitlab.com)

mentioned in commit 37b005bd6a

mentioned in commit 37b005bd6a9fe9a53e5174a6353be63e678066a6
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
code/cl8y-dex-terraclassic!991
No description provided.