fix: PumpSwap PDA derivation + token type test coverage#13
Merged
Conversation
Cover the full token dimensions matrix: Token-2022 vs SPL, mayhem vs standard, cashback vs non-cashback, bonding curve vs PumpSwap. Tests verify correct account counts, fee routing, ATA derivation, and instruction data for every combination. - 15 protocol-level tests (instruction builders, account layouts) - 13 core-level tests (buy/sell with mocked RPC for all state flags) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… lookup getProgramAccounts is an expensive RPC call that scans the entire program account space. On slower RPC endpoints (e.g. Chainstack free tier) it consistently times out at the 30s default, making all graduated token trading fail with "Failed to query PumpSwap pools". Replace with deterministic PDA derivation (same seeds as the migrate instruction) followed by a single getAccountInfo call. This is: - Instant (~1s vs ~45s) - Works on any RPC endpoint (no getProgramAccounts support needed) - No timeout issues Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Extend surfpool test infrastructure to discover and test cashback-enabled bonding curve tokens. The autodiscover script finds a cashback token from the pump.fun API (new --cashback flag), and tests verify buy + sell work correctly with the user_volume_accumulator in remaining accounts. Also update PumpSwap test comments to reflect the PDA-based pool lookup. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Cashback tokens need more compute units due to extra account processing (user_volume_accumulator). Bumped from 100k to 200k in the buy+sell cashback surfpool test to avoid ComputationalBudgetExceeded. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Extract shared tmp_keystore/mock_keypair fixtures to test_core/conftest.py - Extract shared build_pool_data() to test_core/helpers.py - Remove duplicate fixture definitions from test_trade, test_pumpswap, test_token_types - Inline derive_pool_authority in build_migrate_instruction, remove _derive_migrate_pool_authority wrapper - Simplify test_pumpswap_buy_passes_token_program to use shared helpers Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ock helpers - Remove test_buy_cashback_token and test_buy_mayhem_cashback_token (buy is cashback-agnostic, these tested nothing different from standard buy) - Merge _setup_buy_mocks/_setup_sell_mocks into single _setup_mocks(side=) - Use _setup_mocks for graduated error tests instead of inline mock setup - Remove section separator comments (docstrings suffice) Co-Authored-By: Claude Opus 4.6 (1M context) <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 (15)
📝 WalkthroughWalkthroughThe pull request adds cashback-enabled bonding-curve token support to the Solana CLI and refactors pool account discovery from expensive program account filtering to deterministic PDA derivation. Updates shell scripts for cashback mint discovery, introduces PDA-based pool lookup functions, and adds comprehensive test coverage across protocol and integration layers. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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
1 participant
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
getProgramAccountswith PDA derivation for PumpSwap pool lookup — singlegetAccountInfocall instead of expensive account scanderive_amm_poolandderive_pool_authorityintoprotocol/address.py, consolidating the duplicate ininstructions.pyTest plan
uv run pytest tests/ -q— all unit tests pass./scripts/surfpool-autodiscover.sh— surfpool integration tests pass (cashback mint auto-discovered)🤖 Generated with Claude Code
Security & Protocol Analysis
Critical Protocol Changes ✓
This PR modifies the PumpSwap pool lookup mechanism, a code path that directly affects real fund transfers in buy/sell operations. The change replaces expensive
getProgramAccountsaccount scanning with deterministic PDA (Program Derived Address) derivation:MemcmpOptsto find the pool (vulnerable to account scan RPC timeouts and attacks)["pool", [0,0], pool_authority, mint, WSOL_MINT]and fetches it with a singlegetAccountInfocallSecurity assessment: ✅ Improved security and reliability
Transaction Construction Impacts
Code that builds and sends signed transactions is affected:
get_pool_by_mint()inprotocol/pumpswap.py: Used in hot paths forbuy_pumpswap()andsell_pumpswap()operations (both send real transactions to blockchain)build_migrate_instruction()inprotocol/instructions.py: Extractedderive_pool_authority(mint)into shared address module to ensure consistent PDA derivation across bonding-curve graduation and trading pathsCorrectness verification: Three protocol-level tests confirm:
derive_amm_poolis deterministic (same mint → same address)derive_amm_poolmatches the inline derivation inbuild_migrate_instructionComprehensive Test Coverage
The PR adds 48 new tests across three execution layers:
Token-type matrix covers:
{Token-2022, SPL} × {Mayhem, Standard} × {Cashback, Non-cashback}for both bonding-curve and PumpSwap trading paths.No Wallet/Key Handling Changes
No modifications to wallet management, key derivation, or signature logic. Only pool discovery and instruction account derivation are affected.