Add ed25519 support to the p2p/crypto package

[mildred]

This is a follow-up of pull request #677. It has been completely re-made from zero. I started with two commits to clean up things a little bit, then implemented the ed25519 algorithm using the agl/ed25519 pure go library and with tests first.

[jbenet]

maybe we can put these inside an ed25519 package so importing them is:

ed25519.PublicKey

[mildred]

I was thinking of implementing it the same way for RSA and ed25519. Do you think it is also desirable to put the RSA functions in a separate package as well ?

Also, I'm a bit reluctant here because we would have two packages containing different things and having the same name. the ed25519 from github.com/agl/ed25519 an the version from p2p/crypto/ed25519. Do you think this should be avoided, or not ? Same with the rsa package.

[jbenet]

I was thinking of implementing it the same way for RSA and ed25519. Do you think it is also desirable to put the RSA functions in a separate package as well ?

Yeah, and they can follow a common interface.

Also, I'm a bit reluctant here because we would have two packages containing different things and having the same name. the ed25519 from github.com/agl/ed25519 an the version from p2p/crypto/ed25519. Do you think this should be avoided, or not ? Same with the rsa package.

Thanks for caring about this detail :) -- I think in this case it will be fine. Kind of like path and crypto and net. the common names are overloaded-- the fully qualified import path is the truth. thankfully, in Go the local variable being bound at the top of the file let's the user quickly tell which it's coming from.

[mildred]

The builds are failing because of missing package github.com/agl/ed25519. I was under the impression that go is automatically downloading dependencies, and this isn't the case here. Does anyone knows why?

[cryptix]

I was under the impression that go is automatically downloading dependencies, and this isn't the case here.

In general, running 'go get' in a project does fetch all dependencies but we don't want to depend on the most current version (i.e. sudden breakage) that's why we copy the deps into Godeps/_workspace and rewrite the import paths.

We have a target to do this automatically, try 'make vendor' in the project root.

ps: you sadly need to squash the commits that had the unvendored paths in it. Jenkins is setup to test every commit. @jbenet we should have contributor guidelines for this.

[mildred]

Thank you.

After running make vendor, I had a few modifications about a package go-humanize that disappeared from Godeps, I didn't commit that, because I didn't know if this was correct.

[cryptix]

go-humanize disappeared

Oh right. I saw that as well on my spring-cleaning spree yesterday. Didn't finish it though.. I think you were right to leave it out of this, really isn't related.

[cryptix]

Two Failures on travis, but only one relevant. race: limit on 8192 simultaneously alive goroutines is exceeded, dying from exchange/bitswap creeps up from time to time, right @jbenet?

But the build-walk-branch want's every commit to pass the test, so a commit with unvendorded deps will always fail.. You need to squash the offending commits and force push this branch to trigger jenkins again.

ps: I found this document useful on how to do the squashing, otherwise feel free to reach out on IRC.

[mildred]

Ok, I'll rebase my branch soon, thanks

[mildred]

Travis build using Go 1.3 are failing with:

not ok 24 - 'ipfs daemon' is ready
#
# IPFS_PID=$! &&
# test_wait_output_n_lines_60_sec actual_daemon 2 &&
# test_run_repeat_60_sec "grep \"API server listening on $ADDR_API\" actual_daemon" ||
# test_fsh cat actual_daemon || test_fsh cat daemon_err
# 

for both the continuous-integration/travis-ci/pr and continuous-integration/travis-ci/push builds. Go 1.4 builds are fine. Do you have an idea where it comes from ? This pull request doesn't touch sharness tests.

Jenkins fails with the race: limit on 8192 simultaneously alive goroutines is exceeded, dying error that you told me could be ignored.

[whyrusleeping]

that failure on 1.3 isnt really just 1.3. ive seen it on 1.4, but for some reason its most common on 1.3. No idea. but its not your fault. And the race error isnt something you need to worry about either.

[jbenet]

@mildred other than the packaging move, this LGTM.

[whyrusleeping]

Looks good to me. All the right tests seem to pass

[whyrusleeping]

I was diagnosing some performance issues tonight and I found out that rsa's signing algorithms are eating up a TON of our time. ed25519 claims to have fast signing? @jbenet @mildred Lets get this merged in, i want better perf! 😄

[jbenet]

Agreed, let's get this merged. if @mildred is not around, maybe we can rebase + move the package ourselves.

[dylanPowers]

This was labeled as "in progress", but we don't have anyone assigned 😨 Please assign someone 😃

[jbenet]

Let's keep this in the backlog (or icebox). Can maybe land after upcoming keyspec anyway

—
Sent from Mailbox

On Mon, Mar 30, 2015 at 8:32 PM, Dylan Powers notifications@github.com
wrote:

This was labeled as "in progress", but we don't have anyone assigned 😨 Please assign someone 😃

Reply to this email directly or view it on GitHub:
#827 (comment)

[whyrusleeping]

closing due to inactivity, please reopen as necessary

note: all pull requests older than three weeks may be closed in an effort to keep our open pull requests more focused.

[dylanPowers]

@whyrusleeping I like the idea of putting issues/pull-requests in the icebox when there's been a period of inactivity. That way we can have a flow of only closing issues when they have actually been resolved. We can later come back to close this issue when it's no longer relevant and has thus been "resolved" because of it. I should add this case to ipfs/community#20

[whyrusleeping]

thats a good idea, can you bring it up during the sprint meeting tomorrow?

[dylanPowers]

I can try to remember to