Make FeeHandler able to distribute tokens #10247
Conversation
| .fromFixed(); | ||
| celo.burn(balanceToBurn); | ||
|
|
||
| tokenState.toDistribute = balanceToProcess - balanceToBurn; |
There was a problem hiding this comment.
Shouldn't we also add tokenState.toDistribute back ?
tokenState.toDistribute = tokenState.toDistribute + balanceToProcess - balanceToBurn;
There was a problem hiding this comment.
forgot the +=, nice catch
|
|
||
| return | ||
| totalAmount | ||
| .subtract((price.multiply(maxSlippageFraction)).multiply(amountFraction)) |
There was a problem hiding this comment.
Nit: we can remove the inner brackets
.subtract(price.multiply(maxSlippageFraction).multiply(amountFraction))
| // } | ||
|
|
||
| // TODO move this to abstract class | ||
| function calculateMinAmount( |
There was a problem hiding this comment.
Since this method is identical in MentoFeeHandlerSeller and UniswapFeeHandlerSeller we could extract it to library
There was a problem hiding this comment.
will fix after merging
| require(routerAddresses[sellTokenAddress].length > 0, "routerAddresses should be non empty"); | ||
|
|
||
| // IERC20 token = IERC20(sellTokenAddress); | ||
| uint256 balanceToBurn = IERC20(sellTokenAddress).balanceOf(address(this)); |
There was a problem hiding this comment.
Should we check whether the balanceToBurn > 0 ?
There was a problem hiding this comment.
This contract assumes a bunch of checks were done by FeeHandler.sol, so those checks would be redundant.
| emit ReceivedQuote(poolAddress, wouldGet); | ||
| if (wouldGet > bestRouterQuote) { | ||
| bestRouterQuote = wouldGet; | ||
| bestRouterIndex = i; |
There was a problem hiding this comment.
Instead of saving bestRouterIndex we could save
bestRouterAddress
bestRouter
In such case we wouldn't have to retrieve it from storage on line 89 again
There was a problem hiding this comment.
great point, this piece of code is not currently in use, so will fix after merging
Description
Makes FeeHandler able to burn just a fraction of the fees (rather than 100%) and send it to the distribution. Please not this is not a production ready implementation, and as nonMento tokens will not be available in the short term, burning nonMento tokens is not fully supported (code is written but not properly tested). Some other features are also pending further testing, like rate limits.
Other changes
Describe any minor or "drive-by" changes here.
Tested
An explanation of how the changes were tested or an explanation as to why they don't need to be.
Related issues
Backwards compatibility
Brief explanation of why these changes are/are not backwards compatible.
Documentation
The set of community facing docs that have been added/modified because of this change