-
Notifications
You must be signed in to change notification settings - Fork 1k
[Add] GetRandom(MaxValue) to StdLib Contract
#3968
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
base: dev
Are you sure you want to change the base?
[Add] GetRandom(MaxValue) to StdLib Contract
#3968
Conversation
|
@shargon maybe you help me with and setting up hardfork. |
I will do it |
| var maxValueSize = BigInteger.Pow(2, maxValueBits); | ||
|
|
||
| var nthMask = (BigInteger.One << maxValueBits) - 1; | ||
| var randomProduct = maxValue * (engine.GetRandom() & nthMask); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The engine.GetRandom() returns 128-bit filled buffers.
So there is another idea:
- If the
maxValueis less than or equal to 128bit, generate a 128-bit random number(between [0, maxValue)) once. Like this: https://cs.opensource.google/go/go/+/master:src/math/rand/rand.go;l=120 - If the
maxValueis greater than 128bit, generate a random number(between [0, maxValue >> 128bit), i.e.rand1) once, andengine.GetRandom()once, the result isengine.GetRandom()& (rand1<< 128).
The operation maxValue * is not needed in this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bigger the number (and these numbers can be huge, if you look at the tests) the longer it takes to loop. So in this case (so it doesnt take long time) we only want to take no bigger than maxValue bits. That could be a number of 10 or Pow(100, 100). In other words its faster this way or else you run out of gas or takes to long. Also can be up to 256-bits.
roman-khimov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still need some time, likely can be simplified a bit.
| var s2 = BitConverter.ToUInt64(buffer[0..8]); | ||
| var s3 = BitConverter.ToUInt64(buffer[8..16]); | ||
|
|
||
| // Update PRNG state. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what this buys you. Either we have pseudorandom bits from murmur128 and trust it to have them or this doesn't change much.
|
| if (IsHardforkEnabled(Hardfork.HF_Aspidochelone)) | ||
| if (IsHardforkEnabled(Hardfork.HF_Faun)) | ||
| { | ||
| buffer = Cryptography.Helper.Murmur128(nonceData, ProtocolSettings.Network + random_times++); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
random_times is not defined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where did it go?
|
@roman-khimov when are you going to review this? |

Description
Adds two new methods to
StdLibcontract. Still usessyscallGetRandomfor getting ranges. UpdatedGetRandomsyscall to have 256 bit integer.Methods
Fixes #3817
Type of change
How Has This Been Tested?
Checklist: