Skip to content

Commit

Permalink
Fixed potential accounting error, removed duplicated code, made desig…
Browse files Browse the repository at this point in the history 
…n more flexible
  • Loading branch information
@martinvol
martinvol committed Oct 25, 2024
1 parent 62b2b54 commit 61ee665
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 30 deletions.
68 changes: 40 additions & 28 deletions packages/protocol/contracts/common/FeeHandler.sol
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// SPDX-License-Identifier: LGPL-3.0-only
pragma solidity ^0.5.13;

import { console } from "forge-std/console.sol";

import "openzeppelin-solidity/contracts/math/SafeMath.sol";
import "openzeppelin-solidity/contracts/math/Math.sol";
import "openzeppelin-solidity/contracts/ownership/Ownable.sol";
Expand Down Expand Up @@ -72,7 +74,7 @@ contract FeeHandler is

address public ignoreRenaming_carbonFeeBeneficiary;

uint256 public celoToBeBurned; // TODO deprecate
uint256 private celoToBeBurned; // TODO deprecate

// This mapping can not be public because it contains a FixidityLib.Fraction
// and that'd be only supported with experimental features in this
Expand Down Expand Up @@ -147,6 +149,10 @@ contract FeeHandler is
_setCarbonFraction(newFraction);
}

function setDistributionAndBurnAmounts(address tokenAddress) external {
return _setDistributionAndBurnAmounts(tokenStates[tokenAddress], IERC20(tokenAddress));
}

function changeOtherBeneficiaryAllocation(
address beneficiary,
uint256 _newFraction
Expand Down Expand Up @@ -316,6 +322,10 @@ contract FeeHandler is
return IERC20(token).transfer(recipient, value);
}

function getCeloToBeBurned() external view returns (uint256) {
return getCeloTokenState().toBurn;
}

/**
* @param token The address of the token to query.
* @return The amount burned for a token.
Expand Down Expand Up @@ -385,6 +395,10 @@ contract FeeHandler is
return tokenStates[tokenAddress].toDistribute;
}

function getTokenToBurn(address tokenAddress) external view returns (uint256) {
return tokenStates[tokenAddress].toDistribute;
}

function getCarbonFraction() external view returns (uint256) {
return getCarbonFractionFixidity().unwrap();
}
Expand Down Expand Up @@ -446,19 +460,15 @@ contract FeeHandler is
function getActiveTokens() public view returns (address[] memory) {
return activeTokens.values;
}
// TODO to allow this function to be called publicly, it should
// keep track of amounts to be burned, like the Celo token does
function _setDistributionAmounts(
TokenState storage tokenState,
IERC20 token
) internal returns (uint256) {

function _setDistributionAndBurnAmounts(TokenState storage tokenState, IERC20 token) internal {
uint256 balanceOfToken = token.balanceOf(address(this));
console.log("balanceOfToken", balanceOfToken);
uint256 balanceToProcess = balanceOfToken.sub(tokenState.toDistribute).sub(tokenState.toBurn);

uint256 balanceToBurn = _setDistributeAfterBurn(tokenState, balanceToProcess);
console.log("balanceToProcess", balanceToProcess);
_setDistributeAfterBurn(tokenState, balanceToProcess);

emit DistributionAmountSet(address(token), tokenState.toDistribute);
return balanceToBurn;
}

function _executePayment(address tokenAddress, address beneficiary, uint256 amount) internal {
Expand Down Expand Up @@ -487,9 +497,9 @@ contract FeeHandler is
.newFixed(balanceToProcess)
.multiply(getBurnFractionFixidity())
.fromFixed();
tokenState.toBurn.add(balanceToBurn);
tokenState.toBurn = tokenState.toBurn.add(balanceToBurn);
tokenState.toDistribute = tokenState.toDistribute.add(balanceToProcess.sub(balanceToBurn));
return balanceToBurn;
return tokenState.toBurn;
}

function checkTotalBeneficiary() internal view {
Expand Down Expand Up @@ -554,17 +564,12 @@ contract FeeHandler is
* @notice Burns all the Celo balance of this contract.
*/
function _burnCelo() private {
TokenState storage tokenState = tokenStates[getCeloTokenAddress()];
ICeloToken celo = ICeloToken(getCeloTokenAddress());
address celoTokenAddress = getCeloTokenAddress();
TokenState storage tokenState = tokenStates[celoTokenAddress];
_setDistributionAndBurnAmounts(tokenState, IERC20(celoTokenAddress));

uint256 balanceOfCelo = address(this).balance;

uint256 balanceToProcess = balanceOfCelo.sub(tokenState.toDistribute).sub(tokenState.toBurn);
uint256 balanceToBurn = _setDistributeAfterBurn(tokenState, balanceToProcess);
uint256 totalBalanceToBurn = balanceToBurn.add(tokenState.toBurn);
getCeloTokenState().toBurn = 0;

celo.burn(totalBalanceToBurn);
ICeloToken(celoTokenAddress).burn(tokenState.toBurn);
tokenState.toBurn = 0;
}

