Closed msporny closed 2 years ago
iherman
commented
2 years ago A minor comment, though not strictly to §4.2: do we have to say somewhere what "raise XYZ ERROR" means (e.g., in 2.1), or do we intend to leave it at that and leave it to the implementations?
TallTed
commented
2 years ago I would reword your original comment slightly, from algorithms only presume a single proof to algorithms presume only a single proof.
iherman
commented
2 years ago The issue was discussed in a meeting on 2022-10-19
msporny
commented
2 years ago A minor comment, though not strictly to §4.2: do we have to say somewhere what "raise XYZ ERROR" means (e.g., in 2.1), or do we intend to leave it at that and leave it to the implementations?
Ultimately, it's up to the implementations to expose those errors. Exactly how each one does it is implementation specific at the moment. We might want to do something like what the JSON-LD API did, though some have complained about the use of text strings instead of error codes.
We do plan to expose those error types in protocols such as the VC API via a code field, but that's out of scope for this spec (probably). In short, we're still working things out. I'm trying to be consistent with the error codes so that when it comes time to expose them further up the software stack, people aren't caught off guard.
There is an inconsistency in the value references. $4.2/(2) uses "dotted" term references like proof.type, proof.verificationMethod, etc. However, §4.1/(5) uses references to the same terms without the "dotted" formalism. (I did not check all items, there may be such discrepancies elsewhere, too.) Even §4.2 is not fully consistent; shouldn't (5) and (6) say proof.cryptosuite rather than "the cryptosuite specified in proof"?
Yes, thanks for spotting that, it is an oversight and I'll fix that in an upcoming commit. I was going back and forth on the easiest way to mark up these variables.
Technically, it is not clear to me where the parameters like options come from. Or is this the ISSUE right after the algorithm (that only refers to verificationMethod)?
options is specified as an input to the algorithm in the opening paragraph. I'm still trying to figure out if the ReSpec var parameter approach is more helpful than harmful... I think it's helping, but the github preview doesn't show the markup, so it's difficult to see it without checking out the source and rendering the document locally. The var parameter highlights everywhere the variable is used, so in theory, you should be able to click on the word options and it will highlight everywhere the variable is used in the algorithm. In short, if you had a properly rendered document, this might have been easier to find for you (or I screwed up the markup and I'll fix it :P ).
msporny
commented
2 years ago @iherman wrote:
There is an inconsistency in the value references. $4.2/(2) uses "dotted" term references like proof.type, proof.verificationMethod, etc. However, §4.1/(5) uses references to the same terms without the "dotted" formalism. (I did not check all items, there may be such discrepancies elsewhere, too.) Even §4.2 is not fully consistent; shouldn't (5) and (6) say proof.cryptosuite rather than "the cryptosuite specified in proof"?
Yes, thanks for spotting that, it is an oversight and I'll fix that in an upcoming commit. I was going back and forth on the easiest way to mark up these variables.
Fixed in 520a357a4454f9a2342ab7220ae43831a3a57535.
msporny
commented
2 years ago Normative, multiple reviews, changes requested and made, no objections.
This PR updates the algorithm used to verify proofs to align with the language used to generate proofs (now merged into
main).At present, both the proof generation and proof verification algorithms presume only a single proof. The algorithms will need to be updated to support proof sets and proof chains in a future PR.
Preview | Diff