-
Notifications
You must be signed in to change notification settings - Fork 177
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
CAIP-345: Wallet service URL property #345
base: main
Are you sure you want to change the base?
Conversation
|
||
## Abstract | ||
<!--A short (~200 word) description of the technical issue being addressed.--> | ||
A short (~200 word) description of the technical issue being addressed. |
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.
TODO write this
walletService.find(s => s.methods.includes(method)).url | ||
``` | ||
|
||
The `walletService` key can be used in both `sessionProperties` and `scopedProperties`, depending on what namespaces the method(s) in question are supported on. |
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.
What about conflicting scenarios here?
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 would think you'd want to scope it to individual chains, right, with a slightly different URL/token/query-params/etc per-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.
Why sessionProperties
? Can't we limit this to scopedProperties
only? Where eip155
stands for the EVM chains. If there's a need to define support per chain, scopedProperties
also allows that.
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.
well, the way CAIP-25 was written, sessionProperties
were meant to be things about the app<>wallet connection that had nothing to do with specific VMs/L1s, like "expiry" (of the connection) for example. which this kinda feels like to me, actually, unless it's a different URL for each 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.
so yeah, kinda depends on the API design, all options seem justifiable (as does picking one and excluding the others for this specific CAIP)
|
||
By defining a way for wallets to send requests to an external URL, the requests can be satisfied without needing the wallet app to be open. | ||
|
||
## Specification |
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.
TODO specify wallet service endpoint design. E.g. use POST and be JSON-RPC compatible
Also are these responses cacheable or not? I guess not since POST.
|
||
```ts | ||
type WalletService = { | ||
url: string, |
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.
TODO mention the case where wallet wants authentication on this service and should include a token in this URL. What about an RPC to refresh the token?
|
||
```json | ||
"scopedProperties": { | ||
"eip155": { |
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.
What's the example of wallet_getAssets
, which is supported on eip155 and solana?
LIke that?
"scopedProperties": {
"eip155": {
"walletService": [{
"url": "<wallet service URL>",
"methods": ["wallet_getAssets"]
}],
},
"solana": {
"walletService": [{
"url": "<wallet service URL>",
"methods": ["wallet_getAssets"]
}],
}
}
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.
Yeah that's how it should look. I'll add some examples
But nothing that wallet_getAssets
is not supported on Solana anyway (at least currently)
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 assume you mean this one from an open PR? https://eip.tools/eip/7811
I'm thinking maybe this should be "Draft PR" until most of those TODOs get filled out? Interesting stuff, overall... |
Co-authored-by: Bumblefudge <[email protected]>
Co-authored-by: Bumblefudge <[email protected]>
Co-authored-by: Bumblefudge <[email protected]>
|
||
Apps SHOULD use the wallet service when available for a method, instead of calling the wallet directly. | ||
|
||
Wallets SHOULD implement fallback handling for all wallet service methods. |
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.
What is the relationship between methods listed in walletService
and methods available according to CAIP-25? If a method is listed in walletService
must it also be part of the CAIP-25 session payload?
Not requiring them to be present in the session payload could allow wallets to indicate support for a method ONLY on the wallet service, avoiding the wallet potentially needing to forward methods to the wallet service (if apps don't implement the wallet service but do implement the method).
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.
However, it is possible that having methods only listed in walletService
may cause problems for existing CAIP-25 implementations. For example if a provider pre-validates a method is valid before checking if it should be forwarded to the walletService
.
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.
For example if a provider pre-validates a method is valid before checking if it should be forwarded to the walletService.
It's not only providers that validate the methods; it's also the dapp/wallet that does that. E.g. when the method is received and is not listed in CAIP-25, it would most likely be invalid. Hence, I think it would be good to make the CAIP-25 listing obligatory.
|
||
```ts | ||
type WalletService = { | ||
url: string, |
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.
Should this be uri
instead to account for other non-HTTP protocols? For example a wallet could indicate to use p2p or a light client of some kind in order to handle the method.
Furthermore, we could allow listing a wallet service with multiple URIs and the client could select the first one it is able to support/use.
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.
sounds complicated but future-proof.
uri: [string]
sounds highly decentralized and would save us having to ducktype it as an array five years from now, although you end up having to catch the case of strings not-in-an-array-of-1 because humans 😅
"scopedProperties": { | ||
"eip155": { | ||
"walletService": [{ | ||
"url": "<wallet service URL>", |
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.
what about adding headers?
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.
What is an example use case?
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.
Auth token?
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.
Auth token can be added as query param IMO. Avoids pre-flight requests on web apps.
No description provided.