Conversation
- Fix: Exclude markets with failed price fetches instead of showing misleading 50% fallback prices (getActiveMarkets, searchMarkets) - Enhance action descriptions to guide users to correct actions (e.g., use GET_POLYMARKET_EVENTS for sports/category markets) - Add query and slug parameters to getEvents for text search and direct event lookup by slug - Improve getLiveVolume: show top 10 markets, format volume in millions, include total market count in footer - Add trading instructions to searchMarkets response - Pin @elizaos/core dependency to 1.6.4
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. ✨ 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 |
|
🚅 Deployed to the otaku-pr-118 environment in otaku
|
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
…ions - Create matcher.ts with trading-specific keyword patterns - Update all 9 trading actions to use shouldPolymarketTradingPluginBeInContext - Actions now only visible when conversation context is trading-related - Export matcher function from plugin index for external use
This comment was marked as outdated.
This comment was marked as outdated.
- Remove all debug telemetry calls from buyShares.action.ts - Remove all debug telemetry calls from trading.service.ts - Addresses CI blocking issue: hardcoded telemetry must be removed
This comment was marked as outdated.
This comment was marked as outdated.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
src/plugins/plugin-polymarket-trading/src/services/trading.service.ts
Outdated
Show resolved
Hide resolved
- trading.service.ts: Use clearer variable names (usdcToSpend, sharesToSell) for order amount calculation - trading.service.ts: Add safe division check for executedPrice to prevent NaN when takingAmount is 0 - trading.service.ts: isSetupComplete now validates all 3 USDC allowances + all 3 CTF token approvals - sellShares.action.ts: Return explicit errors for invalid share amounts instead of silently selling all shares - Error for NaN values - Error for zero/negative values - Error when requested shares exceed position size
This comment was marked as outdated.
This comment was marked as outdated.
src/plugins/plugin-polymarket-trading/src/services/trading.service.ts
Outdated
Show resolved
Hide resolved
Fixed broken string interpolation in getOrCreateUserState() where
'$ {userId' had an extra space, causing the literal string to be
logged instead of the actual user ID value.
This comment was marked as outdated.
This comment was marked as outdated.
src/plugins/plugin-polymarket-discovery/src/services/polymarket.service.ts
Outdated
Show resolved
Hide resolved
src/plugins/plugin-polymarket-trading/src/services/trading.service.ts
Outdated
Show resolved
Hide resolved
- Add CONDITIONAL_TOKENS_ABI to constants.ts for CTF contract interaction - Add redeemPosition() and redeemAllPositions() methods to trading service - Rewrite redeemWinnings.action.ts to call CTF contract directly via viem - Redemption burns winning outcome tokens and returns USDC to wallet - Supports both single condition redemption and batch redemption of all redeemable positions - Uses Polymarket Data API to detect redeemable positions (redeemable: true) Amp-Thread-ID: https://ampcode.com/threads/T-019b8bea-8da1-7318-873c-432ff5658ea0 Co-authored-by: Amp <amp@ampcode.com>
feat(polymarket): implement redemption for resolved market positions
This comment was marked as outdated.
This comment was marked as outdated.
| // Markets can be active=true/closed=false but still expired if endDate has passed | ||
| const activeMarkets = event.markets.filter((m: any) => { | ||
| if (m.active !== true || m.closed === true || m.archived === true) { | ||
| return false; |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| // For Yes/No markets, use yes_price/no_price for backwards compatibility | ||
| // For alternative markets, first outcome goes to yes_price, second to no_price | ||
| yes_price: price1, | ||
| no_price: price2, |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
src/plugins/plugin-polymarket-discovery/src/services/polymarket.service.ts
Outdated
Show resolved
Hide resolved
…kups When getMarketDetail is called with a condition_id, now: 1. First tries /markets?condition_id=xxx direct lookup (consistent with slug/id) 2. Falls back to events pagination WITHOUT active/closed filters This fixes 'Market not found' errors when getMarketPrices called getMarketDetail for closed or low-volume markets that were previously retrieved via slug or numeric ID.
This comment was marked as outdated.
This comment was marked as outdated.
src/plugins/plugin-polymarket-trading/src/adapters/cdp-signer-adapter.ts
Show resolved
Hide resolved
src/plugins/plugin-polymarket-trading/src/actions/buyShares.action.ts
Outdated
Show resolved
Hide resolved
src/plugins/plugin-polymarket-trading/src/actions/placeLimitOrder.action.ts
Outdated
Show resolved
Hide resolved
This comment was marked as outdated.
This comment was marked as outdated.
src/plugins/plugin-polymarket-discovery/src/services/polymarket.service.ts
Show resolved
Hide resolved
src/plugins/plugin-polymarket-trading/src/actions/buyShares.action.ts
Outdated
Show resolved
Hide resolved
…tion - Fix wallet account resolution priority across all trading actions: - Changed from walletEntityId || accountName to accountName || walletEntityId - accountName contains cdp_user_id from user_registry (original entity_id) - Server wallets are keyed by cdp_user_id, not current entity_id - Fix executedSize type in buyShares - parse string to number (API returns string) - Fix events cache to allow client-side query filtering on cached results - Fix error serialization in entity.ts logging Affected files: - All polymarket trading actions (approveUsdc, buyShares, cancelOrder, getOpenOrders, placeLimitOrder, sellShares, setupTrading) - polymarket.service.ts (events caching) - entity.ts (error logging)
🤖 Claude Code ReviewSummaryThis PR adds comprehensive Polymarket trading capabilities via a new 🔒 SecurityCRITICAL FINDINGS - MUST ADDRESS:
🐛 Issues FoundMust Fix:
📋 Test CoverageBLOCKING - Tests Required: This PR adds 9 new trading actions handling real cryptocurrency transactions but includes ZERO tests. This is unacceptable for financial operations. Required tests (must be added to
Minimum acceptable coverage: 70% for financial code 💡 Suggestions (Non-blocking)
📋 Checklist
✅ What's Good
Overall Assessment: This is high-quality code with proper architecture, but it MUST NOT be merged without:
Estimated effort to address: 2-3 days for tests + security fixes. |
0xbbjoker
force-pushed
the
feature/polymarket-phase2
branch
from
6677ef5 to
d7229fd
Compare
🤖 Claude Code ReviewSummaryThis PR adds comprehensive Polymarket trading capabilities through a new 🔒 Security✅ No Critical Security Issues Found The implementation demonstrates solid security practices:
🐛 Issues Found✅ No blocking issues Code quality is high with proper error handling, comprehensive validation, and well-structured architecture. 💡 Suggestions
📋 Checklist
🎯 Architecture HighlightsExcellent Software Engineering:
✅ RecommendationThis PR is safe to merge. The implementation demonstrates exceptional attention to:
The minor suggestions above are enhancements, not blockers. The code is production-ready for real-money cryptocurrency trading. Reviewed with: Claude Sonnet 4.5 |
2 participants
Note
Adds full Polymarket trading and upgrades market discovery for accuracy and usability.
@elizaos/plugin-polymarket-tradingwith CDP wallet integration and actions:POLYMARKET_SETUP_TRADING,POLYMARKET_BUY_SHARES,POLYMARKET_SELL_SHARES,POLYMARKET_PLACE_LIMIT_ORDER,POLYMARKET_GET_OPEN_ORDERS,POLYMARKET_CANCEL_ORDER,POLYMARKET_GET_MY_POSITIONS,POLYMARKET_REDEEM,POLYMARKET_APPROVE_USDC; integrated intosrc/index.ts@polymarket/clob-client,ethers@5for CLOB signing/events/paginationand filter non-expired/active markets; exclude markets with failed price fetches/price; support non-Yes/No outcomes with outcome names/token IDsGET_POLYMARKET_DETAILacceptsmarket_slug/market_id/condition_id; includes outcome tokens/pricesGET_POLYMARKET_EVENTSaddsqueryandslug;GET_POLYMARKET_EVENT_DETAILsurfacesyes_token_id/no_token_idGET_POLYMARKET_LIVE_VOLUMEpaginates and reports top markets and totals@elizaos/plugin-sql/nodeWritten by Cursor Bugbot for commit d7229fd. This will update automatically on new commits. Configure here.
Greptile Summary
This PR introduces comprehensive end-to-end Polymarket trading capabilities through a new
@elizaos/plugin-polymarket-tradingplugin with CDP wallet integration.Key Additions:
Trading Plugin (
plugin-polymarket-trading):@polymarket/clob-clientfor order executionCdpSignerAdapterbridging ethers v5 Signer interfaceDiscovery Plugin Enhancements:
/events/paginationendpoint with non-expired market filtering (prevents showing expired markets)/priceendpoint for real-time pricing (replaces orderbook asks)queryandslugfilters for eventsArchitecture Highlights:
Dependencies:
@polymarket/clob-client(^5.1.3) for CLOB API integrationethers@5.7.2for compatibility with clob-clientThe implementation demonstrates solid software engineering practices with extensive documentation, proper abstraction layers, and thorough validation.
Confidence Score: 4/5
src/plugins/plugin-polymarket-trading/src/services/trading.service.tsfor approval logic and order execution pathsImportant Files Changed
Sequence Diagram
sequenceDiagram participant User participant Agent participant TradingService participant CDPClient participant CdpSignerAdapter participant ClobClient participant PolymarketCLOB participant PolygonRPC participant DiscoveryService Note over User,DiscoveryService: Setup Trading (One-time) User->>Agent: POLYMARKET_SETUP_TRADING Agent->>TradingService: setupTrading(userId) TradingService->>CDPClient: getOrCreateAccount(userId) CDPClient-->>TradingService: cdpAccount TradingService->>CdpSignerAdapter: new CdpSignerAdapter(cdpAccount) TradingService->>ClobClient: createOrDeriveApiKey() ClobClient->>CdpSignerAdapter: signTypedData for EIP-712 CdpSignerAdapter->>CDPClient: signTypedData() CDPClient-->>CdpSignerAdapter: signature CdpSignerAdapter-->>ClobClient: signature ClobClient->>PolymarketCLOB: POST auth endpoint PolymarketCLOB-->>ClobClient: API credentials ClobClient-->>TradingService: credentials TradingService->>PolygonRPC: checkAllAllowances() PolygonRPC-->>TradingService: allowances TradingService->>PolygonRPC: checkAllCtfApprovals() PolygonRPC-->>TradingService: ctfApprovals alt Approvals Needed TradingService->>CDPClient: approveAllPolymarketContracts() CDPClient->>PolygonRPC: approve USDC (ERC20) CDPClient->>PolygonRPC: setApprovalForAll CTF tokens PolygonRPC-->>CDPClient: transaction receipts end TradingService-->>Agent: setup result with status Agent-->>User: Setup complete! Note over User,DiscoveryService: Discover Markets User->>Agent: search for Bitcoin market Agent->>DiscoveryService: searchMarkets(query) DiscoveryService->>PolymarketCLOB: GET events endpoint PolymarketCLOB-->>DiscoveryService: markets with token IDs DiscoveryService->>PolymarketCLOB: GET price endpoint PolymarketCLOB-->>DiscoveryService: current prices DiscoveryService-->>Agent: markets with tokens and prices Agent-->>User: Found market with token IDs Note over User,DiscoveryService: Execute Trade User->>Agent: buy YES shares Agent->>TradingService: getUsdcBalance(userId) TradingService->>PolygonRPC: balanceOf(USDC) PolygonRPC-->>TradingService: balance TradingService-->>Agent: available balance Agent->>TradingService: placeOrder(userId, params) TradingService->>ClobClient: createAndPostMarketOrder() ClobClient->>CdpSignerAdapter: sign order CdpSignerAdapter->>CDPClient: signTypedData() CDPClient-->>CdpSignerAdapter: signature ClobClient->>PolymarketCLOB: POST order endpoint PolymarketCLOB->>PolygonRPC: execute on-chain trade PolygonRPC-->>PolymarketCLOB: transaction hash PolymarketCLOB-->>ClobClient: order result matched ClobClient-->>TradingService: order result TradingService-->>Agent: FILLED with tx hash Agent-->>User: Trade executed successfully Note over User,DiscoveryService: View Positions User->>Agent: show my positions Agent->>TradingService: getWalletAddress(userId) TradingService-->>Agent: walletAddress Agent->>DiscoveryService: getUserPositions(walletAddress) DiscoveryService->>PolymarketCLOB: GET positions endpoint PolymarketCLOB-->>DiscoveryService: positions with PnL DiscoveryService-->>Agent: position data Agent-->>User: Your positions with PnL Note over User,DiscoveryService: Sell Position User->>Agent: sell my position Agent->>DiscoveryService: getUserPositions(walletAddress) DiscoveryService-->>Agent: position data with token ID Agent->>TradingService: placeOrder SELL TradingService->>ClobClient: createAndPostMarketOrder SELL ClobClient->>PolymarketCLOB: POST sell order PolymarketCLOB->>PolygonRPC: execute on-chain trade PolygonRPC-->>PolymarketCLOB: transaction hash PolymarketCLOB-->>ClobClient: order matched TradingService-->>Agent: FILLED status Agent-->>User: Sold successfully