Update to schnorrkel 0.8.0

[bkchr]

CC @burdges

[burdges]

It looks good! Just one new signature test vector, but that's fine for a new network like kusama.

We'll need to add more test vectors into schnorkel itself of course. w3f/schnorrkel#36

Just fyi, w3f/schnorrkel#9 contains my internal debate about using ed25519 style secret key derivation, never a clear decision.

[jacogr]

Derives are different, i.e 0.8

$ cargo run --release -- inspect "//Alice"
    Finished release [optimized] target(s) in 3.14s
     Running `.../target/release/subkey inspect //Alice`
Secret Key URI `//Alice` is account:
  Public key (hex): 0xe2d5259549e73bffe82a6132d0bb4b6ca9fa8d78ef063c4b58d28da7ec319767
  Address (SS58): 5HC7zmvyyESMevkCTJi7YQnPwujJc1d5K5Bya5kJAtxgSQn7

vs 0.1.1

$ cargo run --release -- inspect "//Alice"
    Finished release [optimized] target(s) in 1.52s
     Running .../target/release/subkey inspect //Alice`
Secret Key URI `//Alice` is account:
  Public key (hex): 0xd43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d
  Address (SS58): 5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY

Update: Solved with 0.8.3, also has tests for both pair hard & soft derivations now

[jacogr]

So not 100% sure I should "approve" since I did some, however can "comment"...

lgtm, with the extended tests it does cater for checking the derives (against known) and well as older sigs as well, which are the 2 major compatibility pain-points in the 0.8 bump. (And has ripple-effects into the ecosystem, e.g. wallets and already generated addresses)

[jacogr]

One thing to be aware of (which is biting me on the JS-side). When storing a keypair from 0.1.1 (i.e. the secretKey/publicKey or just the combined 96-bytes) and trying to import that via the new schnorrkel, well, it is not that happy.

For any minisecret it would seem the pair derivation yields the same publicKey, but the secretKey part of the generated bundle is not the same. As an example for the same values...

0.1.1 -

sk: 28b0ae221c6bb06856b287f60d7ea0d98552ea5a16db16956849aa371db3eb51fd190cce74df356432b410bd64682309d6dedb27c76845daf388557cbac3ca34
pk: 46ebddef8cd9bb167dc30878d7113b7e168e6f0646beffd77d69d39bad76b47a

0.8.3 -

sk: 05d65584630d16cd4af6d0bec10f34bb504a5dcb62dba2122d49f5a663763d0afd190cce74df356432b410bd64682309d6dedb27c76845daf388557cbac3ca34
pk: 46ebddef8cd9bb167dc30878d7113b7e168e6f0646beffd77d69d39bad76b47a

[jacogr]

There is a way around this which is employed in the polkadot-js/wasm to remain compatible with pairs created/stored on exiting wallets. (Basically using the Keypair from_half_ed25519 and to_half_ed25519_bytes)

If required here, it should be available in 0.8.4 when that is published.

[kianenigma]

This looks to be good as far as I have talked around: The only case where something might go wrong is if: similar to wallet restore some keys are generated, dumped, and need to be later restored. Looks like we don't have this.

Hence, this PR should be good to be merged and any API should put some effort into supporting networks prior to this (i.e. Alex).

[gavofyork]

@burdges @jacogr do we have test vectors for both hard and soft derivation? i don't want people with derived kusama keys (like me :-) ) to end up with different addresses.

Code looks like it, but just want to be sure :)

[jacogr]

@gavofyork I added those since @bkchr suggested them (they were massaged into shape by @kianenigma). The only difference between them and the existing derivation tests are that they actually check the resulting public keys against known, which previously was missing.

If the pair serialization format is important, we should certainly add tests for checking against known for those as well, those are different here to the current 0.1.1 formats. (It is important for wallets, hence the WASM has extended tests for those in addition to the public key parts catered for here)

For the w3f/schnorrkel repo itself, similar tests are now the code as comments, however the test vectors there are WIP.

[burdges]

If you use ExpandionMode::Ed25519 then all derivations should now agree with 0.1.1, and I think @jacogr old test vectors confirm this. I'd suggest running more tests with a version of subkey using 0.8.4 of course. ;)

We've a very minor change in the serialization format for expanded secret keys, but the from_*_ed25519_bytes methods do the old method. A deserializer can test if the public key still agrees to figure out if you're deserializating the old or new format, but any code compiled with 0.1.1 cannot deserialize the new keys.

I've one change w3f/schnorrkel@38f726e in derive.rs since 0.8.4 but it only impacts the nonce of the derived key, and thus cannot impact key or signature validity. In fact, it makes it possible to write a full test vector including the nonce seed using attach_rng. :)