code/cl8y-dex-terraclassic

Fork 0

Security: pool provide/withdraw has no pre-sign summary card [SEC-I05] (F-03) #462

New issue

Closed

opened 2026-06-30 18:32:59 +00:00 by totdking · 19 comments

totdking commented

2026-06-30 18:32:59 +00:00

(Migrated from gitlab.com)

No description provided.

totdking commented

2026-06-30 18:35:45 +00:00

(Migrated from gitlab.com)

Summary

Swap operations render SwapPreSubmitSummary before the Keplr wallet dialog, showing action, pair, amounts, slippage protection, min return, and chain label. Pool provide and withdraw operations have no equivalent pre-sign summary. A user providing or withdrawing liquidity sees only token amounts and a submit button -- the chain, pair contract, and action type are not surfaced in a structured card before the wallet dialog appears. This is inconsistent with the SEC-D11 anti-phishing pattern already applied to swaps.

What Was Checked

frontend-dapp/src/pages/PoolPage.tsx lines 780-822 (provide): no SwapPreSubmitSummary or equivalent component rendered before submission.
frontend-dapp/src/pages/PoolPage.tsx lines 953-971 (withdraw): same absence.
frontend-dapp/src/pages/SwapPage.tsx line 1700: SwapPreSubmitSummary rendered conditionally before dispatch.
frontend-dapp/src/components/swap/SwapPreSubmitSummary.tsx: chain label, action, and amounts all rendered.
No unit or e2e test for a pool pre-sign summary component because the component does not exist.

Expected (per checklist)

Pool provide and withdraw operations render a pre-sign summary card showing: action ("Provide Liquidity" or "Withdraw Liquidity"), pair, token amounts, and chain label, before the Keplr wallet dialog opens. Matches the SEC-D11 anti-phishing protection already applied to swaps and consistent with limit order pre-sign expectations.

Actual

No pre-sign summary card exists for provide or withdraw. Chain, pair, and action type are absent from the in-app flow before Keplr signing.

Suggested Fix

Add a PoolPreSubmitSummary component (or reuse SwapPreSubmitSummary with actionLabel="Provide Liquidity" or "Withdraw Liquidity") to PoolPage.tsx provide and withdraw flows. Show: action, pair tokens, amounts, and chain label before the Keplr dialog opens.

Verification Checklist

Pool provide flow renders a pre-sign summary with action, pair tokens, amounts, and chain label before Keplr dialog
Pool withdraw flow renders equivalent pre-sign summary
Unit test added for PoolPreSubmitSummary confirming chain label and action type are rendered
Keplr wallet dialog is not the first place chain or action is identified to the user during pool operations

Cc: @PlasticDigits

### Summary Swap operations render `SwapPreSubmitSummary` before the Keplr wallet dialog, showing action, pair, amounts, slippage protection, min return, and chain label. Pool provide and withdraw operations have no equivalent pre-sign summary. A user providing or withdrawing liquidity sees only token amounts and a submit button -- the chain, pair contract, and action type are not surfaced in a structured card before the wallet dialog appears. This is inconsistent with the SEC-D11 anti-phishing pattern already applied to swaps. --- ### What Was Checked - `frontend-dapp/src/pages/PoolPage.tsx` lines 780-822 (provide): no `SwapPreSubmitSummary` or equivalent component rendered before submission. - `frontend-dapp/src/pages/PoolPage.tsx` lines 953-971 (withdraw): same absence. - `frontend-dapp/src/pages/SwapPage.tsx` line 1700: `SwapPreSubmitSummary` rendered conditionally before dispatch. - `frontend-dapp/src/components/swap/SwapPreSubmitSummary.tsx`: chain label, action, and amounts all rendered. - No unit or e2e test for a pool pre-sign summary component because the component does not exist. --- ### Expected (per checklist) Pool provide and withdraw operations render a pre-sign summary card showing: action ("Provide Liquidity" or "Withdraw Liquidity"), pair, token amounts, and chain label, before the Keplr wallet dialog opens. Matches the SEC-D11 anti-phishing protection already applied to swaps and consistent with limit order pre-sign expectations. --- ### Actual No pre-sign summary card exists for provide or withdraw. Chain, pair, and action type are absent from the in-app flow before Keplr signing. --- ### Suggested Fix Add a `PoolPreSubmitSummary` component (or reuse `SwapPreSubmitSummary` with `actionLabel="Provide Liquidity"` or `"Withdraw Liquidity"`) to `PoolPage.tsx` provide and withdraw flows. Show: action, pair tokens, amounts, and chain label before the Keplr dialog opens. --- ### Verification Checklist - [ ] Pool provide flow renders a pre-sign summary with action, pair tokens, amounts, and chain label before Keplr dialog - [ ] Pool withdraw flow renders equivalent pre-sign summary - [ ] Unit test added for `PoolPreSubmitSummary` confirming chain label and action type are rendered - [ ] Keplr wallet dialog is not the first place chain or action is identified to the user during pool operations Cc: @PlasticDigits

