Skip to content

QA: SSOT establishment + test foundation (v1.0.12 baseline) - #2

Merged
Hiksang merged 11 commits into
mainfrom
qa/2026-05-05-v1-0-12-baseline
May 6, 2026
Merged

QA: SSOT establishment + test foundation (v1.0.12 baseline)#2
Hiksang merged 11 commits into
mainfrom
qa/2026-05-05-v1-0-12-baseline

Conversation

@Hiksang

@Hiksang Hiksang commented May 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

Two back-to-back QA cycles on top of v1.0.12 (ae5bb65):

Cycle 1 — baseline pass (3 commits): established docs/QA_WORKFLOW.md as the autonomous-QA SSOT plus CLAUDE.md as the AI-agent entrypoint, then fixed three doc/CLI drift bugs (slug hyperswaphyperswap-v3, nest correctly marked inactive, banner protocol count 3938).

Cycle 2 — test foundation (8 commits): added a Dockerfile for SSOT 2.1 compliance, four new SSOT-pinned QA test files (slug-parity, approve-safety, slippage, chain-id), one Docker-side lint fix, and two report files. Code/config behaviour change: 0 lines.

Test count: monorepo 78 → 107 (+29, +37%); defi-cli package 29 → 58 (+100%).

Highlights

  • F1 (active, follow-up): 4 DEX adapters ship swap/LP builders with hard-coded min* : 0n (15 occurrences across algebra_v3.ts, balancer_v3.ts, thena_cl.ts, uniswap_v3.ts). Every --broadcast user is currently exposed to MEV/sandwich with no floor. Snapshot-fenced via KNOWN_INFINITE_SLIPPAGE so new sites fail the build; full fix ships as a follow-up v1.0.13 patch (IDex/lp trait change + adapters + CLI --slippage flag).
  • F2 (active, follow-up): tokens/{arbitrum,ethereum}.toml exist but chains.toml does not yet route them — README marks them 🟡 staged. Tracked via KNOWN_ORPHAN_TOKEN_TABLES; resolution requires explicit Section 3 (chain-list) decision.
  • F3 (hardening): viem walletClient/publicClient created without explicit chain parameter. Currently safe (RPC auto-fetch); separate hardening PR.
  • Dockerfile ROI: the strict tsc lint inside the container caught a wrapped_native ?? "" regression that the host vitest run had passed — concrete first-cycle ROI for SSOT 2.1.

What's in this PR

  • docs/QA_WORKFLOW.md — SSOT for autonomous QA actions in this repo (211 lines).
  • CLAUDE.md — AI-agent entrypoint pointing at the SSOT (36 lines).
  • Dockerfile + .dockerignorenode:20-alpine + corepack pnpm. Default CMD = pnpm test && pnpm -r lint.
  • ts/packages/defi-cli/src/qa/:
    • slug-parity.test.ts (10 tests) — README/SKILL mirror byte-equality, count parity, slug active/inactive matrix.
    • approve-safety.test.ts (5 tests) — buildApprove encoding, infinite-approval & wrong-spender source-grep guards, defi token approve max sentinel pin.
    • slippage.test.ts (4 tests) — KNOWN_INFINITE_SLIPPAGE snapshot + aggregator quote consumption + slippage default ≤ 100 bps.
    • chain-id.test.ts (10 tests) — chains.toml × protocols × tokens integrity, KNOWN_ORPHAN_TOKEN_TABLES snapshot, cross-chain leak guard.
  • docs/qa-reports/:
    • 2026-05-05-v1-0-12-baseline.md (Cycle 1 report).
    • 2026-05-05-test-foundation.md (Cycle 2 report, refined with monorepo-wide test counts).

Test plan

  • Host: pnpm -C ts --filter @hypurrquant/defi-cli test → 58/58 pass.
  • Docker: docker build -t defi-cli-qa . && docker run --rm defi-cli-qa → 107/107 pass (defi-core 28 + defi-protocols 21 + defi-cli 58), 3 packages lint clean.
  • CLI smoke: defi --version → 1.0.12, defi status --jsonchains=5 total_active_protocols=38, defi --chain hyperevm lp discover --protocol hyperswap-v3[], defi ... --protocol hyperswapProtocol not found.
  • Reviewer to validate: F1 follow-up scoping, F2 chains.toml policy decision, F3 hardening priority.

Follow-up PRs (separate)

  1. F1 slippage fix (recommended priority: immediate)IDex trait + 4 DEX adapters + defi swap/defi lp --slippage <bps> flag. ~700–900 lines. Target: v1.0.13 patch.
  2. F2 chain-list decision — extend chains.toml for arbitrum/ethereum, or remove from README + tokens/.
  3. F3 viem chain anchor — explicit chain: parameter on createWalletClient/createPublicClient.

🤖 Generated with Claude Code

Hiksang added 11 commits May 5, 2026 13:41
- docs/QA_WORKFLOW.md is the SSOT for autonomous QA actions in this repo.
- CLAUDE.md is a thin entrypoint pointing AI agents at the SSOT.
- Establishes hard rules for branch model (qa/<date>-<topic>),
  no-mainnet-funds, no-main-push, no-publish without explicit chat approval.
