-
Notifications
You must be signed in to change notification settings - Fork 13
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
WT-1662 Bridge and swap route #907
Conversation
Conflicts: packages/checkout/sdk/src/smartCheckout/routing/bridge/bridgeRoute.test.ts packages/checkout/sdk/src/smartCheckout/routing/bridge/bridgeRoute.ts
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.
Awesome work @imx-mikhala 🎉 Just some minor comments.
|
||
const ownerAddress = '0xOWNER'; | ||
|
||
const getTestDexQuotes = (): DexQuotes => { |
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.
I like this work, at some future point I think worth considering to refactor into mockUtils
or mockDexQuotes
as mocking functions we can share across checkout.
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.
100%
); | ||
}); | ||
|
||
it('should return no bridge and swap routes', async () => { |
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.
Do you think it's worth expanding on what the logic/business rule is here that is being validated, rather than the focus only on the response.. for example
it('should return no bridge and swap routes', async () => { | |
it('should return no bridge and swap routes if no bridgeable and swappable tokens available', async () => { |
import { L1ToL2TokenAddressMapping } from '../indexer/fetchL1Representation'; | ||
import { getDexQuotes } from './getDexQuotes'; | ||
|
||
export const abortBridgeAndSwap = ( |
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.
Nice encapsulation here, looks nice and clean 🌮
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.
got the idea from a comment you left on another pr haha
Summary
This PR adds the bridge -> swap route.
Demo
This demo has a requirement of zkYEET. The wallet I am testing with has a small about of IMX on L2 but not quite enough to perform the swap into zkYEET. The wallet also has enough ETH to pay for bridge fees. However, the wallet has enough IMX on L1 that can be bridged across and then swapped into zkYEET. Smart checkout suggests a Bridge->Swap route since I can bridge across enough IMX to perform the swap and cover all fees.
Screen.Recording.2023-09-28.at.8.51.57.am.mov