totdking commented

2026-06-30 18:37:45 +00:00

(Migrated from gitlab.com)

mentioned in issue #454

totdking commented

2026-06-30 18:37:56 +00:00

(Migrated from gitlab.com)

mentioned in issue #381

Brouie commented

2026-06-30 19:28:35 +00:00

(Migrated from gitlab.com)

mentioned in merge request !990

Brouie commented

2026-06-30 19:29:04 +00:00

(Migrated from gitlab.com)

Done. Pool provide/withdraw had no pre-sign summary — only token amounts + a submit button, with chain/action/pair absent before the Keplr dialog, unlike swaps.

Added a PoolPreSubmitSummary component (action, pair, amount line(s), chain label from getNetworkBadgeCopy().fullLabel) and wired it into PoolPage: rendered before the provide button (gated on amountA && amountB, showing both deposit amounts) and before the withdraw button (gated on lpAmount, showing the LP burn amount). Matches the SEC-D11 anti-phishing anchor pattern.

Tests: new component suite — provide (2 amount lines + chain), withdraw (LP line + chain), env-badge fallback — 3/0; PoolPage suite green, tsc clean. Source + unit verified here; the before-the-wallet-dialog placement/UX is a laptop browser-pass item.

MR !990, branch qa/462-pool-presign-summary, commit on the branch. Needs review/merge @PlasticDigits — leaving open for verification.

Done. Pool provide/withdraw had no pre-sign summary — only token amounts + a submit button, with chain/action/pair absent before the Keplr dialog, unlike swaps. Added a `PoolPreSubmitSummary` component (action, pair, amount line(s), chain label from `getNetworkBadgeCopy().fullLabel`) and wired it into `PoolPage`: rendered before the provide button (gated on amountA && amountB, showing both deposit amounts) and before the withdraw button (gated on lpAmount, showing the LP burn amount). Matches the SEC-D11 anti-phishing anchor pattern. Tests: new component suite — provide (2 amount lines + chain), withdraw (LP line + chain), env-badge fallback — 3/0; PoolPage suite green, tsc clean. Source + unit verified here; the before-the-wallet-dialog placement/UX is a laptop browser-pass item. MR !990, branch `qa/462-pool-presign-summary`, commit on the branch. Needs review/merge @PlasticDigits — leaving open for verification.

PlasticDigits commented

2026-06-30 22:07:48 +00:00

(Migrated from gitlab.com)

mentioned in commit a19bb72260

mentioned in commit a19bb72260d4d439d51d0764baddcb9631bf8189

PlasticDigits commented

2026-06-30 22:08:16 +00:00

(Migrated from gitlab.com)

Fix needs review to reduce user cognitive overload

PlasticDigits commented

2026-06-30 22:21:26 +00:00

(Migrated from gitlab.com)

mentioned in commit 81fc27c822

mentioned in commit 81fc27c8220a5c3af82bc8e9a0e89c8fcac44e8e

PlasticDigits commented

2026-06-30 22:21:48 +00:00

(Migrated from gitlab.com)

mentioned in merge request !997

PlasticDigits commented

2026-07-01 00:11:30 +00:00

(Migrated from gitlab.com)

mentioned in commit b2b067ec8f

mentioned in commit b2b067ec8f19d60bd0a60a460cfc79acbd3c737b

PlasticDigits commented

2026-07-01 00:40:22 +00:00

(Migrated from gitlab.com)

mentioned in commit 772a7dec45

mentioned in commit 772a7dec457e3824c3adfe02172a47900e5b6e1e

PlasticDigits commented

2026-07-01 01:01:36 +00:00

(Migrated from gitlab.com)

Verification — #462 (SEC-I05 F-03) — PASS

Verified on main @ 772a7dec (MR !990 + refine !997 merged). No repo changes required.

Acceptance checklist

