novitski / bitcoinj

Automatically exported from code.google.com/p/bitcoinj
Apache License 2.0
0 stars 0 forks source link

Upgrade to Bouncy Castle 1.50 #497

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
BC 1.50 has been released, it will get us at least two useful enhancements:

- Deterministic signatures
- Faster ECDSA

Currently Spongy Castle has not uploaded new builds to Maven but I can see from 
SC git that they're working on it. Once SC 1.50 becomes available, let's update.

Original issue reported on code.google.com by mh.in.en...@gmail.com on 14 Dec 2013 at 7:59

GoogleCodeExporter commented 9 years ago
There's now a new Spongy Castle release that matches BC1.50: should be easy to 
update!

Original comment by mh.in.en...@gmail.com on 6 Feb 2014 at 7:03

GoogleCodeExporter commented 9 years ago
Here's a patch (include the deterministic ECDSA changes also) that updates the 
pom to sc 1.50.0.0 and a few corresponding code changes, mainly around point 
compression, for which we now have a getEncoded(boolean compressed) method.

Please note the use of ECPoint.normalize() in a couple of places. Since we now 
implement non-affine coordinates, points need to be normalized before their 
coordinates will be correctly affine. For the most part, this is automatic, but 
I noted a couple of places in bitcoinj where for performance it should be done 
explicitly.

I can't reliably run the tests through cleanly here (the failures seem like 
unrelated networking type problems), so someone definitely needs to check this.

Original comment by peter.de...@gmail.com on 7 Feb 2014 at 9:05

GoogleCodeExporter commented 9 years ago
Hmm, missed captcha ate my attachment?

Original comment by peter.de...@gmail.com on 7 Feb 2014 at 9:07

Attachments:

GoogleCodeExporter commented 9 years ago
Hmm, if you could open a bug for the test failures that'd be good.

Original comment by mh.in.en...@gmail.com on 7 Feb 2014 at 9:09

GoogleCodeExporter commented 9 years ago
Thanks for the patch! The use of deterministic ECDSA makes the full block unit 
tests fail for some reason, unfortunately when they fail it is a bit hard to 
debug so I won't have time to investigate this today most likely. I CCd Matt 
who wrote those tests in case he feels like taking a look before I do.

Re: the normalisation. So if I understand correctly in 1.5 an ECPoint can be 
Jacobian and normalize() gets us back to affine form?

I have a branch where deterministic wallets are implemented, which involves a 
bunch of changes and refactorings to ECKey and other things. When it's done I 
was wondering if you'd mind looking over the ECC parts, as you have much more 
expertise than me? Probably would be done some time in March.

Original comment by mh.in.en...@gmail.com on 7 Feb 2014 at 9:40

GoogleCodeExporter commented 9 years ago
As for the broken test, testGeneratedChain fails at block "b17". Since the only 
real difference is that DetECDSA can repeat a signature, I checked where this 
was happening in the b17 setup. There's exactly one repeat: for 'out6' which 
was also passed to the b16 setup. i.e the calls to createNextBlock for b16, b17 
both then call
    addOnlyInputToTransaction(t, prevOut);
where prevOut is the same 'out6' outpoint for both, and generate the same 
signature:
    coinbaseOutKey.sign(...)
I figure the subsequent error for b17 must somehow relate to this, but I got 
lost trying to look further...

Yes, in 1.50, the points use Jacobian coordinates internally (technically 
Jacobian-Modified). Normalizing adjusts the projective coordinate (Z) to be 1, 
so that the X, Y values are then the affine coordinate values. This requires a 
field inversion, so it's somewhat expensive (basically free though if already 
normalized). It also returns a new point, since the class is immutable. It only 
really becomes an issue if you keep a point around for more than one call to 
say, getEncoded (which will normalize internally if necessary - each time).

I will be quite happy to review the deterministic wallet ECC code - just let me 
know.

Original comment by peter.de...@gmail.com on 7 Feb 2014 at 12:34

GoogleCodeExporter commented 9 years ago
May I ask if there has been any progress in analyzing the test failure?

Original comment by peter.de...@gmail.com on 19 Feb 2014 at 3:01

GoogleCodeExporter commented 9 years ago
Not yet but I didn't forget about this!

Original comment by mh.in.en...@gmail.com on 21 Feb 2014 at 12:52

GoogleCodeExporter commented 9 years ago
I finally got around to debugging this and upgrading. Thanks for the patch!

Original comment by mh.in.en...@gmail.com on 8 Apr 2014 at 2:28

GoogleCodeExporter commented 9 years ago
This issue was closed by revision feba332e6e02.

Original comment by mh.in.en...@gmail.com on 8 Apr 2014 at 2:40