code/cl8y-dex-terraclassic

Fork 0

frontend: compact pool pre-sign summary card (#462) #997

Merged

PlasticDigits merged 2 commits from qa/462-pool-presign-summary-refine into main

2026-07-01 00:39:50 +00:00

PlasticDigits commented

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

(Migrated from gitlab.com)

Summary

Refines the pool provide/withdraw pre-sign summary merged in !990 (#462 / SEC-I05 F-03) to address cognitive-overload feedback: drops the swap-style intro paragraph (chain was duplicated), consolidates provide deposit amounts into one Amount row, and keeps the four SEC-D11 security anchors (action, pair, amount, chain).

Adds PoolPage wiring tests, skills/AGENTS_FRONTEND_POOL_SIGNING_CONFIRMATION.md, and cross-links in docs/frontend.md and docs/exploit-replay-matrix.md.

Acceptance checklist

Criterion	Verification	Result
Pool provide renders pre-sign summary (action, pair, amounts, chain) before wallet dialog	`cd frontend-dapp && npm run test:run -- src/pages/PoolPage.test.tsx -t "provide pre-sign"`; manual: `/pool` → Provide → enter both amounts → card above submit	PASS
Pool withdraw renders equivalent pre-sign summary	`cd frontend-dapp && npm run test:run -- src/pages/PoolPage.test.tsx -t "withdraw pre-sign"`; manual: `/pool` → Withdraw → enter LP amount	PASS
Unit test for `PoolPreSubmitSummary` (chain label + action type)	`cd frontend-dapp && npm run test:run -- src/components/pool/__tests__/PoolPreSubmitSummary.test.tsx`	PASS (3/3)
Keplr is not the first place chain/action are identified	DOM order test: summary precedes submit button; card shows Action + Chain rows	PASS (unit); manual Keplr placement SKIP (no browser run this session)

Third-party verification

cd frontend-dapp && npm run test:run -- \
  src/components/pool/__tests__/PoolPreSubmitSummary.test.tsx \
  src/pages/PoolPage.test.tsx
bash scripts/with-node.sh --cwd frontend-dapp -- npx tsc --noEmit

Manual (needs make dev + wallet):

Open http://127.0.0.1:5173/pool, expand a pair, click Provide Liquidity.
Enter amounts for both assets — confirm compact card shows Action, Pair, Amount (A + B), Chain above the submit button.
Switch to Withdraw Liquidity, enter LP amount — same card pattern with Withdraw Liquidity and N LP.
Click submit — Keplr should open after the in-app summary was visible.

Blockers

None for merge from automated checks. Manual Keplr browser pass recommended post-merge.

## Summary Refines the pool provide/withdraw pre-sign summary merged in !990 ([#462](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/issues/462) / SEC-I05 F-03) to address cognitive-overload feedback: drops the swap-style intro paragraph (chain was duplicated), consolidates provide deposit amounts into one **Amount** row, and keeps the four SEC-D11 security anchors (action, pair, amount, chain). Adds `PoolPage` wiring tests, `skills/AGENTS_FRONTEND_POOL_SIGNING_CONFIRMATION.md`, and cross-links in `docs/frontend.md` and `docs/exploit-replay-matrix.md`. ## Acceptance checklist | Criterion | Verification | Result | |-----------|--------------|--------| | Pool provide renders pre-sign summary (action, pair, amounts, chain) before wallet dialog | `cd frontend-dapp && npm run test:run -- src/pages/PoolPage.test.tsx -t "provide pre-sign"`; manual: `/pool` → Provide → enter both amounts → card above submit | **PASS** | | Pool withdraw renders equivalent pre-sign summary | `cd frontend-dapp && npm run test:run -- src/pages/PoolPage.test.tsx -t "withdraw pre-sign"`; manual: `/pool` → Withdraw → enter LP amount | **PASS** | | Unit test for `PoolPreSubmitSummary` (chain label + action type) | `cd frontend-dapp && npm run test:run -- src/components/pool/__tests__/PoolPreSubmitSummary.test.tsx` | **PASS** (3/3) | | Keplr is not the first place chain/action are identified | DOM order test: summary precedes submit button; card shows Action + Chain rows | **PASS** (unit); manual Keplr placement **SKIP** (no browser run this session) | ## Third-party verification ```bash cd frontend-dapp && npm run test:run -- \ src/components/pool/__tests__/PoolPreSubmitSummary.test.tsx \ src/pages/PoolPage.test.tsx bash scripts/with-node.sh --cwd frontend-dapp -- npx tsc --noEmit ``` Manual (needs `make dev` + wallet): 1. Open `http://127.0.0.1:5173/pool`, expand a pair, click **Provide Liquidity**. 2. Enter amounts for both assets — confirm compact card shows **Action**, **Pair**, **Amount** (`A + B`), **Chain** above the submit button. 3. Switch to **Withdraw Liquidity**, enter LP amount — same card pattern with **Withdraw Liquidity** and `N LP`. 4. Click submit — Keplr should open **after** the in-app summary was visible. ## Blockers None for merge from automated checks. Manual Keplr browser pass recommended post-merge.

PlasticDigits commented

2026-06-30 22:23:12 +00:00

(Migrated from gitlab.com)

Security review — MR !997

Commit reviewed: 81fc27c8220a5c3af82bc8e9a0e89c8fcac44e8e

Scope: PoolPreSubmitSummary compact layout (consolidated amount row, removed swap-style intro copy), PoolPage wiring tests, docs/skills cross-links for SEC-I05 / SEC-D11 pool parity.

Method: Diff-only review with sink tracing for attacker-controlled inputs (amountLines, pairLabel, actionLabel, chainFullLabel). Compared against existing SwapPreSubmitSummary / PoolPage wiring (unchanged in this MR).

Outcome: FINDINGS: 0 medium+

Summary

No new medium, high, or critical vulnerabilities on this diff.

XSS / HTML injection: SummaryRow renders all props as React text nodes ({value}); no dangerouslySetInnerHTML. Unchanged from prior PoolPreSubmitSummary; indexer/CW20 symbol trust is pre-existing and identical to other pool/swap surfaces.
Signing confusion (SEC-D11): The four labeled security anchors remain — Action, Pair, Amount (now one consolidated row), Chain — with stable data-testids. Removing the intro paragraph is a UX compaction; it does not remove chain identification or pair context, and does not decouple the card from form snapshot values (amountA/amountB/lpAmount wiring in PoolPage.tsx is outside this diff).
Injection / SSRF / authz / secrets: Not introduced; changes are presentational React + tests + documentation.

Inline threads: None (no findings).

Prior security-review comments: None on this MR; nothing to re-report.

## Security review — MR !997 **Commit reviewed:** `81fc27c8220a5c3af82bc8e9a0e89c8fcac44e8e` **Scope:** `PoolPreSubmitSummary` compact layout (consolidated amount row, removed swap-style intro copy), `PoolPage` wiring tests, docs/skills cross-links for SEC-I05 / SEC-D11 pool parity. **Method:** Diff-only review with sink tracing for attacker-controlled inputs (`amountLines`, `pairLabel`, `actionLabel`, `chainFullLabel`). Compared against existing `SwapPreSubmitSummary` / `PoolPage` wiring (unchanged in this MR). **Outcome:** `FINDINGS: 0` medium+ ### Summary No new medium, high, or critical vulnerabilities on this diff. - **XSS / HTML injection:** `SummaryRow` renders all props as React text nodes (`{value}`); no `dangerouslySetInnerHTML`. Unchanged from prior `PoolPreSubmitSummary`; indexer/CW20 symbol trust is pre-existing and identical to other pool/swap surfaces. - **Signing confusion (SEC-D11):** The four labeled security anchors remain — Action, Pair, Amount (now one consolidated row), Chain — with stable `data-testid`s. Removing the intro paragraph is a UX compaction; it does not remove chain identification or pair context, and does not decouple the card from form snapshot values (`amountA`/`amountB`/`lpAmount` wiring in `PoolPage.tsx` is outside this diff). - **Injection / SSRF / authz / secrets:** Not introduced; changes are presentational React + tests + documentation. **Inline threads:** None (no findings). **Prior security-review comments:** None on this MR; nothing to re-report.

PlasticDigits commented

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

(Migrated from gitlab.com)

added 9 commits

81fc27c8...4621116e - 8 commits from branch main
b2b067ec - Merge branch 'main' into qa/462-pool-presign-summary-refine

Compare with previous version

added 9 commits <ul><li>81fc27c8...4621116e - 8 commits from branch <code>main</code></li><li>b2b067ec - Merge branch 'main' into qa/462-pool-presign-summary-refine</li></ul> [Compare with previous version](/PlasticDigits/cl8y-dex-terraclassic/-/merge_requests/279/diffs?diff_id=1879513117&start_sha=81fc27c8220a5c3af82bc8e9a0e89c8fcac44e8e)

PlasticDigits commented

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

(Migrated from gitlab.com)

Resolved merge conflicts with main.

Conflict: skills/AGENTS_FRONTEND_SWAP_SIGNING_CONFIRMATION.md — merged main's #461 limit-order labeled signing fields wording with this branch's #462 pool pre-sign rule (items 4 and 5).

Checks: tsc --noEmit, PoolPreSubmitSummary + PoolPage unit tests (22/22 pass).

Resolved merge conflicts with `main`. **Conflict:** `skills/AGENTS_FRONTEND_SWAP_SIGNING_CONFIRMATION.md` — merged `main`'s #461 limit-order labeled signing fields wording with this branch's #462 pool pre-sign rule (items 4 and 5). **Checks:** `tsc --noEmit`, `PoolPreSubmitSummary` + `PoolPage` unit tests (22/22 pass).

PlasticDigits commented

2026-07-01 00:13:17 +00:00

(Migrated from gitlab.com)

Security review — MR !997

Commit reviewed: b2b067ec8f19d60bd0a60a460cfc79acbd3c737b

Scope: PoolPreSubmitSummary compact layout (consolidated Amount row via amountLines.join(' + '), removed swap-style intro copy), PoolPage wiring tests, docs/skills cross-links for SEC-I05 / SEC-D11 pool parity; factory test formatting-only hunk.

Outcome: FINDINGS: 0 medium+

Summary

Security review: no medium+ findings on this diff.

XSS / HTML injection: SummaryRow renders all props as React text nodes ({value}); no dangerouslySetInnerHTML. Unchanged pattern from prior PoolPreSubmitSummary; indexer/CW20 symbol trust is pre-existing and identical to other pool/swap surfaces.
Signing confusion (SEC-D11): The four labeled security anchors remain — Action, Pair, Amount (now one consolidated row), Chain — with stable data-testids. Removing the intro paragraph is UX compaction; chain identification and pair context are preserved. Summary values still snapshot form state (amountA/amountB/lpAmount + displayLabel wiring in PoolPage.tsx is outside this diff).
Consolidated amount delimiter: New join(' + ') could let a malicious CW20/indexer symbol containing + blur amount boundaries in the summary line; separate form inputs and wallet signing still reflect actual assets/amounts. Treat as pre-existing metadata-display trust, not a new executable sink — below medium threshold.
Injection / SSRF / authz / secrets: Not introduced; changes are presentational React + tests + documentation.

Inline threads: None (no findings).

Prior security-review comments: Reviewed note on 81fc27c; findings still do not apply after rebase to b2b067ec (no new sinks introduced).

## Security review — MR !997 **Commit reviewed:** `b2b067ec8f19d60bd0a60a460cfc79acbd3c737b` **Scope:** `PoolPreSubmitSummary` compact layout (consolidated `Amount` row via `amountLines.join(' + ')`, removed swap-style intro copy), `PoolPage` wiring tests, docs/skills cross-links for SEC-I05 / SEC-D11 pool parity; `factory` test formatting-only hunk. **Method:** Diff-only review with sink tracing for attacker-controlled inputs (`amountLines`, `pairLabel`, `actionLabel`, `chainFullLabel`). Compared against existing `SwapPreSubmitSummary` / `PoolPage` provide-withdraw wiring (unchanged in this MR). **Outcome:** `FINDINGS: 0` medium+ ### Summary Security review: **no medium+ findings** on this diff. - **XSS / HTML injection:** `SummaryRow` renders all props as React text nodes (`{value}`); no `dangerouslySetInnerHTML`. Unchanged pattern from prior `PoolPreSubmitSummary`; indexer/CW20 symbol trust is pre-existing and identical to other pool/swap surfaces. - **Signing confusion (SEC-D11):** The four labeled security anchors remain — Action, Pair, Amount (now one consolidated row), Chain — with stable `data-testid`s. Removing the intro paragraph is UX compaction; chain identification and pair context are preserved. Summary values still snapshot form state (`amountA`/`amountB`/`lpAmount` + `displayLabel` wiring in `PoolPage.tsx` is outside this diff). - **Consolidated amount delimiter:** New `join(' + ')` could let a malicious CW20/indexer symbol containing ` + ` blur amount boundaries in the summary line; separate form inputs and wallet signing still reflect actual assets/amounts. Treat as pre-existing metadata-display trust, not a new executable sink — below medium threshold. - **Injection / SSRF / authz / secrets:** Not introduced; changes are presentational React + tests + documentation. **Inline threads:** None (no findings). **Prior security-review comments:** Reviewed note on `81fc27c`; findings still do not apply after rebase to `b2b067ec` (no new sinks introduced).

PlasticDigits (Migrated from gitlab.com) merged commit 772a7dec45 into main

2026-07-01 00:39:51 +00:00

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 00:41:31 +00:00

(Migrated from gitlab.com)

Conflict resolution

Merged latest main into qa/462-pool-presign-summary-refine (commit 84ff936b). The prior conflict state was already cleared in an earlier merge (b2b067ec); this update integrates main through 9b98249d (#451 factory-address docs) with a clean merge — no file-level conflicts remained.

Sanity checks:

PoolPreSubmitSummary.test.tsx + PoolPage.test.tsx — 22/22 passed
npx tsc --noEmit — clean

## Conflict resolution Merged latest `main` into `qa/462-pool-presign-summary-refine` (commit `84ff936b`). The prior conflict state was already cleared in an earlier merge (`b2b067ec`); this update integrates `main` through `9b98249d` (#451 factory-address docs) with a clean merge — no file-level conflicts remained. **Sanity checks:** - `PoolPreSubmitSummary.test.tsx` + `PoolPage.test.tsx` — 22/22 passed - `npx tsc --noEmit` — clean

PlasticDigits commented

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

(Migrated from gitlab.com)

mentioned in issue #462