Wallet Multi-Network Support + PancakeSwap CLMM LP Management & MasterChef Integration on BSC#638
Conversation
… all routes
All endpoints previously showed only 'mainnet'/'mainnet-beta' in Swagger examples,
leaving BSC (and other EVM networks: arbitrum, base, polygon, avalanche, optimism,
celo) invisible to users browsing /docs.
Updated files and what changed:
- src/schemas/chain-schema.ts: EstimateGasRequest, BalanceRequest, TokensRequest,
PollRequest, StatusRequest — add network description + examples incl. bsc,
arbitrum, base (used by /chains/ethereum/{tokens,balances,status,estimateGas,poll})
- src/schemas/amm-schema.ts: GetPoolInfoRequest (chainNetwork+network), AddLiquidity,
RemoveLiquidity, GetPositionInfoRequest, QuoteSwapRequest, ExecuteSwapRequest —
add network/chainNetwork descriptions + examples incl. ethereum-bsc, bsc
- src/schemas/clmm-schema.ts: FetchPoolsRequest, GetPositionsOwnedRequest,
GetPoolInfoRequest (chainNetwork+network), GetPositionInfoRequest, OpenPosition,
AddLiquidity, RemoveLiquidity, CollectFees, ClosePosition, QuoteSwap, ExecuteSwap —
add network/chainNetwork descriptions + examples incl. ethereum-bsc, bsc
- src/tokens/schemas.ts: TokenListQuerySchema, TokenViewQuerySchema,
TokenAddRequestSchema, TokenRemoveQuerySchema — chain description updated to clarify
'ethereum = all EVM networks incl. BSC'; network examples add bsc/arbitrum/base/polygon;
FindTokenQuerySchema chainNetwork examples add ethereum-bsc first
- src/pools/schemas.ts: PoolListRequestSchema, PoolAddRequestSchema,
GetPoolRequestSchema — network examples add bsc/arbitrum/base; connector
examples add pancakeswap; chain descriptions updated; FindPoolsQuerySchema
chainNetwork examples put ethereum-bsc first, connector examples updated
- src/pools/routes/removePool.ts: inline querystring network examples add bsc
- src/pools/routes/getPool.ts: tradingPair examples add WBNB-USDT, CAKE-USDT
Pre-existing test failures (chain-network-parsing: 2 tests) unchanged.
fengtality
left a comment
fengtality
left a comment
There was a problem hiding this comment.
Thanks for the substantial contribution. The MasterChef integration is architecturally sound (safeTransferFrom is the correct deposit path, approvals are checked pre-flight, CAKE harvested via withdraw). A few blocking issues and a scope concern:
📦 Scope concern (please read first)
Bundling three loosely-related changes in one 54-file / 6892-line PR is hard to review safely:
- Wallet storage redesign (
wallet/utils.ts+223/-64,wallet/schemas.ts+144/-20) — touches core infrastructure used by every connector - PancakeSwap CLMM fixes (
positionsOwned,quotePosition,poolInfo,executeSwap) - New PancakeSwap MasterChef NFT staking (4 route files, 363-line ABI, ~580 LoC of tests)
A bug in (1) affects Ethereum, Solana, Uniswap, Raydium, Meteora; (3) is high-risk new custody code. Recommend splitting into 3 PRs, with (1) landing first and (3) depending on it. Reviewers can give each the attention it needs.
🔴 Critical (must fix before merge)
1. pancakeswap.ts stakeNft — poolId === 0 falsely rejects valid MasterChef pool #0.
v3PoolAddressPid is a Solidity mapping(address => uint256) returning 0 for both unregistered keys AND for legitimately registered pid-0 pools. PancakeSwap V3 MasterChef pools start at pid 0 (typically CAKE/WBNB), so the first registered pool can never be staked through this code path. Use the poolInfo(0).v3Pool check or the dedicated poolExists mapping to distinguish.
2. All 4 NFT staking route handlers use raw reply.status(500).send instead of fastify.httpErrors.
masterchef-stake.ts, masterchef-unstake.ts, masterchef-unstake-and-close.ts, masterchef-knows-pool.ts all catch errors and respond with reply.status(500).send({ error: ... }). The repo convention (CLAUDE.md) is fastify.httpErrors.*. This also collapses 400-class precondition failures (not-approved, not-staked, zero-liquidity) into opaque 500s that callers can't distinguish from server crashes.
3. AddWalletRequestSchema — chain made Optional without oneOf(chain, chainNetwork) constraint.
If a caller submits { privateKey } with neither chain nor chainNetwork, validateChainName(undefined) returns false and the error message becomes Unrecognized chain name: undefined. getWalletBalance correctly adds a !resolvedChain guard; addWallet and createWallet should match. Better: add a schema-level oneOf so Swagger / Hummingbot callers get a clear validation error.
🟡 Important
pancakeswap.tsunstakeNft— callsmasterChef.withdraw(tokenId, walletAddress)with no precondition check that the NFT is actually staked. Will surface as an opaque ethers revert with no user-friendly message. Add an ownership check viauserPositionInfosorownerOffirst.masterchef-unstake-and-close.ts:1122-1123— hardcodedsetTimeout(2000)between unstake and close is shorter than a BSC block (~3s). Useawait tx.wait(1)on the unstake receipt instead.wallet/utils.tsgetWallets— corrupted/unreadable wallet files silently default to[defaultNetwork]with no warning log. Operators won't notice a wallet was misrouted. Add alogger.warnin the catch.pancakeswap.tsisBaseToken0heuristic (~line 1639) — uses WETH as the "wrapped native" special case, but on BSC the wrapped native is WBNB. Prices will invert on BSC pairs. The check should derive from the active network's wrapped-native address.
🟢 Nits
masterchef-stake.tsandmasterchef-unstake.tsdefine TypeBox schemas inline rather than inpancakeswap/schemas.ts— inconsistent with the rest of the connector..github/copilot-instructions.md(+57) — does this belong committed, or only in dev tooling? Worth confirming with maintainers.getPoolMasterchefData(line ~1428) silently returns zeros on error. Combined with the pid==0 check, an RPC failure could mislead callers — but this is post-stake so cosmetic.
🔁 Related — please port the two CLMM fixes from #642
Since this PR is the one touching PancakeSwap CLMM right now, would you mind picking up the changes from #642? PancakeSwap V3 is a Uniswap V3 fork, so both apply directly:
-
BUY-side price inversion in
clmm-routes/quoteSwap.ts— Orca and Meteora computedprice = outputAmount / inputAmountregardless ofside, returning base/quote on BUY and quote/base on SELL. Please verify PancakeSwap CLMMquoteSwapreturns the same price unit on both sides (quote per base) — same pattern as Uniswap CLMM in #642. Regression tests intest/connectors/orca/clmm-routes/quoteSwap.test.tsshow the assertion shape. -
binCountparam onclmm-routes/poolInfo.ts— #642 adds an optional?binCount=Nparam that returns N tick-spacing-wide bins centered on the active tick with{ binId, price, baseTokenAmount, quoteTokenAmount }entries. Default behavior unchanged (binCount=0→ nobins[]field). The Uniswap V3 implementation insrc/connectors/uniswap/uniswap.utils.ts::computeUniswapBinDistributionis the closest reference — it uses parallelpool.ticks(tick)reads, propagates L vialiquidityNet, and computes amounts viaSqrtPriceMath.getAmount{0,1}Delta. The schema additions (bins[]inPoolInfoSchema,binCountinGetPoolInfoRequest) live insrc/schemas/clmm-schema.tsand apply to PancakeSwap automatically.
Happy to help if either is non-obvious to port — most of the logic transfers directly.
Verdict
Once split into 3 PRs and the pid-0 / httpErrors / schema-constraint issues are addressed, the MasterChef pieces look mergeable. The wallet refactor is more invasive and would benefit from focused review on its own. Happy to re-review once split.
|
Created #646 and will add the other one here and then will close this PR |
|
Created the scoped branch/pr for #647 and this can now be closed |
2 participants
PR Summary: Wallet Multi-Network Support + PancakeSwap CLMM LP Management & MasterChef Integration on BSC
What's Included
✅ Existing in
developmentBranchPart 1: Wallet Multi-Network Support
walletAddresses[]field🆕 Proposed Additions (This PR)
Part 2: PancakeSwap CLMM & MasterChef Integration
Part 1: Wallet Multi-Network Support (✅ Existing Foundation)
This section documents the wallet multi-network layer that enables proper per-network wallet tracking and automatic network detection for balance queries across multiple EVM networks.
Key Features Added
1. Network-Aware Wallet Storage
networks: string[]array with primary network atnetworks[0]/wallet/balanceautomatically uses wallet's registered network if address is found in wallet storewalletAddresses: string[]remains unchanged (Hummingbot lens: maintain Python strategy compatibility)2. Enhanced Balance Endpoint
POST /wallet/balance- Improved network detectionnetworkparam omitted: checks wallet store for address, uses primary registered network3. Chain Instance Management
Ethereum.resetInstance(network)- Evict cached provider instancenodeURLchangesSolana.resetInstance(network)- Same functionality for Solana4. RPC Reliability Fix
https://binance.llamarpc.com→https://bsc-rpc.publicnode.comWallet Schema Changes (Backwards Compatible)
Before:
{ "address": "0xAddress", "network": "mainnet", // Single network (redundant) "networks": ["mainnet", "bsc"] // Optional array (confusing) }After:
{ "address": "0xAddress", "networks": ["bsc", "mainnet"] // Single source of truth }Why: Removes ambiguity and simplifies routing logic.
networks[0]is the primary/registered network.File Structure (Wallet Changes)
Modified Files:
src/wallet/schemas.ts- SimplifiedWalletEntrySchema(removednetworkfield)src/wallet/utils.ts- Updated wallet details generation, added wallet store lookup in balance endpointsrc/wallet/routes/balance.ts- Enhanced description and network resolution logicsrc/chains/ethereum/ethereum.ts- Addedstatic resetInstance(network)methodsrc/chains/solana/solana.ts- Addedstatic resetInstance(network)methodsrc/config/routes/updateConfig.ts- Added chain singleton eviction on nodeURL changesrc/templates/chains/ethereum/bsc.yml- Updated BSC RPC endpointconf/chains/ethereum/bsc.yml- Updated BSC RPC endpointJest Test Coverage (35 Tests)
All tests passing ✅
test/wallet/wallet-balance.test.ts(12 tests)test/wallet/wallet-multinetwork.test.ts(12 tests)test/wallet/wallet-network-support.test.ts(11 tests)networkparameter handlingchainNetworkshorthand parsing (e.g.,ethereum-bsc)Part 2: PancakeSwap CLMM & MasterChef Integration (🆕 New Features)
PancakeSwap CLMM (Concentrated Liquidity Market Maker) position management and MasterChef staking, now fully functional on BSC with the wallet multi-network support above as foundation.
New Endpoints (3 additions)
MasterChef NFT Staking Operations:
/connectors/pancakeswap/nft-staking/masterchef-stakePOST/connectors/pancakeswap/nft-staking/masterchef-unstakePOST/connectors/pancakeswap/nft-staking/masterchef-unstake-and-closePOST/connectors/pancakeswap/nft-staking/masterchef-knows-poolPOSTFeatures:
walletAddressparameter for transaction signingCritical Fixes (BigInt & Amount Handling)
openPositionandquoteSwapto properly handle numeric valuesEnhanced Parameters
positionsOwned- AddedactiveOnlyfilter to show only positions with active liquidityexecuteSwap- Added optionalpoolAddressparameter for manual pool selectionmasterchef-stake/unstake- AddedwalletAddressparameter for transaction signingFile Structure (PancakeSwap Changes)
Core Implementation Files
src/connectors/pancakeswap/pancakeswap.tssrc/connectors/pancakeswap/clmm-routes/index.tssrc/connectors/pancakeswap/masterchef-stake.tssrc/connectors/pancakeswap/masterchef-unstake.tssrc/connectors/pancakeswap/masterchef-unstake-and-close.tssrc/connectors/pancakeswap/masterchef-knows-pool.tssrc/connectors/pancakeswap/PancakeswapV3Masterchef.abi.jsonFixed/Enhanced Files
src/connectors/pancakeswap/clmm-routes/quoteSwap.tssrc/connectors/pancakeswap/clmm-routes/openPosition.tssrc/connectors/pancakeswap/clmm-routes/positionsOwned.tsactiveOnlyparametersrc/connectors/pancakeswap/clmm-routes/positionInfo.tssrc/connectors/pancakeswap/schemas.tsTesting Checklist
Part 1: Wallet Multi-Network (✅ Existing - Already Verified)
These tests are already in the development branch and passing:
test/wallet/wallet-balance.test.ts→ 12/12 passing ✅test/wallet/wallet-multinetwork.test.ts→ 12/12 passing ✅test/wallet/wallet-network-support.test.ts→ 11/11 passing ✅Part 2: PancakeSwap MasterChef (🆕 Verification Required)
Before merging, verify these new features:
Unit Tests
Integration Tests
(use Step-by-Step Testing Guide below)
POST /connectors/pancakeswap/nft-staking/masterchef-stakesucceeds (after NFT approval)POST /connectors/pancakeswap/execute-swaphandles decimal amounts correctlyPOST /connectors/pancakeswap/open-positionaccepts string and number inputsPOST /connectors/pancakeswap/masterchef-unstake-and-closecompletes both operationsPOST /connectors/pancakeswap/nft-staking/masterchef-unstakereturns collected rewardsPOST /connectors/pancakeswap/nft-staking/masterchef-knows-poolcorrectly identifies registered poolsValue Conversions Corrected
All the following value conversion issues were fixed:
Fix 2: Execute-Swap BigInt Conversion Error
Problem
{ "statusCode": 500, "error": "HttpError", "message": "Failed to execute swap: Cannot convert 200000000000000000 to a BigInt" }When calling with
amount: 0.2, getting BigInt conversion error.File:
src/connectors/pancakeswap/clmm-routes/executeSwap.tsBackwards Compatibility
✅ All changes are backwards compatible:
walletAddresses: string[]field unchanged (Hummingbot Python strategies rely on this){encryptedKey, network}still supported with automaticnetworks[]conversionnetworks[]arrayNo Breaking Changes ✅
Step-by-Step Testing Guide
Replace the following placeholders with your actual values:
<YOUR_WALLET_ADDRESS>- Your BSC wallet address (e.g.,0x742d35Cc6634C0532925a3b844Bc9e7595f0bEb)<YOUR_POOL_ADDRESS>- Pool contract address (e.g.,0x7f51c8aaa6b0599abd16674e2b17fec7a9f674a1)<YOUR_NFT_ID>- NFT token ID from opened position (e.g.,12345)Prerequisites
Start Gateway (from
~/trash/hummingbot-core/)Verify Gateway is Running
Add Your Wallet to Gateway
Test 1: Get Pool Information
curl "http://localhost:15888/connectors/pancakeswap/clmm/pool-info?network=bsc&poolAddress=<YOUR_POOL_ADDRESS>"Expected Response:
{ "address": "0x7f51c8aaa6b0599abd16674e2b17fec7a9f674a1", "baseTokenAddress": "0x0E09FaBB73Bd3Ade0a17ECC321fD13a19e81cE82", "quoteTokenAddress": "0x55d398326f99059fF775485246999027B3197955", "feePct": 0.25, "price": 3.75, "baseTokenAmount": 1250000, "quoteTokenAmount": 4687500, "activeBinId": 85176 }Test 2: Quote a New Position
Expected Response:
{ "baseTokenAmount": 100, "quoteTokenAmount": 375.25, "lowerPrice": 3.5, "upperPrice": 4.0, "liquidity": "150000000000000000", "baseLimited": true }Test 3: Open a Position (Decimal Amount Handling)
Before running: Ensure you have approved both tokens for the Position Manager (
0xEfF92A263d31888d860bD50809A8D171709b7b1c)Expected Response:
{ "signature": "0x1234567890abcdef...", "status": 1, "data": { "fee": 0.00123, "positionAddress": "12345", "positionRent": 0, "baseTokenAmountAdded": 0.5, "quoteTokenAmountAdded": 1.875 } }Save the
positionAddressvalue as<YOUR_NFT_ID>for subsequent tests.Test 4: Get Position Information
curl "http://localhost:15888/connectors/pancakeswap/clmm/position-info?network=bsc&positionAddress=<YOUR_NFT_ID>"Expected Response:
{ "address": "12345", "poolAddress": "0x7f51c8aaa6b0599abd16674e2b17fec7a9f674a1", "baseTokenAddress": "0x0E09FaBB73Bd3Ade0a17ECC321fD13a19e81cE82", "quoteTokenAddress": "0x55d398326f99059fF775485246999027B3197955", "baseTokenAmount": 0.5, "quoteTokenAmount": 1.875, "baseFeeAmount": 0, "quoteFeeAmount": 0, "lowerBinId": 85150, "upperBinId": 85200, "lowerPrice": 3.5, "upperPrice": 4.0, "price": 3.75 }Test 5: Execute Swap (BigInt Fix Test)
Before running: Ensure you have approved both tokens for the SwapRouter02 (
0x1b81D678ffb9C0263b24A97847620C99d213eB14)Expected Response:
{ "signature": "0xabcdef1234567890...", "status": 1, "data": { "tokenIn": "0x55d398326f99059fF775485246999027B3197955", "tokenOut": "0x0E09FaBB73Bd3Ade0a17ECC321fD13a19e81cE82", "amountIn": 0.75, "amountOut": 0.2, "fee": 0.00089, "baseTokenBalanceChange": 0.2, "quoteTokenBalanceChange": -0.75 } }Test 6: Check if Pool is Registered in MasterChef
Expected Response (if registered):
{ "status": 1, "data": { "isRegistered": true, "poolId": 42 } }Expected Response (if not registered):
{ "status": 1, "data": { "isRegistered": false } }Test 7: Stake NFT in MasterChef (Optional)
Before running: Ensure you have approved MasterChef for NFT transfers via the Position Manager's
setApprovalForAllfunction.Expected Response:
{ "signature": "0x9876543210fedcba...", "status": 1, "message": "NFT staked successfully in MasterChef" }Test 8: Unstake NFT from MasterChef
Expected Response:
{ "signature": "0xfedcba0987654321...", "status": 1, "message": "NFT unstaked successfully from MasterChef", "rewardsCollected": 12.5 }Test 9: Close Position
Expected Response:
{ "signature": "0x1122334455667788...", "status": 1, "data": { "fee": 0.00095, "baseTokenAmountRemoved": 0.5, "quoteTokenAmountRemoved": 1.875, "baseFeeAmount": 0.002, "quoteFeeAmount": 0.0075 } }Test 10: Get Wallet Balances
curl "http://localhost:15888/chains/bsc/balances?walletAddress=<YOUR_WALLET_ADDRESS>"Expected Response:
{ "balances": { "BNB": "1.25", "CAKE": "150.5", "USDT": "500.25" } }Test 11: List All Positions Owned (with activeOnly filter)
curl "http://localhost:15888/connectors/pancakeswap/clmm/positions-owned?network=bsc&walletAddress=<YOUR_NFT_ID>&activeOnly=true"Expected Response:
[ { "address": "12345", "poolAddress": "0x7f51c8aaa6b0599abd16674e2b17fec7a9f674a1", "baseTokenAmount": 0.5, "quoteTokenAmount": 1.875, "lowerPrice": 3.5, "upperPrice": 4.0, "price": 3.75 } ]Common Error Scenarios to Test
Error 1: Insufficient Allowance for Swap
Expected Error:
{ "statusCode": 400, "error": "Bad Request", "message": "Insufficient allowance for USDT. Current: 0 USDT, Required: 0.75 USDT. To swap with PancakeSwap CLMM, you need to approve the spender \"pancakeswap/clmm/swap\" instead of \"pancakeswap/clmm\". This will approve the SwapRouter02 address (0x1b81D678ffb9C0263b24A97847620C99d213eB14)..." }Error 2: Pool Not Registered in MasterChef
Expected Error:
{ "statusCode": 400, "error": "Bad Request", "message": "Pool 0x... is not registered in MasterChef. Only pools registered in MasterChef can be staked." }Changes Made:
Comprehensive Logging - Added logging at every step to trace where conversion happens:
Raw Amount Formatting - Ensured all quote values are properly formatted strings:
Direct String Passing - Pass raw strings directly to contract functions (in
src/connectors/pancakeswap/clmm-routes/executeSwap.ts):Error Tracing - Enhanced error messages with stack traces:
Expected Behavior Now
When you call the endpoint again with the same payload, you'll see detailed logs like:
This will show us EXACTLY where any conversion issue occurs, allowing us to pinpoint and fix it.
Follow-up Testing
After deploying these changes:
/connectors/pancakeswap/clmm/execute-swapwith your test payloadPrerequisites Summary
Before running any integration tests, ensure you have:
Position Manager Approval (
0xEfF92A263d31888d860bD50809A8D171709b7b1c)setApprovalForAll(positionManager, true)on the Position Manager contractSwapRouter02 Approval (
0x1b81D678ffb9C0263b24A97847620C99d213eB14)approve(swapRouter, maxUint256)on each token being swappedMasterChef Approval (for NFT staking only)
setApprovalForAll(masterchef, true)on the Position Manager contractRelated Issues