Update JCS to fix proof configuration. #43
Conversation
Sounds good. Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
|
(Admin: @Wind4Greg, @msporny @dmitrizagidulin : these proposed changes do not seem to raise IPR concerns, ie, I could clear the PR issues raised by the automatic control. Please confirm before I do that.) |
|
iherman marked as non substantive for IPR from ash-nazg. |
msporny
requested review from
dlongley,
peacekeeper,
iherman,
TallTed,
philarcher,
brentzundel and
Sakurann
There was a problem hiding this comment.
For readability's sake I would prefer to spell out the equivalents of 3.1.3 and 3.1.5 instead of essentially defining jcs-eddsa-2022 as a series of diffs. Actually, one may go one step further, and factor out the sections that are common to both, and then specify the transformation and the proof configuration sections explicitly for eddsa-2022 and jcs-eddsa-2022, respectively.
But these are editorial choices, and may reflect personal taste. Ie, I will not lie down the road on this.
| </p> | ||
|
|
||
| <p style="padding-left: 2em;"> | ||
| <strong>4)</strong> If <var>options</var>.<var>type</var> is not set to | ||
| `DataIntegrityProof` and <var>proofConfig</var>.<var>cryptosuite</var> is not | ||
| set to `json-eddsa-2020`, an `INVALID_PROOF_CONFIGURATION` error MUST be raised. | ||
| set to `jcs-eddsa-2022`, an `INVALID_PROOF_CONFIGURATION` error MUST be raised. |
There was a problem hiding this comment.
It's OK as is, but I would also prefer that each of these step-by-step sections are complete, as suggested above by @iherman. Priority of Constituencies, and all that.
|
@iherman and @TallTed -- I've raised #46 to track your (valid) concerns. I'm tracking it in an issue because putting the right solution in place is going to take time. It will require writing a re-spec extension that can take a base algorithm and parameterize it. The end goal will be that one doesn't have to jump back and forth between vc-data-integrity and vc-di-eddsa, or within sections in vc-di-eddsa. We do want to also be clear with implementers that the text is almost exactly the same between two sub-algorithms, except for a single value or two. /cc @Wind4Greg In any case, tracking in issue 46 so we can merge this into the spec. |
|
Normative, multiple reviews, changes requested and made, no objections, merging. |
This PR fixes the issues raised in #41. In particular:
@contextinto proofConfig for canonicalization.Preview | Diff