search

w3c / vc-di-ecdsa

Data Integrity specification for ECDSA using NIST-compliant curves
https://w3c.github.io/vc-di-ecdsa/
Other
9 stars 9 forks source link

Ensure `created` proof option is optional #47

Closed dlongley closed 19 hours ago

dlongley commented 5 months ago

The current proof configuration algorithm seems to make created required instead of optional, this should be fixed.

Wind4Greg commented 5 months ago

Hi @dlongley (and @msporny) I'm attempting to write up a PR for this but have a question on how we should document things. In the VC-Data-Integrity Spec the various proof attributes are carefully defined and properly marked as OPTIONAL, REQUIRED, or other appropriate indicators. It seems that in the DI-ECDSA specification (and others) we should refer back to VC-Data-Integrity and not repeat ourselves and possibly insert errors.

Would something like the following work for the DataIntegrityProof section of VC-DI-ECDSA:

The attributes of the DataIntegrityProof proof field are defined in VC-Data-Integrity Spec for ECDSA the following attributes take on the following restricted values:

Or should we just repeat the language from Data Integrity for the created attribute. Note that we will want to do whatever change looks best to VC-DI-EDDSA and VC-DI-BBS since they also use this language.

msporny commented 5 months ago

Or should we just repeat the language from Data Integrity for the created attribute. Note that we will want to do whatever change looks best to VC-DI-EDDSA and VC-DI-BBS since they also use this language.

Your instinct is correct, don't repeat language that exists in other specifications... instead, point to the other specification for the definitions and normative language. We can add constraining language in the spec if necessary, but in general, follow "don't repeat yourself" principles for exactly the reasons you outline.

dlongley commented 4 months ago

So PR #57 accidentally reverted the work done in #50 to ensure created was optional (here). It should be optional in all suites.

cc: @msporny

msporny commented 4 months ago

So PR #57 accidentally reverted the work done in #50 to ensure created was optional (here). It should be optional in all suites.

Thanks for catching that, the update should be an editorial fix since the right thing was committed in #50 and then accidentally undone in #57.

Wind4Greg commented 19 hours ago

This was implemented in PR https://github.com/w3c/vc-di-ecdsa/pull/50 and has been correctly applied after the accidental reversion. See the first sentence here https://w3c.github.io/vc-di-ecdsa/#dataintegrityproof.