AnonCreds Credentials using the W3C Standard - wrappers #273

Artemkaaas · 2023-11-14T14:28:10Z

Updated Python wrapper to expose new methods
Update NodeJS and React Native wrappers to expose new methods

src/data_types/credential.rs

wrappers/python/anoncreds/bindings.py

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

Signed-off-by: Abdulbois <[email protected]> Signed-off-by: artem.ivanov <[email protected]>

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

TimoGlastra

Mostly looked at the JS wrapper ,as I have more experience with it.

I think overall looks good, but I think we can reduce the number of method exposed in the wrapper, as well as have some more consistency in using w3c in names

TimoGlastra · 2023-12-11T08:10:57Z

wrappers/javascript/packages/anoncreds-nodejs/src/NodeJSAnoncreds.ts

@@ -102,6 +102,16 @@ export class NodeJSAnoncreds implements Anoncreds {
    return handleReturnPointer<string>(ret)
  }

+  public w3cCredentialGetAttribute(options: { objectHandle: ObjectHandle; name: string }) {


Is there a reason why there is a separate method for w3c? Getting an attribute is the same for w3c and non w3c credentials right?

I do not understand your point. w3cCredentialGetAttribute method calls anoncreds_w3c_credential_get_attribute external Rust function which obtain W3C credential object from handle and returns value for requesting attribute.

Yeah I was thinking more of that whether the object is an AnonCreds representation or a W3C representation doesn't really change the input or output of this method, so from an user POV it could be the same method right?

wrappers/javascript/packages/anoncreds-nodejs/src/NodeJSAnoncreds.ts

TimoGlastra · 2023-12-11T08:14:34Z

wrappers/javascript/packages/anoncreds-nodejs/src/NodeJSAnoncreds.ts

+  public createW3CPresentation(options: {
+    presentationRequest: ObjectHandle
+    credentials: NativeCredentialEntry[]
+    credentialsProve: NativeCredentialProve[]
+    linkSecret: string
+    schemas: Record<string, ObjectHandle>
+    credentialDefinitions: Record<string, ObjectHandle>
+  }): ObjectHandle {
+    const { presentationRequest, linkSecret } = serializeArguments(options)
+
+    const credentialEntries = options.credentials.map((value) =>
+      CredentialEntryStruct({
+        credential: value.credential.handle,
+        timestamp: value.timestamp ?? -1,
+        rev_state: value.revocationState?.handle ?? 0
+      })
+    )
+
+    const credentialEntryList = CredentialEntryListStruct({
+      count: credentialEntries.length,
+      data: credentialEntries as unknown as TypedArray<
+        StructObject<{
+          credential: number
+          timestamp: number
+          rev_state: number
+        }>
+      >


I think there's a lot of code here for w3c methods that is basically the same except for the end result. Do you think we can reuse some code here? I think this can lead to inconsistencies in functionality

I added helper methods doing conversion of parameters in order to reduce code duplication.
But left w3c* methods.
I think methods here should be mapped one to one with external C API. Do not see a sense to combine w3c and legacy methods.

wrappers/javascript/packages/anoncreds-nodejs/tests/api.test.ts

wrappers/javascript/packages/anoncreds-react-native/src/ReactNativeAnoncreds.ts

wrappers/javascript/packages/anoncreds-shared/src/api/Credential.ts

wrappers/javascript/packages/anoncreds-shared/src/api/W3CCredential.ts

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

Artemkaaas · 2023-12-11T09:23:33Z

@TimoGlastra I processed and resolved comments connected to using W3c instead W3C.
I will handle the rest of comments (relating to credential-offer/request duplication and credential methods for non-anoncreds proof) after discuss at the WG call. Unfortunately, I may miss today's call due to some personal errands.

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

# Conflicts: # src/data_types/w3c/credential.rs # src/ffi/w3c/credential.rs

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

Signed-off-by: artem.ivanov <[email protected]> # Conflicts: # src/ffi/w3c/credential.rs

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

…anoncreds-wc3-wrappers # Conflicts: # src/services/w3c/prover.rs

…anoncreds-wc3-wrappers # Conflicts: # src/data_types/w3c/proof.rs

TimoGlastra · 2023-12-21T07:29:15Z

@Artemkaaas once #291 is merged, I'll take another look at this PR, but there's currently too much different changes

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

…anoncreds-wc3-wrappers

swcurran · 2024-01-11T21:29:09Z

The plan (per requests from @TimoGlastra and @andrewwhitehead ) is to merge #273 first (once the test failure is fixed), so this is easier to check. Tests are passing here, so hopefully this one will be easy to approve once #273 is merged.

FYI @Artemkaaas

Thanks

swcurran · 2024-01-12T12:16:59Z

@TimoGlastra , @andrewwhitehead -- can you please do a review of this PR. The other two have been merged, so this should be easier to review. Thanks.

andrewwhitehead · 2024-01-12T20:00:11Z

docs/design/w3c/w3c-representation.md

-}
-```
-
-### Examples


Do we not want to keep the examples?

wrappers/javascript/packages/anoncreds-shared/src/api/index.ts

wrappers/python/anoncreds/types.py

andrewwhitehead · 2024-01-12T20:12:02Z

Looks good, with a couple tiny notes. We'll want to add the w3c tests to the CI.

TimoGlastra

LGTM! I already played with the api a bit for creating the test vectors and it made sense to me

Only thing is maybe that there's quite some duplication between the w3c and non w3c code in the JS wrapper, but we can look at that in the future maybe. Would ne good to have this merged
And reelased!

wrappers/javascript/packages/anoncreds-nodejs/src/NodeJSAnoncreds.ts

TimoGlastra · 2024-01-12T15:28:25Z

wrappers/javascript/packages/anoncreds-nodejs/src/NodeJSAnoncreds.ts

+
+    this.nativeAnoncreds.anoncreds_create_w3c_presentation(
+      presentationRequest,
+      credentialEntryList as unknown as Buffer,


Lots of casts should be avoided if possible

TimoGlastra · 2024-01-13T03:54:14Z

Also one more question/
: did you test the react native code already? Or should we do some
Testing in react native?

Artemkaaas · 2024-01-15T12:41:00Z

Also one more question/ : did you test the react native code already? Or should we do some Testing in react native?

I only checked build and compile scripts for wrapper, but not tested integration into a react-native application.

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

TimoGlastra · 2024-01-15T13:35:56Z

Also one more question/ : did you test the react native code already? Or should we do some Testing in react native?

I only checked build and compile scripts for wrapper, but not tested integration into a react-native application.

We can do that once this is merged and released as dev 👍

Artemkaaas marked this pull request as draft November 14, 2023 14:28

Artemkaaas force-pushed the anoncreds-wc3-wrappers branch from 1fbdc06 to fc31dae Compare November 15, 2023 09:51

Artemkaaas marked this pull request as ready for review November 15, 2023 12:22

Artemkaaas force-pushed the anoncreds-wc3-wrappers branch from 19edff7 to 106d702 Compare November 15, 2023 12:56

gvelez17 reviewed Nov 20, 2023

View reviewed changes

src/data_types/credential.rs Outdated Show resolved Hide resolved

Artemkaaas force-pushed the anoncreds-wc3-wrappers branch 2 times, most recently from e2a21ce to 075eba5 Compare November 22, 2023 07:53

Artemkaaas force-pushed the anoncreds-wc3-wrappers branch 2 times, most recently from fc52916 to 6d64da4 Compare December 2, 2023 08:07

gvelez17 reviewed Dec 5, 2023

View reviewed changes

wrappers/python/anoncreds/bindings.py Outdated Show resolved Hide resolved

Artemkaaas and others added 7 commits December 8, 2023 08:41

W3C: Updated Python wrapper

fbc2ecf

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

Added JS wrapper

dfbca25

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

Updated wrappers

6fb3c16

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

Added missing method to NodeJS wrapper

1bed31e

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

Fixes in Python and Node wrappers

0e817d6

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

fix: Replace with w3c call, change order of enum variants

2f97949

Signed-off-by: Abdulbois <[email protected]> Signed-off-by: artem.ivanov <[email protected]>

Python: Added classes for W3CCredentialOffer and W3CCredentialRequest

19ca275

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

Artemkaaas force-pushed the anoncreds-wc3-wrappers branch from 3004b69 to 19ca275 Compare December 8, 2023 05:42

Reverted lerna version change

00d3a15

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

Artemkaaas force-pushed the anoncreds-wc3-wrappers branch 3 times, most recently from 01d1842 to e59d2bc Compare December 8, 2023 08:08

Run prettier

2b903fe

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

Artemkaaas force-pushed the anoncreds-wc3-wrappers branch from e59d2bc to 2b903fe Compare December 8, 2023 08:34

TimoGlastra requested changes Dec 11, 2023

View reviewed changes

TimoGlastra mentioned this pull request Dec 11, 2023

AnonCreds Credentials using the W3C Standard - drop mapping from presentation #274

Closed

Use W3c instead of capitalized W3C

c436949

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

Use W3c instead of capitalized W3C in python wrapper

c276c2a

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

Artemkaaas force-pushed the anoncreds-wc3-wrappers branch from 9f26c7d to c276c2a Compare December 11, 2023 10:04

Artemkaaas added 9 commits December 20, 2023 09:52

Merge branch 'anoncreds-w3c-format-changes' into anoncreds-wc3-wrappers

7fed651

# Conflicts: # src/data_types/w3c/credential.rs # src/ffi/w3c/credential.rs

Updated python wrapper

87312b5

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

Updated JavaScript wrappers

d04299f

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

Merge branch 'anoncreds-w3c-format-changes' into anoncreds-wc3-wrappers

189afbe

Signed-off-by: artem.ivanov <[email protected]> # Conflicts: # src/ffi/w3c/credential.rs

Corrected context definition

1c3c30b

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

Returned rust version to 1.65.0

67d456b

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

Fixed prettier error

5094192

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

Merge remote-tracking branch 'dsr/anoncreds-w3c-format-changes' into …

b39c2c8

…anoncreds-wc3-wrappers # Conflicts: # src/services/w3c/prover.rs

Merge remote-tracking branch 'dsr/anoncreds-w3c-format-changes' into …

5648b15

…anoncreds-wc3-wrappers # Conflicts: # src/data_types/w3c/proof.rs

Artemkaaas added 2 commits December 21, 2023 10:43

Merge branch 'anoncreds-w3c-format-changes' into anoncreds-wc3-wrappers

f4b975c

Renamed parameter from version to w3cVersion

65c7ca6

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

Artemkaaas force-pushed the anoncreds-wc3-wrappers branch from cb837a5 to 65c7ca6 Compare December 21, 2023 07:49

Merge remote-tracking branch 'dsr/anoncreds-w3c-format-changes' into …

93801c8

…anoncreds-wc3-wrappers

Merge remote-tracking branch 'origin/main' into anoncreds-wc3-wrappers

9e6875d

andrewwhitehead reviewed Jan 12, 2024

View reviewed changes

docs/design/w3c/w3c-representation.md

}

```

### Examples

Copy link

Member

andrewwhitehead Jan 12, 2024

Choose a reason for hiding this comment

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

Do we not want to keep the examples?

andrewwhitehead reviewed Jan 12, 2024

View reviewed changes

wrappers/javascript/packages/anoncreds-shared/src/api/index.ts Outdated Show resolved Hide resolved

andrewwhitehead reviewed Jan 12, 2024

View reviewed changes

wrappers/python/anoncreds/types.py Outdated Show resolved Hide resolved

TimoGlastra approved these changes Jan 13, 2024

View reviewed changes

Artemkaaas force-pushed the anoncreds-wc3-wrappers branch from 54a1118 to ab97435 Compare January 15, 2024 12:33

Fixed couple comments

f0c2901

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

Artemkaaas force-pushed the anoncreds-wc3-wrappers branch from ab97435 to f0c2901 Compare January 15, 2024 13:12

swcurran merged commit c5b4f1d into hyperledger:main Jan 15, 2024
26 checks passed

Artemkaaas mentioned this pull request Jan 24, 2024

W3C CredentialSubjectId property not set correctly #308

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AnonCreds Credentials using the W3C Standard - wrappers #273

AnonCreds Credentials using the W3C Standard - wrappers #273

Artemkaaas commented Nov 14, 2023

TimoGlastra left a comment

TimoGlastra Dec 11, 2023

Artemkaaas Dec 11, 2023

TimoGlastra Dec 11, 2023

TimoGlastra Dec 11, 2023

Artemkaaas Dec 12, 2023

Artemkaaas commented Dec 11, 2023

TimoGlastra commented Dec 21, 2023

swcurran commented Jan 11, 2024

swcurran commented Jan 12, 2024

andrewwhitehead Jan 12, 2024

andrewwhitehead commented Jan 12, 2024

TimoGlastra left a comment

TimoGlastra Jan 12, 2024

TimoGlastra commented Jan 13, 2024

Artemkaaas commented Jan 15, 2024

TimoGlastra commented Jan 15, 2024

AnonCreds Credentials using the W3C Standard - wrappers #273

AnonCreds Credentials using the W3C Standard - wrappers #273

Conversation

Artemkaaas commented Nov 14, 2023

TimoGlastra left a comment

Choose a reason for hiding this comment

TimoGlastra Dec 11, 2023

Choose a reason for hiding this comment

Artemkaaas Dec 11, 2023

Choose a reason for hiding this comment

TimoGlastra Dec 11, 2023

Choose a reason for hiding this comment

TimoGlastra Dec 11, 2023

Choose a reason for hiding this comment

Artemkaaas Dec 12, 2023

Choose a reason for hiding this comment

Artemkaaas commented Dec 11, 2023

TimoGlastra commented Dec 21, 2023

swcurran commented Jan 11, 2024

swcurran commented Jan 12, 2024

andrewwhitehead Jan 12, 2024

Choose a reason for hiding this comment

andrewwhitehead commented Jan 12, 2024

TimoGlastra left a comment

Choose a reason for hiding this comment

TimoGlastra Jan 12, 2024

Choose a reason for hiding this comment

TimoGlastra commented Jan 13, 2024

Artemkaaas commented Jan 15, 2024

TimoGlastra commented Jan 15, 2024