-
Notifications
You must be signed in to change notification settings - Fork 310
feat(core): integrate account data size limit #144
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds support for limiting loaded account data size as a Solana Compute Budget parameter. Four bot YAML configs add Changes
Sequence DiagramsequenceDiagram
participant Config as Bot Config
participant Platform as PlatformAware
participant Client as SolanaClient
participant Solana as Solana Network
Config->>Platform: account_data_size = 512_000
Platform->>Platform: _get_cu_override("account_data_size", platform)
Platform->>Client: build_and_send_transaction(..., account_data_size_limit)
Client->>Client: set_loaded_accounts_data_size_limit(limit)
Client->>Client: Prepend size-limit instruction (discriminator 4)
Client->>Client: Add compute-unit & priority-fee instructions
Client->>Solana: Send transaction
Solana-->>Client: Confirm / Response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
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 |
There was a problem hiding this 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/trading/platform_aware.py (2)
156-170: Update documentation to reflect new "account_data_size" operation.The docstring and inline comment state that
operationis limited to "buy" or "sell", but the method now also accepts "account_data_size" as shown in line 114. The implementation correctly handles any key viaself.compute_units.get(operation), but the documentation is misleading.Apply this diff to update the documentation:
def _get_cu_override(self, operation: str, platform: Platform) -> int | None: """Get compute unit override from configuration. Args: - operation: "buy" or "sell" + operation: "buy", "sell", or "account_data_size" platform: Trading platform (unused - each config is platform-specific) Returns: CU override value if configured, None otherwise """ if not self.compute_units: return None - # Just check for operation override (buy/sell) + # Check for operation override (buy/sell/account_data_size) return self.compute_units.get(operation)
312-326: Update documentation and consider DRY refactor.Same documentation issue as the buyer's
_get_cu_overridemethod. Additionally, this method is duplicated betweenPlatformAwareBuyerandPlatformAwareSeller.Fix the documentation:
def _get_cu_override(self, operation: str, platform: Platform) -> int | None: """Get compute unit override from configuration. Args: - operation: "buy" or "sell" + operation: "buy", "sell", or "account_data_size" platform: Trading platform (unused - each config is platform-specific) Returns: CU override value if configured, None otherwise """ if not self.compute_units: return None - # Just check for operation override (buy/sell) + # Check for operation override (buy/sell/account_data_size) return self.compute_units.get(operation)Optional refactor: Consider extracting
_get_cu_overrideinto a shared base class or utility function to eliminate duplication between buyer and seller classes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
bots/bot-sniper-1-geyser.yaml(1 hunks)bots/bot-sniper-2-logs.yaml(1 hunks)bots/bot-sniper-3-blocks.yaml(1 hunks)bots/bot-sniper-4-pp.yaml(1 hunks)src/core/client.py(4 hunks)src/trading/platform_aware.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
bots/**/*.{yaml,yml}
📄 CodeRabbit inference engine (CLAUDE.md)
bots/**/*.{yaml,yml}: Keep bot configurations in YAML files under bots/
Detect target platform from bot configuration files
Do not include sensitive data in bot configuration files
bots/**/*.{yaml,yml}: Edit bot instance configuration in YAML files under bots/
Start with conservative YAML settings (low buy_amount, high min_sol_balance, strict filters)
Files:
bots/bot-sniper-4-pp.yamlbots/bot-sniper-3-blocks.yamlbots/bot-sniper-1-geyser.yamlbots/bot-sniper-2-logs.yaml
bots/**/*.{yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
bots/**/*.{yml,yaml}: Store bot configurations as YAML files in bots/
Support ${VARIABLE} environment interpolation in bot YAML configs
Validate bot configurations before starting bots
Files:
bots/bot-sniper-4-pp.yamlbots/bot-sniper-3-blocks.yamlbots/bot-sniper-1-geyser.yamlbots/bot-sniper-2-logs.yaml
**/*.{yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/trading-bot.mdc)
**/*.{yaml,yml}: Maintain a consistent YAML structure for all bot configuration files, including keys: name, platform, enabled, separate_process, env_file, rpc_endpoint, wss_endpoint, private_key, geyser.{endpoint,api_token,auth_type}, and trade.{buy_amount,buy_slippage,sell_slippage,exit_strategy,extreme_fast_mode}
Use ${VARIABLE_NAME} syntax for environment variable interpolation in YAML configs
Never hardcode sensitive values (e.g., private keys, API tokens) in YAML configuration files
Validate YAML syntax and required configuration fields
Test environment variable interpolation in configuration filesValidate YAML configuration syntax and required fields before running
Files:
bots/bot-sniper-4-pp.yamlbots/bot-sniper-3-blocks.yamlbots/bot-sniper-1-geyser.yamlbots/bot-sniper-2-logs.yaml
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Enforce max line length of 88 characters in Python (Ruff config)
Use 4 spaces for indentation in Python files
Target Python version 3.11+ features and syntax
Prefer double quotes for strings in Python code
Enable and honor import sorting in Python modules
Apply Ruff rule sets: Security (S), Annotations (ANN), Exceptions (BLE, TRY), Complexity (C90), Pylint conventions (PL), and no commented-out code (ERA)
Order imports as: standard library, third-party, then local imports
Use Google-style docstrings for functions and classes
Provide type hints for all public functions
Use a module-level logger via get_logger(name)
Wrap risky operations in try/except with proper logging
Implement rate limiting and retry mechanisms where external calls are made
Perform comprehensive input validation for externally sourced data
**/*.py: Limit lines to 88 characters
Use 4 spaces for indentation (never tabs)
Use double quotes for strings consistently
Target Python 3.11+ features and syntax
Enable automatic import sorting and organization
Order imports: standard library first, third-party second, local last
Add type hints to all public functions and methods
Use modern typing syntax (e.g., X | Y unions)
Include explicit return type annotations
Use typing.Any for complex/unknown types (from typing import Any)
Use Google-style docstrings for all functions and classes
Wrap external operations (RPC calls, file I/O) in try/except blocks
Log exceptions with context using logging.exception()
Provide meaningful error messages when handling exceptions
Do not suppress exceptions without good reason
Use get_logger(name) with logger from utils.logger
Use appropriate log levels (DEBUG, INFO, WARNING, ERROR) with contextual messages
Never hardcode secrets (private keys, API tokens)
Use environment variables for sensitive configuration
Do not log sensitive information
Validate all external inputs
Comply with Ruff security rules (S)
Comply with Ruff type annotation rules (ANN)
Comply...
Files:
src/core/client.pysrc/trading/platform_aware.py
src/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
src/**/*.py: Use snake_case for all Python file names
Avoid circular imports between packages
Use asyncio.create_task() for concurrent operations
Implement proper cleanup on shutdown for async tasks/resources
Files:
src/core/client.pysrc/trading/platform_aware.py
src/core/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
src/core/**/*.py: Core modules must not import from trading or monitoring
core/ may depend only on utils/ and interfaces/
Limit concurrent operations based on RPC provider limits
Files:
src/core/client.py
src/core/client.py
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
src/core/client.py: Use connection pooling for HTTP clients and backoff for failed requests
Cache recent blockhash and relevant account information where appropriate
Files:
src/core/client.py
src/{client,trading,monitoring,platforms}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Separate concerns into client, trading, monitoring, and platforms packages under src/
Files:
src/trading/platform_aware.py
src/trading/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/architecture.mdc)
trading/ may depend on core/, platforms/, and interfaces/
Files:
src/trading/platform_aware.py
🧠 Learnings (1)
📚 Learning: 2025-08-21T12:08:15.475Z
Learnt from: CR
PR: chainstacklabs/pumpfun-bonkfun-bot#0
File: .cursor/rules/trading-bot.mdc:0-0
Timestamp: 2025-08-21T12:08:15.475Z
Learning: Applies to **/*.py : Set compute unit limits and prices (priority fees) on transactions to reduce failures
Applied to files:
src/core/client.py
🔇 Additional comments (7)
bots/bot-sniper-3-blocks.yaml (1)
60-65: LGTM: Well-documented compute unit optimization.The addition of
account_data_size: 512_000is well-documented with clear explanations of the CU reduction benefits. The 512KB limit is reasonable for typical trading operations and aligns with Solana's compute budget best practices. The reference URL is helpful for understanding the optimization.bots/bot-sniper-2-logs.yaml (1)
60-65: LGTM: Consistent configuration across bot instances.The configuration matches other sniper bots, maintaining consistency. The optimization is appropriate for this listener type.
bots/bot-sniper-4-pp.yaml (1)
58-63: LGTM: Appropriate for lets_bonk platform.The account data size optimization is consistently applied across platforms, which is correct since this is a Solana-level optimization independent of trading platform specifics.
bots/bot-sniper-1-geyser.yaml (1)
60-65: LGTM: Completes consistent rollout across all bot instances.All four sniper bot configurations now have the account data size optimization enabled with identical settings, ensuring consistent compute unit behavior across different listener types.
src/trading/platform_aware.py (2)
270-270: Transaction parameter integration looks correct.The
account_data_size_limitis properly propagated to the transaction builder for sell operations, matching the buy flow implementation.
114-114: Configuration parsing is correct and functioning as expected.The YAML parser successfully loads
account_data_size: 512_000into thecompute_unitsdictionary across all bot configs. The code correctly extracts it viacfg.get("compute_units", {})and accesses it through_get_cu_override("account_data_size", ...)using.get(). No configuration structure issues exist.src/core/client.py (1)
172-227: Transaction builder integration is correct.The
account_data_size_limitparameter is properly integrated into the transaction construction flow:
- Parameter added to method signature with appropriate type hint
- Instruction ordering is correct (account data size limit → compute unit limit → priority fee)
- Proper logging added
- Conditional logic ensures instructions are only added when needed
The implementation follows Solana best practices by placing the account data size limit instruction first in the compute budget sequence.
Based on learnings
Summary by CodeRabbit
New Features
Style