# Explorer Code Review

**Date:** 2025-02  
**Scope:** Backend (Go), Frontend (Next.js + SPA), libs, deployment, CI.

---

## 1. Architecture Overview

| Layer | Tech | Purpose |
|-------|------|--------|
| **API** | Go 1.22, net/http | REST API (blocks, transactions, addresses, search, stats, Etherscan compat, auth, feature flags). Tiered tracks (1–4) with optional/required auth. |
| **Indexer** | Go, go-ethereum, pgx | Listens to chain (RPC/WS), processes blocks/txs, writes to PostgreSQL. |
| **Frontend (live)** | Vanilla JS SPA | `frontend/public/index.html` — single HTML + inline script, deployed at https://explorer.d-bis.org. Uses Blockscout-style API, ethers.js from CDN, VMID 2201 RPC. |
| **Frontend (dev)** | Next.js 15, React, TypeScript | `frontend/src/` — app + pages router, dev/build only; uses shared libs (api-client, ui-primitives). |
| **Libs** | In-repo under `backend/libs/`, `frontend/libs/` | go-pgconfig, go-logging, go-chain-adapters, go-rpc-gateway, go-http-middleware, go-bridge-aggregator; frontend-api-client, frontend-ui-primitives. |

---

## 2. Backend (Go)

### 2.1 Strengths

- **Clear layering:** `cmd/main.go` → `rest.NewServer` → `SetupRoutes` / `SetupTrackRoutes`. DB, chainID, and auth are configured in one place.
- **Reusable libs:** DB config (go-pgconfig), RPC gateway (go-rpc-gateway), security headers (go-http-middleware) are extracted and used consistently.
- **Validation:** Centralized in `validation.go`: `isValidHash`, `isValidAddress`, `validateBlockNumber`, `validateChainID`, `validatePagination`, `validateSearchQuery`. Routes validate before calling handlers.
- **Nil-DB safety:** `requireDB(w)` used in all DB-dependent handlers; tests run without a real DB and get 503 where appropriate.
- **Error contract:** `errors.go` defines `ErrorResponse` / `ErrorDetail` and `writeError`, `writeNotFound`, `writeInternalError`, etc. Most handlers use them for consistent JSON errors.
- **Track architecture:** Track 1 (public RPC gateway), 2–4 (auth + tier) with middleware; track routes and auth are clearly separated in `track_routes.go`.
- **Auth:** Wallet nonce + JWT in `auth/wallet_auth.go`; address normalization, nonce expiry, and DB-backed nonce storage.

### 2.2 Issues and Recommendations

