-
Notifications
You must be signed in to change notification settings - Fork 200
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
Return unoptimized quote in SwapQuote #62
Conversation
* on a single quote info instead of using best and worst case | ||
* Orders must be derived from the same path as the quote info | ||
*/ | ||
export function getQuoteInfoMinBuyAmount( |
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 use this in 0x-api to calculate affiliate fees if any
makerTokenDecimals, | ||
takerTokenDecimals, | ||
}; | ||
} |
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.
Did a little refactoring but functionality should be the same
// Calculate the unoptimised alternative | ||
const unoptimizedFillResult = simulateBestCaseFill({ | ||
gasPrice, | ||
orders: optimizedOrders, | ||
orders: unoptimizedPath.orders, | ||
side: operation, | ||
fillAmount: assetFillAmount, | ||
opts: { gasSchedule }, | ||
}); | ||
const unoptimizedQuoteInfo = fillResultsToQuoteInfo(unoptimizedFillResult); |
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.
This is new, the rest is refactoring
bestCaseQuoteInfo, | ||
worstCaseQuoteInfo, | ||
unoptimizedQuoteInfo, | ||
unoptimizedOrders: unoptimizedPath.orders, |
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.
This line is new too, the rest is refactoring
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.
Yeah, I think this makes sense. Have you hooked it up to API to see if it looks right?
const paths = fills.map(singleSourceFills => Path.create(side, singleSourceFills, targetInput, opts)); | ||
// Sort fill arrays by descending adjusted completed rate. | ||
const sortedPaths = paths.sort((a, b) => b.adjustedCompleteRate().comparedTo(a.adjustedCompleteRate())); |
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.
This code is duped below. Can we refactor to avoid the dupe? Maybe add a fillsToPathsSortedByAdjustedCompletedRate()
?
Ya, it looks like this in API: 0xProject/0x-api#435 (comment) Pretty unambiguous! |
aa12232
to
5bc0457
Compare
Description
Supports new feature: calculating user savings.
Blocks: 0xProject/0x-api#435
This PR adds new fields
unoptimizedQuoteInfo
andunoptimizedOrders
to the SwapQuote.These new fields represent the best rate available from a single source, which would be the best alternative to our multi-source optimized path.
Testing instructions
Types of changes
Checklist:
[WIP]
if necessary.