hyperledger-archives / ursa

Hyperledger Ursa (a shared cryptographic library) has moved to end-of-life status, with the components of Ursa still in use moved to their relevant Hyperledger projects (AnonCreds, Indy, Aries and Iroha).
https://wiki.hyperledger.org/display/ursa
Apache License 2.0
321 stars 142 forks source link

Allow to pass pre-generated safe prime numbers to Credential Definition creation #143

Closed wulfraem closed 2 years ago

wulfraem commented 4 years ago

To allow speeding up credential definition creation we wanted to create the safe prime numbers beforehand with ursa::helpers::generate_safe_prime and hold onto them until the actual credential definition is created.

In our use case we have to create multiple credential definitions at a time and having a bunch of safe primes ready for this improves performance of this a lot. :)

brentzundel commented 3 years ago

@wulfraem We discussed this on the Ursa call today, and would like to make it possible for this to happen. We are grateful for contributions and apologize for not responding more fully until now.

Here are the key concerns around your proposed changes:

  1. Lack of input validation for the prime values
  2. A reduction in clarity for the API
  3. The library is currently undergoing significant refactoring.

In greater detail:

  1. At a minimum, the input values (the two safe primes) would need to be checked for:
    1. primality - Since there is no way to guarantee that input values were generated using ursa::helpers::generate_safe_prime, there should be primality checking of the input values. We anticipate this may duplicate some of the prime-checking that has already occurred for valid primes, but it would also add protection for unwary users.
    2. valid range - The call to generate_safe_primes currently expects a size. There would need to be a check to insure that the input values don't exceed this range.
  2. We are concerned that the changed API surface may be confusing to users. Though your added comments are helpful, we feel there could be more done to guide users to the proper understanding of which function best suits their needs.
  3. The ursa library is currently experiencing a large refactoring effort. This is more informational for you than anything else. Because of this refactoring, changes you propose may meet with greater than usual need to make the PR up-do-date with the master branch and deal with conflicts.

Please respond with any questions you may have. We also welcome you to join our biweekly call to discuss this PR with the group directly. You should be able to find meeting time and connection information on the hyperledger calendar

Alexis-Falquier commented 3 years ago

@brentzundel are there any updates to this PR? Any plans to merge it soon?

brentzundel commented 3 years ago

@Alexis-Falquier I have not heard a response from the OP about our concerns. Until those are addressed, I do not support merging this PR.

brentzundel commented 2 years ago

This PR has been dormant for over a year, with no response from @wulfraem Closing