Skip to content
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-25] Add CAIP-211 ("Rpc Authority Negotiation") as extension spec to CAIP-25 #211

Closed
wants to merge 14 commits into from

Conversation

bumblefudge
Copy link
Collaborator

Tried to capture the skeleton of the discussion yesterday and propose one path forward. Note the changes to CAIP-25 as well!

If anyone feels like adding anything or filling in TODO sections, be my guest! use "changes requested", I suppose?

NB @shanejonas

CAIPs/caip-207.md Outdated Show resolved Hide resolved
CAIPs/caip-207.md Outdated Show resolved Hide resolved
Comment on lines 39 to 44
1. `rpcEndpoints` is an array of zero or more URLs of RPC endpoints that
the caller would prefer the respondent to use, ordered by preference. Each
must be a valid URL that addresses an RPC endpoint. The respondent may
return it empty, reordered, with less, the same, or even more conformant URLs
than received.
2. `rpcDocuments` is an array of zero or more URLs of machine-readable RPC
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the expected behavior if zero rpcEndpoints and zero rpcDocuments are provided?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I threw together a straw man in a new commit but we should probably iterate this a bit, it's kinda clunky as-is


### Response

The wallet can respond to this method with either a success result or an error message.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably be defining a test suite for this to make sure implementations are actually conformant. I know this is just a first cut at this but between CAIP-25 and CAIP-207 I see a lot of underspecified states for the protocol occurring between the RPC responder and the RPC requester.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couldn't agree more. I think test vectors might be easier to write into each profile, tho-- i.e. /namespaces/eip155/caip211.md#test-vectors. Perhaps namespace profiles per-CAIP should have a ##Test-Vectors section instead of an ## Examples section more generally?

@shanejonas
Copy link

for a wallet, is rpcEndpoints enough?

for example wallet_addEthereumChain has nativeCurrency so that it can pull out what the name, symbol, decimals to display its currency within the wallet.

},
"eip155:42069": {
"methods": ["get_balance", "chainChanged", "42069_sEcReTbAlAnCe"],
"rpcDocuments": ["https://openrpc.42069-chain.org/", "https://ethereum.github.io/execution-apis/api-documentation/"],
Copy link

@shanejonas shanejonas Feb 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this would not point to a documentation site, which is what https://ethereum.github.io/execution-apis/api-documentation/ is.

it would point to the actual openrpc document which is located here for the ethereum specs repo: https://raw.githubusercontent.com/ethereum/execution-apis/assembled-spec/refs-openrpc.json

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch! curious that https://ethereum.github.io/execution-apis/assembled-spec/refs-openrpc.json doesn't work-- would it make sense to configure the gitpages to publish to, e.g., docs.ethereum.org so that the link would survive a migration off of (Microsoft-owned) infra?

(I'll update it with the accurate raw.githubusercontent.com URL for now regardless)

CAIPs/caip-211.md Outdated Show resolved Hide resolved
CAIPs/caip-211.md Outdated Show resolved Hide resolved
@bumblefudge
Copy link
Collaborator Author

bumblefudge commented Feb 9, 2023

for a wallet, is rpcEndpoints enough?

for example wallet_addEthereumChain has nativeCurrency so that it can pull out what the name, symbol, decimals to display its currency within the wallet.

Hey @shanejonas! Speaking only for myself, I would tend to think that EIP-3085 is a slightly different use-case, since 3085 is for a multi-chain EVM dapp to request/suggest to an EVM wallet adding a chain that the wallet might not already know, a decision which might in turn be passed on to the user over an already-authorized connection. I think enabling the wallet_addEthereumChain method qua method would still work over a connection established by CAIP-25, and would probably be the most logical path in the case where the wallet does not already know/support/properly-display that chainID and its native UX components. To put it another way, I agree that rpcEndpoints isn't enough for adding new [and possibly unknown] chains smoothly to a known namespace; for that, first CAIP-25 authorizing and then calling wallet_addEthereumChain sounds like a better fit.

Conversely, authorizing additional chains with initial or progressive CAIP-25 authorizations make more sense in cases where that chain is already known to the wallet, particularly if that chain has additional methods/capabilities (like a ZK L2 or a non-EVM/EVM-superset L1 or L2). For these, CAIP-25 is asking a more fundamental question which is less about user authorization and more about wallet capability. Here, I should hope rpcEndpoints and/or rpcDocuments IS enough, and user consent and UX parity within the namespace is left for a later stage, whether using wallet_addEthereumChain or not!

@pedrouid
Copy link
Member

No objections on my end 👍

@hmalik88
Copy link
Collaborator

PR should be renamed with [CAIP-211]

CAIPs/caip-211.md Outdated Show resolved Hide resolved
CAIPs/caip-211.md Outdated Show resolved Hide resolved
CAIPs/caip-211.md Outdated Show resolved Hide resolved
CAIPs/caip-211.md Outdated Show resolved Hide resolved
#### Success

A succesful result will be a conformant successful result according to
[CAIP-25][], with the additional presence (if authorized) of the new properties
Copy link
Collaborator

@hmalik88 hmalik88 Feb 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if it makes sense to include the new properties in the response, what use is it to spit back what the dapp has requested from you? The OpenRPC doc would be just used by the wallet for discovery.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my thinking was that the ORDERING of the array might be changed by the wallet (i.e., I'll trust this RPC extension doc, but wherever it redefines something in my core doc, e.g. execution-apis, I'll take the latter's definition). This allows for some flexibility between binary accepting/rejecting each element in the array.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On further thought, it might make sense to include the rpcEndpoints field in an ordered manner like you mentioned for rpcDocuments. If a provider so chooses to use the provided endpoints, it can specify usage + fallbacks in order or otherwise return an empty array (if its using endpoints of its own choice).

Ideally, there should only be one source of truth for the rpcDocument, lets call it that because well it should be one. Imagine a scenario where for some reason the multiple endpoints are not updated to be in line with each other, we have a scenario where the wallet doesn't know what document to trust. Anytime a request has an OpenRPC doc, the provider should be using that to determine/provide functionality, including for core-apis.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I guess I was assuming that multiple RPC docs don't necessarily undermine THE RPC Doc.

Example: custom endpoints and/or proprietary, feature-rich wallets might find it useful to have RPCDocs reinforcing or making more explicit what's custom about them-- whether the custom RPC doc overrides The One is a matter of policy, easily expressed in ordering? Why not let additional RPC docs extend THE RPC Doc, like a class?

Maybe that's too footgunny, though. The real question is whether that ordering mechanism is too prone to human error or too hackable... let's talk about it at the next meeting and see what others in the group have to say, maybe? Feel free to bring Shane or anyone at MM who has opinions and/or topical devrel experience from EthDenver :D

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See also this exchange with ligi, which refers to a discussion with TimDaub about eth clients at the last editorial meeting:
#206 (comment)

It seems multiple only very slightly different RPC documents might be useful until there is 100% conformance across all eth clients! I can definitely imagine that on other namespaces (e.g. Polkadot) where chains can customize their runtimes and add crates/pallets, it would be very useful for cross-chain apps to have multiple RPC docs...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps namespace-specific profiles of 211 would be the place to recommend (or even constrain) behavior here, like "never authorize more than 1 RPC Doc" or "Never put any RPC ahead of The One when authorizing"?

##### TODO: replace following with novel errors

Possible error messages (to be discussed with WG):
- rpcDocuments not conformant syntactically (not openRPC, not served as mime type JSON, etc)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an OpenRPC doc to describe the OpenRPC spec, that would be helpful 😄

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! The error message could even LINK to it (some WC error messages currently link to CAIP URLs!) although I'm not sure where to draw the normativity line on things like error messages

CAIPs/caip-25.md Outdated
the `optionalScopes` array MUST contain 1 or more of them, if present.

There is one special-case scope object, named `wallet`, which refers to methods
and events independent of any namespace but, crucially, no chains or CASA
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This essentially blocks wallet methods that are chain-related then no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, wallet_changeEthereumChain would still be a valid method, but it would be in an eip155 or eip155:{chainId} scope object! the semantics/naming gets a little wonky but hey no one was thinking about non-ethereum wallets or offchain capabilities when the ethereum RPC methods were being named :D

CAIPs/caip-25.md Outdated Show resolved Hide resolved
@bumblefudge bumblefudge changed the title [CAIP-25] Add CAIP-207, Rpc Authority Negotiation, as extension spec [CAIP-25] Add CAIP-211, Rpc Authority Negotiation, as extension spec Feb 17, 2023
@bumblefudge bumblefudge changed the title [CAIP-25] Add CAIP-211, Rpc Authority Negotiation, as extension spec [CAIP-25] Add CAIP-211 ("Rpc Authority Negotiation") as extension spec to CAIP-25 Feb 17, 2023
Bumblefudge and others added 2 commits February 17, 2023 08:20
Co-authored-by: Hassan Malik <41640681+hmalik88@users.noreply.github.com>
@bumblefudge bumblefudge marked this pull request as ready for review February 19, 2023 15:00
Bumblefudge and others added 2 commits February 19, 2023 07:01
Co-authored-by: Hassan Malik <41640681+hmalik88@users.noreply.github.com>
@bumblefudge
Copy link
Collaborator Author

In the interest of simplicity, I'm tempted to just close this in favor of the far less verbose #217 . The example responses illustrating reordering of the arrays might show up again in a future implementation guide or other auxiliary documentation, perhaps even a Informational CAIP?

@bumblefudge bumblefudge marked this pull request as draft March 29, 2023 22:57
@obstropolos
Copy link
Contributor

Closing this in favor of continued work on #217

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants