CIP-0072 | Off-chain schema versioning and schema adjustments #612

marcin-mazurek · 2023-10-23T16:10:33Z

Lace team would like to add a version property for versioning off-chain metadata following the pattern from CIP-25 as well as propose some updates to the schema based on the development done so far.

Additionally, we would like to clarify the algorithm used for root hash calculation to avoid ambiguity.

rphair

Looks straightforward... @marcin-mazurek given that this was filed as Draft (not yet ready for review), are there any planned changes before we review it? (If not you can mark it Ready for Review, or editors can.)

CIP-0072/version_1_offchain.json

marcin-mazurek · 2023-10-24T09:52:23Z

@rphair I marked it as draft to collect some preliminary feedback internally, I'm aiming to mark it as Ready for review today. I'm sorry for the confusion, next time I'm going to ask for internal review in the repository fork.

rphair · 2023-10-24T15:57:07Z

thanks @marcin-mazurek, based on what you've said in #612 (comment) I would be happy to approve it as soon as all your final changes are posted & officially marked ready.

marcin-mazurek · 2023-10-26T15:57:42Z

@rphair this is now ready for review

rphair

thanks @marcin-mazurek, looks good from my own point of view (not working directly with dApps) & any additional technical confirmation would also be welcome so please feel free to tag them here. 😎 cc @matiwinnetou

CIP-0072/version_1_offchain.json

danielmain · 2023-10-31T07:41:35Z

CIP-0072/version_1_offchain.json

            "pattern": "(https?:\/\/(?:www\\.|(?!www))[a-zA-Z0-9][a-zA-Z0-9-]+[a-zA-Z0-9]\\.[^\\s]{2,}|www\\.[a-zA-Z0-9][a-zA-Z0-9-]+[a-zA-Z0-9]\\.[^\\s]{2,}|https?:\/\/(?:www\\.|(?!www))[a-zA-Z0-9]+\\.[^\\s]{2,}|www\\.[a-zA-Z0-9]+\\.[^\\s]{2,})"
          },
          "discord": {
            "type":"string",
            "description": "An optional Discord link.",
+            "maxLength": 200,


What is the benefit by limiting this, if this is offchain?

@marcin-mazurek I'd also like to know why this was added. From experience I know in the vast majority of cases it's a sensible limit, and also that the hard limit will make something fail at some future time. What is the actual risk for these values to have an arbitrary & potentially huge length?

…d other protocols. Renamed twitter to x.com

danielmain · 2023-11-02T11:02:11Z

@rphair I made some changes, regarding the twitter vs x.com, and I modified the regex to accept more URLs and IPFS addresses. Would you pls review it again 🙏🏻

Thank you in advance ❤️

rphair

thanks @danielmain everything looks even better & thanks for attending to the URL form. This is still pending acceptance by 1 other CIP reviewer (I invited them all last week) and hopefully in the meantime @marcin-mazurek will answer your pending question #612 (comment).

Crypto2099

Wondering if this should be changed prior to approval since the intent here is to clean up and enhance the off-chain metadata format.

CIP-0072/version_1_offchain.json

danielmain

Signature has been removed since every cardano transaction has it own signature. The transaction signature (or called Witness) can be used to prove the owner in case of a dApp update or deregistration.

CIP-0072/version_1_offchain.json

ginnun · 2023-11-13T13:58:35Z

CIP-0072/version_1_offchain.json

+              "type": "string",
+              "description": "The platform or resource identifier (GitHub, Website, X.com, etc)"
+            },
+            "link": {


All the other links have max 200 chars allowed. Should we have it here too?

good point, thank you 😉

Ryun1 · 2023-11-13T14:58:46Z

CIP-0072/README.md


-On-chain part has a signature, which has a role to verify that a certain dApp owner made changes. In the initial version, a blake2b-256 signature was needed only for `rootHash` but following discussion, due to security concerns, decision has been made that the signature should attest the whole on-chain canonical JSON except signature field itself (because it would end up in an infinite recursion).
+As any transaction in cardano network has a signature, which has a role to verify that a certain dApp owner made changes.


What is the rationale behind relying on transaction witnesses rather than what was originally proposed?

For me, relying on transaction signatures just adds an extra step to the verification whilst also restricting the type of keys/crypto that can be used. I also liked the property that the one registering did not have to be the one creating and submitting the Tx.

Hi @Ryun1 thank u for the review

What is the rationale behind relying on transaction witnesses rather than what was originally proposed?

After evaluating the situation, my colleagues and I found it puzzling to require two signatures 🤷. I reached out to @matiwinnetou for clarification and discovered the additional signature was initially implemented because Blockfrost did not support witness signatures in transaction fetching a year ago. However, I personally believe that our decision should not be solely based on whether Blockfrost supports a feature or not. With tools like cardano-cli, cardano-wallet, Daedalus, and cardano-services that do support witness signatures, it seems more logical and efficient to make this CIP simpler by adhering to this available technology.

Just adds an extra step to the verification whilst also restricting the type of keys/crypto that can be used.

Indeed, our goal is to streamline the process. By focusing on simplification, we can avoid unnecessary steps in verification and avoid limiting the types of keys/crypto that can be used.

I also liked the property that the one registering did not have to be the one creating and submitting the Tx.

While this flexibility might seem advantageous, I view the requirement for the same key used in initial dApp registration for updates (such as domain changes or rebranding) as a beneficial feature. It adds a layer of security and consistency, ensuring that only authorized updates are made to a dApp.

CIP-0072/README.md

Ryun1 · 2023-11-13T15:15:52Z

Hey @marcin-mazurek,

These are some sizable changes and I am curious for the reasons behind these changes.
With such reasons being reflected within the Rationale section, which also needs a tidy as it is currently somewhat confusing in places such as Schema Version.

Since this is a proposed CIP, I would generally be hesitant to make such changes without incrementing the version.
I don't really know the extent of implementations and such the impact of changing this version 1.
I would consider calling this version 2, if there are any existing implementations.

danielmain · 2023-11-16T09:10:15Z

These are some sizable changes and I am curious for the reasons behind these changes.

1- Simplify the dApp registration for: dApp developers and the team behind implementing the CIP
2- Clear define the minimum off-chain-metadata provided for the dApp
3- Define types and limits of each off-chain-metadata.

danielmain · 2023-11-16T09:15:09Z

With such reasons being reflected within the Rationale section, which also needs a tidy as it is currently somewhat confusing in places such as Schema Version.

Removed

danielmain · 2023-11-16T09:17:24Z

Since this is a proposed CIP, I would generally be hesitant to make such changes without incrementing the version.

We incremented it to 2.0.0 also in file names, is that ok?

danielmain · 2023-11-16T09:18:35Z

I don't really know the extent of implementations and such the impact of changing this version 1.

Reduce effort for dApp developers when registering and reduce abiguity

danielmain · 2023-11-16T09:19:31Z

I would consider calling this version 2, if there are any existing implementations.

We bumped it to 2.0.0 and as far we know there are no implementations out there yet

szymonmaslowski · 2023-11-16T14:53:13Z

CIP-0072/version_2.0.0_onchain.json

       }
    },
    "required":[
       "subject",
       "rootHash",
-       "type",
-       "signature"
+       "type"


I think we are missing the additionalProperties: false here. According to the docs by default any additional properties are allowed, which is not the case for the on-chain json. Accoring to the CIP-72 additional properties are allowed for the off-chain json, but it doesn't say the same for the on-chain one, so I guess we should restrict it.

ginnun · 2023-11-17T11:54:13Z

CIP-0072/README.md

@@ -51,7 +52,7 @@ Stores and auditors should be able to follow the chain and find when a new dApp

 ### **On-chain dApp Registration Certificate**

-The on-chain dApp registration certificate MUST follow canonical JSON and be serialised according to RFC 8785 (https://www.rfc-editor.org/rfc/rfc8785). This stipulation is to avoid any ambigiutines in the signature calculation.
+The on-chain dApp registration certificate MUST follow canonical JSON and be serialised according to RFC 8785 (https://www.rfc-editor.org/rfc/rfc8785).


Since we removed the signature, we don't need to be canonical anymore

removed 🙂. Thank you!

…o-foundation#612) * Specify hashing algorithm for rootHash calculation * Add off-chain version property * Update categories with Lace DApp Store category list * Add DApp screenshot and company detail fields * Remove duplicated/ambiguous website property * Add some property length constraints and describe image requirements * Specify property length limit for the rest `social` props * Fix a typo * Enhanced the regex for URLs to be more permissive by allowing ipfs and other protocols. Renamed twitter to x.com * Removed extra signature in the on-chain metadata * Applied suggestions to have flexible social links * Logo and screenshots gives you the posibility to add base64 images and changed version to follow semver * Added examples using the new schema * Follow semver for all versions * Removed optional links, images only via base64. Added 3 new fields * Changed some descriptions and added some mandatory fields * Added screenshots as mandatory * Removed ftp for links * Added short description to be mandatory * Applied suggestions after review * Bump version to 2.0.0 and removed schema topic in rationale * There is no point defining a schema if it can be violated 🤷‍ * Applied suggestion, we are not signing the off-chain data anymore. * URL settings simplified * Fixed version to follow semver --------- Co-authored-by: Daniel Main <daniel@funktional.dev> Co-authored-by: Daniel Main <daniel.main.cernhoff@icloud.com>

Marcin Mazurek added 3 commits October 23, 2023 18:07

Specify hashing algorithm for rootHash calculation

7c08755

Add off-chain version property

a6e01e5

Update categories with Lace DApp Store category list

94e8bbb

rphair reviewed Oct 24, 2023

View reviewed changes

CIP-0072/version_1_offchain.json Outdated Show resolved Hide resolved

rphair added the Category: Metadata Proposals belonging to the 'Metadata' category. label Oct 24, 2023

Add DApp screenshot and company detail fields

e1cd6a0

Marcin Mazurek added 2 commits October 25, 2023 16:34

Remove duplicated/ambiguous website property

e23c91f

Add some property length constraints and describe image requirements

20c45cd

marcin-mazurek changed the title ~~CIP-0072 | Off-chain schema versioning, new categories~~ CIP-0072 | Off-chain schema versioning and schema adjustments Oct 25, 2023

Specify property length limit for the rest social props

0f47639

marcin-mazurek marked this pull request as ready for review October 26, 2023 15:56

rphair approved these changes Oct 26, 2023

View reviewed changes

rphair requested review from SebastienGllmt, KtorZ, Ryun1 and Crypto2099 October 26, 2023 17:14

Fix a typo

c0d5c6d

matiwinnetou approved these changes Oct 27, 2023

View reviewed changes

danielmain suggested changes Oct 31, 2023

View reviewed changes

Enhanced the regex for URLs to be more permissive by allowing ipfs an…

d99faad

…d other protocols. Renamed twitter to x.com

danielmain approved these changes Nov 2, 2023

View reviewed changes

rphair approved these changes Nov 2, 2023

View reviewed changes

Crypto2099 reviewed Nov 8, 2023

View reviewed changes

CIP-0072/version_1_offchain.json Outdated Show resolved Hide resolved

Removed extra signature in the on-chain metadata

32f259e

danielmain reviewed Nov 9, 2023

View reviewed changes

Applied suggestions to have flexible social links

9ce8444

danielmain and others added 4 commits November 10, 2023 16:30

Changed some descriptions and added some mandatory fields

47fd34a

Added screenshots as mandatory

0320ebe

Removed ftp for links

1dfdec3

Added short description to be mandatory

e271e30

ginnun approved these changes Nov 13, 2023

View reviewed changes

ginnun suggested changes Nov 13, 2023

View reviewed changes

Ryun1 reviewed Nov 13, 2023

View reviewed changes

CIP-0072/README.md Show resolved Hide resolved

Ryun1 reviewed Nov 13, 2023

View reviewed changes

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

Applied suggestions after review

60a9085

Bump version to 2.0.0 and removed schema topic in rationale

d296278

szymonmaslowski suggested changes Nov 16, 2023

View reviewed changes

ginnun approved these changes Nov 17, 2023

View reviewed changes

ginnun reviewed Nov 17, 2023

View reviewed changes

danielmain added 2 commits November 17, 2023 16:46

There is no point defining a schema if it can be violated 🤷‍

f2615d6

Applied suggestion, we are not signing the off-chain data anymore.

8713cfe

szymonmaslowski approved these changes Nov 19, 2023

View reviewed changes

danielmain added 2 commits November 21, 2023 09:46

URL settings simplified

950e50e

Fixed version to follow semver

b625f34

ginnun approved these changes Nov 28, 2023

View reviewed changes

Ryun1 approved these changes Nov 28, 2023

View reviewed changes

Ryun1 merged commit d576faf into cardano-foundation:master Nov 28, 2023

rphair mentioned this pull request Dec 5, 2023

CIP-0072 | Off-chain schema fixes #631

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CIP-0072 | Off-chain schema versioning and schema adjustments #612

CIP-0072 | Off-chain schema versioning and schema adjustments #612

marcin-mazurek commented Oct 23, 2023 •

edited

Loading

rphair left a comment

marcin-mazurek commented Oct 24, 2023

rphair commented Oct 24, 2023

marcin-mazurek commented Oct 26, 2023

rphair left a comment

danielmain Oct 31, 2023

rphair Nov 2, 2023

danielmain commented Nov 2, 2023

rphair left a comment •

edited

Loading

Crypto2099 left a comment

danielmain left a comment

ginnun Nov 13, 2023

danielmain Nov 14, 2023

Ryun1 Nov 13, 2023 •

edited

Loading

danielmain Nov 15, 2023

Ryun1 commented Nov 13, 2023

danielmain commented Nov 16, 2023 •

edited

Loading

danielmain commented Nov 16, 2023

danielmain commented Nov 16, 2023

danielmain commented Nov 16, 2023

danielmain commented Nov 16, 2023

szymonmaslowski Nov 16, 2023

danielmain Nov 17, 2023

ginnun Nov 17, 2023

danielmain Nov 17, 2023


		On-chain part has a signature, which has a role to verify that a certain dApp owner made changes. In the initial version, a blake2b-256 signature was needed only for `rootHash` but following discussion, due to security concerns, decision has been made that the signature should attest the whole on-chain canonical JSON except signature field itself (because it would end up in an infinite recursion).
		As any transaction in cardano network has a signature, which has a role to verify that a certain dApp owner made changes.

CIP-0072 | Off-chain schema versioning and schema adjustments #612

CIP-0072 | Off-chain schema versioning and schema adjustments #612

Conversation

marcin-mazurek commented Oct 23, 2023 • edited Loading

rphair left a comment

Choose a reason for hiding this comment

marcin-mazurek commented Oct 24, 2023

rphair commented Oct 24, 2023

marcin-mazurek commented Oct 26, 2023

rphair left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielmain commented Nov 2, 2023

rphair left a comment • edited Loading

Choose a reason for hiding this comment

Crypto2099 left a comment

Choose a reason for hiding this comment

danielmain left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ryun1 Nov 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ryun1 commented Nov 13, 2023

danielmain commented Nov 16, 2023 • edited Loading

danielmain commented Nov 16, 2023

danielmain commented Nov 16, 2023

danielmain commented Nov 16, 2023

danielmain commented Nov 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcin-mazurek commented Oct 23, 2023 •

edited

Loading

rphair left a comment •

edited

Loading

Ryun1 Nov 13, 2023 •

edited

Loading

danielmain commented Nov 16, 2023 •

edited

Loading