fix(rest-api): apply default slippage #3489
Conversation
WalkthroughThe changes modify the bridge controller in the REST API to introduce a new step for applying slippage adjustments to bridge quotes. The implementation now uses the Changes
Poem
Finishing Touches
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/rest-api/src/controllers/bridgeController.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: test
- GitHub Check: SonarCloud
- GitHub Check: lint
- GitHub Check: changesets-integrity-checker
- GitHub Check: label-statuses
- GitHub Check: lint
- GitHub Check: test
🔇 Additional comments (1)
packages/rest-api/src/controllers/bridgeController.ts (1)
71-72: Verify the impact of slippage adjustments on bridge operations.Since this change affects the critical path handling user funds, please ensure:
- The adjusted queries maintain backward compatibility.
- Comprehensive testing is in place, especially for edge cases.
- The changes have been tested with different bridge modules.
Run the following script to check for existing tests:
✅ Verification successful
Slippage adjustments are safely implemented and well-tested
The changes to the bridge controller correctly utilize the SDK's comprehensive slippage adjustment system, which:
- Has thorough test coverage across multiple bridge modules
- Implements specific safety mechanisms for different bridge types
- Preserves query integrity during adjustments
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for relevant test coverage # Look for bridge-related tests rg -l "bridge.*test" --type ts # Look for specific test cases related to slippage rg "describe.*slippage|test.*slippage" --type tsLength of output: 1244
Script:
#!/bin/bash # Check for imports and usage of bridge controller in test files rg -A 5 "bridgeController|BridgeController" --type ts # Look for the specific query parameters in the codebase rg -A 3 "originQuery|destQuery" --type tsLength of output: 65794
| const { originQuery, destQuery } = Synapse.applyBridgeSlippage( | ||
| quote.bridgeModuleName, | ||
| quote.originQuery, | ||
| quote.destQuery | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and type safety for slippage application.
The new slippage adjustment logic should include:
- Error handling for potential failures in
applyBridgeSlippage. - Type annotations for the destructured values.
- Validation of the returned queries before use.
+ let adjustedQueries;
+ try {
const { originQuery, destQuery } = Synapse.applyBridgeSlippage(
quote.bridgeModuleName,
quote.originQuery,
quote.destQuery
);
+ // Validate the returned queries
+ if (!originQuery?.tokenOut || !destQuery?.tokenOut) {
+ throw new Error('Invalid queries returned from slippage adjustment');
+ }
+ adjustedQueries = { originQuery, destQuery };
+ } catch (error) {
+ logger.error('Failed to apply bridge slippage', {
+ error: error.message,
+ bridgeModule: quote.bridgeModuleName,
+ });
+ // Fallback to original queries
+ adjustedQueries = {
+ originQuery: quote.originQuery,
+ destQuery: quote.destQuery,
+ };
+ }
+ const { originQuery, destQuery } = adjustedQueries;Committable suggestion skipped: line range outside the PR's diff.
Deploying sanguine-fe with
|
| Latest commit: |
03d55bf
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://27b47957.sanguine-fe.pages.dev |
| Branch Preview URL: | https://fix-rest-api-slippage.sanguine-fe.pages.dev |
Description
Isolated changes from #3479 wrt default slippage settings in rest-api.
Summary by CodeRabbit