Skip to content

fix: migrate legacy tests, log silent catches, harden test config, fix TOCTOU in deductCredits - #506

Merged
DeFiVC merged 2 commits into
ChainLearnOfficial:mainfrom
themancalledemma:fix/tests-logging-config-toctou-batch
Sep 30, 2026
Merged

DeFiVC merged 2 commits into
ChainLearnOfficial:mainfrom
themancalledemma:fix/tests-logging-config-toctou-batch

Conversation

@themancalledemma

Copy link
Copy Markdown
Contributor

Summary

  • Migrates the 12 remaining src/test/ files to tests/unit/ or tests/e2e/ by what each actually exercises (mocked service calls vs real HTTP via app.inject), preserving history via git mv, fixing import depths, and removing the now-unused vitest.config.ts include glob. src/test/ is fully removed.
  • Adds structured logger.warn/error calls to 8 catch blocks that previously failed silently, across resilience.ts, the auth refresh-token service, reward claim handling, and the batch enroll/mint loops. The 4 originally-named locations in lock.ts and cache/* were already fixed by prior commits before this branch was cut, confirmed by reading current main.
  • Replaces the hardcoded test-mode config fallbacks: DATABASE_URL, JWT_SECRET, and STELLAR_PLATFORM_SECRET now throw a clear error naming exactly which vars are missing instead of silently defaulting to credential-shaped strings; non-critical vars still default, but to an obviously-fake placeholder rather than something that looks real. Adds .env.test.example documenting every test-mode variable.
  • Fixes a TOCTOU race in deductCredits (admin-users.service.ts): the separate balance-check-then-UPDATE is replaced with a single atomic UPDATE whose WHERE clause enforces credits >= amount under the row lock Postgres takes for the UPDATE, so a concurrent deduction or grant can no longer race the check. A preliminary existence-only check (no balance read) remains since that part genuinely isn't racy.

closes #473
closes #474
closes #475
closes #476

Test plan

  • npx vitest run: 256 passed / 5 failed (261 total), confirmed byte-for-byte against a stashed clean-main baseline run with identical env (same failures, just at relocated paths), zero regressions, net +13 new passing tests
  • npx tsc --noEmit: 0 new errors (pre-existing errors confirmed identical to unmodified main via git stash)
  • eslint: 0 new errors
  • New deduct-credits-toctou.test.ts (6 cases): sufficient/insufficient balance, correct error detail, not-found, sequential-deduction overdraw regression, and an assertion that the fix uses a single atomic UPDATE rather than check-then-act
  • New test-mode-required-vars.test.ts (7 cases): each required var missing throws with the right message, multiple-missing lists all of them, non-critical defaults, success path
  • Local Postgres and Redis used throughout, not mocked away

themancalledemma and others added 2 commits September 29, 2026 15:53
…x TOCTOU in deductCredits

Migrates the 12 remaining src/test/ files to tests/unit/ or tests/e2e/
by what each actually exercises (mocked service calls vs real HTTP via
app.inject), preserving history via git mv and fixing import depths.
Removes the now-unused src/test include glob from vitest.config.ts.

Adds structured logger.warn/error calls to 8 catch blocks that
previously failed silently across resilience.ts, auth/refresh-token
services, reward claim handling, and batch enroll/mint loops. The 4
originally-named locations in lock.ts and cache/* were already fixed
by prior commits before this branch was cut.

Replaces the hardcoded test-mode config fallbacks in config/index.ts:
DATABASE_URL/JWT_SECRET/STELLAR_PLATFORM_SECRET now throw a clear error
naming exactly which vars are missing instead of silently defaulting
to credential-shaped strings; non-critical vars still default, but to
an obviously-fake placeholder. Adds .env.test.example documenting every
test-mode variable.

Fixes a TOCTOU race in deductCredits: the separate balance-check then
UPDATE is replaced with a single atomic UPDATE whose WHERE clause
enforces credits >= amount under the row lock, so a concurrent
deduction or grant can no longer race the check.
@DeFiVC
DeFiVC merged commit 4126fcc into ChainLearnOfficial:main Sep 30, 2026
3 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants