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

Simple Example - Centralized Telescope with BLS #120

Open
wants to merge 28 commits into
base: main
Choose a base branch
from

Conversation

curiecrypt
Copy link
Collaborator

@curiecrypt curiecrypt commented Jan 10, 2025

Content

This PR includes a basic example of integrating a multi-signature scheme into the Centralized Telescope.
It consists of the functionality KeyGen, Sign, Aggregate, and Verify proposed in the Definition 3. of Alba Definitions by @tolikzinovyev.

Pre-submit checklist

  • Branch
    • Tests are provided (if possible)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
  • PR
    • No clippy warnings in the CI
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested

Issue(s)

Relates to #114

@curiecrypt curiecrypt marked this pull request as ready for review January 14, 2025 16:18
rrtoledo
rrtoledo previously approved these changes Jan 14, 2025
Copy link
Collaborator

@rrtoledo rrtoledo left a comment

Choose a reason for hiding this comment

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

LGTM!

I think you can simplify a bit the code (replacing the if lets), optimize it a bit (you can use only one hashmap) and a couple of question on visibility and var name. :)

Cargo.toml Outdated Show resolved Hide resolved
examples/simple_example.rs Outdated Show resolved Hide resolved
examples/simple_example.rs Outdated Show resolved Hide resolved
examples/simple_example.rs Outdated Show resolved Hide resolved
examples/simple_example.rs Outdated Show resolved Hide resolved
examples/simple_example.rs Outdated Show resolved Hide resolved
examples/simple_example.rs Outdated Show resolved Hide resolved
examples/simple_example.rs Outdated Show resolved Hide resolved
Copy link
Member

@tolikzinovyev tolikzinovyev left a comment

Choose a reason for hiding this comment

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

Very good start. One request: let's not commit to the main branch directly. If the element size needs to be changed, it can be done in this PR.

examples/simple_example.rs Outdated Show resolved Hide resolved
examples/simple_example.rs Outdated Show resolved Hide resolved
examples/simple_example.rs Outdated Show resolved Hide resolved
examples/simple_example.rs Outdated Show resolved Hide resolved
examples/simple_example.rs Outdated Show resolved Hide resolved
examples/simple_example.rs Show resolved Hide resolved
examples/simple_example.rs Outdated Show resolved Hide resolved
examples/simple_example.rs Outdated Show resolved Hide resolved
examples/simple_example.rs Outdated Show resolved Hide resolved
rrtoledo
rrtoledo previously approved these changes Jan 21, 2025
Copy link
Collaborator

@rrtoledo rrtoledo left a comment

Choose a reason for hiding this comment

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

LGTM

You could add a some more comments on the functions, but that can be done in a separate PR.

examples/simple_example.rs Outdated Show resolved Hide resolved
examples/simple_threshold_signature/signature.rs Outdated Show resolved Hide resolved
examples/simple_threshold_signature/threshold_signature.rs Outdated Show resolved Hide resolved
examples/simple_example.rs Outdated Show resolved Hide resolved
Co-authored-by: Tolik Zinovyev <aszinovyev@gmail.com>
// Create the telescope structure
let alba = Telescope::create(soundness_param, completeness_param, set_size, lower_bound);

let mut public_key_list: Vec<(usize, PublicKey)> = Vec::with_capacity(nb_elements as usize);
Copy link
Member

Choose a reason for hiding this comment

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

The usize variable always contains its own index, so it is redundant and can be removed.

if let Some(signature_entry) = signatures.iter().find(|entry| entry.signature == *sig) {
if public_key_list
.iter()
.any(|entry| entry.0 == signature_entry.index)
Copy link
Member

Choose a reason for hiding this comment

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

I think this logic wouldn't work if you pass signatures to this function that are not index 0, 1, ..., n_p-1. That's why in my document Aggregate gets triples (i, s_i, w_i). In particular, the index i of the signature is included.

Also, public_key_list seems redundant in this function.

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.

3 participants