test: add PumpSwap auto-routing test coverage#17
Merged
Conversation
- 11 core-layer unit tests verifying graduated→pumpswap routing sequence (mocked RPC) - 10 command-layer unit tests verifying parameter forwarding (slippage, priority-fee, dry-run, confirm) through the fallback path and edge cases (not_found does not trigger fallback, pool-not-found after graduation surfaces correctly) - 3 surfpool integration tests verifying full auto-routing end-to-end against a forked mainnet node No production code changed. Test delta: +21 unit tests (339→360), +3 surfpool tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdded comprehensive test coverage across three layers for auto-routing fallback behavior: when bonding curve tokens are detected as "graduated", the CLI forwards trades to PumpSwap with appropriate flags (slippage, priority-fee, dry-run, confirm) and correctly handles error cases vs. legitimate not-found scenarios. Changes
Sequence DiagramsequenceDiagram
participant CLI as CLI Command
participant Core as Core Trade Logic
participant BC as Bonding Curve (RPC)
participant PS as PumpSwap
CLI->>Core: buy_token(amount, slippage=5)
Core->>BC: Fetch bonding curve account
BC-->>Core: Account data {complete: true}
Core-->>CLI: {error: "graduated"}
Note over CLI: Detect graduated, retry via PumpSwap
CLI->>PS: buy_pumpswap(amount, slippage=5, ...)
PS-->>CLI: {venue: "pumpswap", action: "buy", tokens_received: X, signature: "..."}
CLI-->>CLI: Return success with pumpswap venue
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
📝 Coding Plan
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. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When a pump.fun token graduates from the bonding curve to PumpSwap AMM, the CLI auto-routes buy/sell commands through PumpSwap without requiring
--force-amm. This auto-detection path — the default experience for every user trading a graduated token — had zero test coverage. Only the explicit--force-ammpath was tested.Layers Touched
commands/— no changes (existing auto-routing logic at trade.py:117-130, 206-219 is correct)core/— no changesprotocol/— no changesDoes this affect transaction construction or signing?
No. This is a test-only change. No production code was modified.
Test Plan
🤖 Generated with Claude Code
Security & Architecture Assessment
No protocol code affected: This is a test-only PR. The PR objectives explicitly confirm no changes to production code in
protocol/,commands/, orcore/. The existing auto-routing logic (trade.py:117-130, 206-219) remains untouched.No transaction sending in tests: All
send_txcalls are mocked with return values. Tests explicitly verify thatdry_run=Truepreventssend_txinvocation (e.g.,client.send_tx.assert_not_called()). No actual Solana transactions are sent during test execution.Wallet handling is secure: Test wallets are created using the production
encrypt_keypair()function in isolated temporary directories via pytest'stmp_pathfixture. Environment variables (XDG_CONFIG_HOME,PUMPFUN_PASSWORD) are isolated via pytest'smonkeypatchfixture and confined to test scope. No system-level wallet files are modified. No hardcoded credentials in tests.Proper test isolation and mocking: Core layer tests use
unittest.mock.patch()andAsyncMock()to mock RPC responses, IDL decoding, and program ID selection. Integration tests run against a forked mainnet via Surfpool (non-production environment). No architectural violations—command, core, and integration layers remain independently testable. No subprocess calls, eval, or dynamic code execution.Comprehensive auto-routing coverage:
"not_found"does not trigger fallback; PumpSwap errors propagate correctlyImpact: +21 unit tests (339→360), +3 integration tests; zero changes to production code affecting transaction construction, signing, or fund handling.