Item	Result	How verified
Pool provide renders pre-sign summary (action, pair, amounts, chain) before wallet dialog	PASS	`PoolPage.test.tsx` — summary `data-testid`s appear when both amounts entered; `compareDocumentPosition` confirms card is above submit. Live browser on `http://127.0.0.1:5173/pool` (LocalTerra + indexer + `make dev`): card visible after entering amounts, positioned above Provide button.
Pool withdraw renders equivalent pre-sign summary	PASS	Same tests for withdraw panel (`pool-withdraw-pre-submit-summary-*`); live browser confirms LP amount line + chain.
Unit test for `PoolPreSubmitSummary` (action + chain)	PASS	`npm run test:run -- src/components/pool/__tests__/PoolPreSubmitSummary.test.tsx` → 3/3 (provide 2-amount + chain, withdraw LP + chain, env badge fallback).
Keplr is not the first place chain/action is identified	PASS	Summary card is in-app and gated on form input, rendered above the submit CTA (DOM order assertion in `PoolPage.test.tsx`). Wallet/extension dialog only follows submit — chain (`LocalTerra` via `getNetworkBadgeCopy().fullLabel`) and action labels are on the card first.

Commands run

# Unit / integration
bash scripts/with-node.sh --cwd frontend-dapp -- npm run test:run -- \
  src/components/pool/__tests__/PoolPreSubmitSummary.test.tsx   # 3/3
bash scripts/with-node.sh --cwd frontend-dapp -- npm run test:run -- \
  src/pages/PoolPage.test.tsx -t "462|SEC-I05"                   # 2/2

cd frontend-dapp && npx tsc --noEmit                             # clean

# Stack for browser pass
make setup-cloud-localterra   # fresh deploy after stale-volume fund error
make dev                      # :5173
# Playwright DOM check against live /pool (risk-ack localStorage seed)

Implementation notes (reviewer feedback)

PlasticDigits requested reduced cognitive overload — addressed in qa/462-pool-presign-summary-refine (!997): compact four-row card (action, pair, amount, chain) without swap-style intro paragraph; documented in docs/frontend.md § Pool pre-sign summary and skills/AGENTS_FRONTEND_POOL_SIGNING_CONFIRMATION.md.

Follow-ups

None blocking close. Optional: add a dedicated Playwright smoke asserting pool-*-pre-submit-summary on /pool (currently covered by Vitest only).

## Verification — #462 (SEC-I05 F-03) — **PASS** Verified on `main` @ `772a7dec` (MR !990 + refine !997 merged). No repo changes required. ### Acceptance checklist | Item | Result | How verified | |------|--------|--------------| | Pool **provide** renders pre-sign summary (action, pair, amounts, chain) before wallet dialog | **PASS** | `PoolPage.test.tsx` — summary `data-testid`s appear when both amounts entered; `compareDocumentPosition` confirms card is above submit. Live browser on `http://127.0.0.1:5173/pool` (LocalTerra + indexer + `make dev`): card visible after entering amounts, positioned above Provide button. | | Pool **withdraw** renders equivalent pre-sign summary | **PASS** | Same tests for withdraw panel (`pool-withdraw-pre-submit-summary-*`); live browser confirms LP amount line + chain. | | Unit test for `PoolPreSubmitSummary` (action + chain) | **PASS** | `npm run test:run -- src/components/pool/__tests__/PoolPreSubmitSummary.test.tsx` → **3/3** (provide 2-amount + chain, withdraw LP + chain, env badge fallback). | | Keplr is not the first place chain/action is identified | **PASS** | Summary card is in-app and gated on form input, rendered **above** the submit CTA (DOM order assertion in `PoolPage.test.tsx`). Wallet/extension dialog only follows submit — chain (`LocalTerra` via `getNetworkBadgeCopy().fullLabel`) and action labels are on the card first. | ### Commands run ```bash # Unit / integration bash scripts/with-node.sh --cwd frontend-dapp -- npm run test:run -- \ src/components/pool/__tests__/PoolPreSubmitSummary.test.tsx # 3/3 bash scripts/with-node.sh --cwd frontend-dapp -- npm run test:run -- \ src/pages/PoolPage.test.tsx -t "462|SEC-I05" # 2/2 cd frontend-dapp && npx tsc --noEmit # clean # Stack for browser pass make setup-cloud-localterra # fresh deploy after stale-volume fund error make dev # :5173 # Playwright DOM check against live /pool (risk-ack localStorage seed) ``` ### Implementation notes (reviewer feedback) PlasticDigits requested reduced cognitive overload — addressed in `qa/462-pool-presign-summary-refine` (!997): compact **four-row** card (action, pair, amount, chain) without swap-style intro paragraph; documented in `docs/frontend.md` § Pool pre-sign summary and `skills/AGENTS_FRONTEND_POOL_SIGNING_CONFIRMATION.md`. ### Follow-ups None blocking close. Optional: add a dedicated Playwright smoke asserting `pool-*-pre-submit-summary` on `/pool` (currently covered by Vitest only).

PlasticDigits (Migrated from gitlab.com) closed this issue

2026-07-01 01:01:37 +00:00

PlasticDigits commented

2026-07-12 08:24:53 +00:00

(Migrated from gitlab.com)

mentioned in issue #480