fix BABE randomness calculation #3305
Conversation
| let segment_idx: u32 = <SegmentIndex>::mutate(|s| rstd::mem::replace(s, 0)); | ||
|
|
||
| // overestimate to the segment being full. | ||
| let rho_size = segment_idx.saturating_add(1) as usize * UNDER_CONSTRUCTION_SEGMENT_LENGTH; |
There was a problem hiding this comment.
| let rho_size = segment_idx.saturating_add(1) as usize * UNDER_CONSTRUCTION_SEGMENT_LENGTH; | |
| let rho_size = (segment_idx.saturating_add(1) as usize).saturating_mul(UNDER_CONSTRUCTION_SEGMENT_LENGTH); |
| epoch_index: u64, | ||
| rho: [u8; VRF_OUTPUT_LENGTH], | ||
| rho: impl Iterator<Item=[u8; VRF_OUTPUT_LENGTH]>, | ||
| rho_size_hint: Option<usize>, |
There was a problem hiding this comment.
I thought usize is not allowed due to different size between wasm and native build?
There was a problem hiding this comment.
It is not allowed to be shared between WASM and native, but otherwise you can use it. The size of a vector can not be greater than usize::max anyway.
There was a problem hiding this comment.
yes, that's right -- this isn't exposed anywhere and it's only used to do memory allocation (in which case it is the right unit). the hint is also purely for optimization - it being too large or small shouldn't change the effect of the code
There was a problem hiding this comment.
Interesting, can wasm's usize differ when running on different platforms? If so, we might advise only to run on 64 bit platforms, so no bad parachain code can leak usize::MAX somehow.
There was a problem hiding this comment.
Wasm is currently always only 32bit. Rho can only store 2^32 elments at max.
| s.extend_from_slice(&vrf_output[..]); | ||
| } | ||
|
|
||
| runtime_io::blake2_256(&s) |
There was a problem hiding this comment.
I'm just curious: We do not hash incrementally in situations like this because repeatedly crossing the wasm boundary costs more than an allocation?
|
I like this much more than xor. In future, if it improves anything like avoiding fetches from storage, then we can always change the spec to include some |
|
@burdges yeah, i thought of doing that as well. it is more hashes in the end, but that might be a good alternative. |
|
It's one extra hash and the block header being 32 bytes larger vs an extra fetch from storage. It's minor anyways. :) |
Follow the spec: Use the concatenation of VRF outputs within the epoch as opposed to the XOR.