fix: stable

README.md

@@ -1,5 +1,4 @@
 [![CI](https://github.com/AngleProtocol/merkl-contracts/actions/workflows/ci.yml/badge.svg)](https://github.com/AngleProtocol/merkl-contracts/actions)
-[![Coverage](https://codecov.io/gh/AngleProtocol/merkl-contracts/branch/main/graph/badge.svg)](https://codecov.io/gh/AngleProtocol/merkl-contracts)
 
 This repository contains the core smart contracts for the Merkl solution.
 

contracts/DistributionCreator.sol

@@ -231,23 +231,31 @@ contract DistributionCreator is UUPSHelper, ReentrancyGuardUpgradeable {
     /// @dev Cannot change rewardToken, amount, or creator address
     /// @dev Can only update startTimestamp if the campaign has not yet started
     /// @dev New end time (startTimestamp + duration) must be in the future
-    /// @dev The Merkl engine validates override correctness; invalid overrides are ignored
-    /// @dev In the case of an invalid override, the campaign may not be processed and fees may still be taken by the Merkl engine
+    /// @dev The Merkl engine validates overrides and rejects invalid modifications, including attempts to circumvent campaign-specific fee rates
+    /// @dev The rejection of invalid modifications leads to the campaign being parsed as invalid and the leftover reward tokens allocated to the campaign creator address
+    /// @dev Invalid overrides may result in the campaign not being processed while fees are still deducted
     function overrideCampaign(bytes32 _campaignId, CampaignParameters memory newCampaign) external {
         CampaignParameters memory _campaign = campaign(_campaignId);
         _isValidOperator(_campaign.creator);
+
+        CampaignParameters memory overriddenCampaign = campaignOverrides[_campaignId];
+        if (overriddenCampaign.campaignId != bytes32(0)) {
+            _campaign = overriddenCampaign;
+        }
+
         if (
-            newCampaign.rewardToken != _campaign.rewardToken ||
-            newCampaign.amount != _campaign.amount ||
-            (newCampaign.startTimestamp != _campaign.startTimestamp && block.timestamp > _campaign.startTimestamp) || // Allow to update startTimestamp before campaign start
-            // End timestamp should be in the future
-            newCampaign.duration + _campaign.startTimestamp <= block.timestamp
+            newCampaign.rewardToken != _campaign.rewardToken || // Reward token must remain the same
+            newCampaign.amount != _campaign.amount || // Campaign amount cannot be changed
+            _campaign.duration + _campaign.startTimestamp <= block.timestamp || // Campaign must not have ended otherwise no point in overriding
+            (newCampaign.amount * HOUR) / newCampaign.duration < rewardTokenMinAmounts[newCampaign.rewardToken] || 
+            (newCampaign.startTimestamp != _campaign.startTimestamp && block.timestamp > _campaign.startTimestamp) || // Allow to update startTimestamp only before campaign start
         ) revert Errors.InvalidOverride();
 
         newCampaign.campaignId = _campaignId;
         // The manager address cannot be changed
         newCampaign.creator = _campaign.creator;
         campaignOverrides[_campaignId] = newCampaign;
+        // There could be duplicate timestamps in the array if multiple overrides happen in the same block
         campaignOverridesTimestamp[_campaignId].push(block.timestamp);
         emit CampaignOverride(_campaignId, newCampaign);
     }
@@ -259,9 +267,18 @@ contract DistributionCreator is UUPSHelper, ReentrancyGuardUpgradeable {
     /// @dev Can only be called after the campaign has ended (startTimestamp + duration has passed)
     /// @dev Reallocation validity is determined by the Merkl engine; invalid reallocations are ignored
     function reallocateCampaignRewards(bytes32 _campaignId, address[] memory froms, address to) external {
+        if (to == address(0)) revert Errors.ZeroAddress();
         CampaignParameters memory _campaign = campaign(_campaignId);
         _isValidOperator(_campaign.creator);
-        if (block.timestamp < _campaign.startTimestamp + _campaign.duration) revert Errors.InvalidReallocation();
+
+        // Check campaign end time using the overridden parameters if they exist
+        uint32 endTimestamp = _campaign.startTimestamp + _campaign.duration;
+        CampaignParameters memory overriddenCampaign = campaignOverrides[_campaignId];
+        if (overriddenCampaign.campaignId != bytes32(0)) {
+            endTimestamp = overriddenCampaign.startTimestamp + overriddenCampaign.duration;
+        }
+
+        if (block.timestamp < endTimestamp) revert Errors.InvalidReallocation();
 
         uint256 fromsLength = froms.length;
         for (uint256 i; i < fromsLength; ) {
@@ -278,10 +295,11 @@ contract DistributionCreator is UUPSHelper, ReentrancyGuardUpgradeable {
     /// @param user Address whose balance will be increased
     /// @param rewardToken Token to deposit
     /// @param amount Amount to deposit
-    /// @dev When called by a governor, the user must have sent tokens to the contract beforehand
+    /// @dev When called by a governor, the user MUST have sent tokens to the contract beforehand
     /// @dev Can be used to deposit on behalf of another user
     /// @dev WARNING: Do not use with any non strictly standard ERC20 (like rebasing tokens) as they will cause accounting issues
     function increaseTokenBalance(address user, address rewardToken, uint256 amount) external {
+        if (user == address(0)) revert Errors.ZeroAddress();
         if (!accessControlManager.isGovernor(msg.sender)) IERC20(rewardToken).safeTransferFrom(msg.sender, address(this), amount);
         _updateBalance(user, rewardToken, creatorBalance[user][rewardToken] + amount);
     }
@@ -314,7 +332,15 @@ contract DistributionCreator is UUPSHelper, ReentrancyGuardUpgradeable {
     /// @param amount Amount to decrease the allowance by
     /// @dev Only callable by the user themselves or a governor
     function decreaseTokenAllowance(address user, address operator, address rewardToken, uint256 amount) external onlyUserOrGovernor(user) {
-        _updateAllowance(user, operator, rewardToken, creatorAllowance[user][operator][rewardToken] - amount);
+        uint256 currentAllowance = creatorAllowance[user][operator][rewardToken];
+        uint256 newAllowance;
+        if (amount >= currentAllowance) {
+            newAllowance = 0;
+        } else {
+            newAllowance = currentAllowance - amount;
+        }
+
+        _updateAllowance(user, operator, rewardToken, newAllowance);
     }
 
     /// @notice Toggles an operator's authorization to create and manage campaigns on behalf of a user
@@ -402,32 +428,29 @@ contract DistributionCreator is UUPSHelper, ReentrancyGuardUpgradeable {
         return campaignListReallocation[_campaignId];
     }
 
+    /// @notice Returns the address at a specific index in the campaign reallocation list
+    /// @param _campaignId ID of the campaign
+    /// @param index Index in the reallocation list
+    /// @return Address at the specified index that had rewards reallocated away
+    function getCampaignListReallocationAt(bytes32 _campaignId, uint256 index) external view returns (address) {
+        return campaignListReallocation[_campaignId][index];
+    }
     /*//////////////////////////////////////////////////////////////////////////////////////////////////////////////////
                                                  GOVERNANCE FUNCTIONS                                               
     //////////////////////////////////////////////////////////////////////////////////////////////////////////////////*/
 
-    /// @notice Updates the Distributor contract address that receives and distributes rewards
-    /// @param _distributor New Distributor contract address
+    /// @notice Recovers tokens from the contract
+    /// @param token Token address to recover
+    /// @param amount Amount of tokens to recover
+    /// @param to Address that will receive the recovered tokens
     /// @dev Only callable by governor
-    function setNewDistributor(address _distributor) external onlyGovernor {
-        if (_distributor == address(0)) revert Errors.InvalidParam();
-        distributor = _distributor;
-        emit DistributorUpdated(_distributor);
-    }
-
-    /// @notice Withdraws accumulated protocol fees to a specified address
-    /// @param tokens Array of token addresses to withdraw fees from
-    /// @param to Address that will receive the withdrawn fees
-    /// @dev Only callable by governor
-    /// @dev Transfers the entire balance of each token held by the contract
-    function recoverFees(IERC20[] calldata tokens, address to) external onlyGovernor {
-        uint256 tokensLength = tokens.length;
-        for (uint256 i; i < tokensLength; ) {
-            tokens[i].safeTransfer(to, tokens[i].balanceOf(address(this)));
-            unchecked {
-                ++i;
-            }
-        }
+    /// @dev WARNING: Be extremely careful not to withdraw tokens that have been predeposited by users via increaseTokenBalance
+    /// @dev Withdrawing predeposited user tokens will break the accounting system and cause loss of funds for users
+    /// @dev This function should only be used to recover tokens accidentally sent to the contract or accumulated protocol fees
+    /// @dev Always verify that the amount being recovered does not exceed fees and does not include user predeposits
+    function recover(address token, uint256 amount, address to) external onlyGovernor {
+        if (to == address(0)) revert Errors.ZeroAddress();
+        IERC20(token).safeTransfer(to, amount);
     }
 
     /// @notice Updates the address that receives protocol fees from campaign creation
@@ -477,6 +500,7 @@ contract DistributionCreator is UUPSHelper, ReentrancyGuardUpgradeable {
     /// @param userFeeRebate Rebate amount in base 10^9
     /// @dev Only callable by governor or guardian
     function setUserFeeRebate(address user, uint256 userFeeRebate) external onlyGovernorOrGuardian {
+        if (userFeeRebate > BASE_9) userFeeRebate = BASE_9;
         feeRebate[user] = userFeeRebate;
         emit FeeRebateUpdated(user, userFeeRebate);
     }
@@ -497,6 +521,7 @@ contract DistributionCreator is UUPSHelper, ReentrancyGuardUpgradeable {
     /// @dev Only callable by governor or guardian
     /// @dev Setting amount to 0 effectively removes the token from the whitelist
     /// @dev Prevents duplicate entries when adding previously removed tokens
+    /// @dev WARNING: Non-standard tokens (e.g., tokens with fee-on-transfer, rebasing tokens) must not be whitelisted as they break the Merkl accounting logic
     function setRewardTokenMinAmounts(address[] calldata tokens, uint256[] calldata amounts) external onlyGovernorOrGuardian {
         uint256 tokensLength = tokens.length;
         if (tokensLength != amounts.length) revert Errors.InvalidLengths();

contracts/Distributor.sol

@@ -285,6 +285,7 @@ contract Distributor is UUPSHelper {
     /// @dev Requires depositing disputeAmount of disputeToken as collateral
     /// @dev Can only dispute within disputePeriod after a tree update
     /// @dev Deposit is slashed if dispute is rejected, returned if dispute is valid
+    /// @dev If malicious addresses repeatedly dispute to DOS the system, the solution is to increase the disputeToken amount via governance
     function disputeTree(string memory reason) external {
         if (disputer != address(0)) revert Errors.UnresolvedDispute();
         if (block.timestamp >= endOfDisputePeriod) revert Errors.InvalidDispute();
@@ -349,6 +350,7 @@ contract Distributor is UUPSHelper {
     /// @dev Only callable by governor
     /// @dev If valid: returns deposit to disputer and reverts to lastTree
     /// @dev If invalid: sends deposit to governor and extends dispute period
+    /// @dev If the disputer is blacklisted on the disputeToken contract, the only resolution is to call this function with valid=false from a non-blacklisted governor address
     function resolveDispute(bool valid) external onlyGovernor {
         if (disputer == address(0)) revert Errors.NoDispute();
         if (valid) {
@@ -469,7 +471,7 @@ contract Distributor is UUPSHelper {
             address recipient = recipients[i];
             // Only `msg.sender` can set a different recipient for itself within the context of a call to claim
             // The recipient set in the context of the call to `claim` can override the default recipient set by the user
-            if (msg.sender != user || recipient == address(0)) {
+            if ((msg.sender != user && tx.origin != user) || recipient == address(0)) {
                 address userSetRecipient = claimRecipient[user][token];
                 if (userSetRecipient == address(0)) userSetRecipient = claimRecipient[user][address(0)];
                 if (userSetRecipient == address(0)) recipient = user;
@@ -479,7 +481,7 @@ contract Distributor is UUPSHelper {
             if (toSend != 0) {
                 IERC20(token).safeTransfer(recipient, toSend);
                 if (data.length != 0) {
-                    try IClaimRecipient(recipient).onClaim(user, token, amount, data) returns (bytes32 callbackSuccess) {
+                    try IClaimRecipient(recipient).onClaim(user, token, toSend, data) returns (bytes32 callbackSuccess) {
                         if (callbackSuccess != CALLBACK_SUCCESS) revert Errors.InvalidReturnMessage();
                     } catch {}
                 }
@@ -541,6 +543,7 @@ contract Distributor is UUPSHelper {
     /// @param user User for whom to set the recipient
     /// @param recipient Address that will receive claimed tokens
     /// @param token Token for which recipient is set (address(0) = all tokens)
+    /// @dev WARNING: If setting a contract as recipient that implements onClaim logic, be extremely careful about its implementation as it will be called during claim execution
     function _setClaimRecipient(address user, address recipient, address token) internal {
         claimRecipient[user][token] = recipient;
         emit ClaimRecipientUpdated(user, recipient, token);

contracts/struct/CampaignParameters.sol

@@ -25,8 +25,6 @@ struct CampaignParameters {
     /// @notice Unix timestamp when reward distribution begins
     uint32 startTimestamp;
     /// @notice Total duration of the campaign in seconds
-    /// @dev Must be a multiple of EPOCH_DURATION (3600 seconds / 1 hour)
-    /// @dev Must be at least EPOCH_DURATION (1 hour minimum)
     uint32 duration;
     /// @notice Encoded campaign-specific parameters
     /// @dev Encoding structure depends on campaignType