Allow "diddocContent" in ATTRIB on older networks.

hyperledger / indy-did-method

Indy DID Method Specification

https://hyperledger.github.io/indy-did-method/

Creative Commons Attribution 4.0 International

18 stars 14 forks source link

Allow "diddocContent" in ATTRIB on older networks. #77

Closed peacekeeper closed 1 year ago

peacekeeper commented 1 year ago

Fixes #71.

I know this adds some complexity, but I believe there are advantages in allowing "diddocContent" in an ATTRIB for those older networks that don't support "diddocContent" in a NYM yet.

The main advantage is that full DID document support (e.g. custom verification methods) can already be achieved on all existing HL Indy networks, some of which may take months or years to upgrade to a newer version.
If there is a "diddocContent" in a NYM, then nothing changes, i.e. only a single transaction is required.

dbluhm commented 1 year ago

I think it would be beneficial for some clarity on how implementations should behave in the presence of both an attrib diddocContent and diddocContent in a nym. Additionally, I think it would be beneficial to describe when implementations should check for an attrib diddocContent if they choose to support it; i.e. "resolve the nym and if no diddocContent is found, check for an attrib for diddocContent." Simply stating that implementations "MAY" modify the algorithm to retrieve from an attrib instead of from the nym feels like we're opening the door for compatibility issues down the line, causing more friction for deployment of did:indy.

Otherwise, I think this is likely a prudent way to handle slower than ideal rollout of did:indy.

Thoughts @swcurran?

swcurran commented 1 year ago

I’m happy to add this and promote that this should be added to all resolver implementations.

It would be useful to have an example in of an ATTRIB based DID Doc Content entry — e.g. the JSON for a given DIDDoc and the ATTRIB raw value. What do you think?

Agree with @dbluhm that there should be a definition of what happens if both a DID with diddocContent and an ATTRIB with the same are found. My $0.02CDN is to say that if diddocContent is found in the DID, no check for a corresponding ATTRIB should be done. I believe that is the same wording we have for the endpoint today.

Thanks @peacekeeper !

peacekeeper commented 1 year ago

My $0.02CDN is to say that if diddocContent is found in the DID, no check for a corresponding ATTRIB should be done.

Yes this was my idea as well, to be consistent with what we are saying about endpoint.

I agree with @dbluhm that some more details should be added to this PR, i'll work on that.

peacekeeper commented 1 year ago

@dbluhm @swcurran I updated this PR to move this new content into its own section. I tried to explain better how clients should behave, based on your feedback, which was how I intended it to be anyway. I also added an example.

Maybe you could re-review? Let me know if anything is still unclear or could be improved.

swcurran commented 1 year ago

Looks good — although see my comment above. The submission is missing DCO — please fix that. DCO - Developer Certificate of Origin - https://github.com/apps/dco. See “Details” link above, beside failed test.

peacekeeper commented 1 year ago

@swcurran I agree with you regarding the example, I just added commit https://github.com/hyperledger/indy-did-method/pull/77/commits/27c27ee8ec7d39796b3278afc538e9a93aaf6fb2 to improve that.

I also added the DCO to the commits.

Sorry it took a while to do this.

peacekeeper commented 1 year ago

Could we merge this, or are there any concerns?

swcurran commented 1 year ago

Sorry for the delay — just lost sight of the issue.