-
Notifications
You must be signed in to change notification settings - Fork 122
feat(trade): slippage config for integrators #5906
8000New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
This reverts commit 0b8ad6c.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
WalkthroughThis update implements a flexible, externally configurable slippage system for the COW widget. It introduces new types, utility functions, and hooks to support per-flow (ERC20/ETH) min, max, and default slippage settings, integrates slippage validation, and refactors components and state to use the new configuration throughout the frontend and widget configurator. Changes
Sequence Diagram(s)sequenceDiagram
participant Integrator
participant WidgetConfigurator
participant WidgetFrontend
participant SlippageConfig
participant TradeComponent
Integrator->>WidgetConfigurator: Set slippage (min, max, default) for ERC20/ETH
WidgetConfigurator->>WidgetFrontend: Pass slippage config in widget params
WidgetFrontend->>SlippageConfig: Load slippage config (per trade type/chain)
TradeComponent->>SlippageConfig: Use hook to get min, max, default
TradeComponent->>TradeComponent: Validate user input and show warnings based on config
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
yarn install v1.22.22 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Fixed, thx for highlighting! |
apps/cowswap-frontend/src/modules/injectedWidget/utils/resolveFlexibleConfigValues.ts
Outdated
Show resolved
Hide resolved
Not an issue! I've been able to reproduce this with a SC wallet, but it is an expected behavior for a SC wallet. Sorry about the noise. |
…cowswap into feat/widget-slippage-config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
libs/widget-lib/src/resolveFlexibleConfigValues.ts (1)
13-13
: Remove unnecessary type assertion.The type assertion
as T[]
is redundant since TypeScript can infer thatconfig
is of typeT
when it's not a per-trade-type or per-network configuration.- return [config] as T[] + return [config]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/cowswap-frontend/src/modules/injectedWidget/utils/validatePartnerFee.ts
(1 hunks)apps/cowswap-frontend/src/modules/injectedWidget/utils/validateSlippage.ts
(1 hunks)libs/widget-lib/src/index.ts
(1 hunks)libs/widget-lib/src/resolveFlexibleConfigValues.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- libs/widget-lib/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/cowswap-frontend/src/modules/injectedWidget/utils/validatePartnerFee.ts
- apps/cowswap-frontend/src/modules/injectedWidget/utils/validateSlippage.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: alfetopito
PR: cowprotocol/cowswap#5830
File: apps/cowswap-frontend/src/modules/tradeQuote/hooks/useSmartSlippageFromQuote.ts:5-9
Timestamp: 2025-06-16T15:58:57.402Z
Learning: In the cowswap system, 0 bps will never be suggested as a slippage value, so falsy checks on suggested slippage values are safe and won't drop valid 0 values.
🧬 Code Graph Analysis (1)
libs/widget-lib/src/resolveFlexibleConfigValues.ts (2)
libs/widget-lib/src/types.ts (1)
FlexibleConfig
(10-15)libs/widget-lib/src/flexibleConfig.ts (2)
isPerTradeTypeConfig
(27-31)isPerNetworkConfig
(35-39)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Setup
- GitHub Check: Cypress
🔇 Additional comments (1)
libs/widget-lib/src/resolveFlexibleConfigValues.ts (1)
1-14
: LGTM! Clean implementation with proper type safety.The function correctly handles all variants of the
FlexibleConfig<T>
type, including nested configurations. The logic is clear and the use offlatMap
efficiently flattens nested structures.
@elena-zh I've fixed the case with min == max value in config, thx for catching that edge case! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks!
Summary
Resolves #5858
Implemented support for configuring slippage through widget params.
Screenshots with defaultValue equals to 7%:
Example:
min
/max
— network-specific bounds; values outside the app’s hard limits are ignored.defaultValue
— starting value shown in the UI. Must satisfymin ≤ defaultValue ≤ max
. If higher than auto slippage, it becomes the initial swap slippage.Added validation in the Configurator:
min ≥ 0
,max ≤ 50 %
,min ≤ defaultValue ≤ 50 %
When no
slippage
section is supplied, the widget uses defaults.To Test
Implement config
Open the Configurator → paste the JSON above into “RAW JSON params” → click Update widget btn.
defaultValue
if it's less than auto.Custom slippage warnings
defaultValue
shows “High slippage” warning.defaultValue
shows “Low slippage – transaction may expire” warning.Auto-slippage behaviour
Auto slippage is applied only when it is greater than
defaultValue
.Warning
"Slippage adjusted to {auto}% to ensure quick execution"
appears only if
autoSlippage
>defaultValue
, andautoSlippage
> 5 %(for ETH flow)/2% (for erc20 flow).ETH-Flow rule
defaultValue
< 2 %, ETH-Flow slippage is forced to 2 %.Scope
Validation
min ≥ 0
,max ≤ 50 %
,min ≤ defaultValue ≤ 50 %
).Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores