explorer-monorepo/frontend/FRONTEND_TASKS_AND_REVIEW.md

# Frontend: Full Task List (Critical → Optional) + Detail Review

**Completed (as of last pass):** C1–C4, M1–M4, H4, H5, L2, L4, and React `useEffect` dependency warnings (useCallback + deps). H1/H2/H3: SPA already uses `escapeHtml` and `encodeURIComponent` in breadcrumbs, watchlist, block cards, tx rows, and API error messages; block breadcrumb identifier now escaped. M2: Table has `keyExtractor`; addresses and transactions pages pass stable keys; search uses stable result keys. M3: Named constants (FETCH_TIMEOUT_MS, RPC_HEALTH_TIMEOUT_MS, FETCH_MAX_RETRIES, RETRY_DELAY_MS, API_BASE). H4: `rpcCall` uses `getRpcUrl()`. H5: `_blocksScrollAnimationId` cancelled in `switchToView` and before re-running `loadLatestBlocks`. L4: `addresses.ts` and `transactions.ts` API modules in use.

**Full parallel mode:** Tasks in the same tier can be executed in parallel by different owners. Dependencies are called out where a later task requires an earlier one.

---

## Quick reference (all tasks)

| Priority | ID | One-line description |
|----------|----|----------------------|
| Critical | C1 | Address page: check `response.ok` before setting state |
| Critical | C2 | Transaction detail page: check `response.ok` before setting state |
| Critical | C3 | Search page: check `response.ok` before setting results |
| Critical | C4 | Blocks API: confirm/normalize backend response shape `{ data }` |
| High | H1 | SPA: `escapeHtml(shortenHash(...))` and escape API text in all innerHTML |
| High | H2 | SPA: safe breadcrumb/href (encode identifier, contract, addr in URLs) |
| High | H3 | SPA: escape dynamic values in all onclick attributes |
| High | H4 | SPA: use `getRpcUrl()` in `rpcCall()` for failover |
| High | H5 | SPA: cancel blocks scroll `requestAnimationFrame` on view change |
| Medium | M1 | React: validate block number in `blocks/[number].tsx` |
| Medium | M2 | React: stable list keys (Table + search results) |
| Medium | M3 | SPA: named constants for timeouts/retries |
| Medium | M4 | Blocks API: normalizer for backend shape (if needed after C4) |
| Low | L1 | SPA: extract shared block card markup helper |
| Low | L2 | SPA: DEBUG flag for console logs |
| Low | L3 | A11y: live region for error messages |
| Low | L4 | React: shared API modules for transactions/addresses |

---

## Part 1: Task List (Critical → Optional)

### CRITICAL (P1) – Fix first; blocks correct behavior or security

| ID | Task | Owner / parallel | Notes |
|----|------|------------------|--------|
| **C1** | **Address page: check `response.ok`** – In `src/pages/addresses/[address].tsx`, before `response.json()` and `setAddressInfo(data.data)`, check `response.ok`. On non-2xx: do not set state; set error/empty and optionally `setLoading(false)`. | 1 | Prevents treating error body as address data. |
| **C2** | **Transaction detail page: check `response.ok`** – In `src/pages/transactions/[hash].tsx`, same pattern: check `response.ok` before parsing and `setTransaction(data.data)`. Handle 404/5xx without setting transaction state. | 1 | Same bug as C1. |
| **C3** | **Search page: check `response.ok`** – In `src/pages/search/index.tsx`, check `response.ok` before `setResults(data.results \|\| [])`. On failure, set empty results and optionally show a message. | 1 | Avoids showing error payload as results. |
| **C4** | **Blocks API response shape** – Confirm backend for `/api/v1/blocks` returns `{ data: Block[] }` (or document contract). If it returns `{ items: [] }` or similar, add a normalizer in `src/services/api/blocks.ts` (e.g. `response.data = response.items ?? response.data`) so `response.data` is always an array for list. | 1 | Prevents `blocks.map` / `recentBlocks.map` runtime errors. |

**Parallel:** C1, C2, C3, C4 can all be done in parallel.

---

### HIGH (P2) – Security and correctness; no known exploit but reduces risk

