Add `cryptosuiteString` subtype for the `cryptosuite` property. #162

msporny · 2023-08-07T01:16:21Z

This PR is an attempt to address issue #161 by adding a cryptosuiteString subtype for the cryptosuite property to ensure that it is possible to compress cryptosuite strings when performing semantic compression (CBOR-LD).

Preview | Diff

iherman · 2023-08-07T04:55:48Z

vocab/security/vocabulary.yml

@@ -235,7 +235,7 @@ property:
  - id: cryptosuite
    label: Cryptographic suite
    domain: sec:DataIntegrityProof
-    range: xsd:string
+    range: sec:cryptosuiteString


That is not correct in the vocabulary: the value of range must be a class or a new datatype. Do we really want to go down the (tedious!) process of formally define a new datatype for cryptosuiteString?

I would propose to leave the range a string, and add a comment (or part of the cryptosuite property definition in the spec) what the constraints on the value are. But do not treat it as a datatype with a separate URL.

Just to add to my previous remark. If the WG does decide to define a new datatype, what I would think we should do is as follows.

I do not think we should bother extending the yml2vocab script to add a datatype definition feature in general; this is a very rare occurrence, and this tool is not aimed at being a universal RDF tool. Also, the datatype definition is not part of the RDF Vocabulary Graph, in other words, it is not something we would add to the Turtle and JSON-LD files, only the HTML version. As a consequence, what I would propose to do is to add the formal definition to the integrity spec (see below), and add a separate section into the Template html file that refers back to the definition in the integrity spec, a bit like all the others. I am happy to do that, once the references in the integrity spec is available.

The formal definition of a datatype requires the formal definition of the lexical space, value space, etc. Even for a type that is relatively simple it is, as I said, tedious. I think the formal definition of the rdf:JSON Datatype is a good example for what should be added to the integrity spec (probably in a normative appendix, to avoid breaking the flow of the main text).

Do we really want to go down the (tedious!) process of formally define a new datatype for cryptosuiteString?

Yes, we do, because if we don't do that, the resolution of using long cryptosuite names falls apart due to the burden it places on low-bandwidth environments. In other words, the pain felt in defining a new type will be on the Editors. If we don't define this type, the pain felt will be on every implementer out there in a low bandwidth environment. I'll volunteer to define the data type, thanks for the pointer to rdf:JSON... we'll model it after that.

Yes, we do, because if we don't do that, the resolution of using long cryptosuite names falls apart due to the burden it places on low-bandwidth environments. In other words, the pain felt in defining a new type will be on the Editors. If we don't define this type, the pain felt will be on every implementer out there in a low bandwidth environment. I'll volunteer to define the data type, thanks for the pointer to rdf:JSON... we'll model it after that.

Fine. Then let us do it, but we should follow something like #162 (comment) to make it proper. It is tedious but not too much after all; the only important point is to define more formally the subset of strings that can be used as a datatype value. The rest is obvious (and the rdf:JSON stuff could indeed be used as a pattern).

I am o.k. to expand the formal vocabulary with the datatype stuff once the core spec has it, either in this PR or (probably better) in another one.

@iherman I have added the definition of cryptosuiteString, based on the link to provided for rdf:JSON, in this commit: 40be121

If you are ok with that section (or a variation of it), I can do the same for the multibase datatype in another PR.

Requesting a re-review from you now.

Requesting a re-review from @TallTed as well because I added a bunch of text that's hard to read and I need his help to make it suck less. :)

Re-reviewed. Not too horrible. Editorial changes requested.

index.html

Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>

iherman

Purely editorial: I have the strange feeling of a circular specification. The cryptosuiteString definition refers to the dataIntegrityProof section, which then refers to the cryptosuiteString section... And I did not see anywhere a clear specification (or reference to it) of the fact that cryptosuites have an identifier in terms of an ASCII string, and those are the ones we are talking about. At present, this datatype looks much more mysterious than what it is... But I may have missed some details.
For the records and to other reviewers: the cryptosuiteString datatype definition will be added to the vocabulary in a separate PR once this one settles. The reason for the delay is that the ability to add a datatype specification required an extension of the underlying yml2vocab tool and, although that extension is already done, it has not yet uploaded to the npm repository.

iherman · 2023-08-13T10:51:40Z

index.html

+            <dt>The lexical space</dt>
+            <dd>
+is the set of all cryptosuite types expressed using American Standard Code
+for Information Interchange [[ASCII]] strings that are defined by the sum of all


