Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add example random in range function #21

Closed
sisyphusSmiling opened this issue Sep 21, 2024 · 2 comments · Fixed by #22
Closed

Add example random in range function #21

sisyphusSmiling opened this issue Sep 21, 2024 · 2 comments · Fixed by #22
Assignees
Labels
Documentation Improvements or additions to documentation Improvement

Comments

@sisyphusSmiling
Copy link
Contributor

sisyphusSmiling commented Sep 21, 2024

Description

Recent discussion in onflow/docs#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.

@sisyphusSmiling sisyphusSmiling self-assigned this Sep 21, 2024
@sisyphusSmiling sisyphusSmiling added Improvement Documentation Improvements or additions to documentation labels Sep 21, 2024
@sisyphusSmiling sisyphusSmiling moved this to 🏗 In Progress in 🌊 Flow 4D Sep 21, 2024
@tarakby
Copy link
Collaborator

tarakby commented Sep 21, 2024

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:

  • Cadence's 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[
  • Cadence's revertibleRandom<T> with an argument n implements the non-biased method of rejection sampling, and minimizes the number of iterations.
  • the EVM arch only exports the 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.
  • this won't be a breaking change for the EVM side, 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".

@sisyphusSmiling
Copy link
Contributor Author

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.

@github-project-automation github-project-automation bot moved this from 🏗 In Progress to ✅ Done in 🌊 Flow 4D Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Improvements or additions to documentation Improvement
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants