Github Link
https://github.com/Cyfrin/2024-08-MyCut/blob/946231db0fe717039429a11706717be568d03b54/src/ContestManager.sol#L16C4-L22C66
https://github.com/Cyfrin/2024-08-MyCut/blob/946231db0fe717039429a11706717be568d03b54/src/Pot.sol#L32C1-L34C10
Summary
The audit of the smart contract has identified a potential vulnerability in the ContestManager::createContest function, which arises from the absence of initial filtering for zero rewards. This oversight results in the inclusion of players with zero-value rewards in the reward distribution array, leading to unnecessary gas consumption during the reward distribution process. Specifically, the function iterates over an array of players and assigns rewards without considering whether the rewards are non-zero:
for (uint256 i = 0; i < i_players.length; i++) {
playersToRewards[i_players[i]] = i_rewards[i];
}
Furthermore, the claimCut function checks whether the reward is zero before transferring it to the player, which is also applicable when there are no players in the array itself. Therefore, excluding zeros during deployment directly will not affect the logic of the code, while also avoiding the waste of gas.
Impact
High: The unnecessary inclusion of zero rewards in the reward distribution process can lead to significant gas wastage, especially when dealing with a large number of participants.
High: The inefficient check for zero rewards in the claimCut function can also lead to unnecessary gas consumption, as it does not optimize for the scenario where a player has no reward.
Proof
Use Foundry Tools and Test File:
Tool Used: Foundry (forge 0.2.0)
Test File: TestMyCut.t.sol
A loop from i = 3 to 9999 that creates player names and addresses，with 0 reward,and then `uint gasUsedCreateAndFund = gasStart - gasAfterPot;` is how gas we CreatePot().
pragma solidity ^0.8.20;
import {ContestManager} from "../src/ContestManager.sol";
import {Test, console} from "lib/forge-std/src/Test.sol";
import {IERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
import {ERC20Mock} from "./ERC20Mock.sol";
import {Strings} from "lib/openzeppelin-contracts/contracts/utils/Strings.sol";
import {Pot} from "../src/Pot.sol";
contract TestMyCut is Test {
address conMan;
address player1 = makeAddr("player1");
address player2 = makeAddr("player2");
address[] players = [player1, player2];
uint256 public constant STARTING_USER_BALANCE = 1000;
ERC20Mock weth;
address contest;
address[] totalContests;
uint256[] rewards = [1, 10];
address user = makeAddr("user");
uint256 totalRewards = 11;
function setUp() public {
for (uint256 i = 3; i < 10000; i++) {
string memory playerName = string(abi.encodePacked("player", Strings.toString(i)));
address playerAddr = makeAddr(playerName);
players.push(playerAddr);
rewards.push(uint256(0));
}
vm.startPrank(user);
conMan = address(new ContestManager());
weth = new ERC20Mock("WETH", "WETH", msg.sender, 11);
uint256 managerBalanceafter = ERC20Mock(weth).balanceOf(conMan);
uint256 manageruserafter = ERC20Mock(weth).balanceOf(user);
vm.stopPrank();
}
modifier mintAndApproveTokens() {
console.log("Minting tokens to: ", user);
vm.startPrank(user);
ERC20Mock(weth).mint(user, STARTING_USER_BALANCE);
ERC20Mock(weth).approve(conMan, STARTING_USER_BALANCE);
console.log("Approved tokens to: ", address(conMan));
vm.stopPrank();
_;
}
function testCanCreatePot() public mintAndApproveTokens {
console.log("Contest Manager Owner: ", ContestManager(conMan).owner());
console.log("msg.sender: ", msg.sender);
vm.startPrank(user);
uint gasStart = gasleft();
contest = ContestManager(conMan).createContest(players, rewards, IERC20(ERC20Mock(weth)), totalRewards);
uint gasAfterPot = gasleft();
uint gasUsedCreateAndFund = gasStart - gasAfterPot;
console.log("Gas used for create and fund: %d", gasUsedCreateAndFund);
totalContests = ContestManager(conMan).getContests();
vm.stopPrank();
assertEq(totalContests.length, 1);
}
├─ [0] console::log("Gas used for create and fund: %d", 333627344 [3.336e8]) [staticcall]
The next test is to calculate the gas consumed during the deployment of the pot after excluding participants with zero rewards prior to deployment.
pragma solidity ^0.8.20;
import {ContestManager} from "../src/ContestManager.sol";
import {Test, console} from "lib/forge-std/src/Test.sol";
import {IERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
import {ERC20Mock} from "./ERC20Mock.sol";
import {Strings} from "lib/openzeppelin-contracts/contracts/utils/Strings.sol";
import {Pot} from "../src/Pot.sol";
contract TestMyCut is Test {
address conMan;
address player1 = makeAddr("player1");
address player2 = makeAddr("player2");
address[] players = [player1, player2];
uint256 public constant STARTING_USER_BALANCE = 1000;
ERC20Mock weth;
address contest;
address[] totalContests;
uint256[] rewards = [1, 10];
address user = makeAddr("user");
uint256 totalRewards = 11;
function setUp() public {
vm.startPrank(user);
conMan = address(new ContestManager());
weth = new ERC20Mock("WETH", "WETH", msg.sender, 11);
uint256 managerBalanceafter = ERC20Mock(weth).balanceOf(conMan);
uint256 manageruserafter = ERC20Mock(weth).balanceOf(user);
vm.stopPrank();
}
modifier mintAndApproveTokens() {
console.log("Minting tokens to: ", user);
vm.startPrank(user);
ERC20Mock(weth).mint(user, STARTING_USER_BALANCE);
ERC20Mock(weth).approve(conMan, STARTING_USER_BALANCE);
console.log("Approved tokens to: ", address(conMan));
vm.stopPrank();
_;
}
function testCanCreatePot() public mintAndApproveTokens {
console.log("Contest Manager Owner: ", ContestManager(conMan).owner());
console.log("msg.sender: ", msg.sender);
vm.startPrank(user);
uint gasStart = gasleft();
contest = ContestManager(conMan).createContest(players, rewards, IERC20(ERC20Mock(weth)), totalRewards);
uint gasAfterPot = gasleft();
uint gasUsedCreateAndFund = gasStart - gasAfterPot;
console.log("Gas used for create and fund: %d", gasUsedCreateAndFund);
totalContests = ContestManager(conMan).getContests();
vm.stopPrank();
assertEq(totalContests.length, 1);
}
├─ [0] console::log("Gas used for create and fund: %d", 713439 [7.134e5]) [staticcall]
From the logs:
The excess gas, wasted due to the inclusion of zero rewards participants, is calculated to be about 332.91 million gas units. This highlights the importance of optimizing the contract deployment by filtering out participants with zero rewards to save on gas costs and enhance efficiency. By excluding these participants before deployment, the contract can operate more cost-effectively, making it more sustainable for large-scale applications.
Recommendations
Here's an example of how you might write a JavaScript script using Ethers.js to verify the data before deploying a smart contract that includes checks for zero rewards.
const { ethers } = require("ethers");
const abi = [];
const bytecode = "0x...";
async function verifyContestData(players, rewards) {
if (players.length !== rewards.length) {
throw new Error("Players and rewards arrays must be of the same length.");
}
for (let i = 0; i < rewards.length; i++) {
if (rewards[i] === 0) {
throw new Error(`Invalid reward value at index ${i}: rewards must be greater than zero.`);
}
}
}
async function deployContract(signer, players, rewards, tokenAddress, totalRewards) {
await verifyContestData(players, rewards);
const factory = new ethers.ContractFactory(abi, bytecode, signer);
const contestManager = await factory.deploy();
await contestManager.deployed();
console.log(`Contract deployed to: ${contestManager.address}`);
const potAddress = await contestManager.createContest(players, rewards, tokenAddress, totalRewards);
console.log(`Pot created at: ${potAddress}`);
}
const provider = new ethers.providers.JsonRpcProvider("http://localhost:8545");
const wallet = new ethers.Wallet("YOUR_PRIVATE_KEY", provider);
const signer = wallet.connect(provider);
const players = ["0x...1", "0x...2", "0x...3"];
const rewards = [100, 200, 300];
const tokenAddress = "0x...";
const totalRewards = 600;
deployContract(signer, players, rewards, tokenAddress, totalRewards)
.then(() => console.log("Deployment successful"))
.catch((error) => console.error("Deployment failed:", error));
Please make sure to replace the abi and bytecode variables with the actual ABI and bytecode of your contract. Additionally, YOUR_PRIVATE_KEY should be replaced with the private key of your Ethereum wallet.
This script first defines a verifyContestData function to validate the players and rewards arrays. Then, the deployContract function is used to deploy the contract and call the createContest function. Before deployment, it calls verifyContestData to ensure there are no zero rewards. If the verification fails, an error is thrown and the deployment process is halted.
