Update libsecp256k1, Go wrapper and tests

[Gustav-Simonsson]

NOTE: RecoverPubkey is on the consensus-critical code path. Please review carefully.

Go wrapper update is in it's own commit, files changed:

crypto/secp256k1/secp256.go
crypto/secp256k1/secp256_test.go

benchmark                                  old ns/op     new ns/op     delta
BenchmarkSignRandomInputEachRound-2        148648        149994        +0.91%
BenchmarkRecoverRandomInputEachRound-2     207831        203833        -1.92%

We'll probably see a more significant speedup when we can improve internal conversions (as in remove where possible) of signatures and pubkeys - currently we need two extra calls to the lib for each recovery - one to serialise signature and one to serialise the returned pubkey.

If we can avoid those calls + other internal conversions we could probably gain a few % more on recovery.

• Update libsecp256k1
• Update Go wrapper to new libsecp256k1 API
• Refactor Go wrapper tests to have meaningful names, clearer validations and a few more edge cases.
• Add benchmarks of Sign and RecoverPubkey

[robotally]

Vote	Count	Reviewers
👍	2	@fjl @obscuren
👎	0

Updated: Fri Oct 9 13:02:59 UTC 2015

[fjl]

Did you add zeroing of intermediate buffers yet?

[Gustav-Simonsson]

@fjl not yet. added to Sign

[obscuren]

LGTM 👍

[Gustav-Simonsson]

@obscuren removed unneeded return matchings. SECP256K1_EC_COMPRESSED is from the name of the macro in the lib

[codecov-io]

Current coverage is 47.77%

Merging #1853 into develop will increase coverage by +0.14% as of 97d4d77

Powered by Codecov. Updated on successful CI builds.

[fjl]

It's usually better to use the same input data for all iterations.
package testing runs benchmarks several times to make the
result more stable, so StartTimer/StopTimer is not required.
This also applies to BenchmarkSign.

_, seckey := GenerateKeyPair()
msg := randentropy.GetEntropyCSPRNG(32)
sig, _ := Sign(msg, seckey)
for i := 0; i < b.N; i++ {
    if _, err := RecoverPubkey(msg, sig); err != nil {
        b.Fatal(err)
    }
} 

[Gustav-Simonsson]

Actually in this benchmark it's intended to use a new key for every recovery to avoid potential caching affecting the benchmark. I'll add another benchmark where the same key is used though, that will be interesting to see if caching affects the result. In that one starting and stopping of the timer can indeed be skipped.

[fjl]

What kind of caching? libsecp256k1 shouldn't cache anything.

[Gustav-Simonsson]

I don't know, but it takes much less time if using the same key. See new commit - pushed a separate benchmark for using the same key.

[fjl]

👍 and please squash one more time.