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

Defne public interface for distributed schnorr signature scheme #71

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rantan
Copy link
Contributor

@rantan rantan commented Mar 17, 2020

fix #70

@rantan rantan requested a review from Yamaguchi March 17, 2020 10:36
fn compute_final_signature(
local_sigs: &HashSet<LocalSig>,
threshold: usize,
) -> Result<Signature, Error>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have confidence whether the arguments are enough or not.
@Yamaguchi Can you check it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It need to have signer index to calculate Lagrange interpolation. I will add it into LocalSig.

Copy link
Contributor

Choose a reason for hiding this comment

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

To compute Signatre.r_x, a set of BlockVSS is needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's true. I will add it.

use std::collections::HashSet;
use std::str::FromStr;

struct MockImpl;
Copy link
Contributor

Choose a reason for hiding this comment

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

This struct should be public.

use secp256k1::{constants, SecretKey};
use std::ops::Deref;

pub enum Error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why define these errors here, not in errors.rs?

}

pub trait KeyGenerationProtocol {
fn create_node_vss(
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer the return type of these functions is Result<...., Error> even if it does not return error now.

threshold: usize,
) -> HashSet<NodeVSS>;
fn verify_node_vss(vss: &NodeVSS) -> Result<(), Error>;
fn aggregate_node_vss(vss_set: &HashSet<NodeVSS>) -> NodeShare;
Copy link
Contributor

Choose a reason for hiding this comment

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

This should return Result<NodeShare, Error>

}

fn create_local_sig(
message: &[u8; 32],
Copy link
Contributor

Choose a reason for hiding this comment

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

Use Message type.

@Yamaguchi
Copy link
Contributor

Yamaguchi commented Mar 18, 2020

Some warnings reported when cargo build or cargo test.
Consider to use #[warn(dead_code)] etc...

@rantan rantan added wontfix This will not be worked on don't merge and removed wontfix This will not be worked on labels Mar 18, 2020
@rantan rantan marked this pull request as draft April 10, 2020 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor: Re-define VSS structure and Interface
2 participants