Skip to content

fix: correct slippage conversion from basis points to decimal in PumpFun trades#28

Closed
0x-SquidSol wants to merge 1 commit intonullxnothing:mainfrom
0x-SquidSol:fix/pumpfun-slippage-conversion
Closed

fix: correct slippage conversion from basis points to decimal in PumpFun trades#28
0x-SquidSol wants to merge 1 commit intonullxnothing:mainfrom
0x-SquidSol:fix/pumpfun-slippage-conversion

Conversation

@0x-SquidSol
Copy link
Copy Markdown

Summary

  • Slippage parameter in buyToken and sellToken was divided by 100 instead of 10,000 when converting basis points to the decimal value expected by the pump-sdk
  • A user setting 50 bps (0.5%) slippage was actually submitting trades with 50% slippage tolerance, accepting drastically worse prices
  • Fixed both buyInstructions and sellInstructions calls to divide by 10,000

Test plan

  • Verify slippage value passed to pump-sdk is correct (50 bps → 0.005)
  • Confirm buy/sell trades respect the intended slippage tolerance
  • Run pnpm run typecheck — passes clean
  • Run pnpm run test — all wallet tests pass (19/19)

…Fun trades

The slippage parameter was being divided by 100 instead of 10,000 when
converting basis points to the decimal value expected by the pump-sdk.
This caused users setting 50 bps (0.5%) slippage to actually allow 50%
slippage on buy and sell operations, resulting in significant fund loss.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Owner

@nullxnothing nullxnothing left a comment

Choose a reason for hiding this comment

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

I think this conversion is backwards for the installed pump SDK. In the current code, slippage is passed as input.slippageBps / 100, and the SDK types document that parameter as a percent value, not a decimal fraction. Changing it to / 10_000 would turn 100 bps from 1% into 0.01%, which is much stricter and likely to cause valid trades to fail rather than fix a safety issue. Please verify the SDK contract before merging this change.

@nullxnothing
Copy link
Copy Markdown
Owner

Closed as superseded by #75, which reapplies the corrected security hardening on top of current main. This avoids merging stale/conflicted branches and preserves the fixes with passing CI/CodeQL.

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.

2 participants