feat: make wallet_sendCalls more reliable
#7816
Conversation
jiexi
changed the title
wallet_sendCalls more reliable
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| address: Hex, | ||
| req: JsonRpcRequest, | ||
| { getAccounts }: { getAccounts: (req: JsonRpcRequest) => Promise<string[]> }, | ||
| { getPermittedAccountsForOrigin }: { getPermittedAccountsForOrigin: () => Promise<string[]> }, |
There was a problem hiding this comment.
Could we avoid breaking changes for all clients if we kept the current name, but just ensured the hooks check the origin?
There was a problem hiding this comment.
Yeah Jiexi and I went back and forth on this a bit, but I think for this case the name change is worth it. The bug this PR fixes happened precisely because getAccounts is ambiguous - it doesn't convey that these must be permitted accounts for the requesting origin. A future developer could reasonably implement getAccounts by returning all accounts or the selected account, not realizing this breaks the security model.
Since this is already a breaking change (the signature changed from (req) => Promise<string[]> to () => Promise<string[]>), we'd prefer to make the API self-documenting and reduce the risk of similar bugs.
| // Ensure that an "unauthorized" error is thrown if the requester | ||
| // does not have the `eth_accounts` permission. | ||
| const accounts = await getAccounts(req); | ||
| const accounts = await getPermittedAccountsForOrigin(); |
There was a problem hiding this comment.
How does the client access the origin if we don't provide anything to the hook?
There was a problem hiding this comment.
The hook gets the origin off the request object directly. The 7702 middleware follows the same pattern that I'm hoping to use here
Explanation
Improves reliability of wallet_sendCalls.
Changes
getAccountshook togetPermittedAccountsForOriginhook which takes no params and returns the permitted accounts for the origin of the requestReferences
Checklist
Note
Medium Risk
Moderate risk because it changes RPC middleware authorization/account-selection behavior and the hook contract (from
getAccounts(req)togetPermittedAccountsForOrigin()), which could surface newunauthorizederrors if consumers don’t implement the new hook correctly.Overview
Makes
wallet_sendCallsmore reliable by switching account authorization and defaultfromresolution to an origin-scopedgetPermittedAccountsForOrigin()hook, rather than relying on a genericgetAccounts(req)/selected-account lookup.processSendCallsnow defaultsfromto the first permitted account whenfromis omitted and explicitly throwsproviderErrors.unauthorized()when no permitted account is available;wallet_sendCalls,wallet_getCapabilities, andvalidateAndNormalizeKeyholderwere updated accordingly, with tests and the changelog updated to match.Written by Cursor Bugbot for commit 8f310db. This will update automatically on new commits. Configure here.