| ID | Task | Owner / parallel | Notes |
|----|------|------------------|--------|
| **H1** | **SPA: escape all `shortenHash(...)` in `innerHTML`** – Every place that assigns to `innerHTML` and includes `shortenHash(hash|address|from|to|identifier|...)` must use `escapeHtml(shortenHash(...))`. Locations (grep): breadcrumbs (2243, 2247, 2254, 2259, 2280, 2284, 2288, 2292), block cards (2628), transaction rows (2794–2796, 2909, 2994–2996), bridge table (3108, 3144), internal tx/logs (3526, 3543), token balances (3746), address internal txs (3774), address tx table (3832), token detail (3887, 3905), NFT detail (3935, 3943), statusEl (1639, 1722). Also escape any other API-derived text in those same innerHTML strings (e.g. `displayBalance`, `val` if from API). | 1 | XSS defense in depth. |
| **H2** | **SPA: safe breadcrumb and `href` attributes** – In `updateBreadcrumb`, ensure `identifier` in `href="#/address/' + identifier + '"` cannot break the attribute. Use a safe encoding (e.g. `encodeURIComponent(identifier)` for the path segment, or ensure identifier is validated hex address). Same for token/nft breadcrumb links. | 1 | Prevents attribute breakout. |
| **H3** | **SPA: safe `onclick` attribute values** – Every `onclick="showAddressDetail('...')"` / `showBlockDetail` / `showTransactionDetail` that injects a dynamic value (address, hash, block number) must use an escaped value so a quote in the value cannot break the attribute. Use `escapeHtml(address)` (or equivalent) for the value inside the quoted string. Applies to all dynamic onclick handlers in index.html (see grep list: 3108, 3144, 3526, 3543, 3638, 3746, 3774, 3832, 3887, 3905, 3935, 3943, and template literals with `${address}`, `${hash}`, `${from}`, `${to}`, etc.). | 1 | Prevents attribute injection / XSS. |
| **H4** | **RPC failover in `rpcCall`** – In `public/index.html`, change `rpcCall` to use `getRpcUrl()` (await) instead of a single RPC_IP/RPC_FQDN, so RPC calls benefit from the same health-check and failover as elsewhere. | 1 | Consistency and resilience. |
| **H5** | **Blocks scroll: cancel `requestAnimationFrame` on view change** – When leaving the blocks view (or when `loadLatestBlocks` runs again), cancel the animation loop. Store the `requestAnimationFrame` id (e.g. in a variable or on `scrollContainer.dataset`) and call `cancelAnimationFrame(id)` when switching view or before re-running the block list render. | 1 | Avoids leaking animation loop and unnecessary work. |

**Parallel:** H1, H2, H3 can be done together; H4 and H5 are independent and can run in parallel with each other and with H1–H3.

---

### MEDIUM (P3) – Quality and maintainability

| ID | Task | Owner / parallel | Notes |
|----|------|------------------|--------|
| **M1** | **Block number validation (React)** – In `src/pages/blocks/[number].tsx`, validate `params.number`: reject non-numeric or NaN; show a clear “Invalid block number” (or “Block number required”) message instead of calling API with 0. | 1 | Better UX. |
| **M2** | **Stable list keys (React)** – In `src/components/common/Table.tsx`, accept an optional `keyExtractor` prop; in pages that use Table (e.g. addresses, blocks), pass a stable key (e.g. `tx.hash`, `block.number`). In search results, use `key={result.data?.hash ?? result.data?.address ?? index}` instead of `key={index}`. | 1 | Avoids React reconciliation issues. |
| **M3** | **SPA: named constants** – In `public/index.html`, replace magic numbers with named constants at the top of the script: e.g. `FETCH_TIMEOUT_MS = 15000`, `RPC_HEALTH_TIMEOUT_MS = 5000`, `FETCH_MAX_RETRIES = 3`, `RETRY_DELAY_MS = 1000`. | 1 | Maintainability. |
| **M4** | **API response normalization (blocks)** – If backend is Blockscout-style, add in `blocks.ts` (or client): normalize `{ items: [] }` → `{ data: [] }` so all consumers get a consistent shape without each page handling both. | 1 | Depends on C4 decision; can follow C4. |

**Parallel:** M1, M2, M3, M4 can be done in parallel (M4 after C4 if adding normalizer).

---

### LOW / OPTIONAL (P4)

| ID | Task | Owner / parallel | Notes |
|----|------|------------------|--------|
| **L1** | **SPA: extract shared block card markup** – Introduce a single helper (e.g. `createBlockCardHtml(block, options)`) used by home stats, blocks list, and any other block card HTML to reduce duplication. | 1 | Maintainability. |
| **L2** | **SPA: DEBUG flag for console** – Gate `console.log`/`console.warn` (and optionally `console.error`) behind a flag (e.g. `window.DEBUG_EXPLORER`) so production builds can suppress verbose logs. | 1 | Optional. |
| **L3** | **A11y: live region for errors** – Add an `aria-live="polite"` (or `assertive`) region for error messages and retry buttons so screen readers announce them. | 1 | Accessibility. |
| **L4** | **Shared API modules (React)** – Add `src/services/api/transactions.ts` and `addresses.ts` (or similar) using the same `apiClient`, and refactor `addresses/[address].tsx` and `transactions/[hash].tsx` to use them with consistent error handling. | 1 | Consistency. |

**Parallel:** L1–L4 independent; all optional.

---

## Part 2: Detail Review – Misses, Gaps, Additional Fixes

### 2.1 Additional React fetch bugs (misses from original review)

