Closed sisyphusSmiling closed 1 month ago
Very cool! Thanks for taking the initiative to add a reference method for developers, and very quickly after the feedback 🙏🏼
Before I look at the solidity implementation, I was wondering if an alternative "simpler" way could work: what if we update the EVM arch of revertibleRandom
to export the modulo behaviour from Cadence. I explain:
revertibleRandom<T>
can be called without an argument, or with an argument. If an argument n
is provided, the function returns a random in [0..n[
revertibleRandom<T>
with an argument n
implements the non-biased method of rejection sampling, and minimizes the number of iterations.revertibleRandom<uint64>()
version, without arguments. While it should be fine to export T
as uint64
only, it may make sense to also export the argument modulo behaviour.revertibleRandom()
would still work as before. We just add a new behaviour revertibleRandom(n)
this may be safer for developers, instead of implementing their own methods in solidity (the same safety argument we used on the Cadence side)
This said, the idea only makes sense if this is an easy add, I have no idea how Arch functions work, and how much effort we need to make the change. My wishful is thinking is that this is "just an argument addition".
I think this is the ideal solution @tarakby. It would be much easier to implement a "random range" function from the EVM side if it was simply abstracted in Cadence Arch. I'd be curious to hear thoughts from those more familiar with Arch precompile calls - cc @ramtinms @m-Peter
While I'm not deeply familiar with Cadence Arch, I'm fairly sure the change would require a network upgrade with the next one slated in the order of months. If that is in fact the case, IMO we should still provide some workable solution for acquiring a random number in range which would fall in the realm of a solidity implementation.
Description
Recent discussion in https://github.com/onflow/docs/pull/902 reveals the need for a safe demonstration of generating a random number in a range. @tarakby listed a couple of feasible approaches that can be taken for this implementation. This functionality can be added to the existing
CadencRandomConsumer.sol
base contract for easy consumption in the context of randomness.This implementation should then be added to the docs as reference.