pooltogether / pooltogether-rng-contracts Goto Github PK

Step from readme file:

1. yarn deploy rinkeby fails

yarn run v1.22.17 $ hardhat deploy --write true --network rinkeby (node:35736) Warning: Accessing non-existent property 'VERSION' of module exports inside circular dependency (Usenode --trace-warnings ...` to show where the warning was created)
Nothing to compile
An unexpected error occurred:

Error: ERROR processing skip func of /home/dev/Documents/Projects/Blockchain/pooltogether-rng-contracts/deploy/deploy.ts:
TSError: ⨯ Unable to compile TypeScript:
deploy/deploy.ts:31:20 - error TS7053: Element implicitly has an 'any' type because expression of type 'number' can't be used to index type '{ default: BigNumber; 1: BigNumber; 4: BigNumber; 42: BigNumber; 137: BigNumber; 80001: BigNumber; }'.
No index signature with a parameter of type 'number' was found on type '{ default: BigNumber; 1: BigNumber; 4: BigNumber; 42: BigNumber; 137: BigNumber; 80001: BigNumber; }'.

31 const feeValue = fee[chainId] || fee.default
~~~~~~~~~~~~
deploy/deploy.ts:32:24 - error TS7053: Element implicitly has an 'any' type because expression of type 'number' can't be used to index type '{ default: string; 1: string; 4: string; 42: string; 137: string; 80001: string; }'.
No index signature with a parameter of type 'number' was found on type '{ default: string; 1: string; 4: string; 42: string; 137: string; 80001: string; }'.

32 const keyHashValue = keyHash[chainId] || keyHash.default
~~~~~~~~~~~~~~~~
deploy/deploy.ts:63:56 - error TS2322: Type 'string' is not assignable to type 'never'.

63 RNGChainlink = await _getContract('RNGChainlink', [vrfCoordinator, linkAddress])
~~~~~~~~~~~~~~
deploy/deploy.ts:63:72 - error TS2322: Type 'string' is not assignable to type 'never'.

63 RNGChainlink = await _getContract('RNGChainlink', [vrfCoordinator, linkAddress])
~~~~~~~~~~~

at createTSError (/home/dev/Documents/Projects/Blockchain/pooltogether-rng-contracts/node_modules/ts-node/src/index.ts:750:12)
at reportTSError (/home/dev/Documents/Projects/Blockchain/pooltogether-rng-contracts/node_modules/ts-node/src/index.ts:754:19)
at getOutput (/home/dev/Documents/Projects/Blockchain/pooltogether-rng-contracts/node_modules/ts-node/src/index.ts:941:36)
at Object.compile (/home/dev/Documents/Projects/Blockchain/pooltogether-rng-contracts/node_modules/ts-node/src/index.ts:1243:30)
at Module.m._compile (/home/dev/Documents/Projects/Blockchain/pooltogether-rng-contracts/node_modules/ts-node/src/index.ts:1370:30)
at Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
at Object.require.extensions.<computed> [as .ts] (/home/dev/Documents/Projects/Blockchain/pooltogether-rng-contracts/node_modules/ts-node/src/index.ts:1374:12)
at Module.load (node:internal/modules/cjs/loader:981:32)
at Function.Module._load (node:internal/modules/cjs/loader:822:12)
at Module.require (node:internal/modules/cjs/loader:1005:19)
at DeploymentsManager.executeDeployScripts (/home/dev/Documents/Projects/Blockchain/pooltogether-rng-contracts/node_modules/hardhat-deploy/src/DeploymentsManager.ts:1101:15)
at DeploymentsManager.runDeploy (/home/dev/Documents/Projects/Blockchain/pooltogether-rng-contracts/node_modules/hardhat-deploy/src/DeploymentsManager.ts:1053:16)
at processTicksAndRejections (node:internal/process/task_queues:96:5)
at async SimpleTaskDefinition.action (/home/dev/Documents/Projects/Blockchain/pooltogether-rng-contracts/node_modules/hardhat-deploy/src/index.ts:409:5)
at async Environment._runTaskDefinition (/home/dev/Documents/Projects/Blockchain/pooltogether-rng-contracts/node_modules/hardhat/src/internal/core/runtime-environment.ts:217:14)
at async Environment.run (/home/dev/Documents/Projects/Blockchain/pooltogether-rng-contracts/node_modules/hardhat/src/internal/core/runtime-environment.ts:129:14)
at async SimpleTaskDefinition.action (/home/dev/Documents/Projects/Blockchain/pooltogether-rng-contracts/node_modules/hardhat-deploy/src/index.ts:555:32)
at async Environment._runTaskDefinition (/home/dev/Documents/Projects/Blockchain/pooltogether-rng-contracts/node_modules/hardhat/src/internal/core/runtime-environment.ts:217:14)
at async Environment.run (/home/dev/Documents/Projects/Blockchain/pooltogether-rng-contracts/node_modules/hardhat/src/internal/core/runtime-environment.ts:129:14)
at async SimpleTaskDefinition.action (/home/dev/Documents/Projects/Blockchain/pooltogether-rng-contracts/node_modules/hardhat-deploy/src/index.ts:640:5)

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

`

Seems that RNGBlockhash algorithm is waiting for 2 new blocks ("saved block + 2") instead of one but return seed based on previous for current:

1. current block number saved into "requestLockBlock" array
   

   pooltogether-rng-contracts/contracts/RNGBlockhash.sol

   Line 50 in 1f79ae6

   requestLockBlock[requestId] = lockBlock;

2. following condition will be true when "block number = saved block number + 2":
   

   pooltogether-rng-contracts/contracts/RNGBlockhash.sol

   Line 80 in 1f79ae6

   return block.number > (requestLockBlock[requestId] + 1);

   
   so you are waiting for 2 new blocks and only then function "_isRequestComplete" will start to return true
3. but then you are using previous block for current as seed:
   

   pooltogether-rng-contracts/contracts/RNGBlockhash.sol

   Line 93 in 1f79ae6

   return uint256(blockhash(block.number - 1));

So in general steps are following:

1. save into memory current block number (block A)
2. waiting for new block number (block B)
3. waiting for new block number (block C)
4. generate seed based on block B

This looks like vulnerability because seed will be already known at step (2) and we don't need to wait until full completion

How to fix:

1. pooltogether-rng-contracts/contracts/RNGBlockhash.sol

   Line 80 in 1f79ae6

   return block.number > (requestLockBlock[requestId] + 1);

   
   replace on "return block.number > requestLockBlock[requestId];"

2. pooltogether-rng-contracts/contracts/RNGBlockhash.sol

   Line 93 in 1f79ae6

   return uint256(blockhash(block.number - 1));

   
   replace on "return uint256(blockhash(block.number));"