[Caip-25] Finalize core/extensions model and core terminology (formerly, "move to Last Call")

[bumblefudge]

No description provided.

[kdenhartog]

TL;DR - add an error where no response is provided and instead undefined is returned intentionally and rename the the request method to provider_session_setup so we can define additional methods in separate CAIPs for updating and deleting the session so we can enable progressive permission patterns and reduce the fingerprinting surface area.

Apologies for a request like this so late in the process, but I think this design only just clicked for me how it's going to be used to the point I could provide thoughtful feedback. I've come to realize that we're exposing a large portion of fingerprinting risk (e.g. user supports chain X and Y with namespace A and B) with the current design approach that can be significantly reduced with two improvements. Additionally I see a way that we can make some slight modifications in follow up work to enable progressive permission patterns.

Blocking requests to address fingerprinting risk:

1. We add a response which returns undefined that would be returned when the user doesn't consent to the request. This would effectively mean that a user who doesn't consent to the request is the same as a user who doesn't have a wallet all together decreasing the entropy revealed to sites when they don't want to connect to them.

2. Define how updating the session can occur. I think this may be implicitly understood, but based on my reading of the spec that's not clear. The way this could be done is that the wallet can submit a new request utilizing the same sessionId with updated requested properties. For example, say the DApp wants to request a renewal of the expiration time in the sessionProperties it could submit the request utilizing the same sessionId with an updated expirationTime and if it receives back a new expiration time as confirmation.

I could see number 2 being defined in a separate follow on CAIP, but without the ability to update we would risk exposing a larger fingerprint surface on initial session setup because it could force an all or nothing session request pattern. One way we could address this to minimize impact to this CAIP is to just change the name of the method to provider_session_setup so that we can define a secondary CAIP called provider_session_update defined separately and left out of scope here.

Non blocker requests:

Additionally, it's not clear how the DApp can request that the session be closed or should be notified that the wallet has closed the session. I'm not sure this behavior needs to be defined in this CAIP though as it doesn't affect the primary concern I have around increased fingerprinting surface. Personally I think this could be defined in a separate one since this CAIP seems focused primarily on the interface and not necessarily the implicit contract of the function.

One other thing that's not clear to me is how the interaction of the accountChanged event would interact with the returned authorization response from CAIP-25. For example, if the initial state returned by the eip155 has account 0xABC returned would I expect that I need to call eth_accounts method to signal to the wallet UI that a request for an updated list of accounts is being requested. Alternatively would we prefer this request to be submitted via a follow up CAIP-25 request and if so how would that be conveyed to the wallet so it knows to prompt the user to add support for additional accounts?

The primary reason I bring this account update request up is because I want to move towards decoupling accounts used for SIWX and accounts that are used to manage assets. For example, I would like to see us move to the following pattern:

1. User clicks "Sign in with Wallet" and DApp sends a CAIP-25 request
2. Wallet checks consent with user and displays requested permission to the user asking which account they'd like to user (or automatically generating an ephemeral account for the SIWX response)
3. If the user doesn't consent the wallet sends back undefined if it is consented to it returns the appropriate CAIP-25 response

Then the user could take one of two paths to add an asset account:
4a. The user decides to now add their asset account by clicking "add additional account" and a new CAIP-25 request is submitted with the same sessionId and a CAIP-25 response is provided back with the updated list of accounts.
4b. The user adds a new account from the wallet UI and a corresponding CAIP-25 event similar to the accountsChanged event would notify the DApp of the update via a standardized interface.

These changes would move us towards the paradigm that allows calling patterns to emerge where users can progressively provide more information and access to the wallet rather than presuming that they'll want to set this up all at the same time. Additionally, we could stick with the all or nothing approach by making these changes.

[bumblefudge]

Thanks @kdenhartog , this really helps. I'm going to try translating this into a todo list, and you tell me what details I'm missing or getting wrong, maybe?

Before merge

• add a sentence or two somewhere about fingerprinting risk and entropy (the CASA equiv of a "privacy considerations" section) and discouraging mammoth "all-or-nothing" usage in leiu of progressive patterns
• change response behavior to support anonymizing undefined response for rejected authZ events
• editorial: make idempotent re-call/re-define more explicit with, e.g. an example changing only expirationTime

after merge/nonbreaking:

• define a wallet-initiated method for adding a new account to the session (superseding OR supporting today's walletsChanged)

To decide in this thread:

• method name. Personally, I interpreted the idempotency guarantee as encouraging re-calling with new or additional parameters, so _setup versus _update seems to run counter to that?
• scope of extension CAIPs worth starting soon?
  • should the "contract" between dapp and wallet be spelled out more explicitly? If normative (and , this could be an additional method declared in CAIP-25-- CAIP-25 without CAIP-XXX might have its usecases!

[bumblefudge]

If I've got this right, I can have a PR that hopefully fulfills the first list by Tuesday's RPC meeting :D

[hmalik88]

@kdenhartog Thanks for raising your concerns in detail. My initial thoughts re: your points:

1. I would like to understand and discuss the fingerprinting risk a bit more. I do think it's a bit much to just return undefined in any scenario that the user rejects the CAIP-25 request. The error responses are useful to dapps to respond in those different scenarios no? Wondering how big of a tradeoff just returning undefined can potentially be.
2. I do agree with including sessionId in subsequent CAIP-25 requests as a means of updating the associated session. Where that would live, I'm not sure because we're currently treating sessionProperties as something optional whereas I would think supplying a sessionId for an update on the dapp's side would very much be something intentional.

[kdenhartog]

Detecting metamask alone is pretty unique as it stands today. For example on a default chrome installation on Mac with just Metamask I share a fingerprint with 1.4% of web users.

See https://z0ccc.github.io/extension-fingerprints/ for details about extension fingerprinting and this code for how Metamask is getting fingerprinted today. Notice how they're just checking if window.ethereum is getting injected?

Assuming the only web accessible resource injected by a CAIP-25 supported wallet was the JS that's used to call this RPC request initially then returning undefined when the user doesn't consent to the request would make it so that the user is undistinguishable from a user without a wallet.

From a code flow perspective it would look like this:

If doesn't consent -> return undefined
if user does consent and request object has error -> return error code
if user does consent and request object is good -> return success response

essentially, but having that undefined gating the initial request we have a way to prevent fingerprinting web3 users and make their web fingerprint synonymous with any general web user who doesn't have the extension all together which greatly reduces the fingerprinting risk here.

[kdenhartog]

Regarding 2 @hmalik88 this was the intent of using the additional method name is that we could define separate behavior via the provider_update method. For example this method could require a sessionId property in the request at the top level of the JSON. Alternatively, we could stick with the provider_authorization and overlap the create and update request logic, but this seems to increase the code complexity of the wallet processing logic so it seems like it would be simpler to separate these. This is how most CRUD APIs would handle this as well.

Also, if there was a general expectation that these sessionId's could be represented then it's important that they're cryptographically generated (such as a UUIDv4) so that they can't be guessed and used to bypass permissions on a new origin that didn't initially setup the session.

[hmalik88]

I see now what you're talking about as far as fingerprinting risk. I don't think that will be an issue for us personally when we implement CAIP-25 in our multichain provider as we will ultimately be releasing it in a SDK and no longer injecting our provider. From the doc you linked, it doesn't seem to be an issue for Brave either?

• add a sentence or two somewhere about fingerprinting risk and entropy (the CASA equiv of a "privacy considerations" section) and discouraging mammoth "all-or-nothing" usage in leiu of progressive patterns

I think the current spec already reflects the idea of having optional namespaces which is antithetical to an all or nothing notion, but yeah I think including sessionId in sessionProperties is sufficient to indicate to a wallet that we want to "update".

• should the "contract" between dapp and wallet be spelled out more explicitly? If normative (and , this could be an additional method declared in CAIP-25-- CAIP-25 without CAIP-XXX might have its usecases!

Perhaps, but we've already defined CAIP-171 as session id, do we just then squash that into this CAIP?

[hmalik88]

Also, if there was a general expectation that these sessionId's could be represented then it's important that they're cryptographically generated (such as a UUIDv4) so that they can't be guessed and used to bypass permissions on a new origin that didn't initially setup the session.

I agree.

If doesn't consent -> return undefined if user does consent and request object has error -> return error code if user does consent and request object is good -> return success response

Hmm I think this could work, thoughts? @pedrouid

[kdenhartog]

I see now what you're talking about as far as fingerprinting risk. I don't think that will be an issue for us personally when we implement CAIP-25 in our multichain provider as we will ultimately be releasing it in a SDK and no longer injecting our provider.

That seems compatible with where this would take it. Essentially, the goal is to get to not injecting by default everywhere and instead only inject when the user opts in. This may require DApps to signal web3 support so that we can prompt the user to opt in, but I believe we can cross that bridge later and leave it out of scope for this specific work.

From the doc you linked, it doesn't seem to be an issue for Brave either?

It's an issue if the browser permission isn't set properly because our interface for wallet matches Metamask. If the permission is set to blocked then we just don't inject and the permissioning for it right now by default is set to inject always unless disabled, so our users can be fingerprinted in this way. We looked at how we could stop this, but every direction we considered lead to us breaking compatibility with DApps and Metamask. That's why I wanted to try and address it here so that we can prevent it in the future.

I think the current spec already reflects the idea of having optional namespaces which is antithetical to an all or nothing notion, but yeah I think including sessionId in sessionProperties is sufficient to indicate to a wallet that we want to "update".

Agreed, the optional namespaces does achieve this. What wasn't clear on first read for me was that the session could be updated overtime to add or remove these optional namespaces (say for example a user wants to use not only the swap feature in a DApp but also the staking and DAO features which require additional namespaced interfaces). Now that I understand better about how sessions will be used, I don't believe it ever was intended to be an all or nothing upfront permissioning model.

Perhaps, but we've already defined CAIP-171 as session id, do we just then squash that into this CAIP?

Personally I prefer whichever direction leads to more definitive normative language (MUST, SHALL, MUST NOT, SHALL NOT, REQUIRED). In this case, I think we're going to write separate CAIPs that build on top of the session identifier so keeping it separate will make it easier for us to reference clearly across different specs and also be certain that the language is more definitive. I'm +1 to keeping it separate in 171.

Also, if there was a general expectation that these sessionId's could be represented then it's important that they're cryptographically generated (such as a UUIDv4) so that they can't be guessed and used to bypass permissions on a new origin that didn't initially setup the session.

I agree.

Sweet, I'll get a PR open for 171 to update that.

[bumblefudge]

Since this was my PR to begin with, I just added the requested changes above as new commits after discussing offline with both @hmalik88 and @kdenhartog . We can discuss at tomorrow's meeting or asynchronously with @pedrouid .

[bumblefudge]

Friends, we didn't get to Kyle's changes or my wordy rationales for them in yesterday's meeting, in part because we had other extensions to discuss, and in part because we keep stumbling over the contradictory and overlapping usages of the word namespace. For this reason, I have pushed another commit to solve this problem, because if EVEN WE CAN'T DISCUSS this CAIP without mixing up scopes and namescapes, future developers and namespace authors/profilers won't be able to either!

@hmalik88 @pedrouid
Please review the commit qua commit and/or read the updated PR as a whole, combined with all the new rationale from conversations with Kyle. I will start a draft PR for yesterday's extension model, assuming this commit goes in as-is, and deal with the merge conflicts myself if it does not.

[ligi]

As discussed in today's editorial call: maybe using method names is not completely covering it - there is different behavior in methods between different clients (@TimDaub surfaced this)
My idea to attack it would be to create a registry (like we have namespaces) of possible methods - maybe defining them like here: https://github.com/ethereum/execution-apis/blob/main/src/eth/block.yaml - so we could cover e.g. the case of having one method_name with different parameters
This could even be "backward compatible" - so the id could just be the method name in the default case. Just if there are 2 behaviors the id could be e.g. <method_name>_clientXYZ - so we can have the same method name with different call parameters.

[bumblefudge]

I agree that discrepancies in APIs is an issue worth attacking-- my counterargument, though, is that CASA shouldn't host or run the registry that attacks it! I think the way 211 is written, it presumes that, for example, if geth or erigon use slightly different versions of a method, they can each publish rpcDocuments somewhere canonical* and a dapp can request the the geth RPC doc be prioritized over the execution-apis RPC doc, or a list of Geth clients be requested as rpcEndpoints. the fact that they're ordered arrays allows the wallet a lot of freedom in how it replies to that request, beyond just y/n.

* = that "somewhere canonical" is probably for the EF to decide, right? it could be an eip155-wide registry or domain controlled by a central party, or it could just be https://github.com/ethereum/go-ethereum-rpcDocs/ ; I tried to write 211 such that RPC docs are just URLs, and trust decisions about categorizing or sorting those can happen based on origins, longevity, etc.

[pedrouid]

Actually I disagree with this as clients should not be changing behavior for methods without renaming them

If a client behaves differently for example with eth_call then the clients need to coordinate in a standard spec or the method is renamed

adding a client suffix is only going to incentivize more fragmentation and less collaboration

if clients created this problem then they should fix it because they are mis-implementing methods

can you imagine if every wallet implemented eth_sendTransaction differently??

[ligi]

I agree it is not ideal - but it is reality and something we need to deal with unfortunately. What is your counter-proposal to deal with the situation?

[ligi]

At least with this solution we can detect with a handshake that something would fail and maybe can fallback or handle it gracefully instead of having weird unpredictable errors.

[bumblefudge]

the way I see it, there does not need to be a registry of multiple RPC documents-- there is the one RPC document defined by execution-apis, which namespaces/eip155/caip211 can define as the "default" definition of any methods authorized in eip155 scope objects by CAIP-25 negotiations... and dapps and wallets are free to agree to use additional documents instead of or in addition to the namespace-wide default.

Can you make the RPC WG next week to discuss further, Ligi? We can always invite Shane back to discuss how he/the openRPC team sees it

[bumblefudge]

Merging this PR as-is, but this entire thread is still very pertinent to #211 which lays out the RPC document stuff (in a way that can be profiled for each namespace!), so let's continue the thread there!

[pedrouid]

Agreed that we should renamed the method name to provider_authorize so that we can have future CAIPs for updating and deleting the session

re: returning undefined
I think @kdenhartog brings a good point about browser extension fingerprinting even if this is a little out of scope for this PR as ir covers other wallet providers

While I agree that we should make this PR flexible enough to prevent fingerprinting in the browser… I’m not convinced that returning undefined is the best option

At the end of the day this is about JSON-RPC and there is two response types: Success or Error

undefined is more appropriate for another interface CAIP, potentially JS provider API which will cover all multi-chain session RPCs with authorize, request, update and delete

in this CAIP what we can do is add a new error code which can have code 0 and message unknown that we can then translate to undefined in a JS API

[hmalik88]

This needs to be updated in line with @pedrouid's comment from last week, not sure if this is a point of contention or not, but I agree with his point.

[bumblefudge]

Sure-- will finesse the language in a new commit

[bumblefudge]

Voilà

[hmalik88]

Perfect! Just wondering, how come the error code # change?

[hmalik88]

shouldn't params be an array of two arrays containing various scope objects? (per our description)

type Scope = {
methods: string[];
events: string[];
}
type ChainScope = Scope;
type NamespaceScope = Scope & { chains: Array<CAIP2Id>; }
type RequiredScope = NamespaceScope | ChainScope;
type OptionalScope = RequiredScope;

type CAIP25Params = [NonEmptyArray<RequiredScope>, NonEmptyArray<OptionalScope> | undefined]

[bumblefudge]

Ooh, so the params is an object in the examples and an array in the text? I hesitate to fix that either way by myself, let me check out if JSON-RPC syntax will make my job easier by disqualifying one of the two options 😅

[bumblefudge]

https://www.jsonrpc.org/specification#parameter_structures
^ Seems it can be array ("by-position") OR object ("by-name"), and "by-name" sounds more appropriate to this kind of finicky, versioned, open-world interoperability use-case... by-position sounds too brittle and easy to confuse or break.

[bumblefudge]

so i'm personally leaning towards keep object and make the text make the example :D

[hmalik88]

haha yes, I would also prefer object-based -- well that solves that :)

[bumblefudge]

Discussed out of band, merging for now, but PLEASE FEEL FREE to open a new issue or PR if you want to contest, reverse, or extend any changes here. This PR was getting too multidimensional to be manageable!
Note:

• Please move all rpcDocuments conversation to [CAIP-25] Add CAIP-211 ("Rpc Authority Negotiation") as extension spec to CAIP-25 #211 and
• Please move all CAIP-27 conversation to New CAIP - Community-Powered Assessment of Trust in Discrete Resources (Split of former: Community-powered trust assessment in software components) #271