Skip to content

test(core): add tests for RPC failover behavior mid-cycle - #554

Closed
flexocode442 wants to merge 827 commits into
TegoLabs:mainfrom
flexocode442:fix/511-rpc-failover-tests
Closed

flexocode442 wants to merge 827 commits into
TegoLabs:mainfrom
flexocode442:fix/511-rpc-failover-tests

Conversation

@flexocode442

Copy link
Copy Markdown

What does this PR do?

Adds comprehensive test coverage for multi-endpoint RPC failover behavior during a monitor cycle, ensuring the cycle's fault-isolation guarantees hold even when the RPC layer fails over mid-cycle.

Closes #511

Why?

Once multi-endpoint RPC failover exists (the sibling issue in this phase), the monitor cycle's per-contract fault isolation must remain intact even when the RPC layer itself fails over mid-cycle — not just when a whole contract's RPC call fails outright. These tests verify that invariant.

Does this touch secret-key handling or transaction submission?

  • Yes — see notes above
  • No

Checklist

  • Tests pass (npm test)
  • Type check passes (npx tsc --noEmit)
  • Lint passes (npm run lint)
  • Tests cover the new functionality (TDD preferred — see CONTRIBUTING.md)
  • No unnecessary dependencies added
  • Commit messages follow conventional format
  • No console.log in core logic
  • ADR added if this is a significant design decision (see docs/adr)
  • E2E sandbox tested, if this touches RPC or daemon behavior (see docs/e2e-sandbox.md)

Changes

Test Coverage (tests/core/monitor.test.ts)

  • Mid-cycle failover to a working second endpoint — Simulates a multi-endpoint RPC client where the first two contracts succeed against endpoint 1, and the third contract triggers an internal client-level failover (endpoint 1 fails → transparent fallback to endpoint 2). Asserts the cycle completes with all 3 contracts checked, 3 entries updated, and 0 errors.

  • All endpoints failing for one contract does not block others — Simulates a multi-endpoint client where one contract exhausts all configured endpoints (throws "All 2 RPC endpoints failed"). Asserts 3 contracts checked, 2 entries updated, 1 error referencing the failing contract, and that the other contracts' TTLs are still updated.

Verification Results

npm test -- tests/core/monitor.test.ts
✅ 52/52 passed (2 new failover tests)

npm test
✅ 997 passed / 1 skipped across 77 test files — no regressions
Acceptance Criteria Status
Mid-cycle failover to a working second endpoint results in a complete, accurate cycle result ✅ Test passes — 3 contracts checked, 3 entries updated, 0 errors
All configured endpoints failing for one contract does not prevent other contracts from being processed in the same cycle ✅ Test passes — 3 contracts checked, 2 entries updated, 1 error, other contracts updated

Olagoke22 and others added 30 commits June 29, 2026 16:11
…FIXED (TegoLabs#270)

* TegoLabs#145 feat(core): integrate HashiCorp Vault for key retrieval FIXED

* chore(tests): split mock secrets to evade GitGuardian false positives

* chore(tests): split more mock secrets to evade GitGuardian

---------

Co-authored-by: AbdulmalikAlayande <114596864+AbdulmalikAlayande@users.noreply.github.com>
…FIXED (TegoLabs#270)

* TegoLabs#145 feat(core): integrate HashiCorp Vault for key retrieval FIXED

* chore(tests): split mock secrets to evade GitGuardian false positives

* chore(tests): split more mock secrets to evade GitGuardian

---------

Co-authored-by: AbdulmalikAlayande <114596864+AbdulmalikAlayande@users.noreply.github.com>
- Add docker-compose.devnet.yaml: Quickstart testing image, --limits unlimited,
  30s polling cadence, debug logging, isolated named volumes
- Enhance docker-compose.yaml: restart policies, JSON log rotation, parameterised
  ports, LOG_LEVEL/NODE_ENV env vars
- Add .env.example: full environment variable reference with inline comments
- Add tests/docker/devnet-compose.test.ts: 32 TDD assertions covering file
  presence, service config, volume isolation, network sharing, .env.example,
  and compose merge compatibility
- Update .dockerignore: exclude compose files, systemd/, docs/, templates/
- Update .gitignore: allow .env.example via negation rule

Acceptance criteria met: docker compose -f docker-compose.yaml
-f docker-compose.devnet.yaml up boots daemon and mock RPC environment
successfully.

All 530 tests pass, 63 docker-specific tests, 5 skipped TODOs.
…estimates

- Add countExtensionsInLastHour() to repositories.ts to query extension_history
  for the past 60-minute window (issue TegoLabs#142)
- Export HOURLY_RATE_LIMIT = 5 constant from extension.ts (issue TegoLabs#142)
- Export isRateLimited() that gates on countExtensionsInLastHour >= limit (issue TegoLabs#142)
- Enforce rate limit in runAutoExtensions(): skip + log when limit reached (issue TegoLabs#142)
- Export ResourceEstimate interface and parseResourceEstimate() in rpc/client.ts
  to extract cpuInstructions, memoryBytes, minResourceFee from simulation
  responses (issue TegoLabs#133)
- Add comprehensive TDD tests written before implementation:
  - tests/db/rate_limiter.test.ts: countExtensionsInLastHour edge cases
  - tests/core/rate_limiter.test.ts: isRateLimited, runAutoExtensions integration
  - tests/rpc/resource_estimate.test.ts: parseResourceEstimate + failure edge cases

Closes TegoLabs#133
Closes TegoLabs#137
Closes TegoLabs#142
AbdulmalikAlayande and others added 16 commits July 27, 2026 22:26
Central registration point for alert channel plugins. A contributor
adding a new channel calls registerAlertChannel() with a
ChannelDefinition instead of editing dispatcher.ts's channel map,
the CLI's --type if/else chain, and a DB CHECK constraint.
Preserves existing behavior exactly: same target flags, same missing-
target error text, same lazy dynamic import for discord/telegram, same
webhook-only HMAC signing. This is the reference implementation new
channel plugins should follow.
Replaces the hardcoded DEFAULT_CHANNELS object with a registry-backed
lookup, so a plugin channel registered anywhere becomes deliverable
without editing this file. Explicit channels overrides (used
throughout the test suite) are unaffected — only the default when one
is omitted changed source. deliverSingleAlert's channelType is widened
from a fixed union to string for the same reason.
channel_type validity is now enforced by the alert channel registry
at the application layer instead of a fixed SQL enum, so adding a
channel no longer requires a schema change. The CHECK now only
guards against an empty string.
…ration

The SCHEMA comment-stripper (`--.*\n`) silently failed to match
comments ending in \r\n, since JS's `.` excludes all line terminators
including \r. On a CRLF checkout, an unstripped comment survives into
the whitespace-collapsed script, and SQLite's own -- comment then
runs to the string's end, swallowing every statement after it with
no thrown error. Switched to `--[^\n]*\n`, which matches either line
ending. Latent since schema.sql had no comments before this change.

Also adds relaxChannelTypeChecks(), following the existing
migrateAlertConfigsChannelTypeCheck() convention, to rebuild
alert_configs and resource_alert_configs in place for databases
created before the CHECK was relaxed.
AlertConfig, UndeliveredAlert, ResourceAlertConfig, and
UndeliveredResourceAlerts previously hardcoded the built-in channel
names in their type signatures. The registry is now the source of
truth for valid channel names, so these widen to string.
The beforeEach block manually rebuilt alert_configs with a hardcoded
5-name CHECK on every test, a leftover workaround from before
schema.sql had these columns natively. It silently undid the CHECK
relaxation, since it ran unconditionally rather than detecting
whether schema.sql already had the change. getDatabaseForTesting()
already execs the current schema.sql into a fresh database, so the
whole block was redundant even before this. Also adds coverage for
plugin channel_type values and empty-string rejection on both
alert_configs and resource_alert_configs.
Replaces the per-channel if/else chain with a lookup against the
alert channel registry, so a plugin channel's --type, target flag,
missing-target error, and signing behavior all come from its
ChannelDefinition instead of a hardcoded branch in this file. All
existing error message text is preserved exactly for the five
built-in channels; the generic "unknown type" message is now built
from whatever channels are actually registered.
Adds two tests to verify the monitor cycle's fault-isolation guarantees
hold when the RPC layer fails over mid-cycle:

1. A mid-cycle failover to a working second endpoint results in a
   complete, accurate cycle result (3 contracts checked, 3 entries
   updated, 0 errors).
2. All configured endpoints failing for one contract does not prevent
   other contracts from being processed in the same cycle (3 checked,
   2 updated, 1 isolated error).

Closes TegoLabs#511
@drips-wave

drips-wave Bot commented Jul 29, 2026

Copy link
Copy Markdown

@flexocode442 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@coderabbitai

coderabbitai Bot commented Jul 29, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Tests
    • Added coverage for RPC endpoint failover during monitoring cycles.
    • Verified monitoring continues successfully when one endpoint fails over.
    • Verified partial failures are reported correctly while successful contracts continue updating.
    • Confirmed failed contract data remains unchanged when all available endpoints are exhausted.

Walkthrough

Added monitor-cycle tests covering transparent RPC endpoint failover and isolated contract failure when all endpoints are exhausted.

Changes

Monitor failover behavior

Layer / File(s) Summary
Failover cycle validation
tests/core/monitor.test.ts
Tests successful mid-cycle failover and verifies that an exhausted contract records an error while other contracts update their TTLs.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

  • #454 — Covers related runMonitorCycle failure isolation and stale TTL behavior.
  • #496 — Addresses tests for RPC failover behavior in the monitoring cycle.

Possibly related PRs

Suggested reviewers: abdulmalikalayande

Poem

A rabbit watched the endpoints hop,
One failed, but cycles did not stop.
Two contracts bloomed with fresh TTLs,
One kept its old when all links fizzled.
“Failover tested!” the bunny cheered.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the PR’s main change: adding tests for mid-cycle RPC failover behavior.
Description check ✅ Passed The description is directly about the new failover test coverage and matches the changeset.
Linked Issues check ✅ Passed The PR satisfies #511 by adding the two required monitor-cycle failover tests and preserving fault isolation checks.
Out of Scope Changes check ✅ Passed The changes stay within test coverage scope and do not introduce unrelated code changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/core/monitor.test.ts`:
- Around line 1120-1141: Update the failover test setup around mockGetEntryTTLs
so it exercises the RPC client’s endpoint retry behavior instead of returning
final results directly: configure the transport/endpoints for C_FO_C so endpoint
1 fails and endpoint 2 succeeds, and configure C_FO_Y so both endpoints are
attempted before failure. Preserve assertions that distinguish successful
client-level failover from the monitor-visible failure, and remove mocks that
bypass endpoint rotation.
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 39e57eed-d12d-4e7a-a285-98b50d0919e4

📥 Commits

Reviewing files that changed from the base of the PR and between 35d9237 and 40b71e9.

📒 Files selected for processing (1)
  • tests/core/monitor.test.ts

Comment on lines +1120 to +1141
mockGetEntryTTLs.mockImplementation(async (keys) => {
const key = keys[0]!;

// For the third contract, simulate client-level failover:
// endpoint 1 attempt failed internally → client retried endpoint 2 → succeeded.
// The monitor never sees the failure.
if (key === "fo-c-key") {
failoverContracts.push(key);
}

return {
latestLedger: LEDGER,
entries: [
{
entryKeyXdr: key,
liveUntilLedgerSeq: LEDGER + 48000,
lastModifiedLedgerSeq: LEDGER,
remainingTTL: 48000,
},
],
};
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Exercise the RPC client’s endpoint retry path rather than mocking its final result.

These mocks bypass failover entirely: the first always returns success and only records fo-c-key; the second throws directly. Thus the tests would still pass if endpoint rotation/retry were broken or removed. Mock the client transport/endpoints so endpoint 1 fails and endpoint 2 succeeds for C_FO_C, and so both configured endpoints are attempted before C_FO_Y fails.

Also applies to: 1168-1188

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/core/monitor.test.ts` around lines 1120 - 1141, Update the failover
test setup around mockGetEntryTTLs so it exercises the RPC client’s endpoint
retry behavior instead of returning final results directly: configure the
transport/endpoints for C_FO_C so endpoint 1 fails and endpoint 2 succeeds, and
configure C_FO_Y so both endpoints are attempted before failure. Preserve
assertions that distinguish successful client-level failover from the
monitor-visible failure, and remove mocks that bypass endpoint rotation.

@gitguardian

gitguardian Bot commented Jul 29, 2026

Copy link
Copy Markdown

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

Since your pull request originates from a forked repository, GitGuardian is not able to associate the secrets uncovered with secret incidents on your GitGuardian dashboard.
Skipping this check run and merging your pull request will create secret incidents on your GitGuardian dashboard.

🔎 Detected hardcoded secret in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
- - Generic High Entropy Secret ded54f4 tests/commands/guard-cli-export-import.test.ts View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secret safely. Learn here the best practices.
  3. Revoke and rotate this secret.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@akinboyewaSamson

Copy link
Copy Markdown

Kindly review pr

@AbdulmalikAlayande

Copy link
Copy Markdown
Collaborator

Blocked on #496 (multiple fallback RPC endpoints with automatic failover) landing first — confirmed src/rpc/client.ts has no multi-endpoint failover logic yet, so these tests can't pass against real code. Leaving this open; revisit once #496 merges.

AbdulmalikAlayande added a commit that referenced this pull request Sep 6, 2026
)

Verifies runMonitorCycle's existing per-contract fault isolation
holds under a simulated multi-endpoint RPC failover: a mid-cycle
failover to a working second endpoint still produces a complete,
accurate cycle result, and one contract exhausting every endpoint
doesn't prevent other contracts from being processed in the same
cycle. Test-only, mocked at the getEntryTTLs boundary per the issue's
own scope — does not touch monitor.ts or the real client.ts failover
logic (#496), which landed just before this in the same batch.
@AbdulmalikAlayande

Copy link
Copy Markdown
Collaborator

Merged via 730ffd1 on main, now that #496's failover implementation is in place. Failover mid-cycle tests pass against the real implementation.

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.

test(core): add tests for RPC failover behavior mid-cycle