- **`transactions/[hash].tsx`** – Same pattern as addresses: no `response.ok` check; `setTransaction(data.data)` can run on 4xx/5xx. **Action:** Add to critical list as **C2** (done above).
- **`search/index.tsx`** – No `response.ok` check; `setResults(data.results || [])` can set error payload. **Action:** Add as **C3** (done above).

### 2.2 SPA: Gaps in escapeHtml / innerHTML

- **Wallet status (1639, 1722)** – `statusEl.innerHTML` uses `shortenHash(userAddress)`. If `userAddress` were ever from an untrusted source, it should be escaped. **Action:** Use `escapeHtml(shortenHash(userAddress))` for consistency (in **H1**).
- **loadGasAndNetworkStats (2509)** – `el.innerHTML` uses `gasGwei`, `blockTimeSec`, `tps`. These are from API; escaping is low risk but recommended for defense in depth. **Action:** Escape these values (in **H1** or small follow-up).
- **Token list: `#/token/' + contract`** – The `contract` in `href="#/token/' + contract + '"` can break the attribute if it contains a quote. **Action:** Encode or validate; include in **H2** (safe href/attributes).
- **External link (3800)** – `'https://explorer.d-bis.org/address/' + addr + '/contract'` – `addr` should be validated or encoded so the URL cannot be malformed. **Action:** Use `encodeURIComponent(addr)` for the path segment (in **H2**).

### 2.3 SPA: onclick and attribute injection

- **All dynamic onclick handlers** – Values like `address`, `hash`, `from`, `to`, `block`, `txHash`, `contract`, `contractAddress` are interpolated into onclick strings. A single quote in any of them breaks the attribute and can lead to script execution. **Action:** Consistently use `escapeHtml(value)` for every dynamic part inside the quoted argument (in **H3**). Example: `onclick="showAddressDetail('" + escapeHtml(address) + "')"` (and equivalent for template literals).

### 2.4 SPA: requestAnimationFrame leak (clarification)

- The blocks scroll animation starts inside `loadLatestBlocks()` and is never cancelled. When the user navigates to Transactions or Home, the blocks view is hidden but the animation loop keeps running. **Action:** Store the rAF id (e.g. `let scrollAnimationId` in a scope that can be cleared when switching view). When `switchToView` or the blocks view is left, or before the next `loadLatestBlocks` run, call `cancelAnimationFrame(scrollAnimationId)`. Optionally clear the interval/timeout if any. **In task H5.**

### 2.5 React: API shape and Table key (additional)

- **Home page `recentBlocks`** – Uses `response.data` from `blocksApi.list`; if `response.data` is undefined (e.g. backend returns `items`), `recentBlocks.map` will throw. **Action:** Covered by **C4** and **M4** (normalize in client).
- **Table key** – Using `rowIndex` is acceptable for static lists but can cause unnecessary re-renders or focus issues when list order changes. **Action:** **M2** (stable keys).
- **Search results key** – `key={index}` is weak when results can change; prefer `key={result.data?.hash ?? result.data?.address ?? result.data?.number ?? index}`. **Action:** Include in **M2**.

### 2.6 Additional checks performed

- **rpcCall** – Confirmed it does not use `getRpcUrl()`; single URL used. **Action:** **H4**.
- **Breadcrumb identifier** – Multiple places use `identifier` in innerHTML and in `href`; both display and href need to be safe. **Action:** **H1**, **H2**.
- **No other raw `dangerouslySetInnerHTML`** in React components; no additional React XSS findings.
- **transactions/[hash].tsx** – No validation of `hash` format (e.g. 0x + 64 hex); invalid hash could trigger odd API/UI behavior. Optional: add validation and show “Invalid transaction hash” (can be P4).

### 2.7 Summary of additional fixes required

| Category | Fix |
|----------|-----|
| React fetch | C2 (transaction detail), C3 (search) – check `response.ok` and handle errors. |
| SPA innerHTML | H1 – include statusEl (1639, 1722), loadGasAndNetworkStats (2509); escape all shortenHash and API-derived text. |
| SPA href/attributes | H2 – encode/validate `identifier` and `contract` in hrefs; encode `addr` in external URL. |
| SPA onclick | H3 – escape every dynamic value in onclick (address, hash, from, to, block, txHash, contract, etc.). |
| SPA rAF | H5 – store and cancel requestAnimationFrame when leaving blocks view or re-running loadLatestBlocks. |
| Optional | Validate tx hash in transactions/[hash].tsx (P4); add transactions/addresses API modules (L4). |

---

## Part 3: Execution Order (Full Parallel Mode)

- **Wave 1 (all parallel):** C1, C2, C3, C4, H1, H2, H3, H4, H5.
- **Wave 2 (after C4/M4 if normalizer added):** M1, M2, M3, M4.
- **Wave 3 (optional):** L1, L2, L3, L4.

No task in Wave 2 or 3 blocks another within the same wave; only M4 may depend on C4’s decision (normalize in client vs. backend contract).