sendCalls in executeQuote if the wallet supports it#229
sendCalls in executeQuote if the wallet supports it#229dohaki merged 2 commits intoacross-protocol:masterfrom
Conversation
azf20
requested review from
dohaki,
gsteenkamp89,
mrice32 and
nicholaspai
as code owners
🦋 Changeset detectedLatest commit: 3508a7f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
dohaki
left a comment
dohaki
left a comment
There was a problem hiding this comment.
First of all, thanks for this great PR! And sorry for the delay of reviewing this. We currently don't have a very streamlined process in place to run our CI for forked PRs.
I left some remarks that are related to a failing e2e test and the commented out code. Let me know what you think.
|
|
||
| onProgressHandler(currentTransactionProgress); | ||
|
|
||
| // Check if wallet supports atomic transactions |
There was a problem hiding this comment.
It would be nice to have a flag that disables the flow via getCapabilities, something like disableWalletCapabilites. Background is that we have some e2e test that use local Anvil forks which don't seem to support wallet_* RPC requests. See here.
So we could use this flag to not use the capabilities flow in the tests.
There was a problem hiding this comment.
Thanks for flagging, can definitely make it opt in but i might also try to make it so we can test e2e, i think we could take a similar approach to viem's unit test for sendCalls https://github.com/wevm/viem/blob/feat%2Fcalls-data-suffix/src%2Factions%2Fwallet%2FsendCalls.test.ts
There was a problem hiding this comment.
Will take a look at viem's test setup as well. Thanks for flagging this
| // Simulate both approval and deposit calls | ||
| const { calls } = await prepareAtomicTx({ | ||
| walletClient, | ||
| // publicClient: originClient, #TODO: uncomment this when more public RPCs support eth_simulateV1 |
There was a problem hiding this comment.
Somewhat related to my comment above. But wdyt of making this whole flow opt-in for now? Consumers of the library should then make sure that if they want their users to use this feature, they use RPC providers that rupport eth_simulateV1. We could then remove the inline comments and just throw with a respective error message if the provider doesn't support the call.
There was a problem hiding this comment.
Makes sense, will update!
| error?: Error; | ||
| }; | ||
|
|
||
| export async function prepareAtomicTx( |
There was a problem hiding this comment.
NIT: Based on the naming, I would assume that this function can prepare any atomic tx. A more concise naming like prepareAtomicApproveDepositTx could make sense.
|
Thanks for the review! Will push updates soon |
|
I updated:
|
dohaki
left a comment
dohaki
left a comment
There was a problem hiding this comment.
Looking good! I ran the test here https://github.com/across-protocol/toolkit/actions/runs/15849154219/job/44678376316?pr=230 and they succeed. So this is good to go.
|
Great thanks so much! When do you think it will be merged and released? (working on something that will use this at the moment) |
2 participants
EIP-5792 introduces a new
wallet_sendCallsRPC method which allows a dapp to request multiple calls to be executed in a single transaction (e.g. for smart accounts enabled by EIP-7702). This PR adds sendCalls support toexecuteQuotewhere:wallet_getCapabilities)The goal of this PR is
In some cases the user might need to agree to upgrade their EOA to a smart account - if they reject this request, the quote will fallback to the existing two-step flow.
This was end-to-end tested using the
exampledapp (approve + deposit, fill)Notes:
call.data, becausedataSuffixis not yet supported by Viem (PR is merged but unreleased).eth_simulateV1is not widely supported so this was removed - wallets necessarily simulate transactions when processingsendCalls, but happy to discuss this shortcut.receiptis received, the logic is identical).