feat: Improve Predict Across deposits for proxy setup and direct transfer flows#8208
feat: Improve Predict Across deposits for proxy setup and direct transfer flows#8208pedronfigueiredo wants to merge 4 commits intomainfrom
Conversation
43eb402 to
c770afa
Compare
| ); | ||
|
|
||
| if (transaction.type === TransactionType.predictDeposit) { | ||
| if (transaction.type === TransactionType.predictDeposit) { |
There was a problem hiding this comment.
Why do we need type specific behaviour here?
Can we not just decode the transaction data that is included, and throw if we can't?
| }; | ||
| } | ||
|
|
||
| if (destinationCalls.length === 1 && transferCall) { |
There was a problem hiding this comment.
What if it's a batch including a token transfer?
There was a problem hiding this comment.
Good point. I updated this to the following logic: If at least one transfer transaction exists, the first one should be removed from the post swap actions and used as the swap itself. If there's more transfer actions they get sent through the post swap actions.
| call: AcrossDestinationCall, | ||
| request: QuoteRequest, | ||
| ): AcrossAction { | ||
| if (isTransferCall(call.data)) { |
There was a problem hiding this comment.
Could we make this entirely generic so we simply list a series of signatures in a constant, then iterate through the resulting ABIs to try a parseTransaction or decodeFunctionData as we do for transaction types?
Maybe cleanest in a separate file such as across-actions.ts?
That would mean we don't need to check individual selectors and can generate the args automatically from the result of decodeFunctionData?
| return { | ||
| actions: [], | ||
| recipient: from, | ||
| actions: destinationCalls |
There was a problem hiding this comment.
Minor, some duplication, could we just define a recipient variable and actions, then re-assign and splice the calls before we generate actions?
| toString: () => string; | ||
| }; | ||
|
|
||
| const TOKEN_TRANSFER_INTERFACE = new Interface([TOKEN_TRANSFER_SIGNATURE]); |
There was a problem hiding this comment.
Why do we need to pre-define these, could we initialise as needed?
| : undefined, | ||
| getTarget: (call, request) => call.target ?? request.targetTokenAddress, | ||
| interface: TOKEN_TRANSFER_INTERFACE, | ||
| methodName: 'transfer', |
There was a problem hiding this comment.
Can we not extract this from the signature dynamically?
|
|
||
| function getTransferData(transaction: TransactionMeta): Hex | undefined { | ||
| const { nestedTransactions, txParams } = transaction; | ||
| function getDestinationCalls( |
There was a problem hiding this comment.
Should this be encapsulated inside across-actions too, as it's the main entrypoint?
Get me actions for this TransactionMeta?
| value: '0', | ||
| } | ||
| : undefined, | ||
| getTarget: (call, request) => call.target ?? request.targetTokenAddress, |
There was a problem hiding this comment.
We've added lots of complexity to this list through getArg and getTarget but they are only used for the action we're not even submitting?
Would it simplify to remove token transfer from here and just have a dedicated getTransferRecipient function that returns undefined or an alternate recipient given the TransactionMeta?
Then these actions can just be the signatures we want to build actual actions for and we'd only need to list the signatures themselves in a string array?
| 'function execTransaction(address to, uint256 value, bytes data, uint8 operation, uint256 safeTxGas, uint256 baseGas, uint256 gasPrice, address gasToken, address refundReceiver, bytes signatures)'; | ||
|
|
||
| export const UNSUPPORTED_DESTINATION_ERROR = | ||
| 'Across only supports transfer-style destination flows at the moment'; |
There was a problem hiding this comment.
Do we need to update this? And append the function four bytes we couldn't support?
47d87a1 to
7a8d479
Compare
Explanation
This PR updates predictDeposit Across quote handling to distinguish between initial proxy setup deposits and subsequent transfer-only deposits. When the destination calldata is empty or represents a single ERC-20 transfer, the quote now skips post-swap actions and relays directly to the transfer recipient to avoid unnecessary gas and fees. When calldata is present for proxy creation and setup, the destination transactions are decoded into ordered Across post-swap actions so the proxy initialization, Safe execution, and approval flow can still run after bridging. The change preserves existing non-Predict behavior, adds focused test coverage for both Predict flows and unsupported calldata cases, and keeps the original changelog entry while adding a separate follow-up note for this improvement.
References
https://consensyssoftware.atlassian.net/browse/CONF-1043?atlOrigin=eyJpIjoiOWRmOThmZWRjZjZkNDljYTgyMTE4NTNjYWU5MzgwNDEiLCJwIjoiaiJ9
Checklist
Note
Medium Risk
Changes how Across quotes derive
recipientandactionsby decoding destination calldata (including nested batches), which can alter bridged fund recipients and post-swap execution. Risk is mitigated by extensive new unit coverage but impacts a critical transaction flow.Overview
Across quote destination handling is overhauled to derive
recipientandactionsfrom destination calldata (preferringnestedTransactions), instead of treatingpredictDeposittransfers as post-swap actions.Transfer-only destinations now send directly to the transfer recipient with no actions, while supported setup calls (e.g.,
createProxy,execTransaction) are decoded into ordered post-swap actions; unsupported selectors or missing requiredtargetnow throw clearer errors.Adds a new
across-actionsmodule with focused tests, expandsacross-quotestests to cover transfer-only vs setup flows, batching behavior (first transfer sets recipient, later transfers become actions), and additional failure cases, and documents the behavior change in the changelog.Written by Cursor Bugbot for commit 8457618. This will update automatically on new commits. Configure here.