Correct BABE randomness by calculating InOut bytes directly in pallet

[sorpaas]

This is an alternative to #5788. Fixes #5785.

All Raw* types are removed, and within pallet it will use the types directly in schnorrkel. As a result, the validity of the VRF is now also checked within runtime. With this, we can then use schnorrkel's method to regenerate the InOut directly in the pallet.

[rphmeier]

this could be a map, right?

[burdges]

It could be folded into the previous and_then, no?

[sorpaas]

Yeah this could be a map. I'll change it.
Folded into previous and_then also works, but I think that will make the code more nested.

[rphmeier]

LGTM but should have a review from @burdges and @andresilva as well.

[burdges]

You wrote "the validity of the VRF is now also checked within runtime"? Where? Is it due to removing the distinction between PreDigest and RawPreDigest?

If I read https://github.com/paritytech/substrate/pull/5876/files#diff-7ed093bfcad10b5194ca35132db3dfeaR493-R503 correctly, you just make the VRFInOut without verifying. It's fine to do that assuming as you know it was verified elsewhere.

[sorpaas]

You wrote "the validity of the VRF is now also checked within runtime"? Where? Is it due to removing the distinction between PreDigest and RawPreDigest?

Yeah. So RawPreDigest/RawVRFOutput were just raw 32 byte values. They did not check whether things are valid VRF output or VRF proof. Right now the distinction of raw and non-raw is removed, so all checks happening in schnorrkel now also happens in pallet-babe.

[burdges]

All invocations of vrf_verify still happen in client/consensus/babe/src/verification.rs though, not in frame/babe/src, right? If so, you do not actually verify the VRF here, just compute its output, right? It's fine this way of course.

Can you explain the original distinction between PreDigest and RawPreDigest?

[burdges]

Wait. Are you saying that because the host now supplies the PreDigest and RawPreDigest that the host (not the runtime) much check the VRF again? That's fine too.

[sorpaas]

@burdges Yeah sorry if I wasn't entirely clear before. It's not the entire VRF check -- only the formatting check of those https://github.com/w3f/schnorrkel/blob/master/src/vrf.rs#L514

Previously those formatting check did not happen in runtime -- runtime accepted everything.

[burdges]

Cool. I'm satisfied that I'm not miss-reading the code. It seems fine. :)

I think this and #5788 looks pretty similar from my perspective, so whatever you guys prefer.

[andresilva]

too late but lgtm.