Skip to content

refactor(anoncreds): master secret to link secret #1415

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

Merged
merged 4 commits into from
Apr 1, 2023

Conversation

genaris
Copy link
Contributor

@genaris genaris commented Mar 31, 2023

Updates AnonCredsRsHolderService according to the recent changes in @hyperledger/anoncreds API and applies transformations to credential request metadata (including migration script).

There is a test that is not passing, because anoncreds-rs expects the credential definition to have a link_secret field instead of the former/legacy master_secret field (this is according to this test added recently by @blu3beri).

From my understanding, we should still be tied to master_secret in this case but AnonCreds spec is not consistent about this: in section 7.2.2., it first shows an example where link_secret is used (supposed to be based in an example in Sovrin MainNet which actually uses master_secret). And then, when describing all the fields within a Credential Definition, it states:

master_secret (also known as link secret, but kept as master_secret for backwards compatibility)

@genaris genaris requested a review from a team March 31, 2023 17:44
@genaris genaris linked an issue Mar 31, 2023 that may be closed by this pull request
@TimoGlastra
Copy link
Contributor

Yeah the credential definition should still use master_secret and we should fix that in AnonCreds RS and the specification

Copy link
Contributor

@TimoGlastra TimoGlastra left a comment

Choose a reason for hiding this comment

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

Fabulous!

masterSecret?.handle.clear()
return {
linkSecretId: options?.linkSecretId ?? utils.uuid(),
linkSecretValue: LinkSecret.create(),
Copy link
Contributor

Choose a reason for hiding this comment

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

much better :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I was a really happy moment for me when I did that update :-D

@TimoGlastra
Copy link
Contributor

@TimoGlastra
Copy link
Contributor

@genaris https://github.com/hyperledger/anoncreds-rs/releases/tag/v0.1.0-dev.13 is cooking, after which we can merge this PR I think?

@genaris
Copy link
Contributor Author

genaris commented Apr 1, 2023

Yes, I think we'll need to update some package.json files first so yarn.lock takes this new version.

@TimoGlastra TimoGlastra enabled auto-merge (squash) April 1, 2023 13:45
@TimoGlastra TimoGlastra merged commit 8bc8dbc into openwallet-foundation:main Apr 1, 2023
@genaris genaris deleted the feat/link-secret branch April 1, 2023 14:36
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.

Update master secret to link secret
2 participants