Added issuerId to the schema and cred_def anoncreds objects

[berendsliedrecht]

Partially closes #34

• Adds IssuerId property to the Schema and CredentialDefinition.

This is reflects the PR for adding the issuerId to anoncreds objects.

(revocation needs to be done but is omitted here for conflicts with #37.

Signed-off-by: blu3beri [email protected]

[TimoGlastra]

I see you left the keys as _did. We need to think a bit about backwards interop here. We probably want _did to be valid, but only for legacy (unqualified identifiers). For all others we need to use the _id variant.

This way we keep the old exchange valid

[TimoGlastra]

OR we require the input to be _id and the user of the library must manually transform it from _did to _id. But I think it's maybe easier to support both in here?

[berendsliedrecht]

Hmm yeah I am not too sure. I did not notice the key, so that is a good catch.

Thinking whether this would work okay:

        tag_ @ "issuer_did" => precess_filed("issuer_id", filter.issuer_id.to_owned(), tag_value),
        tag_ @ "issuer_id" => precess_filed(tag_, filter.issuer_id.to_owned(), tag_value),

[TimoGlastra]

Hmm not sure if we should mix them. Where is this code used for? Is it only for internal processing? We don't want to modify the output model. And they probably shouldn't be allowed to be defined at the same time?

[berendsliedrecht]

It is used for verify_presentation which is exposed. Would it not be okay just to remove the issuer_did tag? Would be rather annoying to remove it later with another breaking change.

[TimoGlastra]

Legacy indy identifiers will still be valid for dids, meaning the issuer_id won't be always an uri

[berendsliedrecht]

Okay then I need to redo the validation for the identifiers as it can be a URI or the legacy did indy.

Would this suffice:

1. Check if URI
2. If URI, success
3. If not URI, check legacy indy identifier
  - Check length (not sure which lengths, I thought there were 2 different lengths)
  - Check if all chars are in the base58 alphabet

[TimoGlastra]

Yes. I think the length is 21 or 22 (we have a regex in AFJ)

[berendsliedrecht]

Awesome. Will add that to this PR.

[berendsliedrecht]

It should be fixed with the latest commit.

[TimoGlastra]

@blu3beri on the last WG call we came to the conclusion that:

• Only legacy indy identifiers can use _did for backwards compatiblity
• All other identifier types (qualified indy, but also other methods) MUST use _id
• They can't be used at the same time

That means we need to support both in this library, and that we need to verify the above rules

[berendsliedrecht]

• They can't be used at the same time

Do you mean they can't be used in the same proof request of for s specific group within the request?

[TimoGlastra]

Do you mean they can't be used in the same proof request of for s specific group within the request?

Hm. If you're using the new syntax anywhere in the proof request, you could just as well use it everywhere. But that's probalby more complex to verify?

[TimoGlastra]

Nice 👍

[TimoGlastra]

This uses schema_issuer_id. Are both properties extracted into the same key in the filter struct?

[berendsliedrecht]

Yeah it just maps to the issuer_id in the schema. It got a bit confusing trying to find the answer, but below we get the issuer_id from the schema and map it to the filter.schema_issuer_Id. the schema.issuer_id can be legacy or new. So we check if the provided value for schema_issuer_did is legacy and is inside the schema.issuer_id.

    Ok(Filter {
        schema_id: schema_id.to_owned(),
        schema_name: schema.name.to_owned(),
        schema_version: schema.version.to_owned(),
        schema_issuer_id: schema.issuer_id.to_owned(),
        issuer_id: cred_def.issuer_id.to_owned(),
        cred_def_id: cred_def_id.to_owned(),
    })