chore: audit revision (#90)

* [WP-M1]: avoid sleep on successful fetchDrandResponse * [WP-S2]: cache drand client * [WP-I4]: catch reverts for call to consumer * [WP-S3]: track requested randomness timestamp * chore: reduce sleep time between fetchDrandResponse retries
gelatodigital · Nov 20, 2023 · f5c02fc · f5c02fc
1 parent ccf4134
commit f5c02fc
Show file tree

Hide file tree

Showing 6 changed files with 98 additions and 48 deletions.
diff --git a/contracts/GelatoVRFConsumerBase.sol b/contracts/GelatoVRFConsumerBase.sol
@@ -12,6 +12,7 @@ import {IGelatoVRFConsumer} from "contracts/IGelatoVRFConsumer.sol";
 /// For security considerations, refer to the Gelato documentation.
 abstract contract GelatoVRFConsumerBase is IGelatoVRFConsumer {
     bool[] public requestPending;
+    mapping(uint64 requestId => bytes32 requestHash) public requestedHash;
 
     /// @notice Returns the address of the dedicated msg.sender.
     /// @dev The operator can be found on the Gelato dashboard after a VRF is deployed.
@@ -30,7 +31,14 @@ abstract contract GelatoVRFConsumerBase is IGelatoVRFConsumer {
         requestId = uint64(requestPending.length);
         requestPending.push();
         requestPending[requestId] = true;
+
         bytes memory data = abi.encode(requestId, extraData);
+        // solhint-disable-next-line not-rely-on-time
+        bytes memory dataWithTimestamp = abi.encode(data, block.timestamp);
+        bytes32 requestHash = keccak256(dataWithTimestamp);
+
+        requestedHash[requestId] = requestHash;
+
         emit RequestedRandomness(data);
     }
 
@@ -47,22 +55,34 @@ abstract contract GelatoVRFConsumerBase is IGelatoVRFConsumer {
     /// @notice Callback function used by Gelato VRF to return the random number.
     /// The randomness is derived by hashing the provided randomness with the request ID.
     /// @param randomness The random number generated by Gelato VRF.
-    /// @param data Additional data provided by Gelato VRF, typically containing request details.
+    /// @param dataWithTimestamp Additional data provided by Gelato VRF containing request details.
     function fulfillRandomness(
         uint256 randomness,
-        bytes calldata data
+        bytes calldata dataWithTimestamp
     ) external {
         require(msg.sender == _operator(), "only operator");
+
+        (bytes memory data, ) = abi.decode(dataWithTimestamp, (bytes, uint256));
         (uint64 requestId, bytes memory extraData) = abi.decode(
             data,
             (uint64, bytes)
         );
-        randomness = uint(
-            keccak256(
-                abi.encode(randomness, address(this), block.chainid, requestId)
-            )
-        );
-        if (requestPending[requestId]) {
+
+        bytes32 requestHash = keccak256(dataWithTimestamp);
+        bool isValidRequestHash = requestHash == requestedHash[requestId];
+
+        if (requestPending[requestId] && isValidRequestHash) {
+            randomness = uint(
+                keccak256(
+                    abi.encode(
+                        randomness,
+                        address(this),
+                        block.chainid,
+                        requestId
+                    )
+                )
+            );
+
             _fulfillRandomness(randomness, requestId, extraData);
             requestPending[requestId] = false;
         }

diff --git a/contracts/chainlink_compatible/VRFCoordinatorV2Adapter.sol b/contracts/chainlink_compatible/VRFCoordinatorV2Adapter.sol
@@ -145,7 +145,9 @@ contract VRFCoordinatorV2Adapter is
         for (uint32 i = 0; i < numWords; i++) {
             words[i] = uint(keccak256(abi.encode(randomness, i)));
         }
-        consumer.rawFulfillRandomWords(requestId, words);
+
+        // solhint-disable-next-line no-empty-blocks
+        try consumer.rawFulfillRandomWords(requestId, words) {} catch {}
     }
 
     /// @notice Internal function to add or remove addresses that can create requests.

diff --git a/hardhat.config.ts b/hardhat.config.ts
@@ -2,13 +2,13 @@ import { HardhatUserConfig } from "hardhat/config";
 
 // PLUGINS
 import "@gelatonetwork/web3-functions-sdk/hardhat-plugin";
-import "@typechain/hardhat";
-import "@nomiclabs/hardhat-ethers";
-import "@nomiclabs/hardhat-waffle";
-import "hardhat-deploy";
 import "@matterlabs/hardhat-zksync-solc";
 import "@matterlabs/hardhat-zksync-verify";
+import "@nomiclabs/hardhat-ethers";
+import "@nomiclabs/hardhat-waffle";
 import { getSingletonFactoryInfo } from "@safe-global/safe-singleton-factory";
+import "@typechain/hardhat";
+import "hardhat-deploy";
 
 // ================================= TASKS =========================================
 
@@ -17,8 +17,8 @@ import * as dotenv from "dotenv";
 dotenv.config({ path: __dirname + "/.env" });
 
 // Libraries
-import assert from "assert";
 import { BigNumber } from "@ethersproject/bignumber";
+import assert from "assert";
 
 // Process Env Variables
 const ALCHEMY_ID = process.env.ALCHEMY_ID;
@@ -53,7 +53,7 @@ const config: HardhatUserConfig = {
   w3f: {
     rootDir: "./web3-functions",
     debug: false,
-    networks: ["mumbai", "goerli", "baseGoerli"], //(multiChainProvider) injects provider for these networks
+    networks: ["polygon", "mumbai", "goerli", "baseGoerli"], //(multiChainProvider) injects provider for these networks
   },
 
   namedAccounts: {

diff --git a/src/drand_util.ts b/src/drand_util.ts
@@ -6,7 +6,6 @@ import {
   HttpChainClient,
   fetchBeacon,
   roundAt,
-  roundTime,
 } from "drand-client";
 
 import { quicknet } from "./drand_info";
@@ -24,44 +23,62 @@ async function sleep(duration: number) {
   await new Promise((resolve) => setTimeout(resolve, duration));
 }
 
-async function fetchDrandResponse(round?: number) {
-  // sequentially try different endpoints, in shuffled order for load-balancing
-  const urls = shuffle([
-    // Protocol labs endpoints
-    "https://api.drand.sh",
-    "https://api2.drand.sh",
-    "https://api3.drand.sh",
-    // Cloudflare
-    "https://drand.cloudflare.com",
-    // Storswift
-    "https://api.drand.secureweb3.com:6875",
-  ]);
+class HttpChainClientCache {
+  #chainClients: HttpChainClient[] = [];
 
+  constructor(urls: string[]) {
+    urls.forEach((url) => {
+      const chain = new HttpCachingChain(
+        `${url}/${quicknet.hash}`,
+        DRAND_OPTIONS
+      );
+      const client = new HttpChainClient(chain, DRAND_OPTIONS);
+      this.#chainClients.push(client);
+    });
+  }
+
+  getClients() {
+    return shuffle([...this.#chainClients]);
+  }
+}
+
+const clientCache = new HttpChainClientCache([
+  // Protocol labs endpoints
+  "https://api.drand.sh",
+  "https://api2.drand.sh",
+  "https://api3.drand.sh",
+  // Cloudflare
+  "https://drand.cloudflare.com",
+  // Storswift
+  "https://api.drand.secureweb3.com:6875",
+]);
+
+async function fetchDrandResponse(round: number) {
   console.log("Fetching randomness");
-  const errors: Error[] = [];
-  for (const url of urls) {
-    console.log(`Trying ${url}...`);
-    const chain = new HttpCachingChain(
-      `${url}/${quicknet.hash}`,
-      DRAND_OPTIONS
-    );
-    const client = new HttpChainClient(chain, DRAND_OPTIONS);
+  const errors = [];
+
+  for (const client of clientCache.getClients()) {
     try {
       return await fetchBeacon(client, round);
     } catch (err) {
-      errors.push(err as Error);
+      errors.push(err);
     }
   }
   throw errors.pop();
 }
 
-export async function getNextRandomness(timestampInSec: number) {
-  const requestTime = timestampInSec * 1000;
-  const nextRound = roundAt(requestTime, quicknet) + 1;
-  if (roundTime(quicknet, nextRound) > requestTime) {
-    await sleep(roundTime(quicknet, nextRound) - requestTime);
+export async function getNextRandomness(requestTimeInSec: number) {
+  const nextRound = roundAt(requestTimeInSec * 1000, quicknet) + 1;
+
+  // eslint-disable-next-line no-constant-condition
+  while (true) {
+    try {
+      const { round, randomness } = await fetchDrandResponse(nextRound);
+      console.log(`Fulfilling from round ${round}`);
+      return randomness;
+    } catch (e) {
+      console.log("Failed to fetch randomness", e);
+      await sleep(500);
+    }
   }
-  const { round, randomness } = await fetchDrandResponse(nextRound);
-  console.log(`Fulfilling from round ${round}`);
-  return randomness;
 }
diff --git a/web3-functions/vrf-event-trigger/index.ts b/web3-functions/vrf-event-trigger/index.ts
@@ -28,9 +28,15 @@ Web3Function.onRun(async (context: Web3FunctionEventContext) => {
 
   const event = consumer.interface.parseLog(log);
   const [consumerData] = event.args;
+
+  const consumerDataWithTimestamp = ethers.utils.defaultAbiCoder.encode(
+    ["bytes", "uint256"],
+    [consumerData, timestamp]
+  );
+
   const data = consumer.interface.encodeFunctionData("fulfillRandomness", [
     encodedRandomness,
-    consumerData,
+    consumerDataWithTimestamp,
   ]);
 
   return {

diff --git a/web3-functions/vrf/index.ts b/web3-functions/vrf/index.ts
@@ -70,16 +70,21 @@ Web3Function.onRun(async (context: Web3FunctionContext) => {
   console.log(`Matched ${logs.length} new events`);
   for (const log of logs) {
     const event = consumer.interface.parseLog(log);
-    const [data] = event.args;
+    const [consumerData] = event.args;
     const { timestamp } = await provider.getBlock(log.blockHash);
     const randomness = await getNextRandomness(timestamp);
     const encodedRandomness = ethers.BigNumber.from(`0x${randomness}`);
 
+    const consumerDataWithTimestamp = ethers.utils.defaultAbiCoder.encode(
+      ["bytes", "uint256"],
+      [consumerData, timestamp]
+    );
+
     callData.push({
       to: consumerAddress,
       data: consumer.interface.encodeFunctionData("fulfillRandomness", [
         encodedRandomness,
-        data,
+        consumerDataWithTimestamp,
       ]),
     });
   }