w3f / schnorrkel

Schnorr VRFs and signatures on the Ristretto group
BSD 3-Clause "New" or "Revised" License
315 stars 94 forks source link

Adapt to SURI missuse to ChainCode #45

Open burdges opened 5 years ago

burdges commented 5 years ago

We should swap the chain code and i arguments in https://github.com/paritytech/substrate/issues/3396 because kusama does it wrong.

I'd personally prefer if kusama had a separate key type that kept things wrong, while everything else move on to doing this correctly, and eventually deprecate kusama entirely. We might otoh "miss-label" the arguments passed to merlin here and simultaneously "fix" SURI.

burdges commented 5 years ago

Ain't quite so easy to fix this artfully since i gets added to the transcript in https://github.com/w3f/schnorrkel/blob/master/src/derive.rs#L68 and the chain code gets added in https://github.com/w3f/schnorrkel/blob/master/src/derive.rs#L85

We need to make derive_scalar_and_chaincode and Derivation::derived_key take only a SigningTranscript, add a separate ChainCode::append<T: SigningTranscript>(&self, t: &mut T) function, and make derive_key_simple call them in reverse order and give it an Option<ChainCode> argument that calls t.commit_bytes(CHAIN_CODE_LABEL,b"");. We also wind up with

// Don't ask!
const CHAIN_CODE_LABEL : &'static [u8] = b"sign-bytes";
const SIGN_BYTES_LABLE : &'static [u8] = b"chain-code";

In SURI, we need to pad i to a [u8;32] if less than 32 bytes. We do not need ChainCode to become a Borrow<[u8]> like I originally thought.

In the end, we make chain code less intuitive to use, but maybe make them less likely to missuse too, so meh no biggie.