SakaiDAO - Audit

According to the logic, the initial value of `voteAddress` is zero address, after the first epoch, any user who stakes `minimumStakeForVote` tokens can vote. When `voteAddress` is not zero address, the contract will get current epoch from vote contract for prevent user voting on running epoch. ```solidity shares[account].lastEpochNumberWhenDeposit = voteAddress != address(0) ? ITokenVote(voteAddress).getCurrentEpoch() : 0; ``` However, once the `voteAddress` is set, it cannot be changed back to the zero address. Whether the user can vote will be affected by the `voteAddress` contract. ```solidity function updateVoteAddress(address _voteAddress) external onlyOwner { require(_voteAddress != address(0), "SakaiDAO: Invalid address"); require(voteAddress != _voteAddress, "SakaiDAO: Vote address is already that address"); voteAddress = _voteAddress; emit UpdateVoteAddress(_voteAddress); } ```

If it is intended to prohibit token transfers, it is better to use `revert` to terminate the transaction and throw an error. This is because: 1. Clarity: `revert` clearly indicates that the transaction has failed and provides a reason for the failure. This is helpful for both users and developers as it provides clear feedback on why the operation is not allowed. 2. Gas Savings: When a transaction is reverted, the unused Gas is refunded to the user. Executing empty logic, although it does not change the state, still incurs Gas costs for these invalid computations. 3. Security: Executing empty logic may mislead users into thinking the transaction was successful. Using `revert` can avoid such misunderstandings and ensure that the contract behaves in accordance with its design intentions, enhancing contract security. Therefore, for the sake of transparency, Gas savings, and ensuring security, it is recommended to use `revert` when prohibiting token transfers.

According to the design, when a user unstakes, the contract estimates the fee based on the duration of the staking. The longer the staking period, the lower the fee, and if the staking period exceeds 365 days, no fee is charged when the user unstakes. However, the contract uses `shares[account].lastDepositTimestamp` as the staking start time, and this state variable will be updated both when the user stakes and unstakes. This means that if a user unstakes (possibly only a portion), the staking duration for the user will be recalculated. ```solidity function _estimateTaxPercentEarlyUnstake(address account) internal view returns(uint256) { /** this function for estimate tax if user unstake early */ uint256 lastDepositTimestamp = shares[account].lastDepositTimestamp; uint256 totalDaysStake = (block.timestamp - lastDepositTimestamp) / 86400; uint256 tax = 0; if(totalDaysStake >=0 && totalDaysStake <= 90) tax = 250; else if(totalDaysStake > 90 && totalDaysStake <= 120) tax = 223; else if(totalDaysStake > 120 && totalDaysStake <= 150) tax = 196; else if(totalDaysStake > 150 && totalDaysStake <= 180) tax = 169; else if(totalDaysStake > 180 && totalDaysStake <= 210) tax = 142; else if(totalDaysStake > 210 && totalDaysStake <= 240) tax = 115; else if(totalDaysStake > 240 && totalDaysStake <= 270) tax = 88; else if(totalDaysStake > 270 && totalDaysStake <= 300) tax = 61; else if(totalDaysStake > 300 && totalDaysStake <= 330) tax = 34; else if(totalDaysStake > 330 && totalDaysStake <= 365) tax = 7; return tax; } ```

The owner of the **SakaiDAO** contract can claim all tokens in the contract except the **Sakai** token, but the error message indicates that the owner can only claim native tokens, which is inconsistent with the requirements.

The linked code does not affect the functionality of the codebase and appears to be either remnants of test code or older functionality.

It is not recommended to use Solidity's `transfer()` and `send()` functions for transferring native tokens, since some contracts may not be able to receive the funds. Those functions forward only a fixed amount of gas (2300 specifically) and the receiving contracts may run out of gas before finishing the transfer. Also, EVM instructions' gas costs may increase in the future. Thus, some contracts that can receive now may stop working in the future due to the gas limitation. ```solidity=797 payable(msg.sender).transfer(address(this).balance); ``` - `SakaiDAO.claimStuckBNB` uses `transfer()`.