openwallet-foundation / acapy

ACA-Py is a foundation for building decentralized identity applications and services running in non-mobile environments.
https://aca-py.org
Apache License 2.0
419 stars 512 forks source link

DID Resolver `service_accept` parameter is questionable #2463

Open dbluhm opened 1 year ago

dbluhm commented 1 year ago

https://github.com/hyperledger/aries-cloudagent-python/blob/39cac36d8937ce84a9307ce100aaefb8bc05ec04/aries_cloudagent/resolver/did_resolver.py#L66-L74

The service_accept parameter in DIDResolver.resolve is questionable to me. The value is used exclusively by the Indy resolver and appears to be used to influence the accept parameter in the DIDComm service generated by that resolver from ATTRIB transactions:

https://github.com/hyperledger/aries-cloudagent-python/blob/39cac36d8937ce84a9307ce100aaefb8bc05ec04/aries_cloudagent/resolver/default/indy.py#L119-L130

An abstract "Resolution Options" parameter is accounted for in the DID Resolution specification (https://w3c-ccg.github.io/did-resolution/; ctrl-f for resolution options). However, having a parameter on an abstract interface that is only used by a single concretion of that interface is an anti-pattern. The service_accept parameter should either be generalized to a resolution_options object or else it's utility should be reconsidered (I'm not sure what problem it's solving but it seems odd that we would have the ability to influence the structure and contents of the DID Document we're resolving as the "client" to the resolver interface).

burdettadam commented 1 year ago

Changing the resolve method to reflect the DID Resolution specification definitely would be an improvement. I am in favor of an options parameter.