-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat(swap): handle swap rate v3 #1082
Conversation
0ea4a6e
to
a7c3f5b
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1082 +/- ##
=======================================
Coverage 87.04% 87.04%
=======================================
Files 100 100
Lines 1351 1351
Branches 207 207
=======================================
Hits 1176 1176
Misses 166 166
Partials 9 9
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.
🙌
/* eslint-disable no-param-reassign */ | ||
const adjustDcaQuote = ({ | ||
dcaQuoteParams, | ||
dcaQuote, | ||
originalDepositAmount, | ||
}: { |
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.
my precious 😢
@@ -29,34 +28,32 @@ export default async function getPoolQuote<T extends QuoteType>({ | |||
limitOrders?: LimitOrders; | |||
boostFeeBps?: number; | |||
pools: Pool[]; | |||
quoteType: T; |
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.
if we keep this, the ts will deduce the quote type that has been returned, otherwise we should remove the generic parameter and update the return type
packages/swap/src/routes/v2/quote.ts
Outdated
dcaQuoteParams && | ||
getPoolQuote({ | ||
...quoteArgs, | ||
depositAmount: dcaQuoteParams.chunkSize + ingressFeeSurcharge, | ||
quoteType: 'DCA', | ||
dcaChunks: dcaQuoteParams.numberOfChunks, | ||
dcaParams, | ||
}), | ||
dcaQuoteParams && | ||
estimatedBoostFeeBps && | ||
getPoolQuote({ | ||
...dcaQuoteArgs, | ||
boostFeeBps: estimatedBoostFeeBps, |
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.
should probably check at this point that we have > 1 chunk and not request a quote if we don't
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.
could do the check when setting dcaParams
initially
packages/swap/src/routes/v2/quote.ts
Outdated
.multipliedBy((dcaQuoteParams.numberOfChunks - 1) / dcaQuoteParams.numberOfChunks) | ||
.toFixed(0), | ||
) | ||
: 0n; | ||
|
||
const [dcaQuoteResult, dcaBoostedQuoteResult] = await Promise.allSettled([ |
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 guess we could add this to the Promise.allSettled
above now
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.
the changes look good to me! would be nice to compare the results against mainnet just to be safe
unfortunately can't do that right now because the RPC cannot be queried on mainnet. have to wait for the nodes to update. |
I've tested this against perseverance and did not find any discrepancies except for DCA with boost. But since boost will change in 1.7.2 i think we can ignore that for now. (there is an open PR to add boost to this RPC)
closes WEB-1640