Stale entries in README.md and the skills/defi-cli mirrors caused
agent recipes to silently fail with "Protocol not found":

- `hyperswap` -> CLI exposes the slug as `hyperswap-v3`
- `nest`      -> CLI rejects (is_active = false in nest.toml; the
                 protocol is off-chain claim only via blaze.nest.aegas.it)

Counts corrected to match the live CLI:
- 39 protocols  -> 38 protocols (live count: 10+3+5+16+4)
- HyperEVM (11) -> HyperEVM (10)

Both mirror trees (root skills/ and ts/packages/defi-cli/skills/)
updated in lock-step. Nest is kept as a doc footnote with an
_(inactive)_ marker so the off-chain claim path is still discoverable.

No code or config changes. Found via SSOT QA pass on
qa/2026-05-05-v1-0-12-baseline. Build/test/lint all pass.
Captures SSOT Section 11 report for the qa/2026-05-05-v1-0-12-baseline
branch:

- 3 doc-only defects fixed (hyperswap slug, nest inactive guide,
  protocol count drift)
- 0 code/config changes
- 29/29 vitest pass + 3 packages lint clean on host AND on
  node:20-alpine container (SSOT Section 2.1 deviation resolved)
- 0 mainnet broadcast, 0 secret leaks

Includes deviation log, security checklist, and follow-ups
(Dockerfile addition, nest policy, push/PR approval).
docs/QA_WORKFLOW.md Section 2.1 requires QA work to run inside a
container. The repo lacked a Dockerfile, so the previous QA pass had
to fall back to the host. Adds:

- Dockerfile based on node:20-alpine. corepack honors the
  packageManager pin (pnpm@9.15.0) without a separate global install.
  Two-layer install (manifests first, sources second) keeps the
  install layer cached across source-only changes.
- .dockerignore excludes node_modules, dist, legacy Python tree
  (src/, tests/, config/, .venv), Rust artifacts, and tooling state
  to keep the build context lean.

Default CMD = `pnpm test && pnpm -r lint`, the SSOT QA gate.
Verified: `docker build -t defi-cli-qa .` -> 556 MB image,
`docker run --rm defi-cli-qa` -> 29/29 tests pass, 3 packages
lint clean.
Adds ts/packages/defi-cli/src/qa/slug-parity.test.ts which would have
caught the three drift bugs found by the 2026-05-05 baseline pass at
PR time:

- 'hyperswap' (CLI exposes the slug as 'hyperswap-v3')
- 'nest'      (CLI rejects: is_active = false in nest.toml)
- '39 protocols' (CLI banner reports 38 active protocols)

Strategy: pull ground truth from Registry.loadEmbedded() rather than
hardcoding slug lists, so the test self-updates as protocols are added
or deactivated. Coverage:

1. ts/packages/defi-cli/skills/ vs root skills/ byte-equality
   (SKILL.md, references/protocols.md, references/commands.md)
2. README banner '<N> protocols' == active slug count
3. SKILL.md mirrors '<N> protocols' == active slug count
4. README protocol-table slugs are all active OR explicitly marked
   _(inactive)_ -- carve-out for the 'nest' off-chain footnote
5. SKILL.md catalogue lines (Lending/DEX/Vault/...) reference only
   active slugs
6. Registry.getProtocolsForChain() filters out is_active=false
7. commands.md does not show runnable '--protocol <slug>' usage
   examples for inactive slugs

The README scan is scoped to the '## Supported Protocols' section so
the unrelated Command Reference table (status, ows, ...) doesn't
trigger false positives.

Verified: 39/39 tests pass on host AND in node:20-alpine container.
Pins SSOT Section 7.2 invariants. Today's audit found the codebase
already compliant (every adapter passes params.amount to its
approvals[] entry, and 'defi token approve' only uses maxUint256 with
explicit '--amount max'); this test prevents silent regressions.

Coverage:

1. buildApprove encodes the exact amount with no hidden default
   across {0, 1, 1e6, 2^64, maxUint256} and decode round-trips.