/**
Expand Down Expand Up @@ -633,19 +638,22 @@ contract FeeHandler is
"Max slippage has to be set to sell token"
);

uint256 balanceToBurn = _setDistributionAmounts(tokenState, token);
_setDistributionAndBurnAmounts(tokenState, token);

// small numbers cause rounding errors and zero case should be skipped
if (balanceToBurn < MIN_BURN) {
return;
}
uint256 balanceToBurn = tokenState.toBurn;
console.log("balanceToBurn", balanceToBurn);

if (dailySellLimitHit(tokenAddress, balanceToBurn)) {
// in case the limit is hit, burn the max possible
balanceToBurn = tokenState.currentDaySellLimit;
emit DailyLimitHit(tokenAddress, balanceToBurn);
}

// small numbers cause rounding errors and zero case should be skipped
if (balanceToBurn < MIN_BURN) {
return;
}

token.transfer(tokenState.handler, balanceToBurn);
IFeeHandlerSeller handler = IFeeHandlerSeller(tokenState.handler);

Expand All @@ -656,7 +664,9 @@ contract FeeHandler is
FixidityLib.unwrap(tokenState.maxSlippage)
);

celoToBeBurned = celoToBeBurned.add(celoReceived);
// substract from toBurn only the amount that was burned
tokenState.toBurn = tokenState.toBurn.sub(balanceToBurn);
getCeloTokenState().toBurn = getCeloTokenState().toBurn.add(celoReceived);
tokenState.pastBurn = tokenState.pastBurn.add(balanceToBurn);
updateLimits(tokenAddress, balanceToBurn);

Expand Down Expand Up @@ -733,9 +743,11 @@ contract FeeHandler is
function _distributeAll() private {
for (uint256 i = 0; i < EnumerableSet.length(activeTokens); i++) {
address token = activeTokens.get(i);
_setDistributionAndBurnAmounts(tokenStates[token], IERC20(token));
_distribute(token);
}
// distribute Celo
_setDistributionAndBurnAmounts(getCeloTokenState(), IERC20(getCeloTokenAddress()));
_distribute(getCeloTokenAddress());
}

Expand Down
42 changes: 40 additions & 2 deletions packages/protocol/test-sol/unit/common/FeeHandler.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -691,13 +691,17 @@ contract FeeHandlerTest_SellMentoTokens_WhenTokenEnabled is FeeHandlerTest_SellM
assertEq(stableToken.balanceOf(address(feeHandler)), balanceBefore);
}

function resetLimit() internal {
skip(DAY);
}

function test_ResetSellLimitDaily() public {
fundFeeHandlerStable(3000, address(stableToken), address(exchangeUSD));

feeHandler.setDailySellLimit(address(stableToken), 1000);
feeHandler.sell(address(stableToken));
assertEq(stableToken.balanceOf(address(feeHandler)), 2000);
skip(DAY);
resetLimit();
feeHandler.sell(address(stableToken));
assertEq(stableToken.balanceOf(address(feeHandler)), 1000);
}
Expand All @@ -712,6 +716,39 @@ contract FeeHandlerTest_SellMentoTokens_WhenTokenEnabled is FeeHandlerTest_SellM
assertEq(stableToken.balanceOf(address(feeHandler)), 2000);
}

function test_HitLimitDoesntAffectAccounting() public {
fundFeeHandlerStable(3000, address(stableToken), address(exchangeUSD));
feeHandler.setDailySellLimit(address(stableToken), 1000);

feeHandler.sell(address(stableToken));
assertEq(stableToken.balanceOf(address(feeHandler)), 2000);

assertEq(feeHandler.getTokenToDistribute(address(stableToken)), 600);

resetLimit();

feeHandler.sell(address(stableToken));
assertEq(feeHandler.getTokenToDistribute(address(stableToken)), 600);
assertEq(stableToken.balanceOf(address(feeHandler)), 1000);

resetLimit();

feeHandler.sell(address(stableToken));
assertEq(feeHandler.getTokenToDistribute(address(stableToken)), 600);
assertEq(stableToken.balanceOf(address(feeHandler)), 600);
}

function test_setDistributionAndBurnAmountsDoesntAffectBurn() public {
fundFeeHandlerStable(3000, address(stableToken), address(exchangeUSD));
feeHandler.setDailySellLimit(address(stableToken), 1000);

feeHandler.setDistributionAndBurnAmounts(address(stableToken));
feeHandler.sell(address(stableToken));

assertEq(stableToken.balanceOf(address(feeHandler)), 2000);
assertEq(feeHandler.getTokenToDistribute(address(stableToken)), 600);
}

function test_Sell_WhenOtherTokenHitLimit() public {
fundFeeHandlerStable(3000, address(stableToken), address(exchangeUSD));
feeHandler.setDailySellLimit(address(stableToken), 1000);
Expand Down Expand Up @@ -746,7 +783,7 @@ contract FeeHandlerTest_SellMentoTokens_WhenTokenEnabled is FeeHandlerTest_SellM
assertEq(feeHandler.getPastBurnForToken(address(stableToken)), 8e17);
assertEq(stableToken.balanceOf(address(feeHandler)), 2e17);
assertEq(feeHandler.getTokenToDistribute(address(stableToken)), 2e17);
assertEq(feeHandler.celoToBeBurned(), expectedCeloAmount);
assertEq(feeHandler.getCeloToBeBurned(), expectedCeloAmount);
}

function test_Reverts_WhenNotEnoughReports() public {
Expand Down Expand Up @@ -1001,6 +1038,7 @@ contract FeeHandlerTest_HandleMentoTokens is FeeHandlerTestAbstract {
celoToken.balanceOf(address(0x000000000000000000000000000000000000dEaD)),
398482170620712919
);

assertEq(stableToken.balanceOf(address(feeHandler)), 0);
}
}
Expand Down

0 comments on commit 61ee665

Please sign in to comment.