Skip to content

fix: reject devnet fallback RPC on mainnet (C2)#81

Open
0x-SquidSol wants to merge 1 commit intodcccrypto:mainfrom
0x-SquidSol:fix/c2-mainnet-fallback-rpc-guard
Open

fix: reject devnet fallback RPC on mainnet (C2)#81
0x-SquidSol wants to merge 1 commit intodcccrypto:mainfrom
0x-SquidSol:fix/c2-mainnet-fallback-rpc-guard

Conversation

@0x-SquidSol
Copy link
Copy Markdown
Contributor

@0x-SquidSol 0x-SquidSol commented Apr 9, 2026

Summary

  • The @percolator/shared config defaults FALLBACK_RPC_URL to https://api.devnet.solana.com when the env var is unset
  • On mainnet, this silently routes fallback RPCs (market discovery, liquidation retry, 429 read-only retry) to the wrong network
  • Adds a startup guard in env-guards.ts that requires FALLBACK_RPC_URL to be explicitly set on mainnet and rejects devnet-pointing URLs
  • Guard is skipped for non-mainnet networks so devnet usage is unaffected

Affected codepaths

Caller File Impact if unguarded
Market discovery crank.ts:212 Discovers devnet programs instead of mainnet
Liquidation retry liquidation.ts:49 Reads devnet slab data on retry
429 read-only retry shared/rpc-client.ts:130 Returns devnet account state

Test plan

  • throws when NETWORK=mainnet and FALLBACK_RPC_URL is not set
  • throws when NETWORK=mainnet and FALLBACK_RPC_URL points to devnet
  • throws when NETWORK=mainnet and FALLBACK_RPC_URL contains devnet in Helius URL
  • does not throw when NETWORK=mainnet and FALLBACK_RPC_URL is a mainnet endpoint
  • does not throw when NETWORK=devnet and FALLBACK_RPC_URL is not set
  • All 138 existing tests pass

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • New Features

    • Enhanced environment validation to ensure fallback RPC URLs are configured correctly for mainnet, preventing accidental misconfigurations.
  • Tests

    • Added comprehensive test coverage for fallback RPC URL network validation scenarios.

The @percolator/shared config defaults FALLBACK_RPC_URL to
https://api.devnet.solana.com when the env var is unset. On mainnet
this silently routes fallback RPCs (market discovery, liquidation
retry, 429 read-only retry) to the wrong network.

Add a startup guard in env-guards that:
- requires FALLBACK_RPC_URL to be explicitly set when NETWORK=mainnet
- rejects URLs containing devnet indicators (api.devnet.solana.com,
  devnet.helius-rpc.com, etc.)

The guard is skipped for non-mainnet networks so devnet usage is
unaffected.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 9, 2026

📝 Walkthrough

Walkthrough

Added mainnet-specific validation to environment guards that requires FALLBACK_RPC_URL to be present and non-devnet when NETWORK is set to mainnet. Introduced devnet-detection helper functions and comprehensive test coverage for the new validation rules.

Changes

Cohort / File(s) Summary
Environment Guard Implementation
src/env-guards.ts
Added DEVNET_INDICATORS constants and looksLikeDevnet() helper function for URL pattern matching. Extended validateKeeperEnvGuards() with mainnet-specific validation requiring FALLBACK_RPC_URL to exist and not target devnet based on URL substrings.
Environment Guard Tests
tests/env-guards.test.ts
Added test cases validating mainnet fallback RPC requirements: rejection when FALLBACK_RPC_URL is missing or targets devnet (e.g., https://api.devnet.solana.com), acceptance for mainnet-targeting URLs. Verified devnet network mode does not enforce this requirement.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 A validation rabbit hops with glee,
Checking mainnet URLs with scrutiny!
No devnet sneaking past my keen eye,
Each fallback RPC I verify,
Guards are strong, and tests ring true! 🔐

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding a guard to reject devnet fallback RPC URLs when the network is set to mainnet, which is the primary objective of this PR.

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

✨ 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 and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

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 the current code and only fix it if needed.

Inline comments:
In `@src/env-guards.ts`:
- Around line 4-7: The function looksLikeDevnet currently scans the whole URL
string (using DEVNET_INDICATORS) which can misclassify mainnet URLs; update
looksLikeDevnet to parse the URL with the URL constructor, inspect only the
hostname for any DEVNET_INDICATORS, and also check the explicit query/search
param named "cluster" (or similar param your app uses) for the value "devnet";
if URL parsing fails, fall back to a conservative check but prefer the
parsed-hostname+cluster approach; update references to DEVNET_INDICATORS and
ensure the function returns true only when hostname contains a devnet indicator
or the cluster param equals "devnet".
🪄 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: d290e02c-396b-49cf-b2bd-11fe42ee4293

📥 Commits

Reviewing files that changed from the base of the PR and between 63ad1a0 and daac042.

📒 Files selected for processing (2)
  • src/env-guards.ts
  • tests/env-guards.test.ts

Comment thread src/env-guards.ts
Comment on lines +4 to +7
function looksLikeDevnet(url: string): boolean {
const lower = url.toLowerCase();
return DEVNET_INDICATORS.some((indicator) => lower.includes(indicator));
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid full-string devnet matching; it can reject valid mainnet URLs.

Line 6 checks the entire URL, so a mainnet endpoint can be falsely blocked if devnet appears in query/path text (e.g., API key value). Parse and validate hostname (and explicit cluster param) instead.

Proposed fix
-/** Known devnet RPC hostnames / URL substrings — used to detect network mismatch */
-const DEVNET_INDICATORS = ["devnet", "api.devnet.solana.com"];
+/** Known devnet RPC hostname indicators */
+const DEVNET_HOST_INDICATORS = ["devnet", "api.devnet.solana.com"];

 function looksLikeDevnet(url: string): boolean {
-  const lower = url.toLowerCase();
-  return DEVNET_INDICATORS.some((indicator) => lower.includes(indicator));
+  try {
+    const parsed = new URL(url);
+    const hostname = parsed.hostname.toLowerCase();
+    const cluster = parsed.searchParams.get("cluster")?.toLowerCase();
+
+    return (
+      DEVNET_HOST_INDICATORS.some((indicator) => hostname.includes(indicator)) ||
+      cluster === "devnet"
+    );
+  } catch {
+    return false;
+  }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/env-guards.ts` around lines 4 - 7, The function looksLikeDevnet currently
scans the whole URL string (using DEVNET_INDICATORS) which can misclassify
mainnet URLs; update looksLikeDevnet to parse the URL with the URL constructor,
inspect only the hostname for any DEVNET_INDICATORS, and also check the explicit
query/search param named "cluster" (or similar param your app uses) for the
value "devnet"; if URL parsing fails, fall back to a conservative check but
prefer the parsed-hostname+cluster approach; update references to
DEVNET_INDICATORS and ensure the function returns true only when hostname
contains a devnet indicator or the cluster param equals "devnet".

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