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

WIP Implement anoncreds credential format #901

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

ianco
Copy link
Contributor

@ianco ianco commented Jan 28, 2025

This PR add support for using the anoncreds credential format, and adds a test suite for aca-py running --wallet-type askar-anoncreds.

See issue #872

I have updated the tags so that all anoncreds-specific tests are tagged with @Anoncreds, and I've added a filter to the existing test runs (using --wallet-type askar) to exclude these tests.

I've updated the rest of the tests so that anything using an indy credential exchange is tagged @Indy (or @CredFormat_Indy and these are excluded from the anoncreds test run.

This PR includes a reference to the updated connections plug-in in @jamshale 's branch, so this PR should not be merged until after this connections plug-in is merged (and the relevant files in this PR updated) so that we're not chasing down spurious test errors.

The anoncreds test run looks as follows:

LEDGER_URL_CONFIG=http://test.bcovrin.vonx.io TAILS_SERVER_URL_CONFIG=https://tails.vonx.io BACKCHANNEL_EXTRA_acapy_main="{\"wallet-type\":\"askar-anoncreds\"}" ./manage run -d acapy-main -t @AcceptanceTest -t ~@wip -t ~@T004-RFC0211 -t ~@DidMethod_orb -t ~@Transport_NoHttpOutbound -t ~@Indy -t ~@CredFormat_Indy

...

Failing scenarios:
  features/0183-revocation.feature:61  Credential revoked by Issuer and Holder attempts to prove with a prover that doesn't care if it was revoked -- @1.1 

8 features passed, 1 failed, 6 skipped
48 scenarios passed, 1 failed, 118 skipped
372 steps passed, 1 failed, 1073 skipped, 0 undefined
Took 10m35.382s

There are still some outstanding issues (can be included in this PR before merging or fixed later):

  1. Connections plug-in (as mentioned above). This plug-in should be completed before this PR is merged (and the reference should be updated to the proper repo).
  2. Difference between indy and anoncreds schema and cred def structures. There is some logic in the test harness dealing with the 2 different data formats - this should be cleaned up and a common format used in the test harness (and then only the backchannel needs to deal with the differences). I ran out of time to fix this ...
  3. The above failing test. May be an aca-py issue, I'll have to leave for @jamshale
  4. Review the scope of tests that an askar-anoncreds agent should support. Right now all Indy tests are being excluded.
  5. Review scope of other test scenarios (the anoncreds tests are only excluded from the aca-py/aca-py test suites).

@ianco
Copy link
Contributor Author

ianco commented Jan 28, 2025

Question for @swcurran @jamshale - how should we handle tests tagged with @Anoncreds?

Some tests (that create schemas, issue credentials, etc) interpret the @Anoncreds tag to use the anoncreds credential format, and will only work (with aca-py) with an askar-anoncreds wallet.

Some tests (like those in 0793-peer-did.feature) are tagged with @Anoncreds but don't actually have any anoncreds-specific processing (these tests are just establishing connections using various types of peer DIDs). So these will run regardless of the wallet type.

I'm updating the anoncreds-dependant tests to check that the agent is running with the correct wallet type, and failing with a 400 error agent wallet cannot support anoncreds format if not. (This is done in the backchannel when an anoncreds-specific operation is requested, so some tests tagged with @Anoncreds will still run even without an anoncreds wallet.)

But the question is - should we setup a separate set of test suites that use --wallet-type askar-anoncreds (vs the existing suites that run --wallet-type askar) and then we can filter on the @Anoncreds tag as appropriate for each test run?

Or alternately we can force-restart each agent to run --wallet-type askar-anoncreds for tests tagged with @Anoncreds? (Personally I don't like this option.)

Right now the @Anoncreds tests are passing in the nightly workflows because the @Anoncreds tag test isn't being done properly, but this is something I've fixed in this PR :-)

@jamshale
Copy link
Contributor

I think I intended for this to be a separate suite to make things easy. But, the tests should in theory be an acapy issuer to credo holder, and either being a verifier in the suite. We don't know if these actually pass at this point, though.

@ianco
Copy link
Contributor Author

ianco commented Jan 30, 2025

Note that some tests are failing when running under an askar-anoncreds wallet due to this issue: openwallet-foundation/acapy#3479

ianco added 7 commits January 30, 2025 09:18
Signed-off-by: Ian Costanzo <[email protected]>
Signed-off-by: Ian Costanzo <[email protected]>
Signed-off-by: Ian Costanzo <[email protected]>
Signed-off-by: Ian Costanzo <[email protected]>
@ianco ianco requested a review from jamshale January 31, 2025 20:33
Signed-off-by: Ian Costanzo <[email protected]>
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.

2 participants