I wonder whether term "union" is not better than "sum" ("union" in the sense of a Union datatype in programming languages).

Yes, "union" is clearly better, applied in a commit below.

index.html

TallTed · 2023-08-14T20:01:17Z

vocab/security/vocabulary.yml

@@ -235,7 +235,7 @@ property:
  - id: cryptosuite
    label: Cryptographic suite
    domain: sec:DataIntegrityProof
-    range: xsd:string
+    range: sec:cryptosuiteString


Re-reviewed. Not too horrible. Editorial changes requested.

msporny · 2023-08-17T18:15:04Z

index.html

+            <dt>The lexical space</dt>
+            <dd>
+is the set of all cryptosuite types expressed using American Standard Code
+for Information Interchange [[ASCII]] strings that are defined by the sum of all


Suggested change

for Information Interchange [[ASCII]] strings that are defined by the sum of all

for Information Interchange [[ASCII]] strings that are defined by the union of all

Addressed @iherman's concern in e1ed114.

Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>

msporny · 2023-08-17T18:40:16Z

Normative, multiple reviews, changes requested and made, no objections, merging.

msporny requested review from Wind4Greg and seabass-labrax as code owners August 7, 2023 01:16

msporny requested review from davidlehn, dlongley, peacekeeper, pchampin, martyr280, iherman, dmitrizagidulin, TallTed, philarcher, brianorwhatever and brentzundel August 7, 2023 01:17

msporny mentioned this pull request Aug 7, 2023

Add sec:cryptosuiteString data type to vocabulary #161

Closed

iherman requested changes Aug 7, 2023

View reviewed changes

Wind4Greg approved these changes Aug 7, 2023

View reviewed changes

dlongley approved these changes Aug 7, 2023

View reviewed changes

TallTed suggested changes Aug 7, 2023

View reviewed changes

index.html Outdated Show resolved Hide resolved

dmitrizagidulin approved these changes Aug 8, 2023

View reviewed changes

msporny and others added 2 commits August 12, 2023 16:34

Add cryptosuiteString subtype for the cryptosuite property.

06b5093

Reword subtype sentence of cryptosuiteString definition.

bd6e1d6

Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>

msporny force-pushed the msporny-cryptosuiteString branch from f1b58c8 to bd6e1d6 Compare August 12, 2023 20:34

Add cryptosuiteString datatype section.

40be121

msporny requested review from iherman and TallTed August 12, 2023 21:23

iherman approved these changes Aug 13, 2023

View reviewed changes

TallTed suggested changes Aug 14, 2023

View reviewed changes

msporny commented Aug 17, 2023

View reviewed changes

msporny and others added 2 commits August 17, 2023 14:17

Fix grammar in cryptosuiteString definitions.

27aea77

Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>

Clarify cryptosuiteString definition further.

a71917d

Address set/union concern on cryptosuiteString by @iherman.

e1ed114

msporny merged commit 4981301 into main Aug 17, 2023
2 checks passed

msporny deleted the msporny-cryptosuiteString branch August 17, 2023 18:40

msporny mentioned this pull request Aug 22, 2023

Add multibase datatype definition #176

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `cryptosuiteString` subtype for the `cryptosuite` property. #162

Add `cryptosuiteString` subtype for the `cryptosuite` property. #162

msporny commented Aug 7, 2023 •

edited by pr-preview bot

Loading

iherman Aug 7, 2023

iherman Aug 7, 2023

msporny Aug 7, 2023

iherman Aug 7, 2023

msporny Aug 12, 2023 •

edited

Loading

msporny Aug 12, 2023

TallTed Aug 14, 2023

iherman left a comment

iherman Aug 13, 2023

msporny Aug 17, 2023

TallTed Aug 14, 2023

msporny Aug 17, 2023

msporny Aug 17, 2023

msporny commented Aug 17, 2023

	for Information Interchange [[ASCII]] strings that are defined by the sum of all
	for Information Interchange [[ASCII]] strings that are defined by the union of all

Add cryptosuiteString subtype for the cryptosuite property. #162

Add cryptosuiteString subtype for the cryptosuite property. #162

Conversation

msporny commented Aug 7, 2023 • edited by pr-preview bot Loading

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

msporny Aug 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iherman left a comment

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msporny commented Aug 17, 2023

Add `cryptosuiteString` subtype for the `cryptosuite` property. #162

Add `cryptosuiteString` subtype for the `cryptosuite` property. #162

msporny commented Aug 7, 2023 •

edited by pr-preview bot

Loading

msporny Aug 12, 2023 •

edited

Loading