Different types of endpoints (services) for public DIDs

[domwoe]

ACA-PY currently only uses one type of endpoint for local, as well as public DIDs. If we use the DIF indy resolver driver to resolve an Indy DID, then it generates a DID doc with one entry in the service array of type 'endpoint'. (See: https://uniresolver.io/1.0/identifiers/did:sov:HR6vs6GEZ8rHaVgjg2WodM)

We'd like to define multiple services/endpoints for a public DID (published on the ledger). The use case is to provide public profile/master data information about the entity represented by the DID. *

I started working on integrating this functionality in ACA-PY (setting and getting an endpoint of a different type) and I'd like to know if this would be a feature that might get accepted upstream.

I'd be also interested in getting some feedback concerning the implementation:

Getting an endpoint seems straight forward to add to the /ledger/did-endpoint route:

However setting an endpoint is currently done with the /wallet/set-did-endpoint route. I've implemented setting an endpoint of a different type here, however, I only update the endpoint in the wallet if its of type "endpoint".

The alternative approaches would be

• Add a /ledger/set-did-endpoint that allows to write endpoints of type != endpoint
• Carry the endpoints of type "other than endpoint" also in the wallet.

WiP can be found here: https://github.com/BoschDigitalSolutions/aries-cloudagent-python/tree/feat-endpoint-type

Looking forward to your thoughts.

• The public profile is represented as a JSON-LD Verifiable Presentation signed with the /jsonld/sign endpoint. The aim is to have a base interoperability for public profile information using only the W3C DID & VC spec without reliance on a particular ledger or DIDcomm.

[sklump]

I'd prefer to keep the endpoint update API call in exactly one place. I'm lightly refactoring the wallet API to accommodate your rocket chat issue (provisioning fails to update public did metadata in wallet for command-line endpoint), which might nudge things a bit. Work in progress.

update: it's in https://github.com/hyperledger/aries-cloudagent-python/pull/610

[sklump]

What does the design imagine for endpoint types? User-defined, the sky's the limit, or possible values 'endpoint' and 'profile' (plus possibly others, finitely many). If finite, I'd like to see an enum? Otherwise, this looks good to me.

Quibble: wallet/routes.py lines 281-284 can shorten to:

endpoint_type = body.get("endpoint_type", "endpoint")

[domwoe]

I expect there will be a service type registry (maybe hosted at W3C) at some point. So I think it will be a reasonably small set of service/endpoint types. Hence, I'd agree that an enum would be the better choice and we don't pollute the ledger with mistyped endpoints.

[domwoe]

Hey @sklump

I had sometime to work on this today. I've introduced an Enum. I've put it here for a lack of a better central place. From my point of view it belongs to the ledger but it is not indy specific.

I'm struggling with an issue however. Tests are always failing because I get TypeError: too many positional arguments when calling the following function from tests without the last argument

async def set_did_endpoint(
        self, did: str, endpoint: str, ledger: BaseLedger,
            endpoint_type: EndpointType = EndpointType.ENDPOINT):

E.g in this test

Running the agent and using the function with less arguments, such that the default value is used, works.

[sklump]

Module ledger.util is a fine place for the enum. I am leery about EndpointType.ALL - see below. Lastly, forgive the pedantry but why are "endpoint" and "all" all lower-case but "Profile" and "LinkedDomains" camel-case? I recommend snake-case here like record states (i.e., "endpoint", "profile", "linked_domains", "all"). (Record states should have been enums too, my opinion only, but that's another discussion).

The default value for any parameter such as endpoint_type should always be None, especially if it gets modified. E.g., def f(x=[]): x.append('hello'); print(x) -- f(); f() here prints ['hello'] then ['hello', 'hello'].

The missing formal parameter is in wallet.base.BaseWallet, which wallet.indy.IndyWallet inherits.

The triple-quote docs on wallet.indy.IndyWallet.set_did_endpoint() for endpoint_type (also should repeat for wallet.base.BaseWallet.set_did_endpoint() should explicitly state that the operation sets/clears the endpoint in the wallet only for EndpointType.ENDPOINT (the default value), but in all cases passes the update through to the ledger.

Finally, ledger.indy.IndyLedger.get_endpoint_for_did() will return and endpoint of any type, not necessarily the specified type, unless endpoint_type is EndpointType.ALL. I think a filter here as a sequence of endpoint types of interest (with None for no filter; i.e., all) would do the trick and then you could eliminate EndpointType.ALL - a plural among singulars is a bug factory.

[domwoe]

Thanks @sklump. That was helpful!

I understand your feedback on lower-case/camel-case. The reason is, that in JSON-LD land types are typically defined as camel-case (see https://identity.foundation/.well-known/resources/did-configuration/#example-7), and the indy/sov driver of the universal resolver maps this endpoint_type to the type of the service. Generally, it would like to change the endpoint type 'endpoint' to something like Agent, AriesAgent or DIDcomm, but that's rather wishful thinking I guess ;)

I agree on removing EndpointType.ALL. In general I like the idea of the filter. However, this would be a breaking change of the API. I tried to keep it backwards compatible. Another backwards compatible approach would be to add another internal method ledger.indy.IndyLedger.get_all_endpoints_for_did() that is only used by ledger.indy.IndyLedger.update_endpoint_for_did.

Implementation

[sklump]

Adding another method here would be best I think, to retain backward compatibility

[domwoe]

I've created a pull request PR #634

[domwoe]

Resolved with #667 and #669