Skip to content
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: add IRfqClient #467

Merged
merged 18 commits into from
May 11, 2022
Merged

feat: add IRfqClient #467

merged 18 commits into from
May 11, 2022

Conversation

phil-ociraptor
Copy link
Contributor

@phil-ociraptor phil-ociraptor commented Apr 28, 2022

Description

Allow SwapQuoter to use an IRfqClient to fetch quotes instead of using QuoteRequestor. This allows different implementations for quote fetching.

Testing instructions

Types of changes

Checklist:

  • Prefix PR title with [WIP] if necessary.
  • Add tests to cover changes as needed.
  • Update documentation as needed.
  • Add new entries to the relevant CHANGELOG.jsons.

@phil-ociraptor phil-ociraptor marked this pull request as ready for review May 3, 2022 20:32
@phil-ociraptor phil-ociraptor requested a review from rhinodavid May 3, 2022 20:32
feeToken: undefined,
})
).quotes as V4RFQIndicativeQuoteMM[])
: await rfqt.quoteRequestor.requestRfqtIndicativeQuotesAsync(
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a way we could make quoterequestor conform to IRfqClient so we don't have this weird branching? Or is it not worth it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's better to have the IRfqClient conform to the existing code, otherwise, we need to change some stuff downstream. Riskier, for unclear benefit

@@ -0,0 +1,23 @@
import { FillQuoteTransformerOrderType } from '@0x/protocol-utils';
Copy link
Contributor

Choose a reason for hiding this comment

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

consider just putting this in the interface file -- will be easier to clean up later

export interface RfqClientPriceRequest {
integratorId: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

can you sort

export interface RfqClientV1PriceRequest {
altRfqAssetOfferings: AltRfqMakerAssetOfferings | undefined;
assetFillAmount: BigNumber;
chainId: number;
Copy link
Contributor

Choose a reason for hiding this comment

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

would be beneficial to use enum ChainId here instead?

@@ -663,17 +666,49 @@ export class MarketOperationUtils {
// Timing of RFQT lifecycle
const timeStart = new Date().getTime();
const { makerToken, takerToken } = nativeOrders[0].order;
Copy link
Contributor

Choose a reason for hiding this comment

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

[out of scope] it seems like fooToken sometimes mean the symbol(?) (e.g. RfqClientV1PriceRequest has both
takerAddress, and takerToken) while here it's address.

@phil-ociraptor phil-ociraptor merged commit a7f23a9 into development May 11, 2022
@phil-ociraptor phil-ociraptor deleted the phil/irfq-client branch May 11, 2022 17:35
phil-ociraptor added a commit that referenced this pull request May 12, 2022
phil-ociraptor added a commit that referenced this pull request May 12, 2022
dextracker pushed a commit that referenced this pull request May 13, 2022
dextracker pushed a commit that referenced this pull request May 13, 2022
idokleinman added a commit that referenced this pull request May 17, 2022
* Add BiSwap (as UniV2 clone) on BSC

* changelog PR number

* add BSW

* remove BiSwap from transformer_utils

* Do not initialize BalancerV2SwapInfoCache on unsupported chains [TKR-365] (#472)

* Do not initialize BalancerV2SwapInfoCache on unsupported chains
* Update CHANGELOG.json

* Updated CHANGELOGS & MD docs

* Publish

 - @0x/[email protected]

* chore: Decomission SnowSwap [TKR-356] (#468)

* Decomission SnowSwap

* SnowSwap doesn't have much liquidity anymore (the largest pool has ~$50k)

* Update CHANGELOG.json

* Update CHANGELOG.json

* chore: Offboard Swerve Finance and LinkSwap [TKR-356] (#469)

* Offboard swerve

* Update CHANGELOG.json

* Offboard LinkSwap

* Remove unused import

* Fix CHANGELOG.json

* chore: Offboard Eth2Dai [TKR-356] (#470)

* Offboard Eth2Dai

* Update CHANGELOG.json

* feat: add IRfqClient (#467)

* add message to changelog for #467 (#474)

* Update saddle mainnet pools (#450)

* Add saddle v2 pools

* remove outdated pools

* add two saddle meta pools

* forgot changelog

* remove saddle metapools

* changelog update

* Fix a lint issue (#475)

* Updated CHANGELOGS & MD docs

* Publish

 - @0x/[email protected]

* Add BiSwap (as UniV2 clone) on BSC

* rebase new changes for balv2, up changelog, quotes working

* remove Biswap from transformer_utils once again

Co-authored-by: Kyu <[email protected]>
Co-authored-by: Github Actions <[email protected]>
Co-authored-by: phil-ociraptor <[email protected]>
Co-authored-by: Cece Z <[email protected]>
Co-authored-by: Noah Khamliche <[email protected]>
dextracker pushed a commit that referenced this pull request Jun 15, 2022
dextracker pushed a commit that referenced this pull request Jun 15, 2022
dextracker added a commit that referenced this pull request Jun 15, 2022
* Add BiSwap (as UniV2 clone) on BSC

* changelog PR number

* add BSW

* remove BiSwap from transformer_utils

* Do not initialize BalancerV2SwapInfoCache on unsupported chains [TKR-365] (#472)

* Do not initialize BalancerV2SwapInfoCache on unsupported chains
* Update CHANGELOG.json

* Updated CHANGELOGS & MD docs

* Publish

 - @0x/[email protected]

* chore: Decomission SnowSwap [TKR-356] (#468)

* Decomission SnowSwap

* SnowSwap doesn't have much liquidity anymore (the largest pool has ~$50k)

* Update CHANGELOG.json

* Update CHANGELOG.json

* chore: Offboard Swerve Finance and LinkSwap [TKR-356] (#469)

* Offboard swerve

* Update CHANGELOG.json

* Offboard LinkSwap

* Remove unused import

* Fix CHANGELOG.json

* chore: Offboard Eth2Dai [TKR-356] (#470)

* Offboard Eth2Dai

* Update CHANGELOG.json

* feat: add IRfqClient (#467)

* add message to changelog for #467 (#474)

* Update saddle mainnet pools (#450)

* Add saddle v2 pools

* remove outdated pools

* add two saddle meta pools

* forgot changelog

* remove saddle metapools

* changelog update

* Fix a lint issue (#475)

* Updated CHANGELOGS & MD docs

* Publish

 - @0x/[email protected]

* Add BiSwap (as UniV2 clone) on BSC

* rebase new changes for balv2, up changelog, quotes working

* remove Biswap from transformer_utils once again

Co-authored-by: Kyu <[email protected]>
Co-authored-by: Github Actions <[email protected]>
Co-authored-by: phil-ociraptor <[email protected]>
Co-authored-by: Cece Z <[email protected]>
Co-authored-by: Noah Khamliche <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants