hyperledger / anoncreds-rs

anoncreds-rs
https://wiki.hyperledger.org/display/anoncreds
Apache License 2.0
74 stars 55 forks source link

Full revocation status list is actually required for revocable credential issuance? #227

Open genaris opened 1 year ago

genaris commented 1 year ago

Currently, the library requires us to pass a full revocation status list object when issuing revocable credentials. From my understanding, this is mostly needed to determine whether the revocation status type is issuance_by_default or issuance_on_demand (as seen here), and, in the latter case to determine the new accumulator value (as seen here).

However, as issuance_type has been removed from the spec and now only issuance by default is allowed, I think it would make sense remove this possibility from this library, which will result in a simplified code that will not require the consumer application to provide the revocation status list object every time a credential is issued.

Am I missing something?

swcurran commented 1 year ago

The full revocation status list is there to support Verifiable Data Registry (VDR) methods that require the full state of the RevReg be tracked, vs. Indy’s deltas. I don’t think it has anything to do with the issuance type AFAIK. In the full state usage, the flag could save the first full state entry.

On your second statement, I don’t think it is quite right. The spec. is a ledger-agnostic version of the existing functionality, and since issuance_type is supported in the existing implementation, it should continue to be. It is not needed by “full-state” implementations (as mentioned above), but is for delta-state methods. In the spec, issuance_type of issuance_on_demand is strongly discouraged (since it is silly…), but not removed.

genaris commented 1 year ago

Thanks @swcurran for the answer!

I started my reasoning considering issuance_by_default as the only possible issuance_type because this field is not mentioned in the current spec. issuanceType is in REVOC_REG_DEFs data model (for Indy ledgers) but in AnonCreds spec it is not included in the data model (see section 7.3.1).

There are, however, a few references to this concept:

Should it be included and marked as "deprecated" or "kept for legacy reasons but please please please don't use it"?

TimoGlastra commented 1 year ago

now only issuance by default is allowed

This is not necessarily true. The data model in this spec always contains the full state (so for every index whether it is revoked / unrevoked ). Therefore the issuance_by_default or issuance_on_demand doesn't matter anymore. You could have started with either of them, but the accumulator that you will derive from the full state will be the same as we have all the indexes with their state (instead of how it used to be where you got "this is the starting point (all revoked or all unrevoked) and this is how it changed over time)"

However, as issuance_type has been removed from the spec and now only issuance by default is allowed, I think it would make sense remove this possibility from this library, which will result in a simplified code that will not require the consumer application to provide the revocation status list object every time a credential is issued.

I think this makes sense. From what I remember there is a separate method to update the status list. So if you want to use issuance_on_demand, you can call the issue method first, and then separately call the update_status_list method right?

This way issuance is separate from managing which credentials are revoked

Should it be included and marked as "deprecated" or "kept for legacy reasons but please please please don't use it"?

So as mentioned above. To the spec, it doesn't matter. You always have the full state in the data model, in which case the issuance type doesn't change the end result