Tech Trees security record

In the contract `TechTreesCoin` the role `_owner` has authority over the functions shown in the diagram below. Any compromise to the `_owner` account may allow the hacker to take advantage of this authority, allowing them to receive all future tokens by: 1. removing all users in the royalty whitelist; 2. setting themselves as the royalty receiver; 3. setting the royalty fee to 100%. 

In the contract `TechTreesCoinStaking` the role `_owner` has authority over the functions shown in the diagram below. Any compromise to the `_owner` account may allow the hacker to take advantage of this authority and: - mark every existing pool as obsolete; - set a state of emergency, allowing all users to immediately withdraw all their staked tokens; - set state variables, affecting the pool logic on withdraws and deposits. 

Tokens are sent to the deployer when deploying the contract. This could be a centralization risk as the deployer can distribute tokens without obtaining the consensus of the community.

In the function `emergencyWithdraw()`, `value` is accumulated within a loop that increases it by `_userStakes[msg.sender][pId].amount` each time. Assuming `pId` is a specific pool ID that the user would like to withdraw from in an emergency, there is no need to iterate over indices in `pools` as only the amount for that specific pool is relevant. The intent of the function becomes more unclear due to the unused `index` loop variable. In its current state, obtaining `value` is equivalent to `value = _poolCount * _userStakes[msg.sender][pId].amount;`. Furthermore, the `_userStakes` mapping is not updated despite `ttCoin`s being transferred to the caller. With this, the state variable will not reflect the actual state after the emergency.

The contracts `TechTreesCoin` and `TechTreesCoinStaking` lack comments explaining the methodology and the reasoning behind the calculation and implementation. Detailed documentation could benefit future code reviewers and improve the project's transparency.

The variable `royaltyFee` does not have reasonable boundaries, so it can be given arbitrary values after deploying.

Addresses should be checked before assignment or external call to make sure they are not zero addresses. ```solidity=1247 royaltyReceiver = _royaltyReceiver; ``` - `_royaltyReceiver` is not zero-checked before being used. --- ```solidity=1328 royaltyReceiver = _royaltyReceiver; ``` - `_royaltyReceiver` is not zero-checked before being used. --- ```solidity=1373 if (ethers > 0) payable(recipient).transfer(ethers); ``` - `recipient` is not zero-checked before being used.

Literals with many digits are difficult to read and review. ```solidity=1233 uint256 constant TOTAL_SUPPLY = 6900000000 ether; ```

The return value of the transfer()/transferFrom() call is not checked. ```solidity=1379 if (balance > 0) erc20.transfer(recipient, balance); ```

The `SafeMath` library is used unnecessarily. With Solidity compiler versions 0.8.0 or newer, arithmetic operations will automatically revert in case of integer overflow or underflow. ```solidity=1013 library SafeMath { ``` - An implementation of `SafeMath` library is found. --- ```solidity=1227 using SafeMath for uint256; ``` - `SafeMath` library is used for `uint256` type in `TechTreesCoin` contract. ```solidity=1268 uint256 royaltyAmount = amount.mul(royaltyFee).div(ROYALTY_BASE); ``` - `SafeMath.mul` is called in `transfer` function of `TechTreesCoin` contract. *Note: Only a sample of 2 `SafeMath` library usage in this contract (out of 7) are shown above.*