Add support for revocable credentials in vc_di handler

[EmadAnwer]

This pull request supports issuing revocable verifiable credentials in the vc_di handler.

The main changes include:

• Adding revocation when creating a credential for proof. This includes creating a new revocation registry
• Fixing vc_di proof request issue by updating vc_di/handler.py issue_credential to include revocation missing data
• Adding integration tests to cover revocation of credentials.

This allows Aries to support the W3C Verifiable Credentials Data Model, including revocation of issued credentials. It enables revocable credential issuance and presentation via the vc_di protocols.

[ianco]

This line is incorrect: https://github.com/hyperledger/aries-cloudagent-python/pull/2967/files#diff-f7d9c8d89f2442a02ac7a5924be4046e195d411791e4cbfd0fd9e7e2ecb987adL557

if cred["proof"][0].get("rev_reg_id"):

You can fetch these attributes from the credential, see for example:

ianco@1903eb6

(Not the most elegant approach but you can use it as an example)

[ianco]

Hi @EmadAnwer I've doen a very quick review of the code and it looks ok, I need to review in a bit more detail and get one other dev to do a review.

In the meantime can you add a description to the pull request? It will help anyone who needs to review, and also in the future if someone needs to look back to see what was done it will give them some context. (Also release note re built based on the PR descriptions.)

Also there are a few code issues reported by one of the scanning tools, if you can take a look ...

I've built a couple of integration tests so I'll send those to you once I get them working :-S

[ianco]

FYI the SonarCloud errors (if you don't have access to the details) are:

Failed conditions
 [42.2% Coverage on New Code](https://sonarcloud.io/component_measures?id=hyperledger_aries-cloudagent-python&pullRequest=2967&metric=new_coverage&view=list) (required ≥ 80%)
 [24.7% Duplication on New Code](https://sonarcloud.io/component_measures?id=hyperledger_aries-cloudagent-python&pullRequest=2967&metric=new_duplicated_lines_density&view=list) (required ≤ 3%)

The coverage is just related to the unit tests. The duplication is on the new create_credential_w3c() method which duplicates a lot of the existing create_credential() code (mostly the blocks around the revocation registry handling, which could be re-factored into helper methods).

[EmadAnwer]

Thanks for reviewing, im editing it and i will push asap

[ianco]

@EmadAnwer the coverage test is still failing on the VCDICredFormatHandler class (in formats/vc_di/handler.py).

There are a couple of unit tests already that are marked as "skipped" (since they were added before revocation was implemented, I suspect just copied form the indy version of the unit test) - async def test_issue_credential_revocable(self): and async def test_issue_credential_non_revocable(self): - can you take a look at those and see what it would take to make these work with your updated revocation support?

Otherwise everything in the PR looks good to me.

[EmadAnwer]

@EmadAnwer the coverage test is still failing on the VCDICredFormatHandler class (in formats/vc_di/handler.py).

There are a couple of unit tests already that are marked as "skipped" (since they were added before revocation was implemented, I suspect just copied form the indy version of the unit test) - async def test_issue_credential_revocable(self): and async def test_issue_credential_non_revocable(self): - can you take a look at those and see what it would take to make these work with your updated revocation support?

Otherwise everything in the PR looks good to me.

@ianco Thanks for the review and suggestions, sure I will take a look into it and update the PR

[ianco]

@EmadAnwer it looks like the most recent commit (removing the dup code) is missing the DCO signoff

[sonarcloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
87.5% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[jamshale]

Can we add an @Release tag here? So the tests run on our nightly and pre-release workflows.

[jamshale]

I think we should probably have these as automated tests as well with the @Release tag.

[jamshale]

I would like to have this automatically integration tested with the @Release workflow. But, everything looks good 👍

Similar to work I'm doing right now, I want to make sure revocation registry rotation and other operations are working correctly in anoncreds. See https://github.com/hyperledger/aries-cloudagent-python/blob/main/aries_cloudagent/anoncreds/revocation.py#L875. This isn't in scope for this task. I will look at it separately.

[ianco]

@jamshale I've merged the PR, I'll look at updating the tags on the integration tests later ...