feat: support revocation notification messages

[TheTreek]

Revocation notification support:

• Added revocation notification model for credential records (optional property)
• Added revocation notification message handling
• Added revocation notification received event
• Added support for upcoming revocation notification v2 messages

Revocation notification process:

• Listens for revocation notification message to be received
• Fetches matching credential record
• Adds a new property to record called revocationNotification
• Updates record and triggers revocation notification received event that has the credential record as the payload

revocationRegistryId and credentialRevocationId need to be saved to the credential record. We are storing them as properties of CredentialRecord, I know that storing them in metadata may also be an option. If we wanted to store them as metadata then some guidance will be needed.

[codecov-commenter]

Codecov Report

Merging #579 (b29efea) into main (c9bff93) will increase coverage by 0.14%.
The diff coverage is 93.10%.

@@            Coverage Diff             @@
##             main     #579      +/-   ##
==========================================
+ Coverage   86.66%   86.81%   +0.14%     
==========================================
  Files         355      359       +4     
  Lines        7695     7809     +114     
  Branches     1194     1206      +12     
==========================================
+ Hits         6669     6779     +110     
- Misses       1025     1029       +4     
  Partials        1        1              

Impacted Files	Coverage Δ
.../credentials/repository/credentialMetadataTypes.ts	100.00% <ø> (ø)
.../modules/credentials/services/CredentialService.ts	95.07% <20.00%> (-1.45%)	⬇️
...dentials/handlers/RevocationNotificationHandler.ts	69.23% <69.23%> (ø)
...s/core/src/modules/credentials/CredentialEvents.ts	100.00% <100.00%> (ø)
.../core/src/modules/credentials/CredentialsModule.ts	81.46% <100.00%> (+0.42%)	⬆️
...ges/core/src/modules/credentials/handlers/index.ts	100.00% <100.00%> (ø)
...dentials/messages/RevocationNotificationMessage.ts	100.00% <100.00%> (ø)
...ges/core/src/modules/credentials/messages/index.ts	100.00% <100.00%> (ø)
...dules/credentials/models/RevocationNotification.ts	100.00% <100.00%> (ø)
...kages/core/src/modules/credentials/models/index.ts	100.00% <100.00%> (ø)
... and 4 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c9bff93...b29efea. Read the comment docs.

[TimoGlastra]

This is really great @TheTreek :)

Left a few suggestions

[TimoGlastra]

Should we throw a problem report error here to notify the other party?

[JamesKEbert]

If we don't recognize the format, I'd say yes. If we can't find the credential, then no, I think that's an internal AFJ issue & potentially shouldn't be exposed to the other agent.

[TimoGlastra]

Maybe we can make this object a bit more generic and not specifically about the revocation notification. In the future we could then just store revocation data in the object. E.g we fetch the revocation data ourselves from the ledger and store the result in the credentialrecord.

For the framework user this gives an easy place to find the revocation state whether it was fetched ourselves or informed by the issuer.

{
    "date": "the date",
    "method": "notification" // or "fetched" or something (maybe "ledger", but that wouldn't work for revocation methods that are not anchored on a ledger),
    "comment": "comment only present for notification method"
}

What do you think?

[JamesKEbert]

The issue as I saw it was that the revocation data from the ledger is dependent on the revocation interval--so it doesn't give us a "this was revoked on 2-22-22", but more that is this revoked given a time range--so a question/method vs the revocation notification. Additional the revocation notification is technically optional.
But... even with all that I am okay with making it more generic (especially if we want to support/consider non-indy revocation in the future?).
Happy to elaborate/discuss more too!

[TimoGlastra]

Hmm looking at practical use cases what is often used for non-revocation intervals? Isn't the most often used question now:now? I think according to RFC 0441 we can only ask for the from being the same as to (which gives us an exact timestamp) or we get a single to which means basically the same as an exact timestamp right?

So we would know for sure that at a specific time the credential was revoked/not-revoked. I agree it may not be 100% applicable to every use case, but I think it could be really useful to show the revocation status of a credential next to it, and e.g. maybe showing a refresh button which will fetch the latest revocation state.

[JamesKEbert]

You're correct in that what we're supporting right now based off of RFC 0441 is only supplying the same from and to, such as: now:now, 30 seconds ago/30 second ago, etc.

Right now the ability to query the ledger for a rev reg delta (to get revocation status) is exposed on the LedgerModule, which would enable what you're describing, although it's maybe not as obvious to use for this case.

[TimoGlastra]

Any updates on this @JamesKEbert? What are the blockers on this PR? Would be great to get this merged!

[JamesKEbert]

Any updates on this @JamesKEbert? What are the blockers on this PR? Would be great to get this merged!

This has been primarily waiting on me. The three remaining items/blockers here:

• The implemented version of the v1 implementation is not compliant with the RFC, but instead is compatible with ACA-Py. So my initial proposal would be to merge and note the difference from the RFC in the Divergence from RFCs section as well as in the RFC's implementations section.
• I have a minor addition/tweak to make to the revocation-notification-v2 RFC to add a requirement to ensure the holder is verifying the validity of the issuer.
• We may want to update this to use the metadata vs as a property on the credential.

I expect to be able to have this PR ready to merge within the next week.

[JamesKEbert]

Hey @TimoGlastra, everything's been addressed I think, besides the need to re-fetch the indy credential in order to get the credential revocation id, which we're looking at with our Python team. However, I think I'd be in favor of completing this and having that as a followup if it's a possibility.

[TimoGlastra]

Sounds good @JamesKEbert, thanks! I'm fine with doing it in a follow up. However wouldn't that cause any issues with the tags and the credential not queryable?

Also CodeQL is complaining about the regexes. Maybe we can make them a bit more specific by looking at the ACA-Py regexes: https://github.com/hyperledger/aries-cloudagent-python/blob/main/aries_cloudagent/messaging/valid.py#L403-L436?

[JamesKEbert]

However wouldn't that cause any issues with the tags and the credential not queryable?

Since we're still re-fetching the credential it should be fine querying it as I understand it, so a followup would improve the performance as we wouldn't need to re-fetch it, but it wouldn't change any of the API or interactions.

So far we've been stumped on finding a way to retrieve the cred_rev_id (and maybe we're just missing something obvious).

This is the log of a revocable credential offer that's been decoded, no sign of the cred_rev_id:

{
  "schema_id": "schema_id_here",
  "cred_def_id": "cred_def_id_here",
  "rev_reg_id": "rev_reg_id_here",
  "values": {
    "First Name": {
      "raw": "Patrick",
      "encoded": "19326420889721584150907756434744763515517200385607639812518004696795377894526"
    },
    "Age": {
      "raw": "27",
      "encoded": "27"
    },
    "Last Name": {
      "raw": "Kenyon",
      "encoded": "43826990635406025838022124681260409630854046085697036781736711625850312985001"
    }
  },
  "signature": {
    "p_credential": {
      "m_2": "7305319045799514268272120858355870310534978014147101007311454518968531958054",
      "a": "14844912702470868179521586926134181750894090773047541174695949535671215686561152817635530521869704307303241620428782997615160309203742555018212456152978218558221758570928590138933698774008719170411873022537960519441144504002906569198367737977937615785569935017718931523095844510994104217005719826748598404596206400802335265404233756620288226952475686634699683297431110507274700718547215878693135829962083762313871425803001646999209119309645813904842944499702484567760454564666147804073083555832116498460658343742999144644654083790859145459343410031643523956190170889716781177750508466903312732579813158445784265423377",
      "e": "259344723055062059907025491480697571938277889515152306249728583105665800713306759149981690559193987143012367913206299323899696942213235956742929938668687346298700943854241024183489",
      "v": "8387086101037151305185778556119457911798187212118149201882895432087746857116977574121346243552540856933316283870904265400051433189801970084732357652630551191280289814530004250646369779671677926876342065698678055020818715361339219851297869774270326070869379218860741080941219885760076650703657670342493007005020998285677196559419839635079282556340866370602592434668545433676191202501393312216725437181002777409778133006819702618382461451144768867892011851422649237232055425746841938384818894961237137930145139250225039028636834192319877970553202838304002448925108448011618294118164015842183517754309081270035248554288640329255836932244590610470857416778611921805021796007250433353379898135981810969089320238797076286558928530397515694944345969236224197190013996211670324736368085071986791310897512813454594932035015427059"
    },
    "r_credential": {
      "sigma": "1 01E93D930955B546C5E77DFA68A665293043A223AC1205E2D088A16FFFF68FD9 1 10F98B6EECA6AF330D31BC13EF9FBED03E3E043F8256D2B2D3B7F58270DE1EE8 2 095E45DDF417D05FB10933FFC63D474548B7FFFF7888802F07FFFFFF7D07A8A8",
      "c": "1E62DE3FC03A397FA5B0E5BA5C273E7DB9A9C1BBFCE46EE6D36925D2DA7BF920",
      "vr_prime_prime": "1C76C4D2174338F22E0383465D7ECFC242E99A2CB062AA66D3100AC9698C097C",
      "witness_signature": {
        "sigma_i": "1 07EA72F4064FF3F686886E0FAF4C6D2147714EFD13859E1980A96918CB3908F4 1 075CF56BC08547027C6E944360E404CC3F0E93126D80FAD75BBC097FFBA38F9C 1 1318F030FF17125E34C775B545F652917EBF94A8C52D547172AEF76B8DF1236F 1 06842DB53117BCA5D12E46C02947797D3B24CF18524A52B031A41D5F242AA155 2 095E45DDF417D05FB10933FFC63D474548B7FFFF7888802F07FFFFFF7D07A8A8 1 0000000000000000000000000000000000000000000000000000000000000000",
        "u_i": "1 1E072314806249C297F14C40D3E9A36BCF801845C2755EE0CEBA3E219F70BE79 1 215C68A9133392E4EE95D06B87E0527E8E91551EC25885C1CB44D3B045C2B0C2 1 0C3FEB9FA5812E3E827D78B1B7F585497362815E8F8738376206415701B57D68 1 0215EC379C9EC876ACF53704148A11EC2B3421EB9986A835D982E1C26E9077A2 2 095E45DDF417D05FB10933FFC63D474548B7FFFF7888802F07FFFFFF7D07A8A8 1 0000000000000000000000000000000000000000000000000000000000000000",
        "g_i": "1 22DA34344B797B365BC26C0AF63C967E6CA0E51689E5E2054B833BCD5DD22589 1 04ABB9AF319E46E47C3681F420179C2CE479B89CDCB1C71081A061E66F158631 2 095E45DDF417D05FB10933FFC63D474548B7FFFF7888802F07FFFFFF7D07A8A8"
      },
      "g_i": "1 22DA34344B797B365BC26C0AF63C967E6CA0E51689E5E2054B833BCD5DD22589 1 04ABB9AF319E46E47C3681F420179C2CE479B89CDCB1C71081A061E66F158631 2 095E45DDF417D05FB10933FFC63D474548B7FFFF7888802F07FFFFFF7D07A8A8",
      "i": 44,
      "m2": "1026A9FF5FC7537D0E218AD77501EB0ED3444ACB3F5D10FED1C3095EACE23926"
    }
  },
  "signature_correctness_proof": {
    "se": "6219587800254961292201550431492811823034436241314927998073286276305115636931901662751085349997880880391015795378287411770607013309694403951516553406518853087492054403788074106436923532055092885506047389459184816879685780357222857537219138520912880436522023752833052129901442047554691631307209798739098145843728774312475213640673865199775931882687457308171390891830840408194234886003046777737328890389667057844079428079471442022600389306320449269743101851712438940109680189127232597318314278397254826107750535280251908796135243962700353785986867870470261979597558713018700948536821357259132259131929237281745757292731",
    "c": "114316312050390703566577975573103219557354614093345274619034464414017402213123"
  },
  "rev_reg": {
    "accum": "21 125E18D45CB6A65AC183C3AEAD5193B253911ADF7D1A698DE405D22C305AE60C9 21 1414980198802934B7C14DE988957BA70BA9A40BA37B4E9B4FDCBC3BB5A4EDC88 6 64887278BD596A17F17430CED4CC972E07DBB40AB7D39CE2D33174E0F5F94A77 4 1B6F6588BBFD98D71E14ECEC21FBFD1E11C67299DA118B8C66CFE05FF2644298 6 7A67D9C53E2F7E5C3CD3B4543A52DF7E1DD799EB22A57B85DFA082BA57E13E18 4 21F7BDAEFB60DC00DBE77614B1DC0B65A78C82E273BC4FBBDC5A9795EAB20A8D"
  },
  "witness": {
    "omega": "21 1310F0EAFEFA7B9515E5F221090947B49A8C508779E6B2AFF09E7CB1EE5F4B064 21 135C3B62866A9226AF02884D7E3F64ED25C346B9C09D51377859464798E64C0F8 6 797DA355304B65AF9650F5B1DE3965B18880F720973A4137D5176EC2090BB4DE 4 3BA261017FC9AED454CC80501A3CD7FF4582B5F5F15F151828A5D9BE9ED387B6 6 8FB2082EA9A74E8CD4BAF67F977737E1D7B3531FB328EB3E15A860EBF48F3307 4 2BCB2E3BCDBEBB4D273629A2138AA88C666665896CADD72C0AA20E024B4EB6A3"
  }
}

[TimoGlastra]

Since we're still re-fetching the credential it should be fine querying it as I understand it,

Got it! I thought re-fetching the credential still needs to be added so that's why I was confused the code was already re-fetching it. But now I understand you meant to find a better alternative to re-fetching the credential. Makes sense.

If the tests pass and the regex is updated I'm good to merge this 👍 Thanks for the really great addition

[TimoGlastra]

BTW -- it seems tests are failing due to the snapshots being updated. I think all changes are related to adding the indy metadata tags and look fine. You can update the snapshots by running yarn test -u