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

v0.1.0 candidate - support for EdDSA, Schnorr & fix dependencies #34

Merged
merged 33 commits into from
Oct 27, 2019

Conversation

oleiba
Copy link
Contributor

@oleiba oleiba commented Sep 5, 2019

  • Support Schnorr (as in Zilliqa implementation)
  • Support Ed25519
  • Relevant tests added to integration-tests (all pass)
  • bitcoin dependency upgraded to 0.20.0
  • Dependencies to rely on fixed tagged versions

This PR extends and replaces #30 .

@oleiba oleiba mentioned this pull request Sep 5, 2019
@omershlo omershlo reopened this Sep 9, 2019
@omershlo
Copy link
Contributor

omershlo commented Sep 9, 2019

I would like to explore other options as @gbenattar suggested

jeremyfelder
jeremyfelder previously approved these changes Sep 9, 2019
Copy link
Contributor

@omershlo omershlo left a comment

Choose a reason for hiding this comment

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

Curv should not be split

@jeremyfelder
Copy link

Approved @gbenattar changes to DockerFile and tests

@gbenattar
Copy link
Contributor

I am putting that PR on hold until @omershlo and @oleiba will have a a discussion about the dependency graph.

@oleiba
Copy link
Contributor Author

oleiba commented Sep 24, 2019

After our discussion, changed multi-party-eddsa dependency to use tag [email protected].
It uses https://github.com/KZen-networks/curv-replica, a replica of https://github.com/KZen-networks/curv, and the specific tag points to same commit as tag v0.2.0 in the original repository.

@oleiba oleiba requested a review from omershlo September 24, 2019 17:53
@omershlo
Copy link
Contributor

@oleiba have you tried to use package in Cargo.toml instead of curv2:
https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#renaming-dependencies-in-cargotoml

something like :
curv1 = { git = "https://github.com/KZen-networks/curv", version = "0.2.0", features = ['ec_secp256k1'], package = "curv" }
curv2 = { git = "https://github.com/KZen-networks/curv", version = "0.2.0", features = ['ec_ed25519'], package = "curv" }

@oleiba
Copy link
Contributor Author

oleiba commented Oct 2, 2019

I have updated the Cargo.toml files with using multi-party-eddsa with tag v0.2.1 - this tag is used in https://github.com/multi-party-eddsa master branch, and uses the same commit of curv as the direct dependency here in gotham-city, but only with a different tag name (v0.2.0-ed25519 instead of v0.2.0).

This removes the need of an additional curv repository, and I think it solves the issue nicely.

@gbenattar
Copy link
Contributor

@omershlo is it good to go on your side?

@omershlo
Copy link
Contributor

omershlo commented Oct 6, 2019

still working on it.

@@ -21,32 +24,42 @@ log = "0.4"
time = "*"
clap = { version = "2.32", features = ["yaml"] }
reqwest = "0.9.5"
Copy link
Contributor

Choose a reason for hiding this comment

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

#28 ?

@omershlo
Copy link
Contributor

I approve

@oleiba
Copy link
Contributor Author

oleiba commented Oct 27, 2019

It seems that I can't merge without your "github" approval.
Can you please go to "Files changed" tab, "Review changes" and "approve"?

@oleiba oleiba requested a review from omershlo October 27, 2019 08:40
@oleiba oleiba merged commit a77e05f into master Oct 27, 2019
@oleiba oleiba deleted the v0.1.0 branch February 6, 2020 09:04
@burdges
Copy link

burdges commented Aug 31, 2020

I'm curious: Why only two parties for a Schnorr signature?

@omershlo
Copy link
Contributor

The Gotham city architecture includes a network stack.
For simplicity purposes we wanted to rely on a classical server<>client design where the client makes rest-api calls to the server to facilitate the protocol and so on. This way we avoid p2p and still get distributed computation (although with just 2 parties)

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.

6 participants