-
-
Notifications
You must be signed in to change notification settings - Fork 269
feat: Withdrawal to any token (Predict) #7783
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -80,6 +80,12 @@ export class TransactionPayController extends BaseController< | |
| }); | ||
| } | ||
|
|
||
| setIsPostQuote(transactionId: string, isPostQuote: boolean): void { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than adding more individual setters and actions, could we define a new |
||
| this.#updateTransactionData(transactionId, (transactionData) => { | ||
| transactionData.isPostQuote = isPostQuote; | ||
| }); | ||
| } | ||
|
|
||
| updatePaymentToken(request: UpdatePaymentTokenRequest): void { | ||
| updatePaymentToken(request, { | ||
| messenger: this.messenger, | ||
|
|
@@ -105,6 +111,7 @@ export class TransactionPayController extends BaseController< | |
| const originalPaymentToken = current?.paymentToken; | ||
| const originalTokens = current?.tokens; | ||
| const originalIsMaxAmount = current?.isMaxAmount; | ||
| const originalIsPostQuote = current?.isPostQuote; | ||
|
|
||
| if (!current) { | ||
| transactionData[transactionId] = { | ||
|
|
@@ -122,8 +129,14 @@ export class TransactionPayController extends BaseController< | |
|
|
||
| const isTokensUpdated = current.tokens !== originalTokens; | ||
| const isIsMaxUpdated = current.isMaxAmount !== originalIsMaxAmount; | ||
|
|
||
| if (isPaymentTokenUpdated || isIsMaxUpdated || isTokensUpdated) { | ||
| const isPostQuoteUpdated = current.isPostQuote !== originalIsPostQuote; | ||
|
|
||
| if ( | ||
| isPaymentTokenUpdated || | ||
| isIsMaxUpdated || | ||
| isTokensUpdated || | ||
| isPostQuoteUpdated | ||
| ) { | ||
| updateSourceAmounts(transactionId, current as never, this.messenger); | ||
|
|
||
| shouldUpdateQuotes = true; | ||
|
|
@@ -157,6 +170,11 @@ export class TransactionPayController extends BaseController< | |
| this.setIsMaxAmount.bind(this), | ||
| ); | ||
|
|
||
| this.messenger.registerActionHandler( | ||
| 'TransactionPayController:setIsPostQuote', | ||
| this.setIsPostQuote.bind(this), | ||
| ); | ||
|
|
||
| this.messenger.registerActionHandler( | ||
| 'TransactionPayController:updatePaymentToken', | ||
| this.updatePaymentToken.bind(this), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,8 +59,13 @@ export async function getRelayQuotes( | |
|
|
||
| try { | ||
| const normalizedRequests = requests | ||
| // Ignore gas fee token requests | ||
| .filter((singleRequest) => singleRequest.targetAmountMinimum !== '0') | ||
| // Ignore gas fee token requests (which have both target=0 and source=0) | ||
| // but keep post-quote requests (which have target=0 but source>0) | ||
| .filter( | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The new logic keeps requests where either target > 0 (normal deposit flows) or source > 0 (withdrawal/post-quote flows). Only requests with both at zero (gas fee token placeholders) get filtered out. |
||
| (singleRequest) => | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For simplicity, could we remove this logic and provide |
||
| singleRequest.targetAmountMinimum !== '0' || | ||
| singleRequest.sourceTokenAmount !== '0', | ||
| ) | ||
| .map((singleRequest) => normalizeRequest(singleRequest)); | ||
|
|
||
| log('Normalized requests', normalizedRequests); | ||
|
|
@@ -111,15 +116,20 @@ async function getSingleQuote( | |
| ); | ||
|
|
||
| try { | ||
| // For post-quote (withdrawal) flows, use EXACT_INPUT - user specifies how much | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm concerned we keep mentioning withdrawal everywhere. It's definitely a good example of a post-quote flow, but conceptually we could be moving funds after a transaction for any reason, to a different recipient for example. |
||
| // to withdraw, and we show them how much they'll receive after fees. | ||
| // For regular flows with a target amount, use EXPECTED_OUTPUT. | ||
| const useExactInput = isMaxAmount === true || targetAmountMinimum === '0'; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As mentioned above, could just check |
||
|
|
||
| const body: RelayQuoteRequest = { | ||
| amount: isMaxAmount ? sourceTokenAmount : targetAmountMinimum, | ||
| amount: useExactInput ? sourceTokenAmount : targetAmountMinimum, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we also need to update I assume it is skipping now since it's just a token transfer for the Predict withdraw. |
||
| destinationChainId: Number(targetChainId), | ||
| destinationCurrency: targetTokenAddress, | ||
| originChainId: Number(sourceChainId), | ||
| originCurrency: sourceTokenAddress, | ||
| recipient: from, | ||
| slippageTolerance, | ||
| tradeType: isMaxAmount ? 'EXACT_INPUT' : 'EXPECTED_OUTPUT', | ||
| tradeType: useExactInput ? 'EXACT_INPUT' : 'EXPECTED_OUTPUT', | ||
| user: from, | ||
| }; | ||
|
|
||
|
|
@@ -140,7 +150,7 @@ async function getSingleQuote( | |
|
|
||
| log('Fetched relay quote', quote); | ||
|
|
||
| return normalizeQuote(quote, request, fullRequest); | ||
| return await normalizeQuote(quote, request, fullRequest); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this functionally any different if we're returning from an async function?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't have to be this PR, but since we're adding a transaction to the Relay deposit transaction, we'll need to update the source gas fees in the normalized quote to include the gas fee of the original transaction. Technically, we may also need to move the fee to the target property, and set source to zero. Depends if we consider the fees relative to the transaction or the quote 😅 |
||
| } catch (error) { | ||
| log('Error fetching relay quote', error); | ||
| throw error; | ||
|
|
||
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.
We can nest changes here, so could we have a high level summary like
Support quote execution after transaction in Relay strategywith details in nested lines?