Add ERC: Wallet Signing API #874
Conversation
lukasrosario
commented
Jan 29, 2025
lukasrosario
commented
- adds new ERC defining a wallet signing API
File
|
| version: string; | ||
| address?: `0x${string}`; | ||
| request: { | ||
| type: `0x${string}`; // 1-byte EIP-191 version |
There was a problem hiding this comment.
should probably be version
There was a problem hiding this comment.
ie
...
request: {
version: `0x{string}`
data: any;
}
...
There was a problem hiding this comment.
taking the naming convention from 191 makes sense, but I think the confusion with the top-level version parameter is a net negative
There was a problem hiding this comment.
Yeah, version is kinda ambiguous because it also exists as a top-level property. Think type is fine IMHO. They are analogous to transaction types.
| type SignResult = { | ||
| signature: `0x${string}`; | ||
| capabilities?: Record<string, any>; | ||
| }; |
There was a problem hiding this comment.
Should the user's account also be returned always too? If an app doesn't provide it on request, it probably needs it to verify the signature. Smart contract accounts likely also need chain-specificity (e.g. ERC-7739).
| type SignResult = { | |
| signature: `0x${string}`; | |
| capabilities?: Record<string, any>; | |
| }; | |
| type SignResult = { | |
| account: { | |
| chainId: `0x${string}`; | |
| address: `0x${string}`; | |
| }; | |
| signature: `0x${string}`; | |
| capabilities?: Record<string, any>; | |
| }; |
There was a problem hiding this comment.
Maybe just have address and chainId on top level?
There was a problem hiding this comment.
The intent is for those to be optional though, allowing apps to request a signature without a previous connection step.
There was a problem hiding this comment.
Maybe this and the optional arg should be CAIP-10. I don't like how the chainId of the CAIP-10 address could diverge from the chainId of the inner type (e.g. 712), but maybe that's just extra parsing on the side of the wallet.
| data: { | ||
| ...TypedData // TypedData as defined by EIP-712 | ||
| } |
There was a problem hiding this comment.
Think clarity improves if we just lay out this type completely
| data: { | |
| ...TypedData // TypedData as defined by EIP-712 | |
| } | |
| data: { | |
| domain: { | |
| chainId?: `0x${string}`; | |
| name?: string; | |
| version?: string; | |
| verifyingContract?: `0x${string}`; | |
| salt?: `0x${string}`; | |
| }; | |
| types: Record<string, { name: string; type: string; }[]>; | |
| primaryType: string; | |
| message: Record<string, any> | |
| } |
| requires: 191, 712, 5792 | ||
| --- | ||
|
|
||
| ## Abstract |
There was a problem hiding this comment.
The abstract mentions the new JSON-RPC method, it doesn’t provide a high-level overview of how it works. For example:
What is the name of the new JSON-RPC method?
How does the interaction between the app and wallet work in practice?
What is the expected flow for requesting and receiving signatures?
|
|
||
| The top-level `version` parameter is for specifying the version of `wallet_sign` should the top-level interface change. | ||
|
|
||
| The `request.type` parameter is for specifying the [EIP-191](./eip-191.md) `signed_data` `version` (e.g. `0x01` for structured data, `0x45` for `personal_sign` messaged). The `request.data` parameter is the corresponding data according to the `signed_data` `version`. |
There was a problem hiding this comment.
| The `request.type` parameter is for specifying the [EIP-191](./eip-191.md) `signed_data` `version` (e.g. `0x01` for structured data, `0x45` for `personal_sign` messaged). The `request.data` parameter is the corresponding data according to the `signed_data` `version`. | |
| The `request.type` parameter is for specifying the [EIP-191](./eip-191.md) `signed_data` `version` (e.g. `0x01` for structured data, `0x45` for `personal_sign` messages). The `request.data` parameter is the corresponding data according to the `signed_data` `version`. |
lukasrosario
force-pushed
the
wallet-signature-api
branch
from
3da91ba to
d332df8
Compare
|
The commit d332df8 (as a parent of 7d682c6) contains errors. |
