-
Notifications
You must be signed in to change notification settings - Fork 73
update syntax and example after 5792 syntax changes #139
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
base: main
Are you sure you want to change the base?
Conversation
@llbartekll @jakubuid @pedrouid + anyone else following 5792 prototyping at ReOwn - approvey approvey before 5792 goes to last call so I can update the commit-specific link IN EIP-5792? |
@bumblefudge before merging, according to the last 5792 updates (https://github.com/ethereum/EIPs/pull/9529/files#diff-7a494500c37dc4b57613207d62874322198a4ced215f4a0457a0af908bb6364eR353) can we add example of |
E.g. in scopedProperties:
CC @llbartekll |
OK i just added it in the commit above to match the open PR you linked to, although I'm still confused why the other capabilties are: |
@bumblefudge
|
lol no that was a typo on my part, thank you for not making me feel more embarassed than I need to |
the point is that we need an address (caip10), because wallets can have many addresses on given chain and some of them could support atomic batches and the others not |
Oh sorry, I missed that part. EIP-5792 only allows this to be set per-chain, and leaves it up to the wallet to manage which addresses on that chain can do it, and whether or not "from" address in the batch is one it controls (and is an onchain-address). so maybe the solution here is to leave this complexity OUT of the examples in the CAIP-25 profile, but include a solution/workaround in the WC docs, e.g. multiple namespaces for the same chain (with different addresses) where one returns true and one returns false for atomicity? |
The 5792 wallet_getCapabilities request allows a dapp to specify the address on which it needs the capabilities: https://github.com/ethereum/EIPs/blob/master/EIPS/eip-5792.md#wallet_getcapabilities-example-parameters In CAIP-25 handshake, the wallet can define 5792 capabilities in front for all of the addresses it supports, hence the thinking is that scopedProperties can be used for that as @llbartekll suggested:
It should be convenient for wallets to do it this way
I'm not sure if I follow how that structure would look like but I'm open for any suggestions |
oh god you're totally right, i forgot that the REQUEST specifies a wallet, and optionally chains, and the response specifies chains (with the per-address implicit). yeah modeling this inside CAIP-25 (where addresses are properties of namespaces) is... always going to be a little lossy/chaotic. forgive me for complicating things but I think the EIP5792 exchange
as a CAIP-25 handshake would be more accurate to translate as:
(risky/potentially broken if multiple addresses are being requested) or
(more faithfully roundtrip-translateable, less risky of malforming, but extremely verbose). although it's repetitive, I actually prefer the second of the two options I just gave, because the capabilities are being expressed PER ADDRESS inside of a namespace object (and repeated for every chain that address operates on, unless it's a 0x00/sessionProperties cap available on any eip155). that is, at least, the functional complete-equivalent of the EIP5792 model. i don't like the idea of just throwing CAIP-10 partitions into scopedProperties, because each entry in sessionProperties is specified to map to a scope (i.e. a "namespace" in Reown SDK terms) and some implementations (like I believe MM's) would just drop them if they were keyed to anything else. does that make sense? |
Makes sense to me and I also prefer the second option 🙏 |
maybe 2 options should be allowed? |
@llbartekll @bumblefudge Should we update the example with the 1st and 2nd approaches and merge this week? to allow in response:
or
|
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.
lgtm
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.
Thank you! @bumblefudge
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 good to me, just wondering if you want to include the changes just added to 5792
Similarly, wherever possible, session properties should align closely with information passed as JSON objects by `wallet_` RPC methods, like capabilities or permissions. For example, the capability objects keyed to hexidecimal [EIP-155] `chainId`s defined as the expected return content of the `wallet_getCapabilities` method in [EIP-5792] should also be valid keyed the same way in `sessionProperties.capabilities`. | ||
Similarly, wherever possible, session properties should align closely with information passed as JSON objects by `wallet_` RPC methods, like capabilities or permissions. | ||
For example, the capability objects specified in [EIP-5792] get requested per-address via the `wallet_getCapabilities` RPC method, and the returned objects are partitioned by the hexadecimal chainId when chain-specific, and inside of an object keyed `0x00` (following the chainId 0 convention; see the [`eip155`/caip2 profile](caip2)) when universal to the `eip155` namespace. | ||
Should an application request these capabilities and a wallet choose to pre-declare at time of connection, both parties can put such objects in the appropriately-keyed `scopedProperties` partition for chain-specific capabilities and in the `sessionProperties` (unpartitioned) for `eip155`-wide capabilities; |
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.
and in the
sessionProperties
(unpartitioned) foreip155
-wide capabilities;
Hmmm I'm realizing that my gut feeling is that sessionProperties
should be reserved for properties that live fully outside of any given namespace and that scopedProperties
should allow you to map things to a namespace or a full CAIP-2 chainId scope.
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, i feel you, but full CAIP-2 scopes are out, remember? hehe
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.
full CAIP-2 scopes are out, remember
in what context? They're clearly still the keys in scopedProperties
?
Updated the 5792
wallet_getCapabilities
equivalence spec language and examples ofeip155/caip25.md
following the introduction of the "chainId 0" convention in Alex Forshtat's PR on 5792 here.Eyes from ReOwn (@jakubuid @pedrouid) and MM Snaps (@adonesky1 @vandan @ffmcgee725) appreciated so I can update the commit-specific link in 5792 before final call!