2. buildApprove arity = 3 (no implicit-default drift).
3. Adapter sources do not hardcode infinite approval -- bans
     amount: maxUint256
     amount: ethers.MaxUint256
     amount: 2 ** 256
     amount: BigInt("<huge string>")
     amount: 0xfff... (60+ hex F's)
   in any defi-protocols/src/**/*.ts file.
4. Adapter sources do not assign a caller-controlled address as
   spender -- bans spender: params.{user,from,recipient,to,onBehalfOf},
   opts.user, owner, recipient. Spender must be a protocol contract
   (this.router / this.pool / this.comet / gauge / vtoken / vault).
5. CLI 'token approve' resolution is gated by the literal sentinel
   `opts.amount === "max"` (source-pinned) and only that path maps
   to maxUint256. Numeric strings -- including "0" and "1000" --
   never silently become infinite. Malformed input throws.

44/44 tests pass on host (29 existing + 10 slug-parity + 5 new).
ACTIVE FINDING (2026-05-05): 4 DEX adapters ship swap and LP builders
with effectively unlimited slippage (15 occurrences across
algebra_v3.ts, balancer_v3.ts, thena_cl.ts, uniswap_v3.ts -- all hard
'amountOutMinimum: 0n' / 'amount{0,1}Min: 0n'). Calling these in
broadcast mode exposes the user to MEV / sandwich attacks with no
floor.

Fixing all 15 sites requires threading a slippageBps (or explicit
amount{Out,0,1}Min) parameter through the IDex / lp-builder traits,
which is a breaking change for the public adapter surface. That
refactor is intentionally separated from this baseline pass and is
called out in the QA follow-up list.

What this test does today:
1. Pin every known 0n-min site as a snapshot
   (KNOWN_INFINITE_SLIPPAGE) so any *new* hard-coded zero minimum
   added to defi-protocols/src/**/*.ts fails the build.
2. Reject stale snapshot entries (a known site that has been quietly
   fixed must be removed from the set in the same commit so it
   doesn't mask other regressions).
3. Pin commands/swap.ts to keep consuming the aggregator quote so
   user-facing swap commands retain whatever slippage protection the
   aggregator provides.
4. Cap any user-controllable slippageBps default to <= 100 bps (1%)
   per SSOT 7.3.

48/48 tests pass on host (29 existing + 10 slug-parity + 5 approve +
4 slippage).
…tegrity

Pins SSOT Section 7.4 invariants at the registry layer so a tx-builder
cannot quietly land calldata on the wrong network.

Coverage:

1. chains.toml has a non-empty chain set.
2. Every chain has a positive integer chain_id.
3. chain_id values are unique within chains.toml.
4. Canonical mainnet IDs (hyperevm=999, mantle=5000, base=8453,
   bnb=56, monad=143) are anchored so a typo in chains.toml shows up
   immediately.
5. Every protocol's `chain` field references a key registered in
   chains.toml — no orphan adapters.
6. Token tables whose chain is unregistered are tracked as a
   KNOWN_ORPHAN_TOKEN_TABLES snapshot. Today: {arbitrum, ethereum}
   (README marks them 🟡 staged but chains.toml does not yet route
   them — adding routes is a Section 3 change requiring approval).
   Novel orphans = test failure.
7. getProtocolsForChain('X') only returns entries whose `chain == 'X'`
   — cross-chain leak guard.
8. wrapped_native is a non-zero 20-byte hex address per chain.
9. rpc_url is set per chain and uses http(s).
10. Floor sanity: >= 5 chains, >= 30 active protocols.

ACTIVE FINDING (separate follow-up): commands/swap.ts/executor.ts
create viem walletClient/publicClient without an explicit `chain`
parameter. viem auto-fetches chainId from the RPC, so today's
behaviour is correct, but offline-signing or MITM-RPC scenarios would
silently sign with whatever chainId the RPC responds with. Tracked
in the QA report as a hardening item.

58/58 tests pass on host (29 existing + 10 slug-parity + 5 approve +
4 slippage + 10 chain-id).
ChainConfig.wrapped_native is declared as `?: string`, so the
strict-mode tsc lint inside the Docker QA gate flagged the regex
test as receiving `string | undefined`. Coerce to "" and surface a
"missing" diagnostic before the format check. Behaviour unchanged on
valid configs; lint now passes in node:20-alpine.
Captures the second 2026-05-05 QA cycle on
qa/2026-05-05-v1-0-12-baseline:

- 5 commits (Dockerfile + 4 SSOT-pinned test files).
- Test count 29 -> 58 (+100%).
- 2 active findings surfaced and snapshot-fenced rather than
  silently fixed:
    F1: 4 DEX adapters ship swap/LP builders with hard-coded
        infinite slippage (15 occurrences). Fix needs an IDex/lp
        trait change and is split out as a follow-up PR.
    F2: tokens/{arbitrum,ethereum}.toml exist but the chains.toml
        routes do not — README marks them 🟡 staged. SSOT 3 forbids
        chain-list edits without explicit user approval.
- 1 hardening item (F3): viem walletClient/publicClient created
  without an explicit `chain` parameter. Currently safe (RPC
  auto-fetch) but worth pinning in a follow-up.
- 0 code or config changes; 0 mainnet broadcast; 0 secret leaks.

Verified: 58/58 tests pass on host AND in node:20-alpine container,
3 packages lint clean.
Updates the 2026-05-05-test-foundation.md report after the final
Docker run revealed a fuller picture than the host-only verification:

- Commit count corrected from 5 to 7 (Dockerfile + 4 tests + fix +
  prior report commit; this update is the 8th).
- Test totals expanded:
    host:   defi-cli 58/58 (the only package the host quick-check
            ran)
    docker: defi-core 28 + defi-protocols 21 + defi-cli 58 = 107/107
- New section 'Fixed-1' documents the null-safe wrapped_native fix
  that the Docker strict tsc lint caught after host vitest had
  already passed -- the Dockerfile addition's first concrete ROI.
- Verification log split into host (cli-only quick check) vs Docker
  (monorepo-wide full gate).

No code or config changes; documentation refinement only.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant