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

CIP-0090? | Extendable dApp-Wallet Web Bridge #462

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

Ryun1
Copy link
Collaborator

@Ryun1 Ryun1 commented Feb 16, 2023

This document describes a base web-wallet communication bridge accompanied by a scheme from which functionality can be added through API extensions.

This specification defines a wallet-dApp connection standard using injected Javascript object, with an extendable API interface.

This aims to allow the creation of dApp connectors without the need to include the CIP-30 API as proposed in #446.
Instead the CIP-30 API can act as an optional extension to this specification.

This draws direct inspiration from @mkazlauskas' comment, utilizing @KtorZ's design from #446. Thus the author accreditations.

Todo

  • Rewrite to align with current landscape of CIP30 extensions.

(rendered proposal in branch)

CIP-XXXX/CIP-XXXX.md Outdated Show resolved Hide resolved
CIP-XXXX/CIP-XXXX.md Outdated Show resolved Hide resolved
CIP-XXXX/CIP-XXXX.md Outdated Show resolved Hide resolved
CIP-XXXX/CIP-XXXX.md Outdated Show resolved Hide resolved
CIP-XXXX/CIP-XXXX.md Outdated Show resolved Hide resolved
CIP-XXXX/CIP-XXXX.md Outdated Show resolved Hide resolved
CIP-XXXX/CIP-XXXX.md Outdated Show resolved Hide resolved

Extensions provide an extensibility mechanism and a way to negotiate (possibly conflicting) functionality between a dApp and a wallet provider. There's rules enforced as for what extensions a wallet decide to support or enable. The current mechanism only gives a way for wallets to communicate their choice back to a dApp.

We use object as extensions for now to leave room for adding fields in the future without breaking all existing interfaces. At this point in time however, objects are expected to be singleton.

Choose a reason for hiding this comment

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

Don't quite understand what 'singleton' means in this context (or what is the 'object'). Would be nice to elaborate.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This means that extension objects cannot pass additional fields at enable time, currently.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to limit this in this text? Isn't it enough to say that this CIP only defines a single key in the object passed, and that it's up to Extensions to define additional fields in their respective CIP, if any?

Choose a reason for hiding this comment

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

This means that extension objects cannot pass additional fields at enable time, currently

Thanks. Could be just me not understanding it from text, but consider rewording it @Ryun1

Do we need to limit this in this text? Isn't it enough to say that this CIP only defines a single key in the object passed, and that it's up to Extensions to define additional fields in their respective CIP, if any?

The simpler the base dApp connector cip the better imho. The scope of the extension CIPs can then be contained to just the API object that enable() returns. The extensions can have API methods (or arguments for those methods) instead. If we allow configuration at enable() time then we can expect to see extension designs that would require the dApp calling enable multiple times with different arguments. Do we want that? 😅

Copy link
Collaborator Author

@Ryun1 Ryun1 Feb 21, 2023

Choose a reason for hiding this comment

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

Do we want that?

You raise a good point, I think this could become messy quickly.

And I have removed this sentence to avoid the confusing wording.

CIP-XXXX/CIP-XXXX.md Outdated Show resolved Hide resolved
CIP-XXXX/CIP-XXXX.md Outdated Show resolved Hide resolved
@Ryun1 Ryun1 changed the title CIP-???? | Extendable dApp Connector CIP-???? | Extendable dApp-Wallet Web Bridge Feb 20, 2023
@KtorZ KtorZ mentioned this pull request Feb 21, 2023
Copy link
Member

@KtorZ KtorZ left a comment

Choose a reason for hiding this comment

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

Love it. Thanks for the pushing this @Ryun1.

Any thoughts @Quantumplation ?

CIP-XXXX/README.md Outdated Show resolved Hide resolved
CIP-XXXX/README.md Outdated Show resolved Hide resolved

In order to initiate communication from webpages to a user's Cardano wallet, the wallet must provide the following javascript API to the webpage. A shared, namespaced `cardano` object must be injected into the page if it did not exist already. Each wallet implementing this standard must then create a field in this object with a name unique to each wallet containing a `wallet` object with the following methods. The API is split into two stages to maintain the user's privacy, as the user will have to consent to `cardano.{walletName}.enable()` in order for the dApp to read any information pertaining to the user's wallet with `{walletName}` corresponding to the wallet's namespaced name of its choice.

#### cardano.{walletName}.enable(extensions: Extension[] = [{ cip: 30 }]): Promise\<API>
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment about structural vs positionsl as I left on the other CIP30 PR

Copy link
Contributor

@stevenj stevenj Mar 14, 2023

Choose a reason for hiding this comment

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

Given this is defined in CIP-30 and is already implemented by wallets, this is asking for trouble on a backwards compatibility front.

It would be simpler if the wallets just enable ALL extensions they are aware of.

Alternatively, there could be:

cardano.{wallet}.reqExtensions(extensions: Extension[])

This could pre-informs the wallet what extensions the dapp wants to use. The enable then just enables those. The default if its not called should be enable all extensions the wallet supports. However, in this case, I think its informative only (maybe the wallet can use it to display something meaningful to the user?). The wallet should enable all the extensions that it supports, which were requested. If an extension was requested but the wallet does not support it, that should not be an error.

To be clear, even though this reqExtension function could be added, I don't think it's necessary.


A list of extensions supported by the wallet. Extensions may be requested by dApps on initialization. Dapps can repeatably call `wallet.enable()` to add further extensions. Some extensions may be mutually conflicting, and this list does not thereby reflect what extensions will be enabled by the wallet. Yet it informs on what extensions are known and can be requested by dApps if needed.

`undefined` is equivalent to `{cip: 30}`.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is redundant.

I do not see much utility in fine grain switching of extensions.
If the process was just:

cardano.{walletName}.enable();
extensions = cardano.{walletName}.isEnabled();

That tells you everything you need as a dApp.

  1. Does this wallet support this cip, or is it just obeying cip-30.
  2. What extensions does the wallet support.
  3. All supported extensions are enabled.

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 do agree with this, I think I will do a revision of this proposal.


Errors: `ConnectorError`

Returns a list of extensions which have been enabled by the wallet. If no wallet access has been granted then a `ConnectorError` with `Refused` error code should be thrown.
Copy link
Contributor

Choose a reason for hiding this comment

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

Again from a backward compatibility viewpoint, this should be allowed to return either a bool or a list of enabled extensions. a bool set to true is equivalent to CIP-30, and should mean "This wallet does not support this CIP".

Even though a wallet that implements this CIP should not return a bool, its a valid response from a CIP-30 wallet and it should be clear how a dapp reacts to it.

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 agree with this also, I think I had not considered backward compatibility enough... I plan to do a more significant rewrite when/if #486 is merged.

@KtorZ KtorZ changed the title CIP-???? | Extendable dApp-Wallet Web Bridge CIP-0090? | Extendable dApp-Wallet Web Bridge Mar 15, 2023
CIP-XXXX/README.md Outdated Show resolved Hide resolved
@KtorZ KtorZ added the Category: Wallets Proposals belonging to the 'Wallets' category. label Mar 18, 2023
@Ryun1
Copy link
Collaborator Author

Ryun1 commented May 2, 2023

I plan to do a significant rewrite once there is more community consensus around #486.

@Ryun1 Ryun1 added the State: Waiting for Author Proposal showing lack of documented progress by authors. label Jul 16, 2023
@Ryun1 Ryun1 mentioned this pull request Aug 29, 2023
Ryan Williams and others added 13 commits November 17, 2023 18:02
Co-authored-by: Martynas <martynas@firmfirm.co>
Co-authored-by: Martynas <martynas@firmfirm.co>
Co-authored-by: Martynas <martynas@firmfirm.co>
Co-authored-by: Martynas <martynas@firmfirm.co>
Co-authored-by: Martynas <martynas@firmfirm.co>
Co-authored-by: Martynas <martynas@firmfirm.co>
Co-authored-by: Matthias Benkort <5680256+KtorZ@users.noreply.github.com>
Ryan Williams and others added 3 commits November 17, 2023 18:02
@Ryun1 Ryun1 removed the State: Waiting for Author Proposal showing lack of documented progress by authors. label Nov 17, 2023
@rphair
Copy link
Collaborator

rphair commented Aug 20, 2024

@Ryun1 since this still has not moved forward (and I am not rushing you) I'm reapplying the Waiting for Author tag that you removed 9 months ago. As with all those tagged as such, it will help to have a comment indicating what it is "Waiting for Author" for 😅

@rphair rphair added the State: Waiting for Author Proposal showing lack of documented progress by authors. label Aug 20, 2024
@Ryun1
Copy link
Collaborator Author

Ryun1 commented Aug 22, 2024

Leave me to it

I think I will fix up this proposal explaining drawbacks, get it merged
but not pursue implementation because I am writing another proposal instead of this

@klntsky
Copy link
Contributor

klntsky commented Sep 4, 2024

@Ryun1 how would the new proposal differ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Wallets Proposals belonging to the 'Wallets' category. State: Waiting for Author Proposal showing lack of documented progress by authors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants