Skip to content

slippage - #156

Open
Siddharth2207 wants to merge 1 commit into
mainfrom
2026-04-21-slippage
Open

Siddharth2207 wants to merge 1 commit into
mainfrom
2026-04-21-slippage

Conversation

@Siddharth2207

@Siddharth2207 Siddharth2207 commented Apr 21, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add a user-configurable slippage tolerance for market orders with a safe default.
  • Thread selected slippage through market order execution so aggregated route preparation uses a slippage-derived price cap.
  • Preserve backward compatibility: if no explicit value is provided, execution uses default slippage behavior.

What Changed

  • src/lib/services/marketOrderExecution.ts
    • Added slippageBps?: number to MarketOrderInput.
    • Added defaults/guards:
      • DEFAULT_MARKET_ORDER_SLIPPAGE_BPS = 100 (1%)
      • clamp range: 1..5000 bps
    • For market Buy flow, derives ratio multiplier from slippage (1 + slippageBps/10000) and feeds it into existing price-cap derivation.
    • Leaves the rest of the execution pipeline unchanged.
  • src/lib/components/orders/MarketOrder.svelte
    • Added slippage selector in Order Summary.
    • Default is 1%.
    • Passes slippageBps into executeMarketOrder(...).

Why

  • Gives users explicit control over execution tolerance while keeping current aggregated getTakeOrdersCalldata flow and transaction orchestration intact.
  • Reduces hidden/internal-only behavior by exposing tolerance directly in UI.

Test Plan

  • Open market order panel and verify default slippage shows 1%.
  • Change slippage (e.g. 0.5%, 2%) and place Buy orders; confirm preparation/execution still succeeds.
  • Confirm behavior remains unchanged when user does not modify default slippage.
  • Verify order execution still routes through aggregated calldata path.
  • Run: npx vitest run tests/lib/services/marketOrderExecution.test.ts.

@vercel

vercel Bot commented Apr 21, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
st0x Ready Ready Preview, Comment Apr 21, 2026 4:57pm

Request Review

@coderabbitai

coderabbitai Bot commented Apr 21, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Introduces user-configurable slippage tolerance for market orders. The MarketOrder component adds a UI control allowing users to select from predefined slippage options, passing the selected value to the market order execution service, which now computes dynamic ratio multipliers based on the configured tolerance instead of using fixed constants.

Changes

Cohort / File(s) Summary
Market Order UI Component
src/lib/components/orders/MarketOrder.svelte
Adds slippage tolerance state initialized from default constant, imports slippage options, and introduces a select control with handler to update slippageBps. Passes selected slippage value to executeMarketOrder payload.
Market Order Execution Service
src/lib/services/marketOrderExecution.ts
Adds slippageBps parameter to MarketOrderInput, implements slippage clamping logic with min/max bounds, and replaces fixed BUY_EXACT_RATIO_MULTIPLIER with dynamic ratio computation using 1 + effectiveSlippageBps/10_000. Exports slippage constants for UI consumption.

Sequence Diagram

sequenceDiagram
    actor User
    participant MarketOrder as MarketOrder<br/>Component
    participant Service as Market Order<br/>Execution Service
    
    User->>MarketOrder: Select slippage tolerance
    activate MarketOrder
    MarketOrder->>MarketOrder: Update slippageBps state
    deactivate MarketOrder
    
    User->>MarketOrder: Confirm order
    activate MarketOrder
    MarketOrder->>Service: executeMarketOrder(input:<br/>slippageBps)
    deactivate MarketOrder
    
    activate Service
    Service->>Service: Clamp slippageBps<br/>(MIN/MAX bounds)
    Service->>Service: Compute ratioMultiplier<br/>= 1 + effectiveSlippageBps/10_000
    Service->>Service: Execute order with<br/>dynamic ratio
    deactivate Service
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Hop, hop—a choice appears,
Slippage tolerance brings no fears,
From UI to service flow,
Dynamic ratios—watch them grow!
A configurable path, so fair,
For every trade with utmost care. 🎯

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'slippage' is overly vague and generic, using a single non-descriptive term that doesn't convey meaningful information about the specific changes in the changeset. Use a more descriptive title like 'Add user-configurable slippage tolerance for market orders' to clearly communicate the main change to reviewers.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 2026-04-21-slippage

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/lib/services/marketOrderExecution.ts`:
- Around line 217-220: The code ignores the user-selected slippage for sell
orders by only using slippageBps when isBuy and otherwise using
EMERGENCY_RATIO_MULTIPLIER; change ratioMultiplier calculation in
marketOrderExecution.ts to use the clamped effectiveSlippageBps for both sides
(e.g., derive ratioMultiplier from effectiveSlippageBps for sells too) or
alternatively gate the UI selector in MarketOrder.svelte; update the
ratioMultiplier assignment that references isBuy, effectiveSlippageBps,
clampSlippageBps and EMERGENCY_RATIO_MULTIPLIER so sell execution honors the
configured tolerance (or remove/disable the selector in MarketOrder.svelte if
you prefer not to support sell slippage yet).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 20d76759-62e3-46ce-abce-f666486bf3ea

📥 Commits

Reviewing files that changed from the base of the PR and between d400564 and 4445872.

📒 Files selected for processing (2)
  • src/lib/components/orders/MarketOrder.svelte
  • src/lib/services/marketOrderExecution.ts

Comment on lines +217 to +220
const effectiveSlippageBps = clampSlippageBps(slippageBps);
const ratioMultiplier = isBuy
? String(1 + effectiveSlippageBps / 10_000)
: EMERGENCY_RATIO_MULTIPLIER;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Honor the selected slippage for Sell orders too.

slippageBps is only used when isBuy; Sell orders still use EMERGENCY_RATIO_MULTIPLIER. Since MarketOrder.svelte exposes the selector for both sides, a user-selected Sell tolerance is silently ignored. Either apply the configured tolerance to Sell execution as well, or hide/disable the selector for Sell orders until supported.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/services/marketOrderExecution.ts` around lines 217 - 220, The code
ignores the user-selected slippage for sell orders by only using slippageBps
when isBuy and otherwise using EMERGENCY_RATIO_MULTIPLIER; change
ratioMultiplier calculation in marketOrderExecution.ts to use the clamped
effectiveSlippageBps for both sides (e.g., derive ratioMultiplier from
effectiveSlippageBps for sells too) or alternatively gate the UI selector in
MarketOrder.svelte; update the ratioMultiplier assignment that references isBuy,
effectiveSlippageBps, clampSlippageBps and EMERGENCY_RATIO_MULTIPLIER so sell
execution honors the configured tolerance (or remove/disable the selector in
MarketOrder.svelte if you prefer not to support sell slippage yet).

alastairong1 added a commit that referenced this pull request Apr 22, 2026
Resolves conflict in marketOrderExecution.ts: keeps REST API hydration
block and slippage-based priceCap calculation (replaces hardcoded
BUY_EXACT_RATIO_MULTIPLIER with user-configurable slippageBps).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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