Replace ring with ed25519-dalek in primitives

[tomaka]

Contrary to ring, ed25519-dalek successfully compiles to WASM.

The changes are pretty straight-forward, but I'd understand if this gets rejected if ed25519-dalek is not considered mature enough.

cc #2416

[Demi-Marie]

Can we use ed25519-dalek only when targeting wasm? I suspect that ring is faster, though I don’t have benchmarks. ring also is more likely to resist side-channel attacks.

[tomaka]

Can we use ed25519-dalek only when targeting wasm? I suspect that ring is faster, though I don’t have benchmarks. ring also is more likely to resist side-channel attacks.

I could do that if that's the general feeling.

[tomaka]

Can we use ed25519-dalek only when targeting wasm? I suspect that ring is faster, though I don’t have benchmarks. ring also is more likely to resist side-channel attacks.

I pushed another commit that does that.
Depending on the feeling, I can remove this commit or keep it.

[burdges]

It appears ring choose optimizations dynamically at runtime, which likely makes achieving good performance easier, while all dalek ecosystem crates including schnorrkel require compile time parameters for 64 bit architectures, AVX2, etc.

There are some benchmarks in https://lib.rs/crates/ed25519-dalek but ed25519 benchmarks do not yet exist in https://github.com/briansmith/crypto-bench and thus dalek does not appear there. We should also switch to batch verification eventually, which ring does not support but should boost performance by like 40%.

[Demi-Marie]

We should also switch to batch verification eventually, which ring does not support but should boost performance by like 40%.

That can (and should) be added to ring.

[tomaka]

After some discussion in the parity-retreat chat, we've agreed that ed25519-dalek should be robust and that it's fine to replace ring with it. I've removed the commit that puts them side-by-side.

[tomaka]

I took the time to write some benchmarks.

With ring:

• Signing a 1MiB message: ~3.9ms
• Verifying a 1MiB message: ~2.0ms

With ed25519-dalek:

• Signing a 1MiB message: ~6.1ms
• Verifying a 1MiB message: ~3.1ms

[tomaka]

I added multiple payload sizes. With ring:

signing - ed25519/32    time:   [23.411 us 23.624 us 23.869 us]                                  
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) high mild
  1 (1.00%) high severe
signing - ed25519/1024  time:   [28.442 us 28.849 us 29.335 us]                                    
Found 4 outliers among 100 measurements (4.00%)
  2 (2.00%) high mild
  2 (2.00%) high severe
signing - ed25519/1048576                                                                             
                        time:   [4.4844 ms 4.5368 ms 4.5922 ms]
Found 7 outliers among 100 measurements (7.00%)
  5 (5.00%) high mild
  2 (2.00%) high severe

verifying - ed25519/32  time:   [62.375 us 62.517 us 62.664 us]                                   
Found 9 outliers among 100 measurements (9.00%)
  4 (4.00%) high mild
  5 (5.00%) high severe
verifying - ed25519/1024                                                                            
                        time:   [64.546 us 64.865 us 65.357 us]
Found 9 outliers among 100 measurements (9.00%)
  4 (4.00%) high mild
  5 (5.00%) high severe
verifying - ed25519/1048576                                                                             
                        time:   [2.1488 ms 2.1567 ms 2.1673 ms]
Found 5 outliers among 100 measurements (5.00%)
  2 (2.00%) high mild
  3 (3.00%) high severe

With ed25519-dalek (compared to ring):

signing - ed25519/32    time:   [24.820 us 25.124 us 25.500 us]                                  
                        change: [+5.4847% +8.2287% +10.885%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 11 outliers among 100 measurements (11.00%)
  1 (1.00%) high mild
  10 (10.00%) high severe
signing - ed25519/1024  time:   [30.901 us 31.250 us 31.642 us]                                    
                        change: [+5.2411% +6.9521% +8.6778%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 8 outliers among 100 measurements (8.00%)
  6 (6.00%) high mild
  2 (2.00%) high severe
signing - ed25519/1048576                                                                             
                        time:   [6.3128 ms 6.3748 ms 6.4490 ms]
                        change: [+45.076% +48.796% +52.329%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 2 outliers among 100 measurements (2.00%)
  2 (2.00%) high mild

verifying - ed25519/32  time:   [66.867 us 67.141 us 67.467 us]                                   
                        change: [+3.6526% +5.8532% +7.5132%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 6 outliers among 100 measurements (6.00%)
  2 (2.00%) high mild
  4 (4.00%) high severe
verifying - ed25519/1024                                                                            
                        time:   [68.176 us 68.406 us 68.643 us]
                        change: [+4.4798% +5.1915% +5.9339%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 5 outliers among 100 measurements (5.00%)
  1 (1.00%) low mild
  1 (1.00%) high mild
  3 (3.00%) high severe
verifying - ed25519/1048576                                                                             
                        time:   [3.1123 ms 3.1365 ms 3.1659 ms]
                        change: [+42.348% +43.633% +44.843%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 10 outliers among 100 measurements (10.00%)
  3 (3.00%) high mild
  7 (7.00%) high severe

[burdges]

I suspect the hashing impacts this significantly for either ring or dalek to reach those signing times. It appears ring does not support the ed25519 prehashed variants, but if larger messages are first hashed with blake2s then we'd see more the actual signing times. It's maybe work adding a 32 byte variant to these benchmarks.

Also, I'm afraid schnorrkel's SigningTranscript::bytes method might be rather slow. I vaguely recall substrate hashing everything using blake2s before passing it into schnorrkel, but not sure anymore. See w3f/schnorrkel#33

[tomaka]

It's maybe work adding a 32 byte variant to these benchmarks.

That's what I did!

Signing 32 bytes: 25.124us (dalek) vs 23.624us (ring)
Verifying 32 bytes: 67.141us (dalek) vs 62.517us (ring)

As you mention, for the other lengths I'm probably benchmarking the hashing function more than the signing/verifying.

@folsen's opinion is basically that we should merge that, and if any performance issue arises, then fix them. But let's not pre-occupy ourselves with performances too much at this time. I agree with that.

[burdges]

Oops I missed that, thanks! Yes, we can optimize later since we already know the target performance numbers. If dalek saves you some dev time with WASM then just use it. :)

[burdges]

Also, we should mark our two big performance TODOs as being:

• Improve hashing performance by using whatever native code ring uses.
• Switch to batch verification, whatever code base we use.

[burdges]

I'll summarize the situation as I understand it:

I have not quite understood if/when WASM invokes native signature verification code, but if we need ed25519 implemented in WASM, TEEs, etc. then ed25519-dalek works better for the foreseeable future.

We expect some traits for ed25519 signatures to emerge based on dalek-cryptography/ed25519-dalek#80 which might improve HSM support for many projects, although our lives sound more complex there.

Right now, ring has a faster sha2 than the sha2 crate used by ed25519-dalek (RustCrypto project). Ideally, one might simply fix the sha2 crate, but maybe their traits create the issue, ala NRVO or whatever. It's clear one could substitute an ed25519-crate with the sha2 manually replaced throughout though, dramatically reducing the performance difference. We do not care much if everything gets hashed by bake2b first anyways.

Also, curve25519-dalek has Pippenger multiscalar multiplication dalek-cryptography/curve25519-dalek#129 dalek-cryptography/curve25519-dalek#249 which improves large batch verifications dramatically, but realistically ring will never adopt Pippenger, and likely never implement bath verification.