chore(v2): retire V1 paths post-cutover (closes #37)#51
Conversation
Empty marker commit so the draft PR can exist. Real changes will be pushed when this branch is picked up post-soak. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (25)
📝 WalkthroughWalkthroughThis PR completes the Polymarket V2 cutover by removing all V1 code paths, deleting the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
✨ 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 |
V1 endpoints stopped accepting orders at 2026-04-28 11:00 UTC. The POLYMARKET_V2_ENABLED flag flipped default in PR #49 (commit 306cbcb); this PR removes the V1 code path entirely. Backend: - Delete core/feature_flags.py and the V2_CONTRACTS namespace; merge V2 addresses into CONTRACTS (CTF_EXCHANGE / NEG_RISK_CTF_EXCHANGE now hold V2 addresses, plus PUSD / onramp / offramp). - core/wallet/manager.py: drop _collateral_address/_exchange_addresses helpers, drop NEG_RISK_ADAPTER approvals (dead allowance after #45), rename WalletBalances.usdc_e -> pusd. - core/trading/clob_client.py: delete _get_clob_client_v1 and the V1 endpoint; CLOB client is V2 only. - core/trading/clob.py: drop V1 import branch. - core/trading/executor.py and core/positions/manager.py: drop V1 collateral helpers and the vestigial neg_risk parameter on _split_position / _merge_tokens (Gamma's negRisk is a UI hint, not on-chain routing — see #45). - pyproject.toml: drop py-clob-client dep, keep py-clob-client-v2. Tests: - Delete test_v2_collateral.py (V1/V2 branch tests no longer meaningful). - Strip V1 fixtures from test_v2_order_struct.py. Routers / frontend: cascade rename usdc_e -> pusd through server/routers/wallet.py, server/routers/trading.py, hooks/useWallet.tsx, components/PortfolioModal.tsx, components/terminal/WalletDropdown.tsx (label updated to "pUSD"). Experiments: remove duplicated _v2_enabled() in 01_setup_wallet.py, 03_buy_position.py, 04_transfer_tokens.py; hardcode V2 paths. Docs: drop the V2 cutover banner from README.md and the cutover section in CLAUDE.md; drop POLYMARKET_V2_ENABLED line from .env.example. Operator post-merge step (not code): revoke V1 USDC.e approvals. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Update collateral terminology to match V2: rename `usdc_e` -> `pusd` in example wallet/balance JSON, swap "USDC.e" -> "pUSD" in instructional prose. The other four alphapoly skills (feature, experiment, pipeline, portfolios) had no V1 references and were left alone. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Main got a fix at c189a3c — Polymarket consolidated clob-v2.polymarket.com into clob.polymarket.com post-cutover. This branch was rewriting the same file with the old URL, which would have re-introduced the bug at squash-merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Hardcoded gas: 300000 was rejected by Chainstack RPC under stricter
post-cutover limits ('INTERNAL_ERROR: gas limit is too high'). Actual
usage is ~135k for split and ~90k for merge. Estimate live and apply
25% headroom.
Verified live during V2 validation: a stuck $3 merge that failed under
the old hardcoded limit went through cleanly with the estimate.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
# Conflicts: # backend/core/trading/clob_client.py
Phase 3 of the V1 retirement plan. Open during the V2 soak window so the next agent session can pick it up with full context. Scaffold-only right now (empty commit) — real changes land when the soak completes and V2 is verified stable.
Closes #37.
Pre-merge gate
306cbcb) has been live for at least 1 weekPOLYMARKET_V2_ENABLED=falserollbackPOST /trading/buy-pair) still working against V2 booksIf any of those fail, hold this PR — V1 paths are the rollback escape hatch.
Scope (full V1 deletion)
Code deletion
backend/core/feature_flags.py— delete entire modulebackend/core/trading/clob_client.py— delete_get_clob_client_v1, all V1 branchesbackend/core/trading/clob.py— drop V1 order struct pathbackend/core/trading/executor.py— drop V1 fee math (fees.pyV1 path)backend/core/positions/manager.py— drop V1 branchesbackend/core/wallet/contracts.py— removeUSDC_E, V1CTF_EXCHANGE, V1NEG_RISK_CTF_EXCHANGEentriesbackend/core/wallet/manager.py::set_approvals— drop V1 approval branches_collateral_address()/_exchange_addresses()— inline pUSD/V2 addresses directly at call sites_split_position/_merge_tokens— drop theneg_riskparameter (kept only as log context after fix(v2): route NegRisk-flagged splits/merges through CTF, not the adapter #45; no longer affects routing)set_approvals(dead allowance after fix(v2): route NegRisk-flagged splits/merges through CTF, not the adapter #45)Dependencies
backend/pyproject.toml— removepy-clob-client, keeppy-clob-client-v2onlyuv lockafter the dep removalTests
backend/tests/test_v2_collateral.py— drop V1 fixture + assertions, keep V2-onlybackend/tests/test_v2_order_struct.py— drop V1 dataclass field-delta tests, keep V2-onlycore.feature_flags— delete or rewriteSchema/UI rename (optional but strongly recommended)
WalletBalances.usdc_e→pusd— cascade rename across:backend/core/wallet/manager.pybackend/server/routers/wallet.pyfrontend/components/**andfrontend/hooks/**(wallet status consumers)experiments/trading/*referencesExperiments
_v2_enabled()defaults inexperiments/scripts — currently still default-false because they were left untouched in feat(v2): flip POLYMARKET_V2_ENABLED default to true (Phase 2 — DO NOT MERGE BEFORE 2026-04-28 11:00 UTC) #49. Either remove the function entirely (always V2) or flip default to true.Docs
CLAUDE.md— drop the V2 cutover section +POLYMARKET_V2_ENABLEDenv var docsREADME.md— drop V2 cutover banner (added in docs(readme): V2 cutover banner + v1-final pointer (Phase 1) #48).env.example— removePOLYMARKET_V2_ENABLEDline entirelyMemory hygiene (out of repo)
polymarket_v1_retirement_plan.mdto git so it travels with the repo (currently lives only in user's local memory at~/.claude-corpo/.../memory/)Post-merge operator step (not code)
Reference
v1-finalat94e89ce(https://github.com/chainstacklabs/polymarket-alpha-bot/releases/tag/v1-final)306cbcb) — flag default flipped0xE111180000d2663C0091e4f400237545B87B996B0xe2222d279d744050d28e00520010520000310F590xAdA200001000ef00D07553cEE7006808F895c6F1(only relevant if true multi-outcome trading is added)Findings worth keeping (from live testing 2026-04-27, before deleting V1 code)
negRisk: trueis a UI grouping hint, not on-chain routing. Always route splits/merges through standard CTF using the conditionId Gamma returns (fixed in fix(v2): route NegRisk-flagged splits/merges through CTF, not the adapter #45). Theneg_riskparameter on_split_position/_merge_tokensis now dead weight — remove it in this PR.splitPosition(bytes32,uint256)) is for true multi-outcome flows the bot does NOT use.positions/service.pyenrichment quirk: same wallet with multiple positions on same market+side shares on-chain CTF balance; each position'starget_balanceshows aggregate. Not V2-specific, pre-existing.CTF.balanceOf(addr, gamma_clobTokenIds[0])returns 0 — Gamma'sclobTokenIdsare CLOB identifiers, not on-chain CTF position IDs. Backend stores token IDs from split receipts, so this works in practice; do not "fix" by reading from Gamma.Test plan (when this PR gets real changes)
pytest backend/tests/greenruff check backend/cleancd frontend && npm run typecheckclean (ifusdc_e→pusdrename happens)make pipeline/wallet/statusreturns expected pUSD balancePOST /trading/buy-pairagainst V2 booksPOST /positions/{id}/exitworks (sell + merge fallback)v2.0.0in any version file (semver — breaking change, V1 path removed)Out of scope
🤖 Generated with Claude Code
Live V2 validation (2026-04-28, post-cutover)
Driven on real Polygon mainnet from wallet
0x1a0D3129Cf6bBA90ADC95C1048B60a9981CAFe71after the cutover finished. Two extra fixes shaken out and bundled into this PR:Fix 1 — V2 CLOB endpoint redirect
clob-v2.polymarket.com301-redirects toclob.polymarket.comafter Polymarket consolidated the V2 cutover.py_clob_client_v2's HTTP client doesn't follow redirects, soderive_api_keyraisedPolyApiException[status_code=301]andget_clob_client()returnedNone. PointedCLOB_URLat the canonical host. (Same fix landed onmainasc189a3c; this branch carries it forward so the squash-merge doesn't reintroduce the bug.)Fix 2 — Gas estimation for split + merge
splitandmergehadgas: 300000hardcoded. Chainstack RPC tightened limits post-cutover and started rejecting withINTERNAL_ERROR: gas limit is too high. Real usage is ~135k for split and ~90k for merge. Switched toestimate_gas() * 1.25. Verified live: a stuck $3 merge that failed at 300k went through cleanly with the estimate.What was tested live (txn proofs)
/trading/buy-pairestimate0x0e03f82e29fe4d3aa49456dd52c2691b09094808ab84da6b0616d734146884960xb15ccd996c29a149d724dfe91691fef4db81c2a3860809498422558ca83eb8030x6c6ddbb73e85f7dd34f904b6c78aa959b9c0f17d615d1966e07efbd53b344db40x9c84cb6121edba06151137e573b9353d81bc83484c66962f5035591031cb425b0x14fbf7ef6100dce158a7923069ed9415035d4da56d9502a975d5d0d04c6fd20c0x5a375146c607e83b979a048affee6b83da602a3e078ae2b763a09d3bd589b983derive_api_key)Offramp.unwrap(pUSD → USDC.e)0xff6cfb7e25c728feb3f324c9e14f59bde28e54e296c4410cb2263a2d9ea65c1fOnramp.wrap(USDC.e → pUSD)0x9bea271da849444bad6c9f7c559aa477fb52d7b057f0cc5aef555fc708f4acd9Round-trip math: pUSD started 28.383236, ended 28.383236 after multiple buy/exit cycles + offramp/wrap roundtrip. Exact reversal. POL gas total: ~0.13 across 11 txns.
What's still untested (deferred, not blockers)
403 Trading restricted in your region — enable proxyfrom Polymarket's geo-block. Behaviour-correct for our IP, but the EIP-712 v2 signature post path can only be verified from a permitted region or via residential proxy. No code change indicated.CTF.redeemPositionsafter a market resolves — we have no resolved market to test against. The bot has no built-in redeem endpoint; users redeem manually until that ships.WalletManager.set_approvals()— current wallet already has approvals set. The V2 approval list is correct in code (5 calls: pUSD→CTF/CTF_EXCHANGE/NEG_RISK_CTF_EXCHANGE + CTF→CTF_EXCHANGE/NEG_RISK_CTF_EXCHANGE) but only verified viaapprovals_set: trueread, not by runningset_approvalsfrom scratch.Out of scope for this PR
POST /pipeline/run/productionbackground task wedges silently if the LLM step fails during the run (status keeps readingrunning: trueeven after the bg task dies). Cosmetic, not migration-related; file as a separate issue.Summary by CodeRabbit
Release Notes
Documentation
Chores