Add secure random range reference

README.md

@@ -71,7 +71,7 @@ Given that Flow has both Cadence and EVM runtimes, commit-reveal patterns coveri
 |---|---|---|
 |[CoinToss.cdc](./contracts/CoinToss.cdc)|[0xd1299e755e8be5e7](https://contractbrowser.com/A.d1299e755e8be5e7.CoinToss)|N/A|
 |[Xorshift128plus.cdc](./contracts/Xorshift128plus.cdc)|[0xed24dbe901028c5c](https://contractbrowser.com/A.ed24dbe901028c5c.Xorshift128plus)|[0x45caec600164c9e6](https://contractbrowser.com/A.45caec600164c9e6.Xorshift128plus)|
-|[CoinToss.sol](./contracts/CoinToss.sol)|[0x5FC8d32690cc91D4c39d9d3abcBD16989F875707](https://evm-testnet.flowscan.io/address/0x5FC8d32690cc91D4c39d9d3abcBD16989F875707?tab=contract_code)|N/A|
+|[CoinToss.sol](./contracts/CoinToss.sol)|[0x959922bE3CAee4b8Cd9a407cc3ac1C251C2007B1](https://evm-testnet.flowscan.io/address/0x959922bE3CAee4b8Cd9a407cc3ac1C251C2007B1?tab=contract_code)|N/A|
 
 ## Further Reading
 

solidity/src/CadenceArchWrapper.sol → solidity/src/CadenceArchUtils.sol

@@ -6,7 +6,7 @@ pragma solidity 0.8.19;
  * contracts can use this contract to fetch the current Flow block height and fetch random numbers from the Cadence
  * runtime.
  */
-abstract contract CadenceArchWrapper {
+library CadenceArchUtils {
     // Cadence Arch pre-compile address
     address public constant cadenceArch = 0x0000000000000000000000010000000000000001;
 
@@ -45,17 +45,13 @@ abstract contract CadenceArchWrapper {
      * @param flowHeight The Flow block height for which to get the random source.
      * @return randomSource The random source for the given Flow block height.
      */
-    function _getRandomSource(uint64 flowHeight) internal view returns (uint64) {
+    function _getRandomSource(uint64 flowHeight) internal view returns (bytes32) {
         (bool ok, bytes memory data) =
             cadenceArch.staticcall(abi.encodeWithSignature("getRandomSource(uint64)", flowHeight));
         require(ok, "Unsuccessful call to Cadence Arch pre-compile when fetching random source");
 
-        // Decode the result as bytes32 and then cast it to uint64
+        // Decode the result as bytes32 & return
         bytes32 result = abi.decode(data, (bytes32));
-
-        // Convert the bytes32 result to uint64 by casting, taking the least significant 8 bytes
-        uint64 output = uint64(uint256(result));
-
-        return output;
+        return result;
     }
 }

solidity/src/CadenceRandomConsumer.sol

@@ -1,31 +1,85 @@
 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.19;
 
-import {CadenceArchWrapper} from "./CadenceArchWrapper.sol";
+import {CadenceArchUtils} from "./CadenceArchUtils.sol";
+import {Xorshift128plus} from "./Xorshift128plus.sol";
 
 /**
  * @dev This contract is a base contract for secure consumption of Flow's protocol-native randomness via the Cadence
  * Arch pre-compile. Implementing contracts benefit from the commit-reveal scheme below, ensuring that callers cannot
  * revert on undesirable random results.
  */
-abstract contract CadenceRandomConsumer is CadenceArchWrapper {
+abstract contract CadenceRandomConsumer {
+    using Xorshift128plus for Xorshift128plus.PRG;
+
     // A struct to store the request details
     struct Request {
         // The Flow block height at which the request was made
         uint64 flowHeight;
         // The EVM block height at which the request was made
         uint256 evmHeight;
+        // Whether the request has been fulfilled
+        bool fulfilled;
     }
 
     // Events
-    event RandomnessRequested(uint256 requestId, uint64 flowHeight, uint256 evmHeight);
-    event RandomnessFulfilled(uint256 requestId, uint64 flowHeight, uint256 evmHeight, uint256 randomResult);
+    event RandomnessRequested(uint256 indexed requestId, uint64 flowHeight, uint256 evmHeight);
+    event RandomnessSourced(uint256 indexed requestId, uint64 flowHeight, uint256 evmHeight, bytes32 randomSource);
+    event RandomnessFulfilled(uint256 indexed requestId, uint64 randomResult);
 
     // A list of requests where each request is identified by its index in the array
     Request[] private _requests;
     // A counter to keep track of the number of requests made, serving as the request ID
     uint256 private _requestCounter;
 
+    ///////////////////
+    // PUBLIC FUNCTIONS
+    ///////////////////
+
+    /**
+     * @dev This method checks if a request can be fulfilled.
+     *
+     * @param requestId The ID of the randomness request to check.
+     * @return canFulfill Whether the request can be fulfilled.
+     */
+    function canFulfillRequest(uint256 requestId) public view returns (bool) {
+        uint256 requestIndex = requestId - 1;
+        if (requestIndex >= _requests.length) {
+            return false;
+        }
+        Request storage request = _requests[requestIndex];
+        uint64 flowHeight = CadenceArchUtils._flowBlockHeight();
+        return !request.fulfilled && request.flowHeight < flowHeight;
+    }
+
+    /////////////////////
+    // INTERNAL FUNCTIONS
+    /////////////////////
+
+    /**
+     * @dev This method returns a ***REVERTIBLE** random number in the range [min, max].
+     * NOTE: The fact that this method is revertible means that it should only be used in cases where the initiating
+     * caller is trusted not to revert on the result.
+     *
+     * @param min The minimum value of the range (inclusive).
+     * @param max The maximum value of the range (inclusive).
+     * @return random The random number in the range [min, max].
+     */
+    function _getRevertibleRandomInRange(uint64 min, uint64 max) internal view returns (uint64) {
+        bytes memory seed = abi.encodePacked(_aggregateRevertibleRandom256());
+        bytes memory salt = abi.encodePacked(block.number);
+
+        // Instantiate a PRG with the aggregate bytes and salt with current block number
+        Xorshift128plus.PRG memory prg;
+        prg.seed(seed, salt);
+
+        return _getNumberInRange(prg, min, max);
+    }
+
+    /**
+     * ----- COMMIT STEP -----
+     */
+
     /**
      * @dev This method serves as the commit step in this contract's commit-reveal scheme
      *
@@ -45,7 +99,7 @@ abstract contract CadenceRandomConsumer is CadenceArchWrapper {
         // Store the heights at which the request was made. Note that the Flow block height and EVM block height are
         // not the same. But since Flow blocks ultimately determine usage of secure randomness, we gate requests by
         // Flow block height.
-        Request memory request = Request(_flowBlockHeight(), block.number);
+        Request memory request = Request(CadenceArchUtils._flowBlockHeight(), block.number, false);
 
         // Store the request in the list of requests
         _requests.push(request);
@@ -56,6 +110,106 @@ abstract contract CadenceRandomConsumer is CadenceArchWrapper {
         return requestId;
     }
 
+    /**
+     * ----- REVEAL STEP -----
+     */
+
+    /**
+     * @dev This method fulfills a random request and returns a random number as a uint64.
+     *
+     * @param requestId The ID of the randomness request to fulfill.
+     * @return randomResult The random number.
+     */
+    function _fulfillRandomRequest(uint256 requestId) internal returns (uint64) {
+        bytes memory seed = abi.encodePacked(_fulfillRandomness(requestId));
+        bytes memory salt = abi.encodePacked(requestId);
+
+        // Instantiate a PRG, seeding with the random value and salting with the request ID
+        Xorshift128plus.PRG memory prg;
+        prg.seed(seed, salt);
+
+        uint64 randomResult = prg.nextUInt64();
+
+        emit RandomnessFulfilled(requestId, randomResult);
+
+        return randomResult;
+    }
+
+    /**
+     * @dev This method fulfills a random request and safely returns an unbiased random number in the range inclusive [min, max].
+     *
+     * @param requestId The ID of the randomness request to fulfill.
+     * @param min The minimum value of the range (inclusive).
+     * @param max The maximum value of the range (inclusive).
+     * @return randomResult The random number in the inclusive range [min, max].
+     */
+    function _fulfillRandomInRange(uint256 requestId, uint64 min, uint64 max) internal returns (uint64) {
+        // Ensure that the request is fulfilled at a Flow block height greater than the one at which the request was made
+        // Get the random source for the Flow block at which the request was made
+        bytes memory seed = abi.encodePacked(_fulfillRandomness(requestId));
+        bytes memory salt = abi.encodePacked(requestId);
+
+        // Instantiate a PRG with the random source and the request ID
+        Xorshift128plus.PRG memory prg;
+        prg.seed(seed, salt);
+
+        uint64 randomResult = _getNumberInRange(prg, min, max); // Get a random number in the range [min, max]
+
+        emit RandomnessFulfilled(requestId, randomResult);
+
+        return randomResult;
+    }
+
+    ////////////////////
+    // PRIVATE FUNCTIONS
+    ////////////////////
+
+    /**
+     * @dev This method returns a number in the range [min, max] from the given value with a variation on rejection
+     * sampling.
+     * NOTE: You may be tempted to simply use `value % (max - min + 1)` to get a number in a range. However, this
+     * method is not secure is susceptible to the modulo bias. This method provides an unbiased alternative for secure
+     * secure use of randomness.
+     *
+     * @param prg The PRG to use for generating the random number.
+     * @param min The minimum value of the range (inclusive).
+     * @param max The maximum value of the range (inclusive).
+     * @return random The random number in the range [min, max].
+     */
+    function _getNumberInRange(Xorshift128plus.PRG memory prg, uint64 min, uint64 max) private pure returns (uint64) {
+        require(max > min, "Max must be greater than min");
+
+        uint64 range = max - min;
+        uint64 bitsRequired = _mostSignificantBit(range); // Number of bits needed to cover the range
+        uint256 mask = (1 << bitsRequired) - 1; // Create a bitmask to extract relevant bits
+
+        uint256 shiftLimit = 256 / bitsRequired; // Number of shifts needed to cover 256 bits
+        uint256 shifts = 0; // Initialize shift counter
+
+        uint64 candidate = 0; // Initialize candidate
+        uint256 value = prg.nextUInt256(); // Assign the first 256 bits of randomness
+
+        while (true) {
+            candidate = uint64(value & mask); // Apply the bitmask to extract bits
+            if (candidate <= range) {
+                break; // Found a suitable candidate within the target range
+            }
+
+            // Shift by the number of bits covered by the mask
+            value = value >> bitsRequired;
+            shifts++;
+
+            // Get a new value if we've exhausted the current one
+            if (shifts == shiftLimit) {
+                value = prg.nextUInt256();
+                shifts = 0;
+            }
+        }
+
+        // Scale candidate to the range [min, max]
+        return min + candidate;
+    }
+
     /**
      * @dev This method serves as the reveal step in this contract's commit-reveal scheme
      *
@@ -69,27 +223,64 @@ abstract contract CadenceRandomConsumer is CadenceArchWrapper {
      * @param requestId The ID of the randomness request to fulfill.
      * @return randomResult The random value generated from the Flow block.
      */
-    function _fulfillRandomness(uint32 requestId) internal returns (uint64) {
+    function _fulfillRandomness(uint256 requestId) private returns (bytes32) {
         // Get the request details. Recall that request IDs are 1-indexed to allow for 0 to be used as an invalid value
-        uint32 requestIndex = requestId - 1;
+        uint256 requestIndex = requestId - 1;
         require(requestIndex < _requests.length, "Invalid request ID - value exceeds the number of existing requests");
-        Request memory request = _requests[requestIndex];
 
-        // Ensure that the request is fulfilled at a Flow block height greater than the one at which the request was made
-        uint64 flowHeight = _flowBlockHeight();
-        require(request.flowHeight < flowHeight, "Cannot fulfill request until subsequent Flow network block height");
+        // Access & validate the request
+        Request storage request = _requests[requestIndex];
+        _validateRequest(request);
+        request.fulfilled = true; // Mark the request as fulfilled
 
-        // Get the random source for the Flow block at which the request was made
-        uint64 randomResult = _getRandomSource(request.flowHeight); // returns bytes32
-        // Pack the randomResult into a uint64, hashing with the requestId to vary results across shared block heights.
-        // The random seed returned from Cadence Arch is only 32 bytes. Here only 8 bytes were required, but if an
-        // implementing contract requires more random bytes, the source should be expanded into any arbitrary number of
-        // bytes using a PRG.
-        randomResult = uint64(uint256(keccak256(abi.encodePacked(randomResult, requestId))));
+        // Get the random source for the Flow block at which the request was made, emit & return
+        bytes32 randomSource = CadenceArchUtils._getRandomSource(request.flowHeight);
 
-        emit RandomnessFulfilled(requestId, request.flowHeight, request.evmHeight, randomResult);
+        emit RandomnessSourced(requestId, request.flowHeight, request.evmHeight, randomSource);
 
-        // Return the random result
-        return randomResult;
+        return randomSource;
+    }
+
+    /**
+     * @dev This method aggregates 256 bits of randomness by calling _revertibleRandom() 4 times.
+     *
+     * @return randomValue The aggregated 256 bits of randomness.
+     */
+    function _aggregateRevertibleRandom256() private view returns (uint256) {
+        // Call _revertibleRandom() 4 times to aggregate 256 bits of randomness
+        uint256 randomValue = uint256(CadenceArchUtils._revertibleRandom());
+        randomValue |= (uint256(CadenceArchUtils._revertibleRandom()) << 64);
+        randomValue |= (uint256(CadenceArchUtils._revertibleRandom()) << 128);
+        randomValue |= (uint256(CadenceArchUtils._revertibleRandom()) << 192);
+        return randomValue;
+    }
+
+    /**
+     * @dev This method returns the most significant bit of a uint64.
+     *
+     * @param x The input value.
+     * @return bits The most significant bit of the input value.
+     */
+    function _mostSignificantBit(uint64 x) private pure returns (uint64) {
+        uint64 bits = 0;
+        while (x > 0) {
+            x >>= 1;
+            bits++;
+        }
+        return bits;
+    }
+
+    /**
+     * @dev This method validates a given request, ensuring that it has not been fulfilled and that the Flow block height
+     * has passed.
+     *
+     * @param request The request to validate.
+     */
+    function _validateRequest(Request storage request) private view {
+        require(!request.fulfilled, "Request already fulfilled");
+        require(
+            request.flowHeight < CadenceArchUtils._flowBlockHeight(),
+            "Cannot fulfill request until subsequent Flow network block height"
+        );
     }
 }

solidity/src/CoinToss.sol

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.19;
 
-import "./CadenceRandomConsumer.sol";
+import {CadenceRandomConsumer} from "./CadenceRandomConsumer.sol";
 
 /**
  * @dev This contract is a simple coin toss game where users can place win prizes by flipping a coin as a demonstration
@@ -16,8 +16,8 @@ contract CoinToss is CadenceRandomConsumer {
     // A mapping to store the value sent by the user for each request
     mapping(uint256 => uint256) public openRequests;
 
-    event CoinFlipped(address indexed user, uint256 requestId, uint256 amount);
-    event CoinRevealed(address indexed user, uint256 requestId, uint8 coinFace, uint256 prize);
+    event CoinFlipped(address indexed user, uint256 indexed requestId, uint256 amount);
+    event CoinRevealed(address indexed user, uint256 indexed requestId, uint8 coinFace, uint256 prize);
 
     /**
      * @dev Checks if a user has an open request.
@@ -54,9 +54,10 @@ contract CoinToss is CadenceRandomConsumer {
         // delete the open request from the coinTosses mapping
         delete coinTosses[msg.sender];
 
-        // fulfill the random request
-        uint64 randomResult = _fulfillRandomness(uint32(requestId));
-        uint8 coinFace = uint8(randomResult % 2);
+        // fulfill the random request within the inclusive range [0, 1]
+        // NOTE: Could use % 2 without risk of modulo bias since the range is a multiple of the modulus
+        //  but using _fulfillRandomInRange for demonstration purposes
+        uint8 coinFace = uint8(_fulfillRandomInRange(requestId, 0, 1));
 
         // get the value sent in the flipCoin function & remove the request from the openRequests mapping
         uint256 amount = openRequests[requestId];