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

Add anoncred support #759

Merged
merged 1 commit into from
Jan 8, 2024

Conversation

jamshale
Copy link
Contributor

@jamshale jamshale commented Jan 8, 2024

This allows the askar-anoncreds wallet and endpoints to be tested in the test harness by running the command BACKCHANNEL_EXTRA_acapy_main="{\"wallet-type\":\"askar-anoncreds\"}" ./manage run -d acapy-main -t @Anoncreds

  • has an anoncreds context that is set based on the extras env variable.
  • It adds a schema and cred def in the anoncred format and passes a anoncred param to the backchannel where the endpoints are changed.
  • Adds tags to the tests that are expected to pass. I only added a few tests. v1 tests are still not passing. I figure if they should then another ticket could build off of this. Along with any other tests we feel are important to work with anoncreds.

Some imports and formatting was changed due to my ruff formatting extension. Not sure if anyone cares.

Note: This was based off of my acapy fork which includes the recent anoncred endpoint changes. They have been merged to acapy but for some reason acapy main wasn't getting updated with the latest code for me when I was developing.

@jamshale jamshale force-pushed the add-anoncred-support branch from 5e1451c to 0a56681 Compare January 8, 2024 20:29
Signed-off-by: jamshale <[email protected]>
@jamshale jamshale force-pushed the add-anoncred-support branch from 0a56681 to 2fedc3b Compare January 8, 2024 20:40
@jamshale jamshale marked this pull request as ready for review January 8, 2024 20:42
Copy link
Contributor

@swcurran swcurran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I had to do a rebuild to get the -t @anoncreds to be recognized (probably expected…), but once I did that, the proper tests were executed and succeeded.

@swcurran swcurran merged commit f17f392 into openwallet-foundation:main Jan 8, 2024
1 check passed
@nodlesh
Copy link
Contributor

nodlesh commented Jan 18, 2024

Note that there are other ways to set the wallet-type on an agent besides the EXTRA args. One is to use an ACA-Py config file that contains startup parameters. The other is in a feature file, as seen in the draft PR #760 where we add start_parameters to the Given we have n agents step. I'll probably add support for these other options in that PR.

@ianco
Copy link
Contributor

ianco commented Jan 18, 2024

Note that there are other ways to set the wallet-type on an agent besides the EXTRA args. One is to use an adapt config file that contains startup parameters. The other is in a feature file, as seen in the draft PR #760 where we add start_parameters to the Given we have n agents step. I'll probably add support for these other options in that PR.

I think we generally want to set the wallet type globally. I.e. run me all the tests with aca-py askar wallet, or askar-anoncreds wallet. I don't see the value in setting this in a feature file where we set this on a per-test basis.

@ianco
Copy link
Contributor

ianco commented Jan 18, 2024

Note that the V1 credential and presentation endpoints are not supported for anoncreds (and won't be) so we should add a tag to these (@DEPRECATED or something like that), so we can run ... -t ~@DEPRECATED ...)

@nodlesh
Copy link
Contributor

nodlesh commented Jan 19, 2024

I think we generally want to set the wallet type globally. I.e. run me all the tests with aca-py askar wallet, or askar-anoncreds wallet. I don't see the value in setting this in a feature file where we set this on a per-test basis.

Yes, I agree, generally. It depends on if we want these tests to run in existing runsets or have them separate. All I've done here is open up the start parameters to be fully added to the features file data. It only worked with Transport Protocols previously, and instead of just adding the did peer flags, I opened it all up.

We are adding the @AnonCreds tag to tests that need handling for that. I think that should cover the case for the @Deprecated, maybe?

Right now, this PR, checks for wallet type in the EXTRA Args environment variable to set the handling for AnonCreds, I think it also should check if the tag @AnonCred is set on the tests. Whether or not the wallet type setting comes from the feature file (rarely, if ever), or the config file, the tag will handle all cases.

@ianco
Copy link
Contributor

ianco commented Jan 19, 2024

askar-anoncreds will eventually replace askar and we will be deprecating indy so we don't need to put too much effort into supporting multiple wallet types.

Agree regarding the @AnonCreds tag, however we may deprecate api's for other reasons ...

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

Successfully merging this pull request may close these issues.

4 participants