Skip to content

feat: PancakeSwap CLMM fixes + MasterChef routes (clean branch) - #680

Open
VeXHarbinger wants to merge 9 commits into
hummingbot:developmentfrom
High-Falootin:pancakeswap-bsc-lp
Open

feat: PancakeSwap CLMM fixes + MasterChef routes (clean branch)#680
VeXHarbinger wants to merge 9 commits into
hummingbot:developmentfrom
High-Falootin:pancakeswap-bsc-lp

Conversation

@VeXHarbinger

Copy link
Copy Markdown

Summary

Endpoints added

  • POST /connectors/pancakeswap/nftStaking/masterchef-stake
  • POST /connectors/pancakeswap/nftStaking/masterchef-unstake
  • POST /connectors/pancakeswap/nftStaking/masterchef-unstake-and-close
  • POST /connectors/pancakeswap/nftStaking/masterchef-knows-pool

Validation

  • pnpm build passes
  • Targeted route test passes

Scope

PancakeSwap-related files only.

@greptile-apps

greptile-apps Bot commented Aug 13, 2026

Copy link
Copy Markdown

Greptile Summary

The PR adds PancakeSwap MasterChef staking routes and updates CLMM position, pool, swap, and fee-collection behavior.

  • Registers four NFT-staking endpoints for staking, unstaking, combined unstake-and-close, and pool registration checks.
  • Preserves uint256 token IDs as strings across the new staking transaction paths.
  • Collects trading fees from staked positions directly through the configured MasterChef ABI.
  • Adds live fee-accrual calculations and optional CLMM bin-distribution data.
  • Extends route and fee-collection regression coverage.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
src/connectors/pancakeswap/pancakeswap.ts Adds string-safe MasterChef staking helpers and resolves pid-zero ambiguity through a separate registration query.
src/connectors/pancakeswap/clmm-routes/collectFees.ts Adds direct ABI-backed fee collection for staked positions and updates non-staked collection probing and fee reporting.
src/connectors/pancakeswap/PancakeswapV3Masterchef.abi.json Defines the MasterChef contract interface used by staking and direct staked-fee collection.
src/connectors/pancakeswap/nft-staking/masterchef-stake.ts Adds the MasterChef staking endpoint with token IDs represented as strings.
src/connectors/pancakeswap/nft-staking/masterchef-unstake.ts Adds the MasterChef unstaking endpoint while preserving token ID precision.
src/connectors/pancakeswap/nft-staking/masterchef-unstake-and-close.ts Adds the combined unstake-and-close flow with a string token ID passed through both operations.
src/connectors/pancakeswap/nft-staking/masterchef-knows-pool.ts Reports the pool ID and independently determines registration, including valid pool ID zero.
src/connectors/pancakeswap/clmm-routes/positionInfo.ts Computes current uncollected fees from live pool fee-growth state.
src/app.ts Registers the PancakeSwap NFT-staking route group.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Gateway PancakeSwap routes] --> B[CLMM routes]
    A --> C[NFT staking routes]
    C --> D[MasterChef stake]
    C --> E[MasterChef unstake]
    C --> F[Unstake and close]
    C --> G[Pool registration check]
    B --> H[Collect fees]
    H --> I{NFT owner}
    I -->|Wallet| J[Position Manager collect]
    I -->|MasterChef| K[MasterChef collect]
    F --> E
    F --> L[Close CLMM position]
Loading

Reviews (6): Last reviewed commit: "fix(pancakeswap): use canonical nftStaki..." | Re-trigger Greptile

Comment on lines +17 to +20
tokenId: Type.Number({
description: 'Token ID of the NFT position to unstake and close',
examples: [6450873],
}),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Numeric token IDs lose precision

When a valid uint256 token ID exceeds Number.MAX_SAFE_INTEGER, Type.Number rounds it before it reaches unstakeNft and closePosition, causing the request to fail against a nonexistent ID or to remove liquidity from and burn a different position. The stake and standalone unstake routes use the same lossy representation, so these identifiers need to remain strings throughout the request and transaction paths.

Comment on lines +682 to +686
if (poolId === 0) {
throw new Error(
`Pool for position ${tokenId} is not registered in MasterChef. ` +
`Only positions in MasterChef-registered pools can be staked.`,
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Pool zero is falsely rejected

When a position belongs to the MasterChef pool registered at pid 0, v3PoolAddressPid returns the valid ID 0 and this branch incorrectly classifies the pool as unregistered, preventing the position from being staked. The masterchef-knows-pool route applies the same nonzero test and consequently reports that pool as unknown.

Comment on lines +75 to +84
const encodedArgs = utils.defaultAbiCoder.encode(
['uint256', 'address', 'uint128', 'uint128'],
[positionAddress, walletAddress, UINT128_MAX, UINT128_MAX],
);
const data = `${MASTER_CHEF_COLLECT_SELECTOR}${encodedArgs.slice(2)}`;

const txParams = await ethereum.prepareGasOptions(undefined, CLMM_COLLECT_FEES_GAS_LIMIT);
const tx = await wallet.sendTransaction({
to: masterChefAddress,
data,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 MasterChef collect calldata mismatches ABI

When fees are collected from a staked NFT, this code sends a hard-coded flat collect(uint256,address,uint128,uint128) call even though the supplied MasterChef ABI implements no matching function, causing the submitted transaction to revert and consume gas instead of collecting fees.

@VeXHarbinger

Copy link
Copy Markdown
Author

Addressing found issues

- keep NFT token IDs as strings across stake/unstake routes and connector methods
- handle pid=0 correctly via explicit MasterChef pool registration check
- replace hard-coded staked collect calldata with ABI-backed harvest() call
);

const txParams = await ethereum.prepareGasOptions(undefined, CLMM_COLLECT_FEES_GAS_LIMIT);
const tx = await masterChefContract.harvest(positionAddress, walletAddress, txParams);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Harvest leaves trading fees uncollected

When fee collection is requested for a MasterChef-staked NFT, this branch calls harvest, which claims the farming reward rather than collecting the position's token0/token1 trading fees. The endpoint then reports token0/token1 balance deltas—normally zero—while the requested trading fees remain uncollected.

@VeXHarbinger

Copy link
Copy Markdown
Author

Addressed Greptile P1 findings in commit a605717.

  1. Numeric token ID precision
  • tokenId request schemas switched to string for stake/unstake routes
  • connector methods stakeNft/unstakeNft now accept string IDs
  1. pid=0 false-negative
  • added explicit isMasterChefPoolRegistered(pool) check using getLatestPeriodInfo(pool)
  • masterchef-knows-pool now reports known from registration check instead of pid!=0
  1. Staked fee collect calldata mismatch
  • replaced hard-coded selector payload with ABI-backed MasterChef harvest(tokenId,to) call

Build status: pnpm build passes on branch pancakeswap-bsc-lp.

Use the correct unstake -> NPM collect -> restake sequence for MasterChef-staked NFTs instead of calling harvest(), which only claims farm rewards and leaves trading fees uncollected.
Comment on lines +52 to +60
await pancakeswap.unstakeNft(positionAddress, walletAddress);

try {
collectResult = await collectFees(network, walletAddress, positionAddress);
} catch (error: any) {
collectError = error;
} finally {
try {
await pancakeswap.stakeNft(positionAddress, walletAddress);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Restake failure leaves NFT unstaked

When withdrawal succeeds but fee collection or restaking fails, this sequence returns an error without restoring the NFT's original MasterChef state, leaving the position unstaked and no longer earning farming rewards.

Restore direct MasterChef fee collection for staked NFTs using a typed collect(uint256,address,uint128,uint128) ABI fragment proven by on-chain tx 0x43fbb08a7e6944ae2a1e9bcfcb1fdb4134aed9acc51b809309daca9103c51e99, avoiding both raw calldata and temporary unstake/restake flow.

const masterChefContract = new Contract(masterChefAddress, MASTER_CHEF_STAKED_COLLECT_ABI, wallet);
const txParams = await ethereum.prepareGasOptions(undefined, CLMM_COLLECT_FEES_GAS_LIMIT);
const tx = await masterChefContract.collect(positionAddress, walletAddress, UINT128_MAX, UINT128_MAX, txParams);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 MasterChef collect selector remains invalid

When fee collection is requested for a MasterChef-staked NFT, this branch submits collect(uint256,address,uint128,uint128), which is absent from the configured MasterChef interface, causing the transaction to revert while consuming gas and leaving the trading fees uncollected.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Latest follow-up fix in ffda583:

  • aligned the PancakeSwap MasterChef ABI source-of-truth with the deployed staked fee collect interface by adding collect(uint256,address,uint128,uint128) to PancakeswapV3Masterchef.abi.json
  • updated collectFees.ts to use the configured MasterChef ABI file instead of an inline ABI fragment
  • preserved the direct staked fee collection path (no unstake/restake fallback)
  • added regression coverage proving that staked fee collection calls collect() directly and does not call unstakeNft()

Validation:

  • pnpm build passes
  • targeted regression test passes:
    test/connectors/pancakeswap/clmm-routes/collectFees.test.ts

Additional runtime confirmation:

  • we observed a successful on-chain MasterChef staked collect transaction using this interface:
    0x43fbb08a7e6944ae2a1e9bcfcb1fdb4134aed9acc51b809309daca9103c51e99

- add collect(uint256,address,uint128,uint128) to PancakeswapV3Masterchef.abi.json
- use the configured MasterChef ABI as the source of truth in collectFees
- add regression coverage that staked fee collection calls collect() directly and never unstakeNft()
Register PancakeSwap nftStaking routes in app.ts and add a /nft-staking alias for the live bot's existing close/rebalance path.
Register only the canonical gateway nftStaking prefix and remove the temporary alias. The live bot has been updated to call the canonical path.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant