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-???? | Web-Wallet Bridge - Abstraction Layer for DApps #623

Closed
wants to merge 13 commits into from

Conversation

Fell-x27
Copy link

@Fell-x27 Fell-x27 commented Nov 21, 2023

Original document: https://github.com/Fell-x27/CIPs/tree/master/CIP-Abstraction-Layer-for-DApps

Title: Dapp Bridge Rework

Goal: Develop a new standard for the integration of wallets and DApps, addressing a number of existing issues (details inside the document).

Brief Description: This CIP proposes a new mechanism for integrating wallets and DApps:

  • Clearly separating the roles of DApp and wallet in this interaction.
  • Creating an abstraction layer that eases the interaction for both parties.
  • Enhancing user security.
  • Reducing the impact of human factors by decreasing the number of potential failure points.

This is achieved by transforming the DApp into a client of the wallet, rather than a substitute for it.

@Ryun1 Ryun1 added the Category: Wallets Proposals belonging to the 'Wallets' category. label Nov 21, 2023
@rphair rphair changed the title A new CIP added: Dapp bridge rework CIP-???? | Dapp bridge rework Nov 21, 2023
@Fell-x27
Copy link
Author

Now, this mechanism supports not only DataHash for Movement, but also inline Datum.

@rphair
Copy link
Collaborator

rphair commented Nov 23, 2023

@Fell-x27 I am still trying to understand this material as well as your presentations in the Discord group which may be helping to put it into context with the other wallet interface improvement initiatives. I intend to catch up on all this before next week's meetings...

From your activity on Discord I'm guessing you're interested in attending first the Wallet Connectors Workshop (27 November 4PM UTC) and then the usual CIP meeting 24 hours after? I believe the CIP meeting would be a good time for a quick introduction to your proposal and to review the community's initial reaction to these ideas, so it's on the agenda here (https://hackmd.io/@cip-editors/77) cc @Ryun1 @Crypto2099

…bility to specify an explicit null value has been added.
… a collection of Metadatum + hash. It was already presented as an anonymous type passed in the metadata field of the TransactionScript object, but now it is explicitly specified;

2) The old version of the Metadata type has been renamed to Metadatum;

3) Added the LocalizedString type, allowing DApps to add transaction descriptions in more than one language (where possible);

4) In the TransactionScript type, the type for the metadata field has been changed to match item 1. Also, two new fields have been added: message and draft.
Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

@Fell-x27 thanks for elaborating your ideas in the Discord and I hope you will consider re-posting some of your relevant writing and diagrams into this PR, if not the CIP itself, if & when you are ready. Whenever discussions on Discord clarify an issue raised here or present an item for review, I'll also do what I can to keep the two media & sets of ideas connected.

If this CIP stays in its current form... rather than changing the scope or document type (e.g. a new CPS, or additions to @Ryun1's #619) based on the current discussions... there are some formatting changes to please make while you're editing this material:

CIP-dapp-bridge-rework/CIP-dapp-bridge-rework.md Outdated Show resolved Hide resolved
CIP-dapp-bridge-rework/LICENSE Outdated Show resolved Hide resolved
License: CC-BY-4.0
---

<!-- TOC -->
Copy link
Collaborator

@rphair rphair Nov 27, 2023

Choose a reason for hiding this comment

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

CIP site builders won't be able to parse a Table of Contents here and in fact will create a duplicate. Therefore it's most portable for CIPs to rely on these site builders to build the table of contents rather than include it in markup... which can get inconsistent with edited headers very easily, especially when other authors submit PRs based on your material.

We have a hardcoded TOC in CIP-0001 (yes it's gotten out of sync several times & I've had to correct it myself a couple times) and you can see how kind of silly the duplication looks here in the generated site: https://cips.cardano.org/cips/cip1/

If you continue to mimic CIP-0030 you can see that the generated TOCs on whatever site builder will contain certain heading levels already; currently:

I am hoping @Ryun1 @Crypto2099 @KtorZ will corroborate this but I think the most portable way therefore is to consider the TOC a matter for the presentation layer, not the CIP content itself.

Copy link
Author

Choose a reason for hiding this comment

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

That's why it has a wrapper, which allows third parties to ignore it if needed. My IDE added it, so I thought it was a standard to resolve issues like you described. It was added to improve navigation in the large document, at least during the 'github only' stage. I also use it in my md-editor. If it's not an option to cut it off by the TOC tag, I think it would be OK if it remains here during the 'github only' stage, and then, if it is included in the CIPs list, I'll remove the TOC to avoid any conflicts with document builders. What do you think?

Copy link
Collaborator

@rphair rphair Nov 28, 2023

Choose a reason for hiding this comment

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

I think that's OK as long as we remember to delete the TOC before any merging, or at least if/when this proposal is in Last Check and after you're sure it's no longer going through your IDE. Still I'll leave this "unresolved" in the meantime so other editors & reviewers can be aware of the issue & help remember this if/when merge time comes.

Copy link
Author

Choose a reason for hiding this comment

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

That's why it should be "unresolved" until the end I think.

CIP-dapp-bridge-rework/README.md Outdated Show resolved Hide resolved
CIP-dapp-bridge-rework/README.md Outdated Show resolved Hide resolved
Comment on lines 245 to 255
* **`policyId`**

Contains the [hexadecimal](#hexstring) representation of the asset's `policyID`.<br><br>

* **`name`**

Contains the [hexadecimal](#hexstring) representation of the asset's `name`. If absent, the value should be set to `null`.<br><br>

* **`amount`**

Contains the quantity of the asset [without a decimal part](#bigintstring). That is, if we want to describe 1 token with a decimal part of 6 digits, the amount should be set to 1,000,000, i.e., 1 * 10^6. Setting the value to 1 would be equivalent to 0.000001!<br><br>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is an example of what I was referring to above: loose content which would be better formatted as bullet lists or tables... especially because you're not using headings that would create deep links to these entries.

This pattern is repeated several times throughout the document, and unless addressed it will affect readability by making the CIP look longer & more complicated than it really is.

Copy link
Author

@Fell-x27 Fell-x27 Nov 28, 2023

Choose a reason for hiding this comment

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

Should I remove the brs only, or attach the content of each item to its header in one row?

Copy link
Collaborator

@rphair rphair Nov 28, 2023

Choose a reason for hiding this comment

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

👍 I think without the <br> elements Markup wiil create a white spacing consistent with other documents.... in hindsight that's what was creating the extra line spacing rather than the carriage returns as I suggested above (so resolving that one).

Yes I'd also look to see how it looks as a table with a row for each parameter, if you're also interested, which I think would make it a better match for other CIPs & computer documentation in general.

Copy link
Author

Choose a reason for hiding this comment

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

Yes I'd also look to see how it looks as a table with a row for each parameter, if you're also interested, which I think would make it a better match for other CIPs & computer documentation in general.

I'll check this approach. But there are many variations of docs styles. My reference in most cases was the MDN.

@Fell-x27
Copy link
Author

@rphair , thanks for the review! I'll check this again later and implement everything!

Fell-x27 and others added 2 commits November 28, 2023 10:20
This stub file was only included when CIPs were historically changed to use file base name README rather than the CIP name.

Co-authored-by: Robert Phair <rphair@cosd.com>
these only format properly as tick-boxes on GitHub & other Markdown previewers when either a space or `x`

Co-authored-by: Robert Phair <rphair@cosd.com>
CIP-dapp-bridge-rework/README.md Outdated Show resolved Hide resolved
CIP-dapp-bridge-rework/README.md Outdated Show resolved Hide resolved
CIP-dapp-bridge-rework/README.md Outdated Show resolved Hide resolved
CIP-dapp-bridge-rework/README.md Outdated Show resolved Hide resolved
CIP-dapp-bridge-rework/README.md Outdated Show resolved Hide resolved
CIP-dapp-bridge-rework/README.md Outdated Show resolved Hide resolved
CIP-dapp-bridge-rework/README.md Outdated Show resolved Hide resolved
CIP-dapp-bridge-rework/README.md Outdated Show resolved Hide resolved
CIP-dapp-bridge-rework/README.md Outdated Show resolved Hide resolved
@HinsonSIDAN
Copy link
Contributor

First of all, I really like the idea of streamlining transaction building process specifically. Some brief comments:

  1. On "building" transaction from an object, there is sample implementation on Mesh's latest lower level APIs (https://meshjs.dev/apis/transaction/builderExample). Maybe I would invite discussion based on it, whether we could rely on middle layer libraries like Mesh to handle this logic or putting this logic on wallet side (a concern is that as Adam mentioned it might add extra burden to wallet side).

  2. On the object itself, if we go for standardization, I think it is better to following the native object format (like using validity range as one item rather than splitting as from and to separately) to make the full compatibility, an example object for tx body from aiken's doc:

Transaction {
  inputs: List<Input>,
  reference_inputs: List<Input>,
  outputs: List<Output>,
  fee: Value,
  mint: MintedValue,
  certificates: List<Certificate>,
  withdrawals: Dict<StakeCredential, Int>,
  validity_range: ValidityRange,
  extra_signatories: List<Hash<Blake2b_224, VerificationKey>>,
  redeemers: Dict<ScriptPurpose, Redeemer>,
  datums: Dict<Hash<Blake2b_256, Data>, Data>,
  id: TransactionId,
}

@Fell-x27
Copy link
Author

@SIDANWhatever , hi!

a concern is that as Adam mentioned it might add extra burden to wallet side

Actually, it's not a problem. The thing is, the wallet has likely already implemented a similar mechanism. When we interact with it through the GUI, we don't directly form the transaction. We create some sort of transaction object/description based on input fields, using which we assemble the real one. Essentially, my CIP proposes using the same logic for interaction. So, the only thing needed is to write a "translator" from TransactionScript to a format the wallet understands. And simply extend the logic that is already working.

On the object itself, if we go for standardization, I think it is better to following the native object format

Yes, initially I thought about that, but then decided to move away from it. My idea was to lower the entry barrier and add a layer of abstraction over the existing terms so that the result would be as intuitive as possible for someone "outside the ecosystem". And, if possible, self-documenting. That's why the structure of TransactionScript and its subtypes deliberately do not replicate CDDL. Trying to bring in a more declarative approach. But yes, it can be changed later if we decide that it was the wrong way. It's not a big deal :)

Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

@Fell-x27 the sentiment, as best I understand it, from the CIP meeting today is that this method & CIP would be well suited to a serialization library that will give wallets a well-defined standard interface to offer these services to dApps.

The impact to existing trade practice of changing CIP-30 itself, and obligating all wallet providers to support this interface, would be too much of a burden on Cardano's ecosystem at this developmental stage.

But I think we all agree that there's potential here, enough to promote implementation as an additional library with a CIP here in support of its API which could achieve common but not universal use.

I believe this PR, and the CIP document with appropriate rewriting, would be usable for that idea & resolve the editorial and community difficulty we would have with imposing these changes on CIP-30 and any future wallet interface. cc @Ryun1 @Crypto2099

CIP-dapp-bridge-rework/README.md Outdated Show resolved Hide resolved
@Fell-x27
Copy link
Author

Fell-x27 commented Nov 28, 2023

@rphair

this method & CIP would be well suited to a serialization library that will give wallets a well-defined standard interface to offer these services to dApps. The impact to existing trade practice of changing CIP-30 itself, and obligating all wallet providers to support this interface, would be too much of a burden on Cardano's ecosystem at this developmental stage.

No, this is not a serialization library interface. This is an interface that allows a DApp to interact with a wallet in almost the same way as a user does. In fact, this standard was designed with the idea that the implementation of the API should be as painless as possible for wallets, which bear the brunt of the standard. Because it should be similar to how they already work internally. As I already replied to @SIDANWhatever, this standard should not add new entities but expand existing ones.

As for DApps, there are not so many of them at the moment that it would be a problem. But with this standard, writing DApps will become easier and there will be more of them. This is the main goal of this CIP. Focused on future projects in the ecosystem.

Regarding the need for changes - they are inevitable in any case. If there is a need in the ecosystem to replace CIP30, whatever replaces it will require changes from the ecosystem. Whatever it is. It's already painful now. But still not as painful as, say, in a year.

The ecosystem will need a transition period anyway, during which both the new standard and the old, but already deprecated, will have to be supported.

Moreover, this is not the final implementation. I've already received feedback that it might be more appropriate to implement this standard as a CIP30-like interface, to simplify integration. Or even as a high-level extension of CIP30. I'm still thinking about it.

* CIP renamed to 'Abstraction Layer for DApps' as per @rphair's advice

* Formatting changes as per @rphair's advice

* License file removed as per @rphair's advice

* Connection method revised in favor of CIP-30 extension based on developers' feedback

* Added clarifications for non-obvious points based on feedback in GitHub

* Added ability to explicitly specify withdrawals as proposed by DApp developers
@Fell-x27 Fell-x27 changed the title CIP-???? | Dapp bridge rework CIP-???? | Web-Wallet Bridge - Abstraction Layer for DApps Dec 10, 2023
@rphair rphair mentioned this pull request Dec 11, 2023
4 tasks
@rphair
Copy link
Collaborator

rphair commented Aug 20, 2024

@Fell-x27 since editors never marked this as Waiting for Author in response to your #623 (comment), specifically:

this is not the final implementation. I've already received feedback that it might be more appropriate to implement this standard as a CIP30-like interface, to simplify integration. Or even as a high-level extension of CIP30. I'm still thinking about it.

... I'm applying the (potentially) Abandoned tag that we are putting on PRs that have not progressed in a long time (8 months in this case). If you are still "thinking about" the problem then please indicate here how you want to progress and what exactly we are waiting for. Without a clear idea of your deliberations, this does appear "abandoned" by default & would eventually be closed.

@rphair rphair added the State: Likely Abandoned Close if confirmed abandoned (long waiting). label Aug 20, 2024
@rphair rphair closed this Sep 24, 2024
@Fell-x27
Copy link
Author

@Fell-x27 since editors never marked this as Waiting for Author in response to your #623 (comment), specifically:

this is not the final implementation. I've already received feedback that it might be more appropriate to implement this standard as a CIP30-like interface, to simplify integration. Or even as a high-level extension of CIP30. I'm still thinking about it.

... I'm applying the (potentially) Abandoned tag that we are putting on PRs that have not progressed in a long time (8 months in this case). If you are still "thinking about" the problem then please indicate here how you want to progress and what exactly we are waiting for. Without a clear idea of your deliberations, this does appear "abandoned" by default & would eventually be closed.

Hi, I've become a father, and it's harder to find time for everything. I'll be back as soon as possible.

@Fell-x27
Copy link
Author

Overall, the standard is quite ready, but I need time to implement the reference solution.

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: Likely Abandoned Close if confirmed abandoned (long waiting).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants