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

Small AIP-2 updates #1056

Closed

Conversation

andrewwhitehead
Copy link
Contributor

@andrewwhitehead andrewwhitehead commented Mar 26, 2021

  • Remove --emit-new-didcomm-prefix and --emit-new-didcomm-mime-type in favour of --aip-version (1 or 2). I believe the .NET framework will currently only accept the old MIME type.
  • Change the DID document service type to did-communication when using the did-exchange protocol.
  • Remove DID shortening from DIDDoc and related classes.
  • Do not accept DID documents without an id property.
  • Qualify connection DIDs with did:peer: (does NOT yet qualify other DIDs with did:sov:).
  • Updates the INDY_DID pattern to accept did:sov: or did:peer:, which isn't technically correct. We should be using a generic DID pattern in most places.
  • Should permissively accept DID documents in the current v1.0 format, or in the older format (which I'm calling v0). When using the did-exchange protocol, the new format (with verificationMethod) is used for serialization regardless of the AIP version setting. When using the connections protocol, the old format (with publicKey) is used.

These changes might be better addressed though the generic resolver PR depending on the timeline, but this is for testing compatibility with other agents.

@codecov-io
Copy link

codecov-io commented Mar 27, 2021

Codecov Report

Merging #1056 (e9403a9) into main (46c2265) will decrease coverage by 0.03%.
The diff coverage is 93.75%.

@@            Coverage Diff             @@
##             main    #1056      +/-   ##
==========================================
- Coverage   99.37%   99.33%   -0.04%     
==========================================
  Files         374      374              
  Lines       21603    21591      -12     
==========================================
- Hits        21467    21447      -20     
- Misses        136      144       +8     

pubkey
) in pubkeys: # include all public keys, authentication pubkeys by reference
pubkey_type = PublicKeyType.get(pubkey["type"])
key = PublicKey( # initialization canonicalizes id
Copy link
Contributor

Choose a reason for hiding this comment

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

could lose the comment now: it is an orphan

return ref

return "did:sov:{}{}{}".format(did, delimiter if delimiter else "#", ref) # e.g., 3
return "{}{}{}".format(did, delimiter if delimiter else "#", ref)
Copy link
Contributor

Choose a reason for hiding this comment

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

return f"{did}{delimiter or '#'}{ref}" is slightly briefer but it is not so important

sklump
sklump previously approved these changes Mar 29, 2021
@dbluhm
Copy link
Contributor

dbluhm commented Mar 29, 2021

Indeed, there is some overlap here with #1033. We're aiming for a more "complete" solution that makes it easy to work with DIDs of various methods and DID Documents in various formats; there will be some code conflicts to resolve but I think the Document version distinction introduced in this PR maps well onto our goals. Especially if this PR is already ready to go, I think merging this PR now (or when ready) and incorporating/superseding the changes with #1033 is probably the way to go.

@@ -194,7 +194,7 @@ class IndyDID(Regexp):
"""Validate value against indy DID."""

EXAMPLE = "WgWxqztrNooG92RXvxSTWv"
PATTERN = rf"^(did:sov:)?[{B58}]{{21,22}}$"
PATTERN = rf"^(did:sov:|did:peer:)?[{B58}]{{21,22}}$"
Copy link
Contributor

Choose a reason for hiding this comment

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

https://identity.foundation/peer-did-method-spec/index.html#matching-regex

Not sure whether indy-sdk would take these though

Copy link
Contributor

@sklump sklump Apr 1, 2021

Choose a reason for hiding this comment

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

but note that it's bad regex; the trailing $) characters appear twice. Better:
^did:peer:[01](z)([1-9a-km-zA-HJ-NP-Z]{46,47})$

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.

4 participants