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 crate structure #22

Closed
tplooker opened this issue May 22, 2022 · 5 comments · Fixed by #40
Closed

Refactor crate structure #22

tplooker opened this issue May 22, 2022 · 5 comments · Fixed by #40

Comments

@tplooker
Copy link
Member

tplooker commented May 22, 2022

With the merging of #20 we need to now consider how to restructure the crate, currently the intended design was documented as

├── src - Main source code folder
│   ├ lib.rs - Controls the exposed public API
│   ├ curves - Defines the different pairing based elliptic curves supported by the library
│   │   ├── bls_12381.rs
│   │   └── bls_12381
│   └ schemes - Defines the different cryptographic schemes (e.g signatures) supported by the library
│       ├── bbs.rs
│       └── bbs

However this structure implies we want to build an implementation of BBS that is curve agnostic, which in practise may be difficult/less worthwhile doing now. So I propose we add another layer ciphersuite.

Meaning the structure is as follows

├── src - Main source code folder
│   ├ lib.rs - Controls the exposed public API
│   ├ curves - Defines the different pairing based elliptic curves supported by the library
│   │   ├── bls_12381.rs
│   │   └── bls_12381
│   └ schemes - Defines the different cryptographic schemes (e.g signatures) supported by the library
│       ├── bbs.rs
│       └── bbs
|                    ├── bls_12381.rs
|                    └── bls12_381 // BBS implementation for the BLS12_381 ciphersuite here

In order to do this, I recommend we do the following

  1. Reinstate the curves folder, here will contain curve specific constructs like the public and secret key structures
  2. Shift the current BBS implementation which is BLS12_381 specific down a level

I've started this process here

@dev0x1
Copy link
Contributor

dev0x1 commented May 22, 2022

I would like to pursue a curve agnostic approach as much as possible and suggest we have a generic BBS implementation that mainly depends upon(trait only dependency)

  1. pairing crate and related ff, and group crates
  2. digest
  3. hash-to-curve

Then we can have a BBS-cipher suite implementation with a specific pairing curve, hash function, and hash-to-curve for easy integration in our wrappers.
Specific curve implementations such as similar to blstrs should be kept out in a separate crate, or in a separate cargo workspace in this monorepo.
Here is what I have come up with:

src
├── common
│   ├── error.rs
│   ├── hash_to_curve.rs (traits)
│   └── util.rs
├── curve
│   └── bls12381
│       └── hash_to_curve.rs (implementation, we should be keeping this to wrap `blst` implementation)
├── integration
│   └── bbs
│       └── api
├── lib.rs
└── schemes
    └── bbs
        └── core (curve agnostic implementation, depends only on traits from pairing, digest, etc. )
            ├── public_key.rs
            └── secret_key.rs
            └── other existing code

@tplooker
Copy link
Member Author

tplooker commented May 22, 2022

Im ok with pursuing abstractions provided they aren't too difficult to maintain, w.r.t BBS I think there maybe some operations that will be more difficult to abstract, for example the multi_miller_loop call I believe is BLS12 specific is it not?

In terms of directory structure I think we are functionally talking very similar designs what about the following

blstrs
...contents of the blstrs repository
src
├── lib.rs
├── common
│   ├── error.rs
│   ├── hash_to_curve.rs (traits)
│   └── util.rs
├── curves
│   └── bls12381
│       └── hash_to_curve.rs (implementation, we should be keeping this to wrap `blst` implementation)
└── schemes
    └── bbs
         ├── core (curve agnostic implementation, depends only on traits from pairing, digest, etc. )
         |   ├── public_key.rs
         |   └── secret_key.rs
         |   └── other existing code
         └── ciphersuites (a folder for the different BBS cipher suites)
             └── bls12_381 (either a folder or .rs file for the BLS12-381 cipher as defined in the spec)

@dev0x1
Copy link
Contributor

dev0x1 commented May 22, 2022

^^ Directory structure looks good to me. Agree with maintaiability, allow me code and show something this week. multi_miller_loop is a curve agonstic operation and already abstracted in pairing.

@dev0x1 dev0x1 self-assigned this May 24, 2022
@tplooker
Copy link
Member Author

Move the current src/schemes/core folder into src/schemes/bbs/core and feel free to restructure the api module found in src/schemes/bbs/api

@dev0x1
Copy link
Contributor

dev0x1 commented May 26, 2022

#26 partially closes this issue. Following is a list of pending tasks

  1. update readme with new directory structure
  2. update contribution doc for fmt and clippy on nightly
  3. fix wrappers build

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

Successfully merging a pull request may close this issue.

2 participants