Add farm #13
Add farm #13
Conversation
| address _lpToken, | ||
| uint256 _allocPoint | ||
| ) external override onlyOwner whenNotPaused { | ||
| poolInfos[_lpToken] = PoolInfo({ |
There was a problem hiding this comment.
Need to check if pool already exists. Otherwise we may override existing one with that action
| } | ||
|
|
||
| function addPool( | ||
| address _lpToken, |
There was a problem hiding this comment.
also need to check for lpToken. minimum that it is contract. better - to verify token interface if thats possible
|
|
||
| function addPool( | ||
| address _lpToken, | ||
| uint256 _allocPoint |
There was a problem hiding this comment.
can it be zero? if no - need also to check it
| } | ||
|
|
||
| function deposit(address _lpToken, uint256 _amount) external override whenNotPaused { | ||
| require(_lpToken != address(0), "Zero address"); |
There was a problem hiding this comment.
check for being contract is must, additionally if thats tech possible - check for interface
|
|
||
|
|
||
| function updatePoolReward(address _lpToken) public override { | ||
| PoolInfo storage pool = poolInfos[_lpToken]; |
There was a problem hiding this comment.
we won't have here revert for zero token for example. Thats why I recommend to move all code of this function to internal function _updatePoolReward. And add verifications to updatePoolReward. It will allow to keep code protected when _updatePoolReward is called externally in updatePoolReward and do not duplicate verifications when _updatePoolReward is called internally for example in deposit function
| address _lpToken, | ||
| uint256 _amount | ||
| ) external override whenNotPaused { | ||
| require(_lpToken != address(0), "Zero address"); |
There was a problem hiding this comment.
same need more verification for input
| if (pending > 0) { | ||
| address(rewardToken).safeTransfer(msg.sender, pending); | ||
| emit LogRewardPaid(msg.sender, _lpToken, pending); | ||
| } |
There was a problem hiding this comment.
same, lets think about making reward withdraw in separate function
| function withdraw( | ||
| address _lpToken, | ||
| uint256 _amount | ||
| ) external override whenNotPaused { |
There was a problem hiding this comment.
reentrancy protection
| address public rewardToken; | ||
|
|
||
| uint256 public rewardPerBlock; | ||
| uint256 public totalAllocPoint; |
There was a problem hiding this comment.
Lets make next improvement - have those details not at contract level, but for each particular pool
| import "./IFarm.sol"; | ||
| import "../core/interfaces/IERC20.sol"; | ||
|
|
||
| contract Farm is IFarm, OwnableUpgradeable, PausableUpgradeable { |
There was a problem hiding this comment.
We can rename it to FarmFactory
Add LP token farm