diff --git a/Bootcampers/Manoj/Session03/StakingContractAudit.txt b/Bootcampers/Manoj/Session03/StakingContractAudit.txt new file mode 100644 index 0000000..9548cd4 --- /dev/null +++ b/Bootcampers/Manoj/Session03/StakingContractAudit.txt @@ -0,0 +1,110 @@ +1. The Lockup period and Reward Amount should be immutable, as their values are constant even after the contract is deployed. + Currently, they are not set as immutable. + +2. In the User struct, the stack id is initialized with uint8, which should be uint256 to avoid limiting the number of stakes. + The same issue is present in the code with { mapping(address => uint8) public userStakeCount; }. + +3. The constructor constructor(IERC20 _token) incorrectly uses Ownable(msg.sender), + as there is no need for Ownable to take any parameters in the constructor. + +4. The logic + if (rewardPool >= REWARD_AMOUNT) { + user.stakeAmount += REWARD_AMOUNT; + rewardPool -= REWARD_AMOUNT; + } + needs clarification. Rewards are added to the stake amount only if the reward pool is sufficient, + which might cause confusion and potential fund manipulation. + +5. In the initializeUser function, setting userStakeData[msg.sender][0] to a zero User struct is redundant. + +Final Code : + +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.24; + +import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import "@openzeppelin/contracts/access/Ownable.sol"; + +/** + * @title StakingContract + * @dev A contract that allows users to stake ERC20 tokens, earn rewards, and unstake after a lockup period. + */ +contract BRBStaking is Ownable { + IERC20 public token; + uint256 public totalStaked; + uint256 public rewardPool; + uint256 public immutable LOCKUP_PERIOD; + uint256 public immutable REWARD_AMOUNT; + + /** + * @dev Struct to represent a user's staking information. + */ + struct User { + address userAddress; + uint256 stakeAmount; + bool initialized; + uint256 timeStamp; + uint256 stakeID; + } + + mapping(address => mapping(uint256 => User)) public userStakeData; + mapping(address => uint256) public userStakeCount; + + event UserInitialized(address indexed user); + event TokensStaked(address indexed user, uint256 amount, uint256 stakeID); + event TokensUnstaked(address indexed user, uint256 amount, uint256 stakeID); + event RewardsAdded(uint256 amount); + + constructor(IERC20 _token, uint256 _lockupPeriod, uint256 _rewardAmount) { + token = _token; + LOCKUP_PERIOD = _lockupPeriod; + REWARD_AMOUNT = _rewardAmount; + } + + function initializeUser() external { + require(!userStakeData[msg.sender][0].initialized, "User already initialized"); + User memory user = User(msg.sender, 0, true, 0, 0); + userStakeData[msg.sender][0] = user; + emit UserInitialized(msg.sender); + } + + function stake(uint256 _amount) external { + require(userStakeData[msg.sender][0].initialized, "User not initialized"); + require(token.transferFrom(msg.sender, address(this), _amount), "Token transfer failed"); + + uint256 stakeID = userStakeCount[msg.sender]; + User memory user = User(msg.sender, _amount, true, block.timestamp, stakeID); + userStakeData[msg.sender][stakeID] = user; + + userStakeCount[msg.sender]++; + totalStaked += _amount; + + emit TokensStaked(msg.sender, _amount, stakeID); + } + + function unstake(uint256 _stakeID) external { + User storage user = userStakeData[msg.sender][_stakeID]; + require(user.initialized, "Stake not found"); + require(block.timestamp >= user.timeStamp + LOCKUP_PERIOD, "Lockup period not completed"); + + uint256 amountToTransfer = user.stakeAmount; + if (rewardPool >= REWARD_AMOUNT) { + amountToTransfer += REWARD_AMOUNT; + rewardPool -= REWARD_AMOUNT; + } + + totalStaked -= user.stakeAmount; + delete userStakeData[msg.sender][_stakeID]; + + require(token.transfer(msg.sender, amountToTransfer), "Token transfer failed"); + + emit TokensUnstaked(msg.sender, user.stakeAmount, _stakeID); + } + + function addReward(uint256 _amount) external onlyOwner { + require(token.transferFrom(msg.sender, address(this), _amount), "Token transfer failed"); + rewardPool += _amount; + + emit RewardsAdded(_amount); + } +} \ No newline at end of file