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

Error in resolving service in pydid.deserialize_document() #76

Closed
codespree opened this issue Oct 5, 2023 · 3 comments
Closed

Error in resolving service in pydid.deserialize_document() #76

codespree opened this issue Oct 5, 2023 · 3 comments

Comments

@codespree
Copy link

Given the following document:

{'@context': 'https://www.w3.org/ns/did/v1', 'id': 'did:key:z6MkghJ8qgeLgKTNDpyMuzAk28736HzUhkQffNMriAjyLExk', 'verificationMethod': [{'id': 'did:key:z6MkghJ8qgeLgKTNDpyMuzAk28736HzUhkQffNMriAjyLExk#z6MkghJ8qgeLgKTNDpyMuzAk28736HzUhkQffNMriAjyLExk', 'type': 'Ed25519VerificationKey2018', 'controller': 'did:key:z6MkghJ8qgeLgKTNDpyMuzAk28736HzUhkQffNMriAjyLExk', 'publicKeyBase58': '3F36FSPuLmxu7L8fERCuB2Z3GiidHsAJyMSvstmxR2BN'}], 'authentication': ['did:key:z6MkghJ8qgeLgKTNDpyMuzAk28736HzUhkQffNMriAjyLExk'], 'assertionMethod': ['did:key:z6MkghJ8qgeLgKTNDpyMuzAk28736HzUhkQffNMriAjyLExk#z6MkghJ8qgeLgKTNDpyMuzAk28736HzUhkQffNMriAjyLExk'], 'capabilityDelegation': ['did:key:z6MkghJ8qgeLgKTNDpyMuzAk28736HzUhkQffNMriAjyLExk#z6MkghJ8qgeLgKTNDpyMuzAk28736HzUhkQffNMriAjyLExk'], 'capabilityInvocation': ['did:key:z6MkghJ8qgeLgKTNDpyMuzAk28736HzUhkQffNMriAjyLExk#z6MkghJ8qgeLgKTNDpyMuzAk28736HzUhkQffNMriAjyLExk'], 'keyAgreement': [{'id': 'did:key:z6MkghJ8qgeLgKTNDpyMuzAk28736HzUhkQffNMriAjyLExk#z6LSpykVNke7uXzP9tSw79MgjHr2ffzXezQCSLKkD6rckPM7', 'type': 'X25519KeyAgreementKey2019', 'controller': 'did:key:z6MkghJ8qgeLgKTNDpyMuzAk28736HzUhkQffNMriAjyLExk', 'publicKeyBase58': 'EJaKrSqFp5Ge4W5AaVqjQhdYpXTQxPE3ZMc4ieD631aM'}]}

Current Behaviour:

>>> pydid.deserialize_document(a, strict=True)
DIDDocument(context=['https://w3id.org/did/v1'], id='did:sov:597jeiseQmfqPSykhfAPHm', also_known_as=None, controller=None, verification_method=[Ed25519VerificationKey2018(id='did:sov:597jeiseQmfqPSykhfAPHm#1', type='Ed25519VerificationKey2018', controller='did:sov:597jeiseQmfqPSykhfAPHm', public_key_hex=None, public_key_base58='3FsYZSwaaMwUK7TwaiAJSngVwttwxtLAoaWcmjJU2dFZ', public_key_pem=None, public_key_multibase=None, blockchain_account_id=None, ethereum_address=None, public_key_jwk=None)], authentication=['did:sov:597jeiseQmfqPSykhfAPHm#1'], assertion_method=None, key_agreement=None, capability_invocation=None, capability_delegation=None, service=[UnknownService(id='did:sov:597jeiseQmfqPSykhfAPHm#didcomm-0', type='did-communication', service_endpoint=AnyUrl('https://devmediator.sensecrypt.com', scheme='https', host='devmediator.sensecrypt.com', tld='com', host_type='domain'), recipientKeys=['did:sov:597jeiseQmfqPSykhfAPHm#1'], routingKeys=['did:key:z6MkghJ8qgeLgKTNDpyMuzAk28736HzUhkQffNMriAjyLExk'])])

The service is UnknownService.

Expected Behaviour, service should be:

[DIDCommV1Service(id='did:sov:597jeiseQmfqPSykhfAPHm#didcomm-0', type='did-communication', service_endpoint=AnyUrl('https://devmediator.sensecrypt.com', scheme='https', host='devmediator.sensecrypt.com', tld='com', host_type='domain'), recipient_keys=['did:sov:597jeiseQmfqPSykhfAPHm#1'], routing_keys=['did:key:z6MkghJ8qgeLgKTNDpyMuzAk28736HzUhkQffNMriAjyLExk#z6MkghJ8qgeLgKTNDpyMuzAk28736HzUhkQffNMriAjyLExk'], accept=None, priority=0)]

The above causes a problem in Aries Mediator Service, see:
openwallet-foundation/didcomm-mediator-service#79

The following document works:

{'@context': 'https://w3id.org/did/v1', 'id': 'did:sov:597jeiseQmfqPSykhfAPHm', 'authentication': ['did:sov:597jeiseQmfqPSykhfAPHm#1'], 'service': [{'id': 'did:sov:597jeiseQmfqPSykhfAPHm#didcomm-0', 'type': 'did-communication', 'recipientKeys': ['did:sov:597jeiseQmfqPSykhfAPHm#1'], 'routingKeys': ['did:key:z6MkghJ8qgeLgKTNDpyMuzAk28736HzUhkQffNMriAjyLExk#z6MkghJ8qgeLgKTNDpyMuzAk28736HzUhkQffNMriAjyLExk'], 'serviceEndpoint': 'https://devmediator.sensecrypt.com'}], 'verificationMethod': [{'id': 'did:sov:597jeiseQmfqPSykhfAPHm#1', 'type': 'Ed25519VerificationKey2018', 'controller': 'did:sov:597jeiseQmfqPSykhfAPHm', 'publicKeyBase58': '3FsYZSwaaMwUK7TwaiAJSngVwttwxtLAoaWcmjJU2dFZ'}]}

The difference is the part after the # in the routingKeys:

did:key:z6MkghJ8qgeLgKTNDpyMuzAk28736HzUhkQffNMriAjyLExk#z6MkghJ8qgeLgKTNDpyMuzAk28736HzUhkQffNMriAjyLExk

I manually patched pydid to add #z6MkghJ8qgeLgKTNDpyMuzAk28736HzUhkQffNMriAjyLExk to the routingKeys (before calling return super(DIDDocument, cls).deserialize(value) in pydid/pydid/doc/doc.py and it works.

The Indicio Public Mediator returns routingKeys with did:key:val#val format. However, aca-py returns keys as did:key:val

@dbluhm request a fix for this to ensure the mediation is not broken for aca-py version 0.10.3 (and possibly other versions).

@dbluhm
Copy link
Member

dbluhm commented Oct 5, 2023

I sympathize with the issue and will continue to work to address it from ACA-Py and the mediator service. However, this is the wrong place to fix this issue. PyDID is designed to adhere to spec as closely as possible, with the occasional minor concession. According to Aries RFC 67, routingKeys is:

an array of did key references, ordered from most destward to most srcward, used to denote the individual routing hops in between the sender and recipients.

I'll amend that for clarity by using the updated terms and correcting "did key references" to "verification method references."

A fix is necessary downstream rather than in this library. If, for the sake of backwards compatibility, we decide to accept did:key:val in ACA-Py, then it would be best for corrections to be applied to the document in ACA-Py, converting the value to did:key:val#val, before being deserialized by PyDID.

Given the above, I'm going to close this issue -- I will continue the conversation in the ACA-Py and mediator service repos. If you feel my response hasn't adequately addressed your concerns, feel free to reopen this issue to discuss further.

@dbluhm dbluhm closed this as completed Oct 5, 2023
@dbluhm dbluhm closed this as not planned Won't fix, can't repro, duplicate, stale Oct 5, 2023
@codespree
Copy link
Author

codespree commented Oct 5, 2023

@dbluhm I understand. How does Indicio Public mediator address the issue? It returns val#val in routingKeys (unlike the current version of aca-py). I understand that, perhaps, this issue doesn't need to be fixed here. Nonetheless, the following is the patch I implemented to get it to work:

Filename: pydid/doc/doc.py
pydid-patch-working

I am including the patch as an image as this might not be the right place to apply the patch.

However, If developers are stuck (isn't our goal to make things work for developers?), they can apply this patch, while the issue is resolved in the correct place.

I know this is a hacky fix, but it might unblock some developers while we await the proper fix.

What is the version of aca-py and configuration for Indicio's public mediator?

How does it work for that mediator?

@codespree
Copy link
Author

Protocol level differences between aca-py and Indicio Public Mediator can be found here:
https://github.com/hyperledger/aries-mediator-service/files/12801371/Protocol.Debug.pdf

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

No branches or pull requests

2 participants