test(wallet-service): isolate tests making real fullnode HTTP calls

[andreabadesso]

Motivation

While fixing CI on #370, we discovered the suite was silently making real HTTP requests to the public mainnet fullnode (`node1.mainnet.hathor.network`, our CI `DEFAULT_SERVER`). Two offenders:

• `tests/txProposalUtxoUnlock.test.ts` — failed loudly when axios bumped to 1.x (wouldn't tolerate GET with body)
• `tests/api.test.ts` `POST /tx/proposal` CORS test — silently hitting the fullnode every run (CORS test only asserts on headers, so a 500 still passes)

Root cause is the same in both: handlers call `getFullnodeData()` → `fullnode.version()` at the top of the handler body before any input validation. `src/fullnode.ts` imports `axios` directly, so the in-test mock on `hathorLib.axios.createRequestInstance` does not intercept it. When `version_data` isn't seeded in the DB, the call goes out over the network for real.

Changes

1. `tests/utils.ts` — added `seedFullnodeVersionData(mysql, overrides?)` helper with a sensible default `FullNodeApiVersionResponse`. Replaces the ~20-line inline literals tests used to copy-paste.
2. `tests/txProposalUtxoUnlock.test.ts` — now calls `seedFullnodeVersionData(mysql)` in `beforeEach`.
3. `tests/api.test.ts` — `POST /tx/proposal` CORS test calls `seedFullnodeVersionData(mysql)` before invoking the handler.
4. `tests/jestSetup.ts` — patches `http.request` / `https.request` to throw a clear error on any outbound request. This turns silent network escapes into loud, actionable test failures. An `ALLOWED_HOSTS` escape hatch is exposed (intentionally empty for the unit suite).

Why the global block matters

The CORS test had been silently reaching out to the mainnet fullnode for an unknown amount of time. `_testCORSHeaders` only checks response headers, so a 500 from a dead fullnode still passes the assertion. Without a global block, this class of bug is invisible: tests pass locally, tests pass in CI, but the suite is doing real network I/O on every run. With the block in place, the same mistake fails immediately with a message pointing at the fix.

Audit scope

Swept `packages/wallet-service/tests/` and `packages/daemon/tests/` for the same anti-pattern:

• Wallet-service tests touching `fullnode.*` (fullnodeProxy, healthcheck, /version, getTokenDetails) already mock via `jest.spyOn` or seed `version_data`. Clean.
• Daemon tests: `services.test.ts` does `jest.mock('axios', ...)` globally; `HealthCheckActor.test.ts` spies `axios.post`. Integration tests run against Docker simulators. Clean.

Only the two wallet-service cases were actually making real outbound calls.

Acceptance Criteria

• No test makes implicit HTTP calls to the public fullnode
• Reusable `seedFullnodeVersionData` helper replaces duplicated literals
• New unmocked HTTP escape attempts fail loudly, with a message that points at the fix

Checklist

• If you are requesting a merge into `master`, confirm this code is production-ready and can be included in future releases as soon as it gets merged
• Make sure either the unit tests and/or the QA tests are capable of testing the new features
• Make sure you do not include new dependencies in the project unless strictly necessary and do not include dev-dependencies as production ones. More dependencies increase the possibility of one of them being hijacked and affecting us.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Tests
  • Added runtime guards to intercept and block outbound HTTP/HTTPS requests during test execution.
  • Added test utilities for seeding fullnode version data into the test database cache across multiple test suites.

[coderabbitai]

Warning

Rate limit exceeded

@andreabadesso has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 0 minutes and 36 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 0 minutes and 36 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1bb4c845-65e5-4ba2-b0d7-7cb2e4d2e325

📥 Commits

Reviewing files that changed from the base of the PR and between 08f9706 and 05522c2.

📒 Files selected for processing (1)

• packages/wallet-service/tests/utils.ts

📝 Walkthrough

Walkthrough

Test infrastructure enhancements that prevent real HTTP/HTTPS calls during test execution by installing runtime guards and providing utilities to seed the database cache with fullnode version data, allowing tests to avoid external network dependencies.

Changes

Cohort / File(s)	Summary
Test Setup & Guards packages/wallet-service/tests/jestSetup.ts	Installed runtime intercepts for http/https.request and .get methods to block outbound calls. Validates destination hostname against an allowlist; throws detailed error indicating tests must mock or seed DB cache. Explicitly wraps get methods to prevent bypass.
Test Utilities packages/wallet-service/tests/utils.ts	Added DEFAULT_TEST_VERSION_DATA constant and seedFullnodeVersionData() helper function to insert fullnode version payloads into the version data table, enabling tests to resolve version information from database cache.
Test Cases Using New Utilities packages/wallet-service/tests/api.test.ts, packages/wallet-service/tests/txProposalUtxoUnlock.test.ts	Integrated seedFullnodeVersionData() calls in test setup (beforeEach or before specific test) to populate database cache before test execution.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

enhancement

Suggested reviewers

• pedroferreira1
• luislhl

Poem

🐰 A guard at the gate now stands tall,
Blocking stray HTTP calls,
With seeds sown in the cache so deep,
Your tests run isolated and clean,
No network noise—just what you need! 🌱✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: isolating tests by preventing real fullnode HTTP calls through seeding DB cache and installing runtime guards.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/txproposal-utxo-unlock-test-network-isolation

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

Updates the wallet-service unit test suite to prevent accidental real HTTP calls to public fullnodes by seeding cached version_data where needed and adding a global outbound network blocker in Jest setup.

Changes:

• Added DEFAULT_TEST_VERSION_DATA and seedFullnodeVersionData(mysql, overrides?) helper to consistently seed version_data for tests.
• Updated specific tests (txProposalUtxoUnlock and API CORS test for POST /tx/proposal) to seed version data before invoking handlers that call getFullnodeData().
• Patched Jest setup to throw on outbound http.request / https.request during unit tests to catch network escapes early.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
packages/wallet-service/tests/utils.ts	Introduces reusable default fullnode version payload + seeding helper for version_data.
packages/wallet-service/tests/txProposalUtxoUnlock.test.ts	Seeds version_data in beforeEach to prevent real fullnode calls during tx proposal create.
packages/wallet-service/tests/api.test.ts	Seeds version_data before the POST /tx/proposal CORS header test.
packages/wallet-service/tests/jestSetup.ts	Adds a global outbound HTTP/HTTPS request blocker for the unit test suite.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[andreabadesso]

Addressed Copilot's three comments in f01d1df:

• tests/jestSetup.ts:69 — also patch http.get / https.get (they kept an internal reference to the original request and would have bypassed the blocker). get now delegates through the patched request and calls .end() itself, so allow-listed hosts behave the same as before.
• tests/jestSetup.ts:40 — normalize arg.host via new URL('http://' + host).hostname before matching ALLOWED_HOSTS. Strips port and handles IPv6 bracket form consistently.
• tests/utils.ts:903 — seedFullnodeVersionData now uses getUnixTimestamp() to match the rest of the codebase.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

packages/wallet-service/tests/utils.ts (1)

857-902: Optional: consider upsert in addToVersionDataTable for helper safety.

seedFullnodeVersionData is now the canonical seed path and will typically run in beforeEach after cleanDatabase. However, addToVersionDataTable uses a plain INSERT and the version_data row is keyed on id = 1, so calling the helper twice in a single test (e.g., seeding in beforeEach plus an explicit override later) would fail with a duplicate key error. The production updateVersionData at packages/wallet-service/src/db/index.ts:1619 already uses ON DUPLICATE KEY UPDATE. Routing through updateVersionData (or switching addToVersionDataTable to upsert) would make the helper safer to call multiple times with overrides.

 export const seedFullnodeVersionData = async (
   mysql: ServerlessMysql,
   overrides: Partial<FullNodeApiVersionResponse> = {},
 ): Promise<void> => {
-  await addToVersionDataTable(mysql, getUnixTimestamp(), { ...DEFAULT_TEST_VERSION_DATA, ...overrides });
+  await updateVersionData(mysql, getUnixTimestamp(), { ...DEFAULT_TEST_VERSION_DATA, ...overrides });
 };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/wallet-service/tests/utils.ts` around lines 857 - 902,
addToVersionDataTable currently does a straight INSERT for id=1 which causes
duplicate-key failures when called multiple times; change it to perform an
upsert or reuse the existing DB helper: either (A) modify addToVersionDataTable
to use the same ON DUPLICATE KEY UPDATE semantics as updateVersionData
(preserving id, timestamp and data fields) so repeated calls replace the row, or
(B) have seedFullnodeVersionData call the existing updateVersionData function
instead of addToVersionDataTable; update references to
addToVersionDataTable/seedFullnodeVersionData accordingly so tests can safely
call the helper multiple times.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/wallet-service/tests/utils.ts`:
- Around line 873-890: DEFAULT_TEST_VERSION_DATA currently captures
process.env.NETWORK at module load, causing stale values if tests mutate the env
later; update the code so the network is computed at access time (e.g., replace
the hard-coded network property in DEFAULT_TEST_VERSION_DATA with a getter or
remove it and inline computation into seedFullnodeVersionData) so that functions
like seedFullnodeVersionData use process.env.NETWORK ?? 'mainnet' at call time
rather than at import time.

---

Nitpick comments:
In `@packages/wallet-service/tests/utils.ts`:
- Around line 857-902: addToVersionDataTable currently does a straight INSERT
for id=1 which causes duplicate-key failures when called multiple times; change
it to perform an upsert or reuse the existing DB helper: either (A) modify
addToVersionDataTable to use the same ON DUPLICATE KEY UPDATE semantics as
updateVersionData (preserving id, timestamp and data fields) so repeated calls
replace the row, or (B) have seedFullnodeVersionData call the existing
updateVersionData function instead of addToVersionDataTable; update references
to addToVersionDataTable/seedFullnodeVersionData accordingly so tests can safely
call the helper multiple times.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a464f1d0-912b-46cb-b62e-2ccd01739933

📥 Commits

Reviewing files that changed from the base of the PR and between e25788e and 08f9706.

📒 Files selected for processing (4)

• packages/wallet-service/tests/api.test.ts
• packages/wallet-service/tests/jestSetup.ts
• packages/wallet-service/tests/txProposalUtxoUnlock.test.ts
• packages/wallet-service/tests/utils.ts