-
-
Notifications
You must be signed in to change notification settings - Fork 269
feat: Add types for post-quote (withdrawal) flows #7773
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
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 |
|---|---|---|
|
|
@@ -85,6 +85,18 @@ export type TransactionPayControllerUpdatePaymentTokenAction = { | |
| handler: (request: UpdatePaymentTokenRequest) => void; | ||
| }; | ||
|
|
||
| /** Action to update the selected token for a transaction (used for withdrawals). */ | ||
| export type TransactionPayControllerUpdateSelectedTokenAction = { | ||
| type: `${typeof CONTROLLER_NAME}:updateSelectedToken`; | ||
| handler: (request: UpdateSelectedTokenRequest) => void; | ||
| }; | ||
|
|
||
| /** Action to set the post-quote flag for a transaction. */ | ||
| export type TransactionPayControllerSetIsPostQuoteAction = { | ||
| type: `${typeof CONTROLLER_NAME}:setIsPostQuote`; | ||
| handler: (transactionId: string, isPostQuote: boolean) => void; | ||
| }; | ||
|
|
||
| /** Action to set the max amount flag for a transaction. */ | ||
| export type TransactionPayControllerSetIsMaxAmountAction = { | ||
| type: `${typeof CONTROLLER_NAME}:setIsMaxAmount`; | ||
|
|
@@ -102,7 +114,9 @@ export type TransactionPayControllerActions = | |
| | TransactionPayControllerGetStateAction | ||
| | TransactionPayControllerGetStrategyAction | ||
| | TransactionPayControllerSetIsMaxAmountAction | ||
| | TransactionPayControllerUpdatePaymentTokenAction; | ||
| | TransactionPayControllerSetIsPostQuoteAction | ||
| | TransactionPayControllerUpdatePaymentTokenAction | ||
| | TransactionPayControllerUpdateSelectedTokenAction; | ||
|
|
||
| export type TransactionPayControllerEvents = | ||
| TransactionPayControllerStateChangeEvent; | ||
|
|
@@ -142,7 +156,26 @@ export type TransactionData = { | |
| /** Whether the user has selected the maximum amount. */ | ||
| isMaxAmount?: boolean; | ||
|
|
||
| /** Source token selected for the transaction. */ | ||
| /** | ||
| * Whether this is a post-quote transaction (e.g., withdrawal flow). | ||
| * When true, the selectedToken represents the destination token, | ||
| * and the quote source is derived from the transaction's native token. | ||
|
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. This isn't quite true, the source would be whatever the original transaction generated / it's output. |
||
| * Used for Predict/Perps withdrawals where funds flow: | ||
| * withdrawal → bridge/swap → destination token | ||
| */ | ||
| isPostQuote?: boolean; | ||
|
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'd usually recommend against a type only PR as easier to judge it's value when it's being utilised in new logic. |
||
|
|
||
| /** | ||
| * Token selected for the transaction. | ||
| * - For deposits (isPostQuote=false): This is the SOURCE/payment token | ||
| * - For withdrawals (isPostQuote=true): This is the DESTINATION token | ||
| */ | ||
| selectedToken?: TransactionPaymentToken; | ||
|
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 wouldn't bother renaming this yet until the entire feature is done, given the breaking change, and refactor needed in the clients. Though we can still update the JSDoc to clarify it's broader use. |
||
|
|
||
| /** | ||
| * @deprecated Use selectedToken instead. Kept for backwards compatibility. | ||
| * Source token selected for the transaction. | ||
| */ | ||
dan437 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| paymentToken?: TransactionPaymentToken; | ||
|
|
||
| /** Quotes retrieved for the transaction. */ | ||
|
|
@@ -450,6 +483,18 @@ export type UpdatePaymentTokenRequest = { | |
| chainId: Hex; | ||
| }; | ||
|
|
||
| /** Request to update the selected token for a transaction (used for withdrawals). */ | ||
| export type UpdateSelectedTokenRequest = { | ||
|
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. Also here, wouldn't bother with a new action type yet as we can refactor later with a dedicated breaking change PR. |
||
| /** ID of the transaction to update. */ | ||
| transactionId: string; | ||
|
|
||
| /** Address of the selected token. */ | ||
| tokenAddress: Hex; | ||
|
|
||
| /** Chain ID of the selected token. */ | ||
| chainId: Hex; | ||
| }; | ||
|
|
||
| /** Callback to convert a transaction to a redeem delegation. */ | ||
| export type GetDelegationTransactionCallback = ({ | ||
| transaction, | ||
|
|
||
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.
New action types lack corresponding handler registrations
Medium Severity
The new action types
TransactionPayControllerUpdateSelectedTokenActionandTransactionPayControllerSetIsPostQuoteActionare added to theTransactionPayControllerActionsunion and exported, but no handlers are registered inTransactionPayController. Existing actions likesetIsMaxAmountandupdatePaymentTokenfollow a pattern where they're defined in types, have corresponding controller methods, and handlers are registered viamessenger.registerActionHandler. The new action types break this pattern — calling them via the messenger will fail at runtime.Additional Locations (1)
packages/transaction-pay-controller/src/types.ts#L116-L119