aries-cloudagent-python icon indicating copy to clipboard operation
aries-cloudagent-python copied to clipboard

Use did:key for recipient keys

Open TheTechmage opened this issue 2 years ago • 2 comments

As part of #1859, I have made it so the coordinate-mediation protocol uses did:key representation. Apologies that this took as long as it did. Implementing this impacted more of ACA-Py than I realized it would while keeping it compatible with existing agents.

TheTechmage avatar Aug 04 '22 18:08 TheTechmage

@dbluhm It looks like I could have gotten away without modifying the MediationRecord. I would just have had to replace every instance of record.routing_keys = grant.routing_keys (for example) with the conversion code.

TheTechmage avatar Aug 05 '22 00:08 TheTechmage

@dbluhm It looks like I could have gotten away without modifying the MediationRecord. I would just have had to replace every instance of record.routing_keys = grant.routing_keys (for example) with the conversion code.

In the interest of the principle of least change, would this help us avoid needing to modify connections, didexchange, pack, etc.? Might be a bit of a pain to make that change now but if it results in a cleaner set of changes, that might be worth it.

dbluhm avatar Aug 05 '22 01:08 dbluhm

Codecov Report

Merging #1886 (465a09b) into main (a8101e1) will increase coverage by 0.00%. The diff coverage is 97.29%.

@@           Coverage Diff           @@
##             main    #1886   +/-   ##
=======================================
  Coverage   93.68%   93.68%           
=======================================
  Files         539      540    +1     
  Lines       34162    34187   +25     
=======================================
+ Hits        32003    32027   +24     
- Misses       2159     2160    +1     

codecov-commenter avatar Aug 16 '22 16:08 codecov-commenter

@dbluhm I believe that addresses your feedback

TheTechmage avatar Aug 16 '22 17:08 TheTechmage

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
3.1% 3.1% Duplication

sonarqubecloud[bot] avatar Aug 22 '22 14:08 sonarqubecloud[bot]

@frostyfrog -- can you please check the failed integration test. It's a "can't find a DID" error which may be related to the PR (or not... :-) ). From the test: "7hAcme.agent | 2022-08-23 22:21:35,515 aries_cloudagent.connections.base_manager WARNING No corresponding DID found for recipient verkey: CcGfdN6DgibjWEU3kShAkHC6f1eyMAghfi8hknrZEZFF"

swcurran avatar Aug 23 '22 22:08 swcurran

Interesting that it's failing now. I'll take a look!

TheTechmage avatar Aug 24 '22 16:08 TheTechmage

When I pulled down the latest changes (the two merges), I was unable to reproduce the error locally. Opening up the logs from the failed test reveals that my changes may not be a likely cause here. There was an exception in the revocation code where ACA-Py received something that did not match the revocation schema.

Logs below:

7hBob.agent  | 2022-08-23 22:21:54,679 aries_cloudagent.admin.server ERROR Handler error with exception: Unprocessable Entity
7hBob.agent  | Traceback (most recent call last):
7hBob.agent  |   File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/webargs/asyncparser.py", line 90, in parse
7hBob.agent  |     result = schema.load(parsed)
7hBob.agent  |   File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/marshmallow/schema.py", line 723, in load
7hBob.agent  |     data, many=many, partial=partial, unknown=unknown, postprocess=True
7hBob.agent  |   File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/marshmallow/schema.py", line 904, in _do_load
7hBob.agent  |     raise exc
7hBob.agent  | marshmallow.exceptions.ValidationError: {'rev_reg_id': ['Value None is not an indy revocation registry identifier']}
7hBob.agent  | 
7hBob.agent  | During handling of the above exception, another exception occurred:
7hBob.agent  | 
7hBob.agent  | Traceback (most recent call last):
7hBob.agent  |   File "/home/indy/aries_cloudagent/admin/server.py", line 171, in ready_middleware
7hBob.agent  |     return await handler(request)
7hBob.agent  |   File "/home/indy/aries_cloudagent/admin/server.py", line 208, in debug_middleware
7hBob.agent  |     return await handler(request)
7hBob.agent  |   File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/aiohttp_apispec/middlewares.py", line 34, in validation_middleware
      Traceback (most recent call last):
        File "/home/indy/demo/runners/support/agent.py", line 848, in admin_request
          resp.raise_for_status()
        File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/aiohttp/client_reqrep.py", line 1009, in raise_for_status
          headers=self.headers,
      aiohttp.client_exceptions.ClientResponseError: 422, message='Unprocessable Entity', url=URL('http://10.1.0.158:8031/revocation/registry/None')
      
      The above exception was the direct cause of the following exception:
      
      Traceback (most recent call last):
        File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/behave/model.py", line 1329, in run
          match.run(runner.context)
        File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/site-packages/behave/matchers.py", line 98, in run
          self.func(context, *args, **kwargs)
        File "features/steps/0586-sign-transaction.py", line 375, in step_impl
          f"/revocation/registry/{context.rev_reg_id}",
        File "/home/indy/demo/bdd_support/agent_backchannel_client.py", line 200, in agent_container_GET
          params=params,
        File "/home/indy/demo/bdd_support/agent_backchannel_client.py", line 19, in run_coroutine
          return loop.run_until_complete(coroutine(*args, **kwargs))
        File "/home/indy/.pyenv/versions/3.6.13/lib/python3.6/asyncio/base_events.py", line 488, in run_until_complete
          return future.result()
        File "/home/indy/demo/runners/agent_container.py", line 1015, in admin_GET
          return await self.agent.admin_GET(path, text=text, params=params)
        File "/home/indy/demo/runners/support/agent.py", line 895, in admin_GET
          "GET", path, None, text, params, headers=headers
        File "/home/indy/demo/runners/support/agent.py", line 851, in admin_request
          raise Exception(f"Error: {resp_text}") from e
      Exception: Error: {"rev_reg_id": ["Value None is not an indy revocation registry identifier"]}

TheTechmage avatar Aug 24 '22 17:08 TheTechmage

If you pull these changes and attempt to use a mediator that has not been updated, you'll see problem reports bouncing back from the mediator on keylist update messages:

2022-08-25 09:16:50,493 aries_cloudagent.messaging.base_handler INFO Received problem report from: MGY45PK93sKVJjHRLJxA1p, <ProblemReport(_message_id='7ef74d0d-3801-4573-a6fe-79932a016442', _message_new_id=False, _message_decorators=<DecoratorSet{~thread: <ThreadDecorator(_thid='af21ac95-4492-4e06-b096-c39103909bcd', _pthid=None, _sender_order=None, _received_orders=None)>}>, description={'en': 'Error deserializing message: KeylistUpdate schema validation failed', 'code': 'message-parse-failure'}, problem_items=None, who_retries=None, fix_hint=None, impact=None, where=None, noticed_time=None, tracking_uri=None, escalation_uri=None)>

Mediators can account for clients that continue to use the original raw key format but the inverse is unfortunately more complicated to account for.

dbluhm avatar Aug 25 '22 13:08 dbluhm

I assume we should update the aries-mediator-service to use the latest? How is it determined what version of ACA-Py to use? Do we need to do an "-rc1" for that upgrade? @dbluhm -- do you have someone that can take that on? I assume that Indicio will be updating their public mediator?

Thanks!

swcurran avatar Aug 25 '22 17:08 swcurran

Sure, I'll add that to the to-do list :slightly_smiling_face:

cc @reflectivedevelopment

dbluhm avatar Aug 25 '22 17:08 dbluhm