Add farmer tax to contracts with TaxCollector #36
Conversation
| farmsToTax = farms; | ||
| } | ||
|
|
||
| function addFarm(address farm) external { |
There was a problem hiding this comment.
This feels like it should be locked down, maybe just the beneficiary? Or maybe an owner? Otherwise anyone could create their own FarmPool with their own withdrawTax implementation that could do potentially do naughty things
| } | ||
|
|
||
| function collectTaxes() external { | ||
| for (uint256 i = 0; i < farmsToTax.length; i++) { |
There was a problem hiding this comment.
If farmsToTax is really large (unlikely, but still worth considering), this function could end up consuming a prohibitively large amount of gas, even to the point where this function could not be called. I think a good pattern could be to make this function collectAllTaxes and then have a separate collectTaxes(uint256 farmIndex) that can be used for collecting from specific farms
| uniFactory = uniFactory_; | ||
| uniRouter = uniRouter_; | ||
| hamToken = HamToken(hamToken_); | ||
| beneficiary = beneficiary_; |
There was a problem hiding this comment.
Wonder if the beneficiary should be able to change the beneficiary
| for (uint256 i = 0; i < farmsToTax.length; i++) { | ||
| FarmPool(farmsToTax[i]).withdrawTax(); | ||
| } | ||
| address uniswap_pair = UniswapV2Library.pairFor(uniFactory, weth, address(hamToken)); |
| (uint256 reserves1, uint256 reserves2, ) = pair.getReserves(); | ||
| uint256 balance = hamToken.balanceOf(address(this)); | ||
| uint256 amountOut = IUniswapV2Router01(uniRouter).getAmountOut(balance, reserves1, reserves2); | ||
| address[] memory path = new address[](2); |
There was a problem hiding this comment.
nit: is it possible to pass an array literal to swapTokensForExactETH instead?
| address[] memory path = new address[](2); | ||
| path[0] = address(hamToken); | ||
| path[1] = weth; | ||
| uint256[] memory amounts = IUniswapV2Router01(uniRouter).swapTokensForExactETH(balance, amountOut, path, beneficiary, block.timestamp); |
There was a problem hiding this comment.
I'm curious why you'd want to even convert to eth here instead of just having the taxes remain as ham
| address[] memory path = new address[](2); | ||
| path[0] = address(hamToken); | ||
| path[1] = weth; | ||
| uint256[] memory amounts = IUniswapV2Router01(uniRouter).swapTokensForExactETH(balance, amountOut, path, beneficiary, block.timestamp); |
There was a problem hiding this comment.
I think balance and amountOut are flipped
https://uniswap.org/docs/v2/smart-contracts/router01/#swaptokensforexacteth
Anyways I think if you used https://uniswap.org/docs/v2/smart-contracts/router01/#swapexacttokensforeth you wouldn't have to worry about any getAmountOut voodoo up above
This PR adds a farmer tax of 1% from all farming contracts and adds a TaxCollector contract.