| Item | Severity | Location | Recommendation |
|------|----------|----------|----------------|
| **Block not found response** | Low | `blocks.go` (handleGetBlockByNumber) | Uses `http.Error(w, fmt.Sprintf("Block not found: %v", err), 404)` → plain text. Prefer `writeNotFound(w, "Block")` (or equivalent) for JSON consistency. Same for `handleGetBlockByHash` if it uses `http.Error`. |
| **JWT default secret** | Medium | `server.go` | Default `JWT_SECRET` is logged as warning but still used. Ensure production always sets `JWT_SECRET` (deployment checklist / env template). |
| **CORS for /api/** | Info | `server.go` | `Access-Control-Allow-Origin: *` on all `/api/` is permissive. Acceptable for public API; tighten if you need origin allowlist. |
| **Track 1 routes in routes.go** | Info | `routes.go` | Track 1 handlers are commented out in `SetupRoutes` and only registered in `SetupTrackRoutes`. Intentional; no change needed. |
| **Indexer chainID** | Low | `indexer/main.go` | `chainID := 138` is hardcoded; `CHAIN_ID` env is not read. Add `os.Getenv("CHAIN_ID")` with fallback 138 for consistency with API. |

### 2.3 Security

- **Input validation:** Hash, address, block number, chain ID, and pagination are validated before DB/RPC.
- **Security headers:** Applied via go-http-middleware (CSP, X-Frame-Options, etc.); CSP is configurable via `CSP_HEADER` env.
- **Auth:** Nonce is stored and checked; JWT is validated; track middleware enforces tier.

---

## 3. Frontend (Next.js / React)

### 3.1 Strengths

- **API client:** Single `createApiClient` (frontend-api-client) with `get` / `getSafe` / post/put/delete; `getSafe` allows checking `ok` before setting state and avoids treating 4xx/5xx body as data.
- **Response handling:** Address and transaction detail pages use `getSafe`; search checks `response.ok`; blocks API normalizes `{ data }` / `{ items }` in `blocks.ts`.
- **UI primitives:** Card, Table, Address, Button in frontend-ui-primitives; Table supports `keyExtractor`; pages use stable keys (e.g. `tx.hash`, `block.number`).
- **Hooks:** Data-loading functions wrapped in `useCallback` with correct deps; `useEffect` deps avoid exhaustive-deps warnings.
- **Block number validation:** `blocks/[number].tsx` validates `params.number` and shows "Invalid block number" when invalid.
- **No dangerouslySetInnerHTML** in React components; no raw `dangerouslySetInnerHTML` in the reviewed code.

### 3.2 Issues and Recommendations

| Item | Severity | Location | Recommendation |
|------|----------|----------|----------------|
| **Frontend test script** | Info | `frontend/package.json` | `npm test` runs lint + type-check only. Add unit tests (e.g. React Testing Library) if you want component/API coverage. |
| **Next.js workspace warning** | Low | Build output | Multiple lockfiles (pnpm at parent, npm in repo) trigger Next.js warning. Set `outputFileTracingRoot` in next.config.js or align lockfile strategy. |
| **Transactions list API** | Low | `pages/transactions/index.tsx` | Uses raw `fetch` + `response.json()`; no `response.ok` check before `setTransactions(data.data || [])`. Consider using a transactions API module with `getSafe`-style handling (or add `response.ok` check). |

### 3.3 TypeScript and Lint

- Types are used for API responses (AddressInfo, Transaction, Block, SearchResult).
- Lint and type-check pass; no critical warnings.

---

## 4. SPA (frontend/public/index.html)

### 4.1 Strengths

- **Ethers.js loading:** Multiple CDNs with fallback; `ethersReady` event for dependent code.
- **RPC:** `getRpcUrl()` with health check and failover; `rpcCall` uses `getRpcUrl()`.
- **Constants:** `FETCH_TIMEOUT_MS`, `RPC_HEALTH_TIMEOUT_MS`, `FETCH_MAX_RETRIES`, `RETRY_DELAY_MS`, `API_BASE`; `window.DEBUG_EXPLORER` gates console.log/warn.
- **rAF cleanup:** `_blocksScrollAnimationId` cancelled in `switchToView` and before re-running block list animation.
- **XSS mitigation:** `escapeHtml` and `encodeURIComponent` used in breadcrumbs, watchlist, block cards, tx rows, and API error messages; block breadcrumb identifier escaped.
- **CSP:** Meta CSP present; comment notes `unsafe-eval` required by ethers v5 UMD.

### 4.2 Issues and Recommendations

| Item | Severity | Location | Recommendation |
|------|----------|----------|----------------|
| **File size** | Info | `index.html` | ~4.6k+ lines in one file. Hard to maintain. Consider splitting into separate JS files (e.g. `app.js`, `views.js`, `rpc.js`) or migrating critical flows to the Next app. |
| **Duplicate logic** | Low | SPA vs Next | Blocks/transactions/addresses exist in both SPA and Next.js pages. Document which is canonical for production (SPA) and that Next is for dev/build only. |
| **CSP unsafe-eval** | Info | CSP | Required by ethers v5 UMD. If you upgrade to a build (e.g. ethers v6 without UMD), you can remove `unsafe-eval`. |

---

## 5. Libs (backend/libs, frontend/libs)

- **go-pgconfig:** Used by API and indexer; loads from env, builds pool config. No issues.
- **go-rpc-gateway:** Cache (memory/Redis), rate limiter (memory/Redis), RPC gateway. Track 1 uses it; no duplicate in-tree.
- **go-http-middleware:** Security headers with configurable CSP. Used in server.
- **frontend-api-client:** Axios-based client with `get`/`getSafe`/post/put/delete; ApiResponse/ApiError types. Used by blocks, addresses, transactions services.
- **frontend-ui-primitives:** Card, Table, Address, Button. Used by Next pages; Table has `keyExtractor`.

---

## 6. Deployment and CI

- **CI:** Backend tests + build; frontend install + `npm test` + build; lint job runs `go vet`, `npm run lint`, `npm run type-check`. Submodules checked out recursively.
- **Deployment:** Docs reference VMID 5000, nginx, Blockscout, RPC VMID 2201; env templates and systemd examples exist under `deployment/`.
- **Recommendation:** Ensure production sets `JWT_SECRET`, `RPC_URL`, `CHAIN_ID`, and DB env; run migrations before starting API/indexer.

---

## 7. Testing

- **Backend:** `go test ./...` passes; `api/rest` tests run without DB and accept 200/503/404 as appropriate.
- **Frontend:** `npm test` (lint + type-check) and `npm run build` succeed.
- **E2E:** Playwright (15 tests) against explorer.d-bis.org; paths, nav, breadcrumbs, block detail covered.
- **Gaps:** No backend integration tests with real DB; no frontend unit tests; indexer has no test files. Optional: add integration test job (e.g. with testcontainers) and a few React tests for critical pages.

---

## 8. Summary Table

| Area | Grade | Notes |
|------|--------|--------|
| Backend structure | A | Clear routes, validation, error handling, nil-DB safety. |
| Backend security | A- | Validation and auth in place; JWT default secret and block 404 JSON to tighten. |
| Frontend (Next) | A | Type-safe API usage, getSafe, stable keys, validation. |
| SPA (index.html) | B+ | Escape/encode in place, rAF cleanup, constants; single large file. |
| Libs | A | Used consistently; no duplication. |
| CI/CD | A | Tests, build, lint; submodules. |
| Docs | A- | README, testing, extraction plan, frontend task list. |

**Overall:** The explorer codebase is in good shape: clear architecture, consistent validation and error handling, and sensible use of shared libs. The main follow-ups are minor (block 404 JSON, indexer CHAIN_ID env, transactions list response.ok, and optional tests/docs).

---

## 9. Recommended next steps – completed

| Step | Status |
|------|--------|
| Transactions list: use safe response (ok check) | Done: `transactionsApi.listSafe` used in `frontend/src/pages/transactions/index.tsx`. |
| Production: JWT_SECRET + migrations | Done: `docs/PRODUCTION_CHECKLIST.md` and note in `deployment/ENVIRONMENT_TEMPLATE.env`. |
| Optional: frontend unit test | Done: Vitest + `frontend/libs/frontend-api-client/client.test.ts`; run `pnpm run test:unit` (after `pnpm install` in frontend). |
| Optional: backend integration test | Documented in `backend/README_TESTING.md` (current suite runs without DB; note added for optional DB/testcontainers). |

**Run verification (as of last run):**

- `cd backend && go test ./...`: all pass (api/rest, benchmarks).
- `cd frontend && npm test` (lint + type-check): pass. Test files excluded from `tsconfig` so type-check does not require Vitest.
- `cd frontend && pnpm run test:unit`: 2 Vitest tests pass (api client `getSafe`).
- `make test-e2e`: Playwright against live site; address/root tests made more resilient (timeouts and selector fallbacks). Run when needed; live URL may vary.

---

## 10. Remaining optional items (all completed)

| Item | Status |
|------|--------|
| **Next.js workspace warning** | Done: Comment added in `frontend/next.config.js`; align package manager in frontend or ignore for dev/build. (Next 14 does not support `outputFileTracingRoot` in config; standalone trace uses project root.) |
| **CORS** | Done: `CORS_ALLOWED_ORIGIN` env in `server.go`; default `*`, set to e.g. `https://explorer.d-bis.org` to restrict. Documented in `deployment/ENVIRONMENT_TEMPLATE.env`. |
| **SPA file size** | Done: main app script extracted to `frontend/public/explorer-spa.js` (~3.5k lines); `index.html` now ~1.15k lines. Deploy scripts copy `explorer-spa.js` (e.g. `deploy-frontend-to-vmid5000.sh`, `deploy.sh`). |
| **SPA vs Next canonical** | Done: `README.md` states production serves the SPA, Next.js is for local dev and build validation only. |
| **CSP unsafe-eval** | Done: comment in `index.html` CSP meta updated: "Can be removed when moving to ethers v6 build (no UMD eval)." |
| **Further product work** | See `docs/EXPLORER_ADDITIONAL_RECOMMENDATIONS.md` (i18n, event log decoding, token list, health endpoint, etc.). |