Add credential_revoked state

[DaevMithran]

Pull request for Issue: 1539
@swcurran Added the credential_revoked state.
For now I have considered only issue-credential 1.0, I should check for the record in V20CredentialExchange too.
Will do it soon. If you have any suggestions on my approach please comment below.

Questions:

• Is it required raise an error if the credential record doesn't exist?
• The credential state should be changed only after publishing revocation.
  When publish is set to pending, what is the best way to implement this?. I'm planning to create retrieve credential records using rev_reg_id and cred_rev_id

[ianco]

Typically the credential exchange records will get deleted after the credential is issued, so in most use cases there will be no credential exchange record when the credential is revoked.

I'm not sure if it makes sense to add a status or tag on the credential itself with the status (issued vs revoked)?

[swcurran]

Typically the credential exchange records will get deleted after the credential is issued, so in most use cases there will be no credential exchange record when the credential is revoked.

I'm not sure if it makes sense to add a status or tag on the credential itself with the status (issued vs revoked)?

While "typically" they will get deleted, not always, and the intention of this PR is to updated the non-deleted protocol state objects if they are not deleted.

[ianco]

• Is it required raise an error if the credential record doesn't exist?

It shouldn't raise an error. The credential exchange record can be deleted, and could even be deleted in between the revocation and publishing events.

• The credential state should be changed only after publishing revocation.
  When publish is set to pending, what is the best way to implement this?. I'm planning to create retrieve credential records using rev_reg_id and cred_rev_id

Not completely sure of the intent, but you may want to change the status to revocation_pending if publishing the revocation status will be delayed, otherwise it looks like the credential is still in issued status. Then, when you publish revocations, you can just flip the status from revocation_pending to revoked.

Also note that there are some tests failing on this PR (formatting and unit tests).

[DaevMithran]

Hi @swcurran @ianco
For the current approach

• I added a revoke_credentials method in Credential Manager which retrieves credential records using revoc_reg_id and revocation_id's and changes their state to credential_revoked.

• It updates the state only after publishing revocation and supports the publish_revocation api too.

• This approach supports credentials records issued after this merge. As the previous credential records did not have tags to be retrieved using revoc_reg_id and revocation_id

Questions

• I have added support only for V10Credential records, Should I also add support for V20Credential records

• There is not a way to find if a credential belongs to V10Credential record or V20Credential record. So in worst case I have to search in both.

[codecov-commenter]

Codecov Report

Merging #1545 (f94184d) into main (d7abd7c) will decrease coverage by 0.03%.
The diff coverage is 56.00%.

@@            Coverage Diff             @@
##             main    #1545      +/-   ##
==========================================
- Coverage   95.76%   95.73%   -0.04%     
==========================================
  Files         527      527              
  Lines       32312    32336      +24     
==========================================
+ Hits        30943    30956      +13     
- Misses       1369     1380      +11     

[ianco]

One (fairly minor) comment, plus this functionality should also be provided for the V20 cred exchange.

[shaangill025]

• This approach supports credentials records issued after this merge. As the previous credential records did not have tags to be retrieved using revoc_reg_id and revocation_id

I don't see any changes to TAG_NAMES here and here. I am just confused by the above comment regarding cred_ex_record tags. Just to confirm in regards to cred_ex_record backward compatibility, there should be no breaking change, and only that cred_ex_record state will be updated as credential_revoked upon revocation after the merge?

[DaevMithran]

• This approach supports credentials records issued after this merge. As the previous credential records did not have tags to be retrieved using revoc_reg_id and revocation_id

I don't see any changes to TAG_NAMES here and here. I am just confused by the above comment regarding cred_ex_record tags. Just to confirm in regards to cred_ex_record backward compatibility, there should be no breaking change, and only that cred_ex_record state will be updated as credential_revoked upon revocation after the merge?

Hi @shaangill025 initially I tried the tags approach but V20Cred_ex_record doesn't include values of rev_reg_id and cred_rev_id so went with a different approach.

• First I retrieve IssuerCredRevRecord using rev_reg_id and cred_rev_id
• extract the cred_ex_id from the record
• retrieve credential record using cred_ex_id
• update state of credential record