Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AnonCreds Credentials using the W3C Standard - implementation #271

Merged
merged 34 commits into from
Dec 7, 2023

Conversation

Artemkaaas
Copy link
Contributor

  • W3C credential and presentation definitions: tied to AnonCreds W3C form
  • Method to convert legacy credential into W3C form
  • Method to convert AnonCreds W3C credential into legacy form
  • Methods to issue/process a credential in W3C form
  • Methods to create/verify a presentation in W3C form
  • Helper methods for W3C credential

@Artemkaaas Artemkaaas force-pushed the anoncreds-wc3 branch 4 times, most recently from 9620287 to 511a669 Compare November 13, 2023 14:51
@Artemkaaas Artemkaaas changed the title AnonCreds Credentials and Presentations using the W3C Verifiable Credentials Data Model Standard AnonCreds Credentials using the W3C Standard Nov 14, 2023
@TimoGlastra
Copy link
Member

@Artemkaaas is this ready for review, or should it still be in draft?

@Artemkaaas
Copy link
Contributor Author

@Artemkaaas is this ready for review, or should it still be in draft?

Yes. There is still some work to do internally, but in general it already provides implementation of all necessary APIs

Signed-off-by: artem.ivanov <[email protected]>
Copy link
Contributor

@berendsliedrecht berendsliedrecht left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did not review everything yet, but here are some minor comments.

Cargo.toml Outdated Show resolved Hide resolved
src/data_types/credential.rs Show resolved Hide resolved
src/data_types/w3c/credential.rs Show resolved Hide resolved
src/data_types/w3c/credential.rs Show resolved Hide resolved
src/data_types/w3c/credential.rs Outdated Show resolved Hide resolved
Comment on lines +255 to +261
W3CCredential {
context: ANONCREDS_CONTEXTS.clone(),
type_: ANONCREDS_CREDENTIAL_TYPES.clone(),
issuance_date: Utc::now(),
proof: OneOrMany::Many(Vec::new()),
..Default::default()
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can just be the impl Default for W3CCredential....

}
}

pub fn set_id(&mut self, id: URI) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not the biggest fan of this API right now, but I am also unsure what can be done to improve it.

src/data_types/w3c/credential_proof.rs Outdated Show resolved Hide resolved
src/data_types/w3c/one_or_many.rs Show resolved Hide resolved
src/ffi/cred_offer.rs Outdated Show resolved Hide resolved
@Artemkaaas Artemkaaas force-pushed the anoncreds-wc3 branch 2 times, most recently from 93c08f1 to 410ef34 Compare November 22, 2023 07:51
Signed-off-by: artem.ivanov <[email protected]>
Signed-off-by: artem.ivanov <[email protected]>
@swcurran
Copy link
Member

swcurran commented Dec 4, 2023

@andrewwhitehead @berendsliedrecht -- could you please take a final pass through this PR and, if appropriate, approve it?

Thanks!

Copy link
Contributor

@berendsliedrecht berendsliedrecht left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have reviewed 23 out of 41 files. I will continue friday.


#[derive(Debug, Clone, PartialEq, Eq)]
pub enum CredentialStatusType {
AnonCredsCredentialStatusList2023,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Been a bit out of the loop, will this be a new revocation scheme or just a placeholder for future work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It refers to the same revocation schema as we used before. We need to define a new typelike AnonCredsProof2023 for credentials and AnonCredsPresentationProof2023 for presentations.

src/data_types/w3c/presentation_proof.rs Outdated Show resolved Hide resolved
/// # Returns
/// Error code
#[no_mangle]
pub extern "C" fn anoncreds_create_w3c_credential_offer(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I likely have missed some previous discussions surrounding this, but why is this method added? I have no real objection to it, just more curious.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed during one of the community group calls.
There are 6 methods for running issuance/verification flow methods in total.
We have to duplicate at least 4 of them: create_credential, process_credential, create_presentation, verify_presentation.

credential_offer and credential_request are not changed for the current moment. It's clearer to use the same set of methods for running the whole w3c credential issuance process. Also, potentially offer and request can be adjusted for W3C standard somehow in future.

/// # Returns
/// Error code
#[no_mangle]
pub extern "C" fn anoncreds_w3c_credential_set_id(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a specific reason why we expose these set methods over FFI? IIRC we did not do that for the anoncreds credentials, is this a nice convience for w3c specifically after conversion or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We added these helper methods to provide an ability to set Non-AnonCreds Data Integrity Proof Signatures for generated W3C credential objects. So that W3C credentials may contain multiple signatures to use in future.
It's up to application whether to use these methods or add proof by itself.

}

impl<'a> CLCredentialIssuer<'a> {
pub(crate) fn init(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a specific reason why this is not just new and why does it return a Result?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be updated.
CLCredentialIssuer returns Result only to be aligned with other CL helpers (like CLCredentialProver) which does more steps on the init and truly returns Result.

) -> Result<SubProofRequest> {
trace!("_build_sub_proof_request <<< req_attrs_for_credential: {:?}, req_predicates_for_credential: {:?}",
req_attrs_for_credential, req_predicates_for_credential);
pub(crate) struct CLCredentialProver<'a> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could it be possible to just handle this without the lifetime? Whats the reason we need a ref?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It cannot because CLCredentialProver holds references requiring explicit lifetime definition

src/services/prover.rs Outdated Show resolved Hide resolved
Signed-off-by: artem.ivanov <[email protected]>
# Conflicts:
#	docs/design/w3c/w3c-representation.md
@swcurran swcurran merged commit fd44f1b into hyperledger:main Dec 7, 2023
26 checks passed
@swcurran
Copy link
Member

swcurran commented Dec 7, 2023

w00t!!!! Awesome work @Artemkaaas ! Good stuff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants