search

hyperledger / aries-cloudagent-python

Hyperledger Aries Cloud Agent Python (ACA-Py) is a foundation for building decentralized identity applications and services running in non-mobile environments.
https://wiki.hyperledger.org/display/aries
Apache License 2.0
405 stars 510 forks source link

Feature multikey management #3246

Closed PatStLouis closed 3 days ago

PatStLouis commented 1 week ago

Supersedes #3168.

During the last aca-py call, it was discuss that it would be preferable to create keys instead of dids for associating a verification method.

This PR adds 3 routes to the wallet and adds an optional kid property to the KeyInfo class.

Deleting a key should be looked at in a separate PR as this feature isn't currently supported by aca-py and could have other ramifications to be taken into account.

@dbluhm @jamshale

PatStLouis commented 1 week ago

@jamshale I'm having issues with the linting tests. I have ruff version 0.6.5 and even if I apply formatting it's still causing errors without much details. Do you have a clue what I'm doing wrong here?

jamshale commented 1 week ago

I think it's working correctly. Sometimes the formatter won't be able to automatically fix things.

image

The line too long is annoying sometimes. I'm fine with just adding # noqa: E501 to these lines. The others just need doc string and we have a preference to have is None instead of == None.

Usually I just run poetry run ruff check . when I get a fail. There should be a vscode run option for the devcontianer as well.

PatStLouis commented 1 week ago

now it's asking me to use version ruff==0.5.7 but the pyproject.toml lists 0.6.5

creating virtual environment... installing ruff from spec 'ruff==0.5.7'... Would reformat: aries_cloudagent/wallet/did_info.py 1 file would be reformatted, 1360 files already formatted

PatStLouis commented 1 week ago

ok I think I sorted it out this time!

PatStLouis commented 1 week ago

@jamshale @dbluhm I'm happy with the state of this PR, every new function has a test

PatStLouis commented 5 days ago

Upon further testing I noticed that for it to be compatible with the askar wallet there was some slight changes required. I've implemented those.

One issue is Askar is unable for find the key given a kid. Has the get_key_by_kid function been tested with the askar wallet? If so, could someone point me towards where it's used in the code base? The only place I can find it is in the test_in_memory_wallet.py file.

PatStLouis commented 4 days ago

ok I got it to work as I intended

PatStLouis commented 3 days ago

@jamshale can you add @dbluhm as a reviewer here?

jamshale commented 3 days ago

Done. One thing I was wondering was if we can or should do a scenario integration test for this? They are a lot quicker and easier to write than before.

PatStLouis commented 3 days ago

@dbluhm I've implemented all your recommendations, @jamshale I will have a quick look

PatStLouis commented 3 days ago

@jamshale @dbluhm if we are happy with the state of this work I would like to have it merged sooner than later to start re basing #3181

sonarcloud[bot] commented 3 days ago

Quality Gate Failed Quality Gate failed

Failed conditions
67.7% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud