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

Refactor idemix implementation #2955

Merged
merged 1 commit into from
Sep 30, 2021
Merged

Refactor idemix implementation #2955

merged 1 commit into from
Sep 30, 2021

Conversation

ale-linux
Copy link
Contributor

This PR removes the internal implementation of the idemix protocol and uses
an external dependency that contains the same implementation.

Signed-off-by: Alessandro Sorniotti aso@zurich.ibm.com

Type of change

  • Improvement (improvement to code, performance, etc)

Description

The idemix is factored out of the repo and now imported as a dependency.

@ale-linux ale-linux requested a review from a team as a code owner September 28, 2021 15:58
@yacovm
Copy link
Contributor

yacovm commented Sep 28, 2021

LGTM, perhaps @adecaro can also take a look.

Do we have any concern of non-determinism here, or are we OK since:

and uses an external dependency that contains the same implementation.

can you elaborate on that?

@ale-linux
Copy link
Contributor Author

LGTM, perhaps @adecaro can also take a look.

Do we have any concern of non-determinism here, or are we OK since:

and uses an external dependency that contains the same implementation.

can you elaborate on that?

good points. So:

  • the new module does indeed add new functionalities (e.g. extensions to what's being proved/verified), but these are only usable if the caller of the affected functions (e.g. Sign or Verify) requests it by supplying a different signer/verifier op; this isn't done by the idemix msp, which keeps using the "old" opts which ensure the "old" behaviour
  • furthermore, we have added a set of tests in the new repo to assert that all artefacts (CA PK, params, CA SK, user PK, user SK, nyms, signatures etc) produced by the "old" implementation would cause the same behaviour when supplied to the appropriate function

@yacovm
Copy link
Contributor

yacovm commented Sep 28, 2021

Looks good, let's wait for @adecaro to make a pass too.

This PR removes the internal implementation of the idemix protocol and uses
an external dependency that contains the same implementation.

Signed-off-by: Alessandro Sorniotti <aso@zurich.ibm.com>
@adecaro
Copy link
Contributor

adecaro commented Sep 30, 2021

LGTM. This PR will make Fabric lighter and will give more opportunities to enhance the idemix stack.

@yacovm yacovm enabled auto-merge (squash) September 30, 2021 14:06
@yacovm yacovm merged commit 9636332 into hyperledger:main Sep 30, 2021
@bestbeforetoday
Copy link
Member

bestbeforetoday commented Oct 1, 2021

@ale-linux Would you be interested in adding a client-side Idemix signing implementation to the fabric-gateway client API?

All that needs to be provided at the client end is a mechanism to create a signing function of the form: func(digest []byte) ([]byte, error)

Existing client signing implementations and the API provided to create them can be found at https://github.com/hyperledger/fabric-gateway/tree/main/pkg/identity:

If a specific hash algorithm is required for an Idemix signer, that can be added here.

We would also want a scenario test to ensure an Idemix signer worked at runtime with a real Fabric network, but this should require very little code to extend the existing Cucumber scenario tests, similar to how the HSM signer is tested.

@ale-linux
Copy link
Contributor Author

@bestbeforetoday that sounds indeed like an interesting feature to add. It should be simple enough - see this test for example

https://github.com/IBM/idemix/blob/main/bccsp/bccsp_test.go#L98

lines 98 through 189 basically show all the calls needed to fully set up an idemix stack and sign with it.

Here instead

https://github.com/IBM/idemix/blob/main/bccsp/bccsp_test.go#L853

we show how to load an idemix configuration from a client local msp store.

denyeart pushed a commit to denyeart/fabric that referenced this pull request Jan 23, 2022
This PR removes the internal implementation of the idemix protocol and uses
an external dependency that contains the same implementation.

The backport to release-2.2 is required since the new idemix dependencies
support Go 1.17 while the old does not.

Signed-off-by: Alessandro Sorniotti <aso@zurich.ibm.com>
Signed-off-by" David Enyeart <enyeart@us.ibm.com>
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.

4 participants