chore(ref-imp): add controller back into verification methods #1009
Conversation
isaacJChen
requested review from
csuwildcat,
OR13,
tplooker and
troyronda
as code owners
| @@ -18,6 +18,7 @@ | |||
| "verificationMethod": [ | |||
| { | |||
| "id": "#publicKeyModel1Id", | |||
| "controller": "", | |||
There was a problem hiding this comment.
Controller should not be an empty string. Prefer to omit property rather than have an empty string.
There was a problem hiding this comment.
It has to be there according to did-core spec
There was a problem hiding this comment.
Section 3.1 ABNF doesn't seem to allow for an empty string. What rule are we applying to allow for this case?
There was a problem hiding this comment.
We have a base in @context, which fills it in with JSON-LD
There was a problem hiding this comment.
I see.
Note: personally, I think this style makes the resolution result less clear to the human reader (and also to simpler processors.)
There was a problem hiding this comment.
Ya, I agree. I removed it in my last PR based on WG discussion results, but @OR13 caught that it's not spec compliant. We can further discuss it if we want the plain DID in there since I believe that's what OR13 prefers as well. Having it there as empty string at least is spec compliant and was the previous state so I decided to revert it. Feel free to bring this up in the next WG meeting though :)
There was a problem hiding this comment.
@troyronda, this is a follow up PR for discussion that occurred in #1006, thus @sandrask is aware of this change and I believe we have consensus, thus I am approving this PR to unblock.
There was a problem hiding this comment.
@thehenrytsai I have opened a followup #1010 as I do not think using an empty string is the best outcome.
No description provided.