-
Notifications
You must be signed in to change notification settings - Fork 125
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
Add EIP-5792 support to Safe Apps Provider #605
Conversation
🦋 Changeset detectedLatest commit: c3ab818 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 |
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.
Looks correct. We should maybe update the Test Safe app to easily test this from a Safe App.
const txs = params[0].calls.map( | ||
(call: { to?: `0x${string}`; data?: `0x${string}`; value?: `0x${string}`; chainId?: `0x${string}` }) => { | ||
if (call.chainId !== numberToHex(this.chainId) || !call.to) { | ||
throw new Error('Invalid call'); |
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 should maybe give more details: e.g. "Invalid calldata: The Safe is not available on the chain"
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 improved the error messages in c3ab818. What do you think?
I agree. Considering it will require quite some changes (and that it is not release-dependant), I'd suggest doing it in a follow up PR. What do you think? |
Summary
With EIP-5792, transaction batching is possible. As it is also possible to batch transactions with our Safe Apps SDK, this includes the relevant JSON-RPC methods to propagate the feature into the Safe Apps Provider.
This implementation has been tested with the experimental implementation of EIP-5792 in viem.
Note: this takes inspiration from the
safe-wallet-web
EIP-5792 implementation.wallet_sendCalls
This method is responsible for dispatching the batch. Our implementation maps the incoming "calls" to the relevant
sdk.txs.send
call, which creates amultiSend
call in our interface.The method should return an ID for fetching the status of the calls. In our case, this is the
safeTxHash
of themultiSend
transaction.wallet_getCallsStatus
Accepting the aforementioned
id
(safeTxHash
), this returns the status and receipt(s) of the "calls". By using thesafeTxHash
, we fetch the details from the Transaction Service and transaction receipt, mapping the responses to the expected data structure.wallet_showCallsStatus
This should open the "status" in the wallet UI. As our SDK cannot navigate the UI, this is marked as unsupported. (For reference, see the
safe-wallet-web
implementation - navigating to the transaction details.)wallet_getCapabilities
Responsible for returning the "features" of the wallet, this returns the support for
atomicBatch
(batching of transactions) for the current Safe.Changes
wallet_sendCalls
wallet_getCallsStatus
wallet_getCapabilities
wallet_showCallsStatus