Reimplement random buffer with new runtime api #82
Conversation
janedegtiareva
requested review from
MaksymZavershynskyi and
evgenykuzyakov
assembly/math.ts
Outdated
| // const lastValue = storage.contains(this._LAST_RANDOM_VALUE_KEY) ? storage.get<u32>(this._LAST_RANDOM_VALUE_KEY) : 0; | ||
| // runtime_api.random_seed(0); | ||
| // add random seed to last random value | ||
| const randomSeed = randomSeed(); |
There was a problem hiding this comment.
When this executed the first time we take random seed and add it to itself before hashing. Why do we need to do that?
Also it looks like it can overflow.
There was a problem hiding this comment.
good point, we can do without initializing with random seed.
There was a problem hiding this comment.
The overflow issue is still present. resultValue[i] + randomSeed[i] can overflow.
There was a problem hiding this comment.
why is that a problem for generating a random buffer?
There was a problem hiding this comment.
Overflowing is often UB and should be avoided in general. Do we know whether it is going to truncate it at 255, wrap around, take a random number, take some existing byte from the memory (e.g. neighboring byte)? It depends on AssemblyScript implementation and Wasm spec. Having deterministic UB is still bad. E.g. if it truncates it at 255 then this number is not that random and we will have a loop over the same small set of numbers instead of generating new numbers.
No description provided.