feat(core): add RPC rate limiting and retry handling#154
feat(core): add RPC rate limiting and retry handling#154smypmsa merged 6 commits intochainstacklabs:mainfrom
Conversation
Addresses chainstacklabs#44 — users on free-tier RPC endpoints hit HTTP 429 errors during buy transactions due to no rate limiting or retry handling. - Add TokenBucketRateLimiter (new file: src/core/rpc_rate_limiter.py) - Gate all RPC methods through rate limiter (both post_rpc and solana-py calls) - Rewrite post_rpc() with retry loop, exponential backoff, jitter, and specific 429 detection with Retry-After header support - Replace per-call aiohttp session with shared persistent session - Wire node.max_rps from YAML bot config through to SolanaClient - Fix cleanup manager and learning example to use SolanaClient abstraction instead of bypassing it via get_client() Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a token-bucket RPC rate limiter, shared aiohttp session, and retry/backoff (429) to the Solana client; exposes configurable Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Client as SolanaClient
participant Limiter as TokenBucketRateLimiter
participant RPC as Solana RPC
App->>Client: get_account_info(pubkey)
Client->>Limiter: acquire()
alt token available
Limiter-->>Client: token granted
else wait for tokens
Limiter->>Limiter: compute wait & sleep
Limiter-->>Client: token granted
end
Client->>RPC: POST / (RPC body)
alt 200 OK
RPC-->>Client: 200 + result
Client-->>App: return result
else 429 Too Many Requests
RPC-->>Client: 429 (+Retry-After?)
Client->>Client: backoff + jitter, decrement retries
Client->>RPC: POST / (retry)
RPC-->>Client: result or 429
Client-->>App: return result or None
else network/error
Client->>Client: retry loop until max_retries
Client->>RPC: POST / (retry)
RPC-->>Client: result or error
Client-->>App: return result or None
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
learning-examples/cleanup_accounts.py (1)
40-71:⚠️ Potential issue | 🟠 MajorBurn transaction is not confirmed before attempting close.
On Line 52 the burn transaction is sent without
confirm_transaction, but Line 65 sends a separate close transaction immediately after. If the burn hasn't landed on-chain yet, the close will fail because the account still has a non-zero balance.In contrast,
src/cleanup/manager.pycorrectly batches burn and close into a single transaction. Consider either confirming the burn before closing, or combining both instructions into one transaction as the manager does.Proposed fix: combine into a single transaction
+ instructions = [] balance = await client.get_token_account_balance(account) if balance > 0: logger.info(f"Burning {balance} tokens from account {account}...") burn_ix = burn( BurnParams( account=account, mint=mint, owner=wallet.pubkey, amount=balance, program_id=TOKEN_PROGRAM, ) ) - await client.build_and_send_transaction([burn_ix], wallet.keypair) - logger.info(f"Burned tokens from {account}") + instructions.append(burn_ix) # Account exists, attempt to close it logger.info(f"Closing account: {account}") close_params = CloseAccountParams( account=account, dest=wallet.pubkey, owner=wallet.pubkey, program_id=TOKEN_PROGRAM, ) ix = close_account(close_params) + instructions.append(ix) - tx_sig = await client.build_and_send_transaction( - [ix], + tx_sig = await client.build_and_send_transaction( + instructions, wallet.keypair, skip_preflight=True, ) await client.confirm_transaction(tx_sig) - logger.info(f"Closed successfully: {account}") + logger.info(f"Burned and closed successfully: {account}")
🤖 Fix all issues with AI agents
In `@src/bot_runner.py`:
- Around line 140-141: TokenBucketRateLimiter currently allows non-positive
max_rps which causes ZeroDivisionError in acquire() and broken replenishment;
add validation in TokenBucketRateLimiter.__init__ to check the max_rps parameter
and raise a ValueError if max_rps <= 0 (e.g., "max_rps must be positive, got
{max_rps}") so callers supplying zero or negative values fail fast and avoid
runtime errors in acquire() and token refill logic.
In `@src/core/client.py`:
- Around line 506-508: The JSONDecodeError except block is effectively dead
because response.json() raises aiohttp.ContentTypeError (a ClientError
subclass); update the error handling in the try/except around response.json() so
that malformed JSON fails immediately instead of being retried: either catch
aiohttp.ContentTypeError specifically (in addition to or instead of
json.JSONDecodeError) or move the json decode exception handler before the
broader aiohttp.ClientError catch, and ensure the logger.exception call for
logger.exception(f"Failed to decode RPC response for {method}") is executed in
that specific ContentTypeError handler so malformed payloads are not swallowed
by the ClientError retry logic.
- Around line 471-485: The Retry-After header parsing can raise ValueError when
it's an HTTP-date string; wrap the parsing logic in a try/except in the 429
handling block (the code using response.headers.get("Retry-After"), retry_after,
wait_time, method, attempt, max_retries) so you first try to parse numeric
seconds (float) and on ValueError attempt to parse an HTTP-date into a datetime
and compute seconds until that date, and if that also fails or yields a negative
value fall back to the existing exponential-backoff default (min(2 ** (attempt +
1), 30)); ensure any exceptions are caught so they don't escape the retry loop
and keep the existing logging and sleep behavior.
- Around line 109-119: The _get_session method can create multiple
aiohttp.ClientSession instances under concurrent calls; add an asyncio.Lock
field initialized in __init__ (e.g., self._session_lock = asyncio.Lock()) and
use it to serialize session creation in _get_session (acquire via async with
self._session_lock) so only one caller creates and assigns self._session when
self._session is None or closed, then return the shared session; reference the
_get_session method, __init__, self._session, and aiohttp.ClientSession when
updating the code.
In `@src/core/rpc_rate_limiter.py`:
- Around line 23-28: Validate and normalize the incoming max_rps in
RpcRateLimiter.__init__: raise a clear ValueError if max_rps is not > 0, and
compute burst_size using a non-zero ceiling (e.g., burst_size = max(1,
math.ceil(max_rps))) instead of int(max_rps) so fractional rates like 0.5 don't
produce a zero burst; keep _tokens as float(self._burst_size) and retain
existing refill/acquire logic so tokens and refill math work with fractional
rates without causing ZeroDivisionError or infinite loops.
Validate max_rps > 0 in TokenBucketRateLimiter to prevent ZeroDivisionError and infinite loops with fractional values. Add asyncio.Lock to _get_session to fix race condition, handle non-numeric Retry-After headers gracefully, replace dead json.JSONDecodeError with aiohttp.ContentTypeError, and combine burn+close into a single transaction in cleanup example. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
- Add section on built-in RPC rate limiting with token bucket algorithm - Document configurable max RPS and automatic retry logic - Update roadmap to mark "Configurable RPS" as completed - Clarify benefits of rate limiting for provider compliance Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@README.md`:
- Around line 201-202: The Markdown table rows containing "Error handling |
Improving error handling | ✅ |" and "Configurable RPS | Ability to set RPS in
the config to match your provider's and plan RPS (preferably
[Chainstack](https://console.chainstack.com/) 🤩) | ✅ |" have trailing pipes
that trigger markdownlint MD055; remove the final trailing pipe from each of
these rows so they end with "✅" instead of "✅ |". Ensure you update the lines
that include "Error handling" and "Configurable RPS" in the README table to
match the pipe style used by other rows.
| | | Error handling | Improving error handling | ✅ | | ||
| | | Configurable RPS | Ability to set RPS in the config to match your provider's and plan RPS (preferably [Chainstack](https://console.chainstack.com/) 🤩) | ✅ | |
There was a problem hiding this comment.
Fix MD055 table pipe style (remove trailing pipe).
markdownlint flags trailing pipes on these rows.
🛠️ Proposed fix
-| | Error handling | Improving error handling | ✅ |
-| | Configurable RPS | Ability to set RPS in the config to match your provider's and plan RPS (preferably [Chainstack](https://console.chainstack.com/) 🤩) | ✅ |
+| | Error handling | Improving error handling | ✅
+| | Configurable RPS | Ability to set RPS in the config to match your provider's and plan RPS (preferably [Chainstack](https://console.chainstack.com/) 🤩) | ✅📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | | Error handling | Improving error handling | ✅ | | |
| | | Configurable RPS | Ability to set RPS in the config to match your provider's and plan RPS (preferably [Chainstack](https://console.chainstack.com/) 🤩) | ✅ | | |
| | | Error handling | Improving error handling | ✅ | |
| | | Configurable RPS | Ability to set RPS in the config to match your provider's and plan RPS (preferably [Chainstack](https://console.chainstack.com/) 🤩) | ✅ |
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 201-201: Table pipe style
Expected: leading_only; Actual: leading_and_trailing; Unexpected trailing pipe
(MD055, table-pipe-style)
[warning] 202-202: Table pipe style
Expected: leading_only; Actual: leading_and_trailing; Unexpected trailing pipe
(MD055, table-pipe-style)
🤖 Prompt for AI Agents
In `@README.md` around lines 201 - 202, The Markdown table rows containing "Error
handling | Improving error handling | ✅ |" and "Configurable RPS | Ability to
set RPS in the config to match your provider's and plan RPS (preferably
[Chainstack](https://console.chainstack.com/) 🤩) | ✅ |" have trailing pipes
that trigger markdownlint MD055; remove the final trailing pipe from each of
these rows so they end with "✅" instead of "✅ |". Ensure you update the lines
that include "Error handling" and "Configurable RPS" in the README table to
match the pipe style used by other rows.
- Replace int(max_rps) with math.ceil(max_rps) in burst_size calculation - Prevents infinite loop when max_rps < 1.0 (e.g., 0.5 RPS would result in burst_size=0) - Ensures burst_size is always at least 1 for valid fractional rates - Addresses CodeRabbit feedback on rpc_rate_limiter.py:27 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/core/rpc_rate_limiter.py`:
- Around line 24-31: The constructor __init__ currently has an overlong line and
doesn't validate an explicit burst_size; update it to enforce burst_size > 0
(raise ValueError with a short non-f-string message if burst_size <= 0), keep
the default behavior burst_size = max(1, math.ceil(max_rps)) when None,
initialize _tokens from the validated _burst_size, and ensure any long
expressions are split or assigned to temporaries so no line exceeds the
88-character limit; refer to symbols __init__, _burst_size, _tokens, and
_last_refill when making the changes.
| def __init__(self, max_rps: float, burst_size: int | None = None) -> None: | ||
| if max_rps <= 0: | ||
| raise ValueError(f"max_rps must be positive, got {max_rps}") | ||
| self._max_rps = max_rps | ||
| self._burst_size = burst_size if burst_size is not None else max(1, math.ceil(max_rps)) | ||
| self._tokens = float(self._burst_size) | ||
| self._last_refill = time.monotonic() | ||
| self._lock = asyncio.Lock() |
There was a problem hiding this comment.
Line 28 exceeds the 88-character limit; also validate explicit burst_size.
Two items in __init__:
- Line length: Line 28 is ~96 characters, exceeding the 88-char Ruff/project limit.
- Missing
burst_sizeguard: If a caller explicitly passesburst_size=0(or negative),_tokensstarts at0,_refillcaps at0, andacquire()loops forever — same class of bug that was just fixed formax_rps. Sinceburst_sizecan be supplied externally, a quick guard keeps the contract tight. - TRY003 (Ruff hint, line 26): The f-string message in
ValueErroris flagged by Ruff's TRY003 rule. A minor nit — defining a custom message constant or inlining a short message would silence it.
Proposed fix
def __init__(self, max_rps: float, burst_size: int | None = None) -> None:
if max_rps <= 0:
- raise ValueError(f"max_rps must be positive, got {max_rps}")
+ msg = f"max_rps must be positive, got {max_rps}"
+ raise ValueError(msg)
self._max_rps = max_rps
- self._burst_size = burst_size if burst_size is not None else max(1, math.ceil(max_rps))
+ self._burst_size = (
+ burst_size if burst_size is not None
+ else max(1, math.ceil(max_rps))
+ )
+ if self._burst_size <= 0:
+ msg = f"burst_size must be positive, got {burst_size}"
+ raise ValueError(msg)
self._tokens = float(self._burst_size)
self._last_refill = time.monotonic()
self._lock = asyncio.Lock()As per coding guidelines: "Limit lines to 88 characters", "Perform comprehensive input validation for externally sourced data", and "Comply with Ruff exception handling rules (BLE, TRY)".
🧰 Tools
🪛 Ruff (0.14.14)
[warning] 26-26: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In `@src/core/rpc_rate_limiter.py` around lines 24 - 31, The constructor __init__
currently has an overlong line and doesn't validate an explicit burst_size;
update it to enforce burst_size > 0 (raise ValueError with a short non-f-string
message if burst_size <= 0), keep the default behavior burst_size = max(1,
math.ceil(max_rps)) when None, initialize _tokens from the validated
_burst_size, and ensure any long expressions are split or assigned to
temporaries so no line exceeds the 88-character limit; refer to symbols
__init__, _burst_size, _tokens, and _last_refill when making the changes.
Address remaining CodeRabbitAI review feedback: add burst_size validation guard, fix TRY003 lint (use msg variable for ValueError), break long line under 88 chars, and fix MD055 table pipe style in README. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
429 responses no longer count against max_retries — they use a dedicated max_429_retries counter (default 10) so free-tier users hitting rate limits won't exhaust retries prematurely. Also refresh the aiohttp session inside the retry loop to avoid stale references after network failures, and fix cleanup log message accuracy. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
I pushed one fix on top. The 429 handling was counting rate-limit retries against the same |
Summary
Addresses #44 — users on free-tier RPC endpoints hit HTTP 429 (Too Many Requests) errors during buy transactions.
src/core/rpc_rate_limiter.py) — gates all RPC calls to respectnode.max_rpsfrom bot YAML config (default 25 RPS)post_rpc()— 3 retries with exponential backoff, jitter, andRetry-Afterheader supportpost_rpc(), reducing TCP connection overheadget_account_info(),get_token_account_balance(),get_latest_blockhash(),build_and_send_transaction(),confirm_transaction()node.max_rpsfrom YAML configs now flows throughbot_runner → UniversalTrader → SolanaClientSolanaClientvia directget_client()callsTest plan
node.max_rpsvalue (e.g., 5) to confirm rate limiting throttles requestsSolanaClientabstractionruff checkandruff format— no new lint violations introduced🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Documentation