-
Notifications
You must be signed in to change notification settings - Fork 33
New 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
applyBridgeSlippage() applySwapSlippage() applied #1877
Conversation
WalkthroughThe recent updates across various components and utilities within the Synapse interface revolve around a unified approach to handling slippage. The Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1877 +/- ##
=============================================
Coverage 51.22032% 51.22032%
=============================================
Files 397 397
Lines 27124 27124
Branches 307 307
=============================================
Hits 13893 13893
Misses 11881 11881
Partials 1350 1350
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- packages/synapse-interface/pages/state-managed-bridge/index.tsx (2 hunks)
- packages/synapse-interface/pages/swap/index.tsx (2 hunks)
- packages/synapse-interface/utils/actions/fetchBridgeQuotes.tsx (2 hunks)
Additional comments: 6
packages/synapse-interface/utils/actions/fetchBridgeQuotes.tsx (2)
- 64-71: The integration of
synapseSDK.applyBridgeSlippage
within thefetchBridgeQuote
function correctly replaces the previous slippage logic with a standardized method from the SDK.- 92-93: The updated references to
originQueryWithSlippage
anddestQueryWithSlippage
in the return object offetchBridgeQuote
function are consistent with the new slippage calculation logic.packages/synapse-interface/pages/swap/index.tsx (2)
- 162-163: The use of
synapseSDK.applySwapSlippage
to obtainoriginQueryWithSlippage
directly is a good simplification of the previous logic.- 188-188: The
quote
property in the dispatch call correctly usesoriginQueryWithSlippage
instead of the previously usednewOriginQuery
.packages/synapse-interface/pages/state-managed-bridge/index.tsx (2)
- 235-242: The replacement of the previous slippage logic with
synapseSDK.applyBridgeSlippage
in theStateManagedBridge
component is correctly implemented.- 269-270: The updated references to
originQueryWithSlippage
anddestQueryWithSlippage
in the return object of theStateManagedBridge
component are consistent with the new slippage calculation logic.
Deploying with Cloudflare Pages
|
This error is thrown on a swap attempt in this test build, figuring out if this is present in master (not seeing this in production right now) |
Update: not seeing this error in the latest build from master |
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- packages/synapse-interface/pages/swap/index.tsx (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/synapse-interface/pages/swap/index.tsx
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.
LGTM, the fix above appears to be working.
Actually, let's remove unused
Can't rip this function entirely yet, as it's still used for slippage on add/remove liquidity operations. Opened an issue for updating the SDK: |
…s/sanguine into fe/sdk-slippage-method
Cleaned imports in aca4a49 |
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- packages/synapse-interface/pages/state-managed-bridge/index.tsx (3 hunks)
- packages/synapse-interface/pages/swap/index.tsx (3 hunks)
- packages/synapse-interface/utils/actions/fetchBridgeQuotes.tsx (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- packages/synapse-interface/pages/swap/index.tsx
- packages/synapse-interface/utils/actions/fetchBridgeQuotes.tsx
Additional comments: 2
packages/synapse-interface/pages/state-managed-bridge/index.tsx (2)
- 234-241: The integration of
synapseSDK.applyBridgeSlippage
replaces the previous slippage calculation logic. Ensure that this new method is thoroughly tested, especially since it affects financial transactions.- 268-269: The new slippage calculation results are being used in the
setBridgeQuote
dispatch call. Confirm that the rest of the application is compatible with this new structure oforiginQuery
anddestQuery
to prevent any unintended side effects.
Summary by CodeRabbit
New Features
synapseSDK.applyBridgeSlippage
.synapseSDK.applySwapSlippage
.Refactor
f192e69858a3514d8d23a1aed8fa40f72df5332c: synapse-interface preview link
f473bf9eb23687a821e73e447b7a1b5daa005257: synapse-interface preview link
6d5263a8cc7ea68e04c1de58127669a260f16d39: synapse-interface preview link