-
Notifications
You must be signed in to change notification settings - Fork 199
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
Rename {Rfqt=>Rfq} for types in Asset Swapper #179
Conversation
packages/asset-swapper/src/types.ts
Outdated
nativeExclusivelyRFQT?: boolean; | ||
altRfqtAssetOfferings?: AltRfqtMakerAssetOfferings; | ||
nativeExclusivelyRFQ?: boolean; | ||
altRfqtAssetOfferings?: AltRfqMakerAssetOfferings; |
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.
left this field to be called altRfqtAssetOfferings
because not sure if this will be separate for RFQT and RFQM
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 think we should rename it if we're making this type RFQ-T or M agnostic
} | ||
|
||
/** | ||
* gasPrice: gas price to determine protocolFee amount, default to ethGasStation fast amount | ||
*/ | ||
export interface SwapQuoteRequestOpts extends GetMarketOrdersOpts { | ||
gasPrice?: BigNumber; | ||
rfqt?: RfqtRequestOpts; | ||
rfqt?: RfqRequestOpts; |
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.
also left this field to be called rfqt
because I imagine there might be a separate field for rfqm
but not sure
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 should be left rfqt bc RFQM will be separate from swapQuoter
@@ -119,7 +119,7 @@ export async function returnQuoteFromAltMMAsync<ResponseT>( | |||
makerToken: string, | |||
takerToken: string, | |||
maxResponseTimeMs: number, | |||
altRfqtAssetOfferings: AltRfqtMakerAssetOfferings, | |||
altRfqtAssetOfferings: AltRfqMakerAssetOfferings, |
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 field might need to be renamed?
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.
yea best to rename this as well
Description
Super tiny change to make Rfqt=>Rfq for types in Asset Swapper. This is because these types will be reused for RFQM
Testing instructions
Types of changes
Checklist:
[WIP]
if necessary.