# Gauss Training #Rock Paper Scissors

### Players with more eth than the betSize will not play
For players to `join ()` the match, they need to have a bet amount which is set as `uint public betSize = 10000000000000000000;`.
For the contract to enforce this, in`join ()` function calls the internal `checkBetAmount ()` function.

	function checkBetAmount() internal {
		if(msg.value > 0 ether) {
			require(msg.value == betSize, "betSize eth required to bet");
			betMatch = true;
		}
	}

From the code of the `checkBetAmount ()` function below, the code first checks that the `msg.value > 0 ether` to ensure that the player has enough `eth`. The contract further ensures that the `msg.value == betSize` which was already set as shown above. This condition further restricts playing as it only allows players to only have the exact `betSize`. If they have more than the `betSize` amount they will not be able to play.

***Scenario***
Alice wants to play this `rockpaperscissors` game. She has eth which is equivalent of 20000000000000000000 which is clearly above the `betSize`. When she joins the game, the contract will revert as the eth she poses which will now be `msg.value` will be greater than the `betSize`

### Reentrancy attack ###
In the `win ()` function of the code, the contract declares the winner and further sends eth to the winning person

/**
	 * @notice Declares the winner
	 */
	function win(address payable winner) internal {
		uint winningsAmount = 0;
		uint contractBalance = address(this).balance;
		if(betMatch) {
			(bool success, ) = winner.call.value(contractBalance)("");
				require(success, "Transfer failed.");
			winningsAmount = betSize * 2;
		}
		emit Winner(winner, winningsAmount);
	}

The function uses a low level `call` which is susceptible to reentrancy attacks as this call bypasses type checking, function existence check, and argument packing. Moreover, the function or other functions calling it does not have a reentrancy guard or uses the checks-interactions and effects pattern.

### Declaring a `draw` might fail due to the use of `transfer`
The contract in declaring a draw, it sends the `betSize` amount to each player using the `transfer` call as shown:

/**
	 * @notice Declares a draw
	 */
	function draw() internal {
		if(betMatch) {
			players[0].transfer(betSize);
			players[1].transfer(betSize);
		}
		emit Draw();
	}
}

However, the call to this contract might fail because the `transfer` call has fixed gas cost of 2300. After the istanbul hardfork, the gas cost for `SLOAD` opcode which is used to read a word from the blockchain was increased. As a result, the gas cost is subject to change which might be more than the fixed gas cost of 2300 accompanied by the `transfer` call.
You can read more about it here https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/.

### The winner will wipe out the contract's `balance` ###
The internal `win` function is used to declare the winner and send them the winning amount incase they are the winner as shown

/**
	 * @notice Declares the winner
	 */
	function win(address payable winner) internal {
		uint winningsAmount = 0;
		uint contractBalance = address(this).balance;
		if(betMatch) {
			(bool success, ) = winner.call.value(contractBalance)("");
				require(success, "Transfer failed.");
			winningsAmount = betSize * 2;
		}
		emit Winner(winner, winningsAmount);
	}
	/**

However, in the actual call to tranfer the win, the contract's balance is computed and stored in `contractBalance` variable which is used in the transfer call. Therefore, the winner will be able to get the entire contracts balance. Moreover, the `event` declared in the function outputs the `winner` and the `winningsAmount` which is double the `betSize` and the actual won amount.

***Recommendation***
Consider, using the `winningsAmount` in the low level call to tranfer the winnings.

### The bet Amount is not transferred to the contract
When joining the game via the `join` function, the contract checks if the player has enough balance through `msg.value` and ensures that they have the required `betSize` in order to join through the `checkBetAmount` function.

	/**
	 * @notice Checks if the amount the player is betting is valid
	 */
	function checkBetAmount() internal {
		if(msg.value > 0 ether) {
			require(msg.value == betSize, "betSize eth required to bet");
			betMatch = true;
		}

From the function, the bet amount is not transferred to the contract, it is only checked if it is there. When the player wins the game, the amount they are sent is the balance of the contract `uint contractBalance = address(this).balance;`. This would mean that the `msg.value`(bet Amounts) is never used and players will continously drain the contract by simply participating.
***Recommendation***
The contract should atleast, send the `betSize` amount to the contract and use this amount in awarding the winners their wins and not `contractBalance`