PRNG Library with Pyth Entropy Integration #1733
Conversation
|
@baileynottingham is attempting to deploy a commit to the pyth-web Team on Vercel. A member of the Team first needs to authorize it. |
| /// @notice Generate a random byte between 0 and 255 | ||
| /// @param seed The Pyth Entropy seed (bytes32) | ||
| /// @return A random uint8 value | ||
| function randomByte(bytes32 seed) internal pure returns (uint8) { |
There was a problem hiding this comment.
@baileynottingham can you think of a way to ensure that the user of this library doesn't accidentally pass the same Entropy seed to multiple of these random functions? E.g., the following code would be easy to write but also a security mistake:
bytes32 entropySeed = ... // get from entropy
unit8 rand = PRNG.randomByte(entropySeed);
uint256 rand2 = PRNG.randInt(entropySeed);
In most programming languages, a PRNG has internal state to track the current seed. Each time the user calls a function requesting randomness, the PRNG automatically updates that state.
There was a problem hiding this comment.
Thank you for your input. I have updated the code to be a contract, allowing it to store an internal seed and nonce. The nextBytes32() function now returns a value and updates the nonce. This function is utilized internally by all other functions, ensuring that the nonce is consistently updated. As a result, the following code works correctly and returns a different number each time:
uint256 randomNumber1 = randUint();
uint256 randomNumber2 = randUint();
uint256 randomNumber3= randUint();
uint256 randomNumber4 = randUint();
| /// @notice Convert bytes32 to uint256 | ||
| /// @param value The bytes32 value to convert | ||
| /// @return The converted uint256 value | ||
| function toUint(bytes32 value) internal pure returns (uint256) { |
There was a problem hiding this comment.
Instead of type conversion functions, I would rather see randUint, randUint64 similar to the existing randInt method. The user will know how to do type conversions in solidity (imo).
| /// @param seed The Pyth Entropy seed (bytes32) | ||
| /// @param numHashes The number of random bytes32 values to generate | ||
| /// @return An array of random bytes32 values | ||
| function expandRandomness( |
There was a problem hiding this comment.
(if the PRNG had internal state, then this method could be replaced by the user simply calling randBytes32 repeatedly)
There was a problem hiding this comment.
I like the approach here. I left a couple minor comments. One ask though is for some unit tests.
I realize it's hard to test random code. I think the best approach is probably to hardcode the behavior you expect for a couple cases, just to validate that (1) the code runs, and (2) simple stuff like calling nextUint() repeatedly returns different values.
You can put your unit tests here https://github.com/pyth-network/pyth-crosschain/tree/main/target_chains/ethereum/contracts/forge-test and they will run with the rest of the contract tests.
|
|
||
| /// @notice Generate a random uint256 value within a specified range | ||
| /// @param min The minimum value (inclusive) | ||
| /// @param max The maximum value (inclusive) |
There was a problem hiding this comment.
I think the max should be exclusive, as that's the usual programming convention for ranges
There was a problem hiding this comment.
Can you also document that the result is uniformly distributed between min and max, with a slight bias toward lower numbers that is insignificant as long as (max - min) << MAX_UINT256
(if you consider the case where you put in max = MAX_UINT256 - 1 and min = 0, then 0 has 2x the probability of being chosen as any other number)
There was a problem hiding this comment.
Updated max to be exclusive and added your note in the docstring
| /// @param length The number of random bytes to generate (max 32) | ||
| /// @return A bytes array of random values | ||
| function randomBytes(uint256 length) internal returns (bytes memory) { | ||
| require(length <= 32, "Length must be 32 or less"); |
There was a problem hiding this comment.
if this is limited to 32 bytes then this seems kind of redundant with the random bytes 32 method above. Maybe it's fine to delete and let users use the bytes32 version directly.
There was a problem hiding this comment.
Yeah after thinking about this again, I agree it is redundant now with the nextBytes32() method. I went ahead and deleted it
| bytes32 randomness = nextBytes32(); | ||
| bytes memory result = new bytes(length); | ||
| assembly { | ||
| mstore(add(result, 32), randomness) |
There was a problem hiding this comment.
is this right? if length < 32, isn't the result array shorter than the value you're trying to write into it?
There was a problem hiding this comment.
Not applicable anymore since this is removed
|
Tests have been added! |
This pull request introduces a new Pseudorandom Number Generation (PRNG) library designed to work seamlessly with Pyth Entropy. The library provides a suite of functions for generating, manipulating, and converting random numbers from a Pyth Entropy seed.
Key Features: