Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds Alg 4 #61

Merged
merged 7 commits into from
Nov 1, 2023
Merged

Adds Alg 4 #61

merged 7 commits into from
Nov 1, 2023

Conversation

TelegramSam
Copy link
Collaborator

Adds @dbluhm's Alg 4 into the spec.

Signed-off-by: Sam Curren <[email protected]>
Copy link
Member

@dbluhm dbluhm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of the info from the FAQs section from the hackmd outlining did:peer:4 that I put together might be good to include here in some form but I don't feel strongly about that.

spec/core.md Outdated Show resolved Hide resolved
spec/core.md Outdated Show resolved Hide resolved
spec/core.md Outdated Show resolved Hide resolved
spec/core.md Outdated Show resolved Hide resolved
spec/core.md Outdated Show resolved Hide resolved
spec/core.md Outdated Show resolved Hide resolved
spec/core.md Outdated Show resolved Hide resolved
spec/core.md Show resolved Hide resolved
spec/core.md Outdated Show resolved Hide resolved
spec/core.md Outdated Show resolved Hide resolved
spec/core.md Outdated Show resolved Hide resolved
@swcurran
Copy link
Collaborator

Very minor editorial notes. Looks great!

@Patrik-Stas
Copy link

Hi, we will start implementation in aries-vcx of this perhaps through out the next week or beginning of October.

It would be great if the PR can stay open for a while, so we can provide feedback from our experience implementing this.

@dbluhm
Copy link
Member

dbluhm commented Sep 25, 2023

decentralized-identity/did-peer-4#1 (comment)

My only point against this did:peer:4 is why complicate everything else with two identities. Other than that I love the idea.

IMO is complicated enough controlling one identity and here we are proposing to identities that come always in pairs:

did:peer:4{{hash}}:{{encoded document}}
did:peer:4{{hash}}

Was it considered to use DID query?
Note the the query part of a DID is not part of the identity! On the contrary of DID path.
(The DID fragment is also not part of the identity but can't be used on the DID Comm message field id.)

did:peer:4{{hash}}?doc={{encoded document}} <=> did:peer:4{{hash}}
Those two are equivalent in terms of identity. According to the DID core specs they are one identity.

@dbluhm
Copy link
Member

dbluhm commented Sep 25, 2023

@FabioPinheiro The two forms, long and short, do not represent two separate identities. The whole idea is that they represent exactly the same set of keys and endpoints. The goal of the short vs long form is to enable more compact kids in DIDComm v2. That being said, it is technically perfectly acceptable to always use the long form of the DID.

A DID query was briefly considered; however, there is precedent in the form of did:ion for having a short form and long form style DID. Then, the documents resolved from short and long form are declared to be "aliases" for each other through the use of the alsoKnownAs field. This bidirectional AKA is exactly what is referred to in the note for that field found here: https://www.w3.org/TR/did-core/#dfn-alsoknownas.

Copying note for emphasis:

Applications might choose to consider two identifiers related by alsoKnownAs to be equivalent if the alsoKnownAs relationship is reciprocated in the reverse direction. It is best practice not to consider them equivalent in the absence of this inverse relationship. In other words, the presence of an alsoKnownAs assertion does not prove that this assertion is true. Therefore, it is strongly advised that a requesting party obtain independent verification of an alsoKnownAs assertion.

Given that the DID subject might use different identifiers for different purposes, an expectation of strong equivalence between the two identifiers, or merging the information of the two corresponding DID documents, is not necessarily appropriate, even with a reciprocal relationship.

In my opinion, this is the closest match for our goals and the cleanest way to express the relationship between the short and long form in a way that is supported by the DID Core spec.

Copy link
Collaborator

@swcurran swcurran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — let’s merge it.

Copy link
Contributor

@FabioPinheiro FabioPinheiro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

serviceEndpoint need to be fix to follow the specs

This will change also all the dids in here short and long form

spec/core.md Outdated Show resolved Hide resolved
spec/core.md Outdated Show resolved Hide resolved
spec/core.md Outdated Show resolved Hide resolved
spec/core.md Outdated Show resolved Hide resolved
@dbluhm
Copy link
Member

dbluhm commented Sep 27, 2023

Agreed with @FabioPinheiro, we should correct these examples to make sure we're promoting correctly constructed didcomm v2 services and not causing any confusion.

@swcurran
Copy link
Collaborator

Definitely want the examples right!!! Thanks @FabioPinheiro !

TelegramSam and others added 2 commits September 29, 2023 08:34
service endpoint corrections

Co-authored-by: Fabio Pinheiro <[email protected]>
@dbluhm
Copy link
Member

dbluhm commented Sep 29, 2023

TelegramSam#1

fix: make sure examples follow didcomm v2 spec
@TelegramSam
Copy link
Collaborator Author

TelegramSam#1

I did not know you could do that. 👍

@TelegramSam
Copy link
Collaborator Author

@FabioPinheiro examples corrected.

@dbluhm
Copy link
Member

dbluhm commented Oct 2, 2023

I built a tool for quickly parsing/resolving and generating did:peer:4 DIDs:

https://dbluhm.github.io/did-peer-4-ts/

And another link with a DID queued up.

@TelegramSam
Copy link
Collaborator Author

Now merging, given the approvals, implementations, and time.

@TelegramSam TelegramSam merged commit da263d4 into decentralized-identity:master Nov 1, 2023
@TelegramSam TelegramSam deleted the alg4 branch November 1, 2023 02:56
@TelegramSam TelegramSam mentioned this pull request Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants