msporny
commented
1 year ago Nice work! Some initial comments from a close reading:
Thanks for the thorough review @Wind4Greg! :)
Some preliminary responses below:
- Section 1.1 How it Works: This seems to only show basic proof generation and verification process. It seems we could use a diagram like figure 2 of DID-core to show relationship with controller, controller document, and the proof gen/verify processes.
Yes, certainly a complete diagram showing all processes would be useful. We do have the ability to embed mermaid diagrams into the spec, if that helps: https://w3c-ccg.github.io/vc-api/#exchange-examples
- Section 1.3 Terminology: "controller" and "controller document" are a bit circular. This parallels the definitions of DID controller and DID document. Can we call this something more specific. Scanning the current document it seems that the terms "controller" and "controller document" are only associated with "verification methods and material". Could we call it something like a "verification material controller"? Somebody else may have a better name. Couldn't a DID be used for this purpose? Or isn't this just a DID? i.e., verificationMethod stuff is the same as in DID-core specification.
The verificationMethod stuff is the same as in DID Core because we needed a place to put the language and this spec didn't exist at the time. Over time, that language should move to this specification and the DID Core specification should refer to this specification for the section on verification methods/material/relationships.
A DID Document is a type of "controller document". An HTTP-served such as https://controller.example/foo.json could also be a type of "controller document". I'm not sure if "verification material controller" is entirely accurate. It is expected that the "controller" controls the private key material, public key material, and the "controller document". A "controller" is a role (an entity that controls the cryptographic material) and that entity is typically described by the "controller document". We could call it a "verification material document", since it does list cryptographic material that can be used for the purposes of cryptographic verification... but that language doesn't exactly roll off of the tongue. We've tried to endeavor to use simpler (but accurate) words for terminology if we can get away with it.
- Section 2.1 Proofs: "attributes defined for use a data integrity proof"... challenge SHOULD be included in proof if a domain is specified... Is this always true? Or dependent on the proof type?
It is dependent on the proof type. We wanted to provide properties that could be used in any challenge-response protocol so that each proof type didn't have to keep re-inventing their own. So, we /want/ this to always be true... and haven't had any push back on it yet (implementers seem to be happy with the notion that challenge/domain are there in any proof type for them to use if they need them).
- Section 2.1.1 Proof Sets: "A proof set, which has no order, is represented by associating a set of proofs with the proof key in a document." Icky! Can we use the term "attribute" or "field". I know Python dictionaries like to talk about keys and JavaScript objects are "keyed collections" but in a cryptographic context can we use something else? Or at the beginning of section 2 Data Model should we write like in the DID-core "A DID document consists of a map of entries, where each entry consists of a key/value pair."
Yes, agreed, let's not use the word "key". DID Core is probably not a good model to follow because we ended up having to re-invent our own data model there by using INFRA (a years-long debate that we shouldn't re-open here). JSON specifies "name/value" pairs called "members". CBOR specifies "key/value" pairs. INFRA uses "key/value". The VC data model specifies "property-value" (at this layer).
How about "property name/property value" and "property"? That feels workable.
- 2.3 Controller Documents: Needs a simple, clear example/explanation up front of what this is... It seems so close to a DID document. Note that the DID-core document is an informative reference but not mentioned with respect to controller and controller documents.
A controller document is the super-class of a DID Document. The reference is informative because we shouldn't have the normative reference go from DI -> DIDCore... it should be the other way around (eventually)... DIDCore should normatively reference Data Integrity (in time) as that's the intended layering.
- 2.3.1 Verification Methods: Cut and pastes error: "id The value of the id property for a verification method MUST be a string that conforms to the conforms to the [URL] syntax." Should be: "id The value of the id property for a verification method MUST be a string that conforms to the [URL] syntax."
Yes, agreed.
- 2.3.2 Verification Relationships; These seem like a repeat of the items in section 2.2 on "Proof Purposes". Something is odd here...
Hmm, I'm not seeing an issue in my copy of the spec or the published version?
- Section 4.1 could use an example and maybe a bit of explanation of the psuedo-code approach since it seems somewhat JavaScript oriented, i.e., dot notation for objects...
When people want to look at code, we typically point them towards the implementations. That said, the spec isn't supposed to favor one implementation over another (believe it or not, that has led to very long, time consuming debates in the past). So, we try to do our best to be precise with algorithmic language while leaving implementation details up to implementers. This has worked fairly well in the past (and is how most W3C specifications are written).
If someone wrote some pseudo-code in JavaScript, what I would expect is the non-JS people would complain about the spec "skewing towards Web-based development languages" (yes, that's happened before). Dot-notation seems to be as much as we can get away with w/o raising the spectre of implementation and/or language favoritism.
That said, totally willing to try something if it'll lead to a better spec... just be ready for people to complain about the example being "too low level" or "not using the ideal language". :)
- Issue redundancy with DID-Core: Verification Methods, Verification Relationships.
Yes, there will be redundancy there until we publish the Data Integrity spec as a W3C standard and then update the DID Core spec to point to that standard. That cycle is expected to take 2-4 more years. In the meantime, there will be duplicated content in each spec (DIDWG isn't allowed to make normative changes anymore, they're in maintenance mode... and VCWG hasn't published a DI standard yet).
Thanks for the review, it has highlighted a number of things that need to change in the spec. A future PR should address those concerns. Please feel free to raise a PR to fix any of the above.
Wind4Greg
Nice work! Some initial comments from a close reading:
Cheers Greg B.