Verify #413: fix Advanced settings toggle for E2E #949

Merged
PlasticDigits merged 2 commits from issue-413-verify-advanced-toggle into main 2026-06-26 08:17:18 +00:00
PlasticDigits commented 2026-06-26 07:07:39 +00:00 (Migrated from gitlab.com)

Summary

  • Main already shipped retail vs Advanced Swap Settings (#413); verification found the controlled <details> disclosure did not expand under Playwright.
  • Replaced it with a button + conditional panel (aria-expanded) so hybrid book leg and indexer route check remain behind Advanced while E2E helpers can expand reliably.
  • Updated unit/E2E helpers to assert collapsed state via aria-expanded and swap-indexer-route-check mount.

Acceptance checklist

Criterion Result How verified
First open of Settings shows retail prefs only (no hop addresses, no indexer BFS) PASS SwapPage.test.tsx — shows retail prefs only when Settings first opens; Playwright screenshot issue-413-settings-retail.png
Advanced panels require explicit expand PASS aria-expanded=false when collapsed; toggle click required
Hybrid book leg functional when expanded PASS SwapPage.test.tsx hybrid tests; e2e/hybrid-swap.spec.ts UI + on-chain after fix
Indexer route check functional when expanded PASS SwapPage.test.tsx; expandSwapAdvancedSettings waits on swap-indexer-route-check
Screenshot: default collapsed retail view PASS frontend-dapp/test-results/issue-413-settings-retail.png (local verify run)
Screenshot: Advanced expanded (hybrid + route check) PASS frontend-dapp/test-results/issue-413-settings-advanced.png
make test-frontend PASS 184 files / 1084 tests
Swap E2E subset PASS playwright test --project=e2e-smoke swap.spec.ts (11); --project=e2e-tx swap-tx.spec.ts hybrid-swap.spec.ts wrap-swap.spec.ts (17 after redeploy)
Expert-mode block >30% with Advanced collapsed PASS SwapPage.test.tsx GitLab #293; E2E snapshot shows Slippage is too high + retail Enable Expert Mode with Settings closed
Hybrid max makers 256 cap unchanged PASS SwapAdvancedSettings.tsx max={256}
No alert stack on default Settings open PASS Manual/E2E: opening Settings shows slippage/deadline/expert only, no execution alerts in settings panel

Test plan

make test-frontend
CI=1 bash scripts/with-node.sh --cwd frontend-dapp -- ./node_modules/.bin/playwright test --project=e2e-smoke swap.spec.ts
CI=1 bash scripts/with-node.sh --cwd frontend-dapp -- ./node_modules/.bin/playwright test --project=e2e-tx swap-tx.spec.ts hybrid-swap.spec.ts wrap-swap.spec.ts
## Summary - Main already shipped retail vs Advanced Swap Settings (#413); verification found the controlled `<details>` disclosure did not expand under Playwright. - Replaced it with a button + conditional panel (`aria-expanded`) so hybrid book leg and indexer route check remain behind Advanced while E2E helpers can expand reliably. - Updated unit/E2E helpers to assert collapsed state via `aria-expanded` and `swap-indexer-route-check` mount. ## Acceptance checklist | Criterion | Result | How verified | |-----------|--------|--------------| | First open of Settings shows retail prefs only (no hop addresses, no indexer BFS) | **PASS** | `SwapPage.test.tsx` — `shows retail prefs only when Settings first opens`; Playwright screenshot `issue-413-settings-retail.png` | | Advanced panels require explicit expand | **PASS** | `aria-expanded=false` when collapsed; toggle click required | | Hybrid book leg functional when expanded | **PASS** | `SwapPage.test.tsx` hybrid tests; `e2e/hybrid-swap.spec.ts` UI + on-chain after fix | | Indexer route check functional when expanded | **PASS** | `SwapPage.test.tsx`; `expandSwapAdvancedSettings` waits on `swap-indexer-route-check` | | Screenshot: default collapsed retail view | **PASS** | `frontend-dapp/test-results/issue-413-settings-retail.png` (local verify run) | | Screenshot: Advanced expanded (hybrid + route check) | **PASS** | `frontend-dapp/test-results/issue-413-settings-advanced.png` | | `make test-frontend` | **PASS** | 184 files / 1084 tests | | Swap E2E subset | **PASS** | `playwright test --project=e2e-smoke swap.spec.ts` (11); `--project=e2e-tx swap-tx.spec.ts hybrid-swap.spec.ts wrap-swap.spec.ts` (17 after redeploy) | | Expert-mode block >30% with Advanced collapsed | **PASS** | `SwapPage.test.tsx` GitLab #293; E2E snapshot shows `Slippage is too high` + retail `Enable Expert Mode` with Settings closed | | Hybrid max makers 256 cap unchanged | **PASS** | `SwapAdvancedSettings.tsx` `max={256}` | | No alert stack on default Settings open | **PASS** | Manual/E2E: opening Settings shows slippage/deadline/expert only, no execution alerts in settings panel | ## Test plan ```bash make test-frontend CI=1 bash scripts/with-node.sh --cwd frontend-dapp -- ./node_modules/.bin/playwright test --project=e2e-smoke swap.spec.ts CI=1 bash scripts/with-node.sh --cwd frontend-dapp -- ./node_modules/.bin/playwright test --project=e2e-tx swap-tx.spec.ts hybrid-swap.spec.ts wrap-swap.spec.ts ```
PlasticDigits commented 2026-06-26 07:09:39 +00:00 (Migrated from gitlab.com)

Security review

Commit reviewed: 6deb64a6ff242e06fd5e99063435544a4b38879a
Scope: MR !949 — Advanced Swap Settings disclosure refactor (<details> → controlled <button> + conditional panel); E2E/unit test selector updates.

Outcome: FINDINGS: 0 medium+

Security review: no medium+ findings on this diff.

Review summary

Traced attacker-controlled inputs through changed paths:

  • SwapAdvancedSettings.tsx — Structural UI change only. Advanced panel content (hybrid book leg, indexer route check) is now conditionally mounted when open is true; no new props, handlers, or rendering sinks. indexerRouteError and route hop display remain React text children (auto-escaped); unchanged from pre-MR behavior.
  • swap-ui.ts / SwapPage.test.tsx — Test helper assertions only; no production attack surface.

No injection, authn/authz bypass, secret leakage, SSRF, XSS, or permission-boundary regressions introduced. Collapsing advanced settings does not bypass expert-mode slippage guards or on-chain swap validation (state/logic unchanged in SwapPage.tsx).

Inline threads: none (no findings).

## Security review **Commit reviewed:** `6deb64a6ff242e06fd5e99063435544a4b38879a` **Scope:** MR !949 — Advanced Swap Settings disclosure refactor (`<details>` → controlled `<button>` + conditional panel); E2E/unit test selector updates. **Outcome:** `FINDINGS: 0` medium+ Security review: no medium+ findings on this diff. ### Review summary Traced attacker-controlled inputs through changed paths: - **SwapAdvancedSettings.tsx** — Structural UI change only. Advanced panel content (hybrid book leg, indexer route check) is now conditionally mounted when `open` is true; no new props, handlers, or rendering sinks. `indexerRouteError` and route hop display remain React text children (auto-escaped); unchanged from pre-MR behavior. - **swap-ui.ts / SwapPage.test.tsx** — Test helper assertions only; no production attack surface. No injection, authn/authz bypass, secret leakage, SSRF, XSS, or permission-boundary regressions introduced. Collapsing advanced settings does not bypass expert-mode slippage guards or on-chain swap validation (state/logic unchanged in `SwapPage.tsx`). **Inline threads:** none (no findings).
PlasticDigits commented 2026-06-26 08:00:34 +00:00 (Migrated from gitlab.com)

added 5 commits

  • 6deb64a6...a399b53f - 4 commits from branch main
  • 4ca88cb4 - Merge branch 'main' into issue-413-verify-advanced-toggle

Compare with previous version

added 5 commits <ul><li>6deb64a6...a399b53f - 4 commits from branch <code>main</code></li><li>4ca88cb4 - Merge branch &#39;main&#39; into issue-413-verify-advanced-toggle</li></ul> [Compare with previous version](/PlasticDigits/cl8y-dex-terraclassic/-/merge_requests/231/diffs?diff_id=1872559844&start_sha=6deb64a6ff242e06fd5e99063435544a4b38879a)
PlasticDigits commented 2026-06-26 08:00:36 +00:00 (Migrated from gitlab.com)

Merged latest main into issue-413-verify-advanced-toggle.

Conflict resolved: frontend-dapp/e2e/helpers/swap-ui.ts — kept the branch’s aria-expanded check on swap-advanced-settings-toggle (instead of main’s route-check visibility probe) so Playwright can expand Advanced settings reliably (#413).

Sanity: SwapPage.test.tsx (27 tests) passed locally.

Merged latest `main` into `issue-413-verify-advanced-toggle`. **Conflict resolved:** `frontend-dapp/e2e/helpers/swap-ui.ts` — kept the branch’s `aria-expanded` check on `swap-advanced-settings-toggle` (instead of main’s route-check visibility probe) so Playwright can expand Advanced settings reliably (#413). Sanity: `SwapPage.test.tsx` (27 tests) passed locally.
PlasticDigits commented 2026-06-26 08:02:45 +00:00 (Migrated from gitlab.com)

Security review

Commit reviewed: 4ca88cb49f8efdc0dca782312be0d92d3e9c5788
Scope: MR !949 — Advanced Swap Settings disclosure refactor (<details> → controlled <button> + conditional panel); E2E/unit test selector updates (includes post-merge conflict resolution in swap-ui.ts).

Outcome: FINDINGS: 0 medium+

Security review: no medium+ findings on this diff.

Review summary

Re-reviewed after merge of main into issue-413-verify-advanced-toggle (head 4ca88cb). Prior review on 6deb64a still applies; merge conflict resolution in swap-ui.ts is test-helper only.

Traced attacker-controlled inputs through changed paths:

  • SwapAdvancedSettings.tsx — Structural UI change only. Advanced panel (hybrid book leg, indexer route check) is conditionally mounted when open is true; no new props, handlers, or rendering sinks. indexerRouteError and route hop display remain React text children (auto-escaped); unchanged from pre-MR behavior. Collapsing unmounts controls but does not alter parent swap state or submission guards in SwapPage.tsx.
  • swap-ui.ts / SwapPage.test.tsx — Test helper assertions only (aria-expanded, DOM presence checks); no production attack surface.

No injection, authn/authz bypass, secret leakage, SSRF, XSS, or permission-boundary regressions introduced. Expert-mode slippage block (routeSlippageBlocked) and on-chain swap validation are unchanged; hybridMaxMakers cap (max={256}) unchanged.

Inline threads: none (no findings).

## Security review **Commit reviewed:** `4ca88cb49f8efdc0dca782312be0d92d3e9c5788` **Scope:** MR !949 — Advanced Swap Settings disclosure refactor (`<details>` → controlled `<button>` + conditional panel); E2E/unit test selector updates (includes post-merge conflict resolution in `swap-ui.ts`). **Outcome:** `FINDINGS: 0` medium+ Security review: no medium+ findings on this diff. ### Review summary Re-reviewed after merge of `main` into `issue-413-verify-advanced-toggle` (head `4ca88cb`). Prior review on `6deb64a` still applies; merge conflict resolution in `swap-ui.ts` is test-helper only. Traced attacker-controlled inputs through changed paths: - **SwapAdvancedSettings.tsx** — Structural UI change only. Advanced panel (hybrid book leg, indexer route check) is conditionally mounted when `open` is true; no new props, handlers, or rendering sinks. `indexerRouteError` and route hop display remain React text children (auto-escaped); unchanged from pre-MR behavior. Collapsing unmounts controls but does not alter parent swap state or submission guards in `SwapPage.tsx`. - **swap-ui.ts / SwapPage.test.tsx** — Test helper assertions only (`aria-expanded`, DOM presence checks); no production attack surface. No injection, authn/authz bypass, secret leakage, SSRF, XSS, or permission-boundary regressions introduced. Expert-mode slippage block (`routeSlippageBlocked`) and on-chain swap validation are unchanged; `hybridMaxMakers` cap (`max={256}`) unchanged. **Inline threads:** none (no findings).
PlasticDigits commented 2026-06-26 08:17:18 +00:00 (Migrated from gitlab.com)

mentioned in commit 9bc362308d

mentioned in commit 9bc362308dbcca9a729bd12effaf6dcfadda186f
PlasticDigits (Migrated from gitlab.com) merged commit 9bc362308d into main 2026-06-26 08:17:18 +00:00
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!949
No description provided.