Skip to content
This repository has been archived by the owner on Apr 25, 2024. It is now read-only.

feat: add router trade adapter #168

Merged
merged 28 commits into from
Mar 29, 2024
Merged

feat: add router trade adapter #168

merged 28 commits into from
Mar 29, 2024

Conversation

zhongeric
Copy link
Contributor

The code for converting a response from our routing services (classic quote) into Trade SDK objects is fragmented and duplicated in many areas: notably in the interface and external api.

This PR introduces a helper class RouterTradeAdapter which has a static method fromClassicQuote that constructs the underlying RouterTrade entity (from router-sdk). The RouterTrade entity can easily be used to create the UniversalRouterTrade entity in this SDK.

Comment on lines 57 to 60
if(this.options.payerIsUser === undefined) {
// default to user as payer
Object.assign(this.options, { payerIsUser: true })
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR also allows for the overriding of payerIsUser within SwapOptions. This is intended to be fully backward compatible - in that the default value is true if not specified.

Copy link
Collaborator

@marktoda marktoda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good just few thoughts!

src/entities/protocols/uniswap.ts Outdated Show resolved Hide resolved
src/utils/constants.ts Outdated Show resolved Hide resolved
src/utils/routerTradeAdapter.ts Outdated Show resolved Hide resolved
test/uniswapTrades.test.ts Show resolved Hide resolved
@zhongeric zhongeric requested a review from marktoda March 28, 2024 23:08
@@ -31,6 +31,7 @@ export type FlatFeeOptions = {
// so we extend swap options with the permit2 permit
// when safe mode is enabled, the SDK will add an extra ETH sweep for security
export type SwapOptions = Omit<RouterSwapOptions, 'inputTokenPermit'> & {
directSend?: boolean
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

little comment explaining what this means would be useful, and not 100% sure on the name but can't think of anything much better haha

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok maybe something like useRouterBalance is more appropriate and comment like // when useRouterBalance is enabled the SDK will use the balance in the router for the swap

src/entities/protocols/uniswap.ts Outdated Show resolved Hide resolved
@zhongeric zhongeric merged commit afbaf31 into main Mar 29, 2024
5 checks passed
@zhongeric zhongeric deleted the feat-router-trade-adapter branch March 29, 2024 16:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants