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-0020 | Update with new Implementors, Screenshots ... #394

Merged
merged 35 commits into from
Dec 8, 2022

Conversation

gitmachtl
Copy link
Contributor

@gitmachtl gitmachtl commented Dec 1, 2022

  • Updated integration list
  • Updated screenshots

@gitmachtl gitmachtl changed the title CIP-0020 | Adding encrypted message specification CIP-0020 | Adding encrypted message specification (Update) Dec 1, 2022
@rphair rphair changed the title CIP-0020 | Adding encrypted message specification (Update) CIP-0020 | Adding encrypted message specification Dec 1, 2022
@rphair rphair added the Update Adds content or significantly reworks an existing proposal label Dec 1, 2022
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.

@gitmachtl @Crypto2099 this is robustly well documented and it's also helpful to have an update of the supporting dApps in the list "current Tools/Sites/Explorers".

This is going to be introduced at the CIP meeting today if any authors are going to be there. I'm not the metadata expert (@KtorZ @SebastienGllmt) but I would readily approve this pending their acceptance & will say so at the meeting.

Also a new CIP specification came out in the meantime which dictates a slightly different preamble format (https://github.com/cardano-foundation/CIPs/tree/master/CIP-0001#header-preamble), e.g. Authors: should be a bulleted list, with some fields added and others dropped.

CIP-0020/README.md Show resolved Hide resolved
``` json
"basic": {
"desc": "symmetrical encryption via openssl and a passphrase (default=cardano)",
"type": "openssl",
Copy link
Contributor

Choose a reason for hiding this comment

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

Not super convinced by this format. I think if you try to go down this route, this is what will happen:

  1. At some point, people will start asking for other encryption schemes and you will keep modifying this list and it will get bigger
  2. At some point, somebody will suggest that instead of specifying some totally custom format for this, you should just standardize it to something like https://github.com/panva/jose so that you automatically get SDKs for a bunch of langauges, etc.
  3. Somebody will point out that instead of using a JSON-based SDK, you should use a CBOR-based standard since that's what Cardano is actually storing on-chain
  4. You will realize that the Cardano message signing spec is actually already meant to handle mostly (3)

so if you actually want to do this route, it may make more sense to make a separate CIP from CIP20 that talks about how to take the message signing spec and building some metadata standard that uses it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @SebastienGllmt thx for the feedback!

The current format of this "lookup" file was choosen as a simple first start. It was not intended to be compatible for all possible future updates with all kinds of different encryption types. It could also be a simple text description file. It is ment currently as a place to collect the key parameters.

I agree that CIP8 has some elements in it, but is to complicated to act as an easy upgrade to the existing transaction message/memo/comment format imo. When we started with CIP-0020 the intention was to keep it simple so many can implement it, but also leave it open enough to upgrade it. For the CIP8 style encryption, its ChachaPoly instead of AES256 for the method via passphrase. But CIP8 is oriented in signing in first place. Going with this format would make it totally incompatible with the current CIP-0020 format.

Let me think about it...

Copy link
Contributor

Choose a reason for hiding this comment

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

What I like about this update is the simplicity of implementing openssl encryption for wallets. This isn't designed to be the end-all-be-all security solution for privacy on the blockchain. There's better options and upcoming Midnight for that. This is just so your receipt for a purchase at seedywebsite.com isn't in cleartext for everyone to see.

@KtorZ
Copy link
Member

KtorZ commented Dec 6, 2022

Given that CIP-0020 is already active and in-use by different providers and, given that this proposal is a quite substantial addition; it would be better to propose this as a new CIP coming as an extension of CIP-0020.

In particular, the new CIP should formulate what is motivating the introduction of encryption in the message scheme (I reckon privacy, but some supporting use cases would be nice).

@gitmachtl
Copy link
Contributor Author

gitmachtl commented Dec 6, 2022

@KtorZ maybe a good idea to bring it in as an own CIP, however, its an update to an existing one. Should the new one than replace the old one?

@gitmachtl
Copy link
Contributor Author

Also a new CIP specification came out in the meantime which dictates a slightly different preamble format

Thanks for pointing this out, i wasn't aware of that.

@rphair
Copy link
Collaborator

rphair commented Dec 7, 2022

@gitmachtl: ... Should the new one than replace the old one?

no, definitely what we talked about the meeting was to leave CIP-0020 as-is and create a new CIP, to be assigned a new number, defining the encrypted message standard. It wouldn't have to an all-new CIP... and would be mostly just the diff with CIP-0020 seen here at this time.. because most of the sections like Motivation and parts of the implementation could refer back to the CIP-0020 document, with just the new parts being documented in the new CIP. Of course then there would be no reason to deprecate CIP-0020.

As with other "extensions" like new parts coming in to extend the API like CIP-0030, this would have the advantage of allowing developers and users to say "supports CIP-00NN" when indicating their support for the encrypted message standard, rather than referring to a title or section of CIP-0020 which is less concise. And it also would prevent documents like CIP-0020 and CIP-0030 from becoming bloated by who-knows-how-many extensions may be added over time. cc: @KtorZ

@gitmachtl
Copy link
Contributor Author

Should we start to name such new ones like CIP-0020a ? Because its a bit difficult to track the CIPs only on there numbers if they relate to each other. At least there should be an Info in the original CIP than that there are is an Extension CIP available xxx.

@rphair
Copy link
Collaborator

rphair commented Dec 7, 2022

name such new ones like CIP-0020a

This would break our scripts that build cips.cardano.org and the Developer Portal, and probably some future scripts 🤣

there should be an Info in the original CIP than that there are is an Extension CIP available xxx.

Yes, that's sensible, and adding that link (maybe with a brief introduction) to the CIP-0020 document would be welcome in whatever PR creates the newer CIP. If somehow we see that such a link is missing I would add it myself (we've had to do this for other related CIPs like protocol parameter updates).

@gitmachtl
Copy link
Contributor Author

Ok, i will modify this PR for the original CIP-0020 than to keep the updated pictures and tools/services in as a normal info update and add a new PR with the additions.

@gitmachtl gitmachtl changed the title CIP-0020 | Adding encrypted message specification CIP-0020 | Update with new Implementors, Screenshots ... Dec 8, 2022
@gitmachtl
Copy link
Contributor Author

@rphair @KtorZ I have updated this PR to only add new implementors, updated screenshots and using the new header format

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.

@gitmachtl looks good & will await the next PR that covers encrypted messaging.

@KtorZ I've seen this with other CIP PRs and can't figure out why some of these preambles render as tables, and others just the raw text, when viewed on GitHub even when the --- format & field arrangements are the same. I'm just including this in the review in case there's somehow a formatting problem that I've missed (not just here, but in those other PRs).

@rphair rphair requested a review from KtorZ December 8, 2022 12:58
@KtorZ
Copy link
Member

KtorZ commented Dec 8, 2022

@rphair the problem comes from the authors list here; the frontmatter data do not support markdown and the markdown renderer therefore treats the whole section as a blob of code.

@KtorZ
Copy link
Member

KtorZ commented Dec 8, 2022

@rphair
Copy link
Collaborator

rphair commented Dec 8, 2022

thanks @KtorZ; this likely was also the problem with the other CIPs: links in the frontmatter. I see from your last commit that it has no trouble with email style syntax (will check for this when we're working on #389).

@gitmachtl
Copy link
Contributor Author

looks good & will await the next PR that covers encrypted messaging.

here it is #409

@KtorZ KtorZ merged commit 6f50596 into cardano-foundation:master Dec 8, 2022
Ryun1 pushed a commit to Ryun1/CIPs that referenced this pull request Nov 17, 2023
…ndation#394)

* Create cip0020-encryption-modes.json

* Update cip0020-encryption-modes.json

* Update README.md

* Update README.md

* Update README.md

* Create demo_via_nodeJS.js

* Create demo_via_PHP.php

* Update demo_via_nodeJS.js

* Update README.md

* Update README.md

* Update README.md

* Create cip0020-democode-PHP.php

* Delete demo_via_PHP.php

* Create cip0020-democode-NODEJS.js

* Delete demo_via_nodeJS.js

* Create normal-message-metadata.json

* Create encrypted-message-metadata.json

* Create cip0020-democode-BASH.sh

* Update cip0020-democode-BASH.sh

* Update README.md

* Update README.md

* Update README.md

* Update README.md

* Update README.md

* Update README.md

* Update README.md

* Added 'SPO Scripts' as an encrypted msg integration example

* Delete cip0020-encryption-modes.json

* Delete cip0020-democode-BASH.sh

* Delete cip0020-democode-NODEJS.js

* Delete cip0020-democode-PHP.php

* Delete encrypted-message-metadata.json

* Delete normal-message-metadata.json

* Updating README with updated Implementors

- This update uses the new Header-Scheme, added Implementors and updated some Screenshots

* Adjust authors list (remove markdown) + adjust section titles levels.

Co-authored-by: KtorZ <matthias.benkort@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Update Adds content or significantly reworks an existing proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants