feat: production readiness - 7 critical fixes before prod#2
Draft
Ageree wants to merge 1 commit into
Draft
Conversation
1. Fix pending_withdrawals status column: migration converts VARCHAR(32) to proper withdrawalstatus enum, fallback _ensure_pending_withdrawals() now creates the enum type before the table. 2. Rate limits on deposit subaddress creation: per-agent rate limiting on POST /v2/balance/deposit to prevent wallet RPC abuse. 3. DepositMonitor alerting: consecutive poll failures now fire alerts through the HealthMonitor system, dispatching to Slack/Telegram/Discord webhooks. Added Slack Incoming Webhook support alongside existing Telegram and Discord. 4. DB backup script: scripts/backup_db.sh with pg_dump, gzip compression, retention rotation, optional S3 upload, cron install mode, and restore. 5. Withdraw API tests: 10 end-to-end tests for POST /v2/balance/withdraw with agent auth (not admin), covering success, insufficient balance, auth rejection, validation, idempotency, and sequential withdrawals. 6. Test agent cleanup: POST /v2/admin/cleanup-test-agents endpoint with pattern matching, dry-run mode, and balance safety guard. 7. WEB_CONCURRENCY + worker guard: default bumped to 2 workers, PG advisory lock-based leader election ensures background tasks (DepositMonitor, escrow, SLA, recurring) only run on primary worker. Added --max-requests 1000 for worker recycling. https://claude.ai/code/session_013YXpQQTzpN8ueQWUuQArAz
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Ageree
added a commit
that referenced
this pull request
Apr 27, 2026
Follow-up to fd6382b addressing 5-agent review of the PR1 commit. Real bugs introduced by PR1, now fixed: - F-5b — channel_signing.verify_channel_state regression (Agent#3 finding): PyNaCl's CryptoError is the parent of BadSignatureError, not the other way around. The narrowed `except BadSignatureError` left raw CryptoError uncaught, so a corrupt key would propagate as an unhandled 500 instead of cleanly rejecting the channel state. Add explicit `except CryptoError` and drop the redundant `binascii.Error` (it's a ValueError subclass in Python 3). Demote the BadSignatureError branch to DEBUG since it's the expected reject path. - F-9c — validate_session AttributeError on non-dict JSON (Agent#2 #1): json.loads accepts any valid JSON, not just objects. A stray "42" or "[1,2]" in the session payload would let the parse succeed but then `payload.get(...)` would AttributeError → 500. Guard with `isinstance(payload, dict)`. - F-9d — short-circuit before compare_digest (Agent#2 #2): `if not client_ip or not hmac.compare_digest(...)` returns faster on a missing IP than on a wrong IP, leaking presence of binding via timing. Always pass strings (`client_ip or ""`) so compare_digest runs in every branch. - F-9e — bound IP leaked in log (Agent#5 #1): The mismatch warning logged the bound IP in plaintext. Anyone with read access to Railway/Logtail logs would learn where each admin token was issued. Drop the IP from the log message. - F-13b — _secure_shuffle in-place mutation (Agents #1, #5): Violated the project-wide "ALWAYS create new objects, NEVER mutate" rule from ~/.claude/rules/common/coding-style.md. Now returns a fresh list; call site simplified. Documentation / config: - F-9b — trusted_proxy_hosts: documented Railway requirement in sthrip/config.py and .env.railway.example. Without ``TRUSTED_PROXY_HOSTS=*`` on Railway, request.client.host is the Railway edge IP, collapsing every admin session and rate-limit key to a single IP and bypassing F-9 binding. (Agent#3 finding.) - F-15b — CORS_DEV_AUTOEXTEND documented in .env.railway.example. - F-16 — _fix_pg_enums comment clarified: code "log+skips" unsafe entries, it does not raise.
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
2 participants
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
All 7 production-readiness items from the checklist, implemented and tested:
pending_withdrawalsstatus column — migration convertsVARCHAR(32)→withdrawalstatusenum; runtime fallback now creates the enum type before the tablePOST /v2/balance/depositto prevent wallet RPC abusescripts/backup_db.shwith pg_dump, gzip, rotation, S3 upload, cron install, and restore modesPOST /v2/balance/withdrawwith agent auth (not admin), covering success/failure/validation/idempotencyPOST /v2/admin/cleanup-test-agentsendpoint with pattern matching, dry-run, and balance safety guard--max-requests 1000for worker recyclingTest plan
test_withdraw_api.pytests pass (auth, validation, balance deduction, idempotency)test_withdrawal_atomic.pytests passtest_withdraw_self_send.pytests passtest_balance.pytests passPOST /v2/admin/cleanup-test-agentswith dry_run=true on stagingALERT_WEBHOOK_URLon staging and verify Slack/Telegram alerts firescripts/backup_db.shagainst staging DB and verify restoreWEB_CONCURRENCY=2and verify only one worker runs DepositMonitorhttps://claude.ai/code/session_013YXpQQTzpN8ueQWUuQArAz