-
Notifications
You must be signed in to change notification settings - Fork 0
Hotfix rm #230
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Hotfix rm #230
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ import {Initializable} from "@openzeppelin-upgradeable/contracts/proxy/utils/Ini | |
| import {AccessControlUpgradeable} from "@openzeppelin-upgradeable/contracts/access/AccessControlUpgradeable.sol"; | ||
| import {ReentrancyGuardUpgradeable} from "@openzeppelin-upgradeable/contracts/security/ReentrancyGuardUpgradeable.sol"; | ||
| import {PausableUpgradeable} from "@openzeppelin-upgradeable/contracts/security/PausableUpgradeable.sol"; | ||
| import {EnumerableSetUpgradeable} from "@openzeppelin-upgradeable/contracts/utils/structs/EnumerableSetUpgradeable.sol"; | ||
| import {IRewardsCoordinator} from "@eigenlayer/contracts/interfaces/IRewardsCoordinator.sol"; | ||
| import {IRewardsCoordinatorTypes} from "@eigenlayer/contracts/interfaces/IRewardsCoordinator.sol"; | ||
| import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; | ||
|
|
@@ -25,6 +26,7 @@ contract RewardsManager is | |
| { | ||
| using SafeERC20 for IERC20; | ||
| using Math for uint256; | ||
| using EnumerableSetUpgradeable for EnumerableSetUpgradeable.AddressSet; | ||
|
|
||
| // ------------------------------------------------------------------------------ | ||
| // State | ||
|
|
@@ -44,9 +46,13 @@ contract RewardsManager is | |
| mapping(address => uint256) public unsupportedAssetBalances; | ||
| address[] public unsupportedAssets; | ||
|
|
||
| /// @notice Claimer for earners on EL | ||
| /// @notice Claimer for earners on EL - using EnumerableSet for efficient operations | ||
| mapping(address => bool) public isClaimerFor; | ||
| address[] public claimerFor; | ||
| EnumerableSetUpgradeable.AddressSet private _claimerForSet; | ||
|
|
||
| // ------------------------------------------------------------------------------ | ||
| // Events | ||
| // ------------------------------------------------------------------------------ | ||
|
|
||
| // ------------------------------------------------------------------------------ | ||
| // Init functions | ||
|
|
@@ -61,6 +67,7 @@ contract RewardsManager is | |
| function initialize(Init memory init) public initializer { | ||
| __AccessControl_init(); | ||
| __ReentrancyGuard_init(); | ||
| __Pausable_init(); | ||
|
|
||
| if ( | ||
| address(init.initialOwner) == address(0) || | ||
|
|
@@ -100,15 +107,13 @@ contract RewardsManager is | |
| function processClaims( | ||
| IRewardsCoordinatorTypes.RewardsMerkleClaim[] calldata claims | ||
| ) external override nonReentrant whenNotPaused { | ||
| require(claims.length <= 50, "Too many claims"); // Prevent gas exhaustion | ||
|
|
||
| for (uint256 i = 0; i < claims.length; i++) { | ||
| _processClaim(claims[i]); | ||
| } | ||
| } | ||
|
|
||
| /// @notice For rewards that are in unsupported assets, swaps into supported assets and transfers over to LT | ||
| /// @dev OUT OF SCOPE FOR V2 | ||
| /// function swapAndTransferRewards() external override nonReentrant whenNotPaused onlyRole(DEFAULT_ADMIN_ROLE) {} | ||
|
|
||
| // ------------------------------------------------------------------------------ | ||
| // Getter functions | ||
| // ------------------------------------------------------------------------------ | ||
|
|
@@ -122,96 +127,164 @@ contract RewardsManager is | |
| return balances; | ||
| } | ||
|
|
||
| /// @notice Get all claimers as an array (for backward compatibility) | ||
| function claimerFor() external view returns (address[] memory) { | ||
| return _claimerForSet.values(); | ||
| } | ||
|
|
||
| /// @notice Get number of claimers | ||
| function claimerForLength() external view returns (uint256) { | ||
| return _claimerForSet.length(); | ||
| } | ||
|
|
||
| // ------------------------------------------------------------------------------ | ||
| // Internal functions | ||
| // ------------------------------------------------------------------------------ | ||
|
|
||
| /// @dev Called by `processClaim` and `processClaims` | ||
| /// @dev Called by `processClaim` and `processClaims` - Fixed for reentrancy and balance calculation | ||
| function _processClaim(IRewardsCoordinatorTypes.RewardsMerkleClaim calldata claim) internal { | ||
| address earner = claim.earnerLeaf.earner; | ||
| if (!_verifyAndUpdateClaimerFor(earner)) revert NotClaimerFor(earner); | ||
|
|
||
| // Call for claim on EL | ||
| rewardsCoordinator.processClaim(claim, address(this)); | ||
| // Record balances BEFORE the external call to prevent reentrancy issues | ||
| IERC20[] memory uniqueTokens = _getUniqueTokensFromClaim(claim); | ||
|
|
||
| // Record current balances of all expected assets from this claim | ||
| IERC20[] memory expectedSupportedAssets = new IERC20[](claim.tokenLeaves.length); | ||
| IERC20[] memory expectedUnsupportedAssets = new IERC20[](claim.tokenLeaves.length); | ||
| uint256[] memory supportedBalances = new uint256[](claim.tokenLeaves.length); | ||
| uint256[] memory unsupportedBalances = new uint256[](claim.tokenLeaves.length); | ||
| // Create properly sized arrays instead of using assembly manipulation | ||
| // Fixed: Renamed to avoid shadowing | ||
| IERC20[] memory supportedTokens = new IERC20[](uniqueTokens.length); | ||
| IERC20[] memory unsupportedTokens = new IERC20[](uniqueTokens.length); | ||
| uint256[] memory supportedBalancesBefore = new uint256[](uniqueTokens.length); | ||
| uint256[] memory unsupportedBalancesBefore = new uint256[](uniqueTokens.length); | ||
|
|
||
| IERC20[] memory allSupportedAssets = liquidTokenManager.getSupportedTokens(); | ||
| uint256 supportedCount = 0; | ||
| uint256 unsupportedCount = 0; | ||
|
|
||
| for (uint256 i = 0; i < claim.tokenLeaves.length; i++) { | ||
| IERC20 currentToken = claim.tokenLeaves[i].token; | ||
| bool tokenAlreadyProcessed = false; | ||
| IERC20[] memory allSupportedAssets = liquidTokenManager.getSupportedTokens(); | ||
|
|
||
| for (uint256 k = 0; k < supportedCount; k++) { | ||
| if (expectedSupportedAssets[k] == currentToken) { | ||
| tokenAlreadyProcessed = true; | ||
| break; | ||
| } | ||
| // Categorize tokens and record pre-claim balances | ||
| for (uint256 i = 0; i < uniqueTokens.length; i++) { | ||
| IERC20 currentToken = uniqueTokens[i]; | ||
| bool isSupported = _isTokenSupported(currentToken, allSupportedAssets); | ||
|
|
||
| if (isSupported) { | ||
| supportedTokens[supportedCount] = currentToken; | ||
| // Only count newly claimable balance, not existing contract balance | ||
| supportedBalancesBefore[supportedCount] = | ||
| currentToken.balanceOf(address(this)) - | ||
| unsupportedAssetBalances[address(currentToken)]; | ||
| supportedCount++; | ||
| } else { | ||
| unsupportedTokens[unsupportedCount] = currentToken; | ||
| unsupportedBalancesBefore[unsupportedCount] = | ||
| currentToken.balanceOf(address(this)) - | ||
| unsupportedAssetBalances[address(currentToken)]; | ||
| unsupportedCount++; | ||
| } | ||
| } | ||
|
|
||
| if (!tokenAlreadyProcessed) { | ||
| for (uint256 k = 0; k < unsupportedCount; k++) { | ||
| if (expectedUnsupportedAssets[k] == currentToken) { | ||
| tokenAlreadyProcessed = true; | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| // Resize arrays to actual counts | ||
| supportedTokens = _resizeTokenArray(supportedTokens, supportedCount); | ||
| unsupportedTokens = _resizeTokenArray(unsupportedTokens, unsupportedCount); | ||
| supportedBalancesBefore = _resizeUintArray(supportedBalancesBefore, supportedCount); | ||
| unsupportedBalancesBefore = _resizeUintArray(unsupportedBalancesBefore, unsupportedCount); | ||
|
|
||
| if (!tokenAlreadyProcessed) { | ||
| bool isSupported = false; | ||
| // Make the external call to EigenLayer AFTER recording pre-state | ||
| rewardsCoordinator.processClaim(claim, address(this)); | ||
|
|
||
| for (uint256 j = 0; j < allSupportedAssets.length; j++) { | ||
| if (allSupportedAssets[j] == currentToken) { | ||
| isSupported = true; | ||
| break; | ||
| } | ||
| } | ||
| // Calculate actual received amounts by comparing post-claim balances | ||
| uint256[] memory supportedReceivedAmounts = new uint256[](supportedCount); | ||
| uint256[] memory unsupportedReceivedAmounts = new uint256[](unsupportedCount); | ||
|
|
||
| if (isSupported) { | ||
| expectedSupportedAssets[supportedCount] = currentToken; | ||
| supportedBalances[supportedCount] = currentToken.balanceOf(address(this)); | ||
| supportedCount++; | ||
| } else { | ||
| expectedUnsupportedAssets[unsupportedCount] = currentToken; | ||
| unsupportedBalances[unsupportedCount] = currentToken.balanceOf(address(this)); | ||
| unsupportedCount++; | ||
| } | ||
| } | ||
| for (uint256 i = 0; i < supportedCount; i++) { | ||
| uint256 currentBalance = supportedTokens[i].balanceOf(address(this)) - | ||
| unsupportedAssetBalances[address(supportedTokens[i])]; | ||
| supportedReceivedAmounts[i] = currentBalance > supportedBalancesBefore[i] | ||
| ? currentBalance - supportedBalancesBefore[i] | ||
| : 0; | ||
| } | ||
|
|
||
| // Trim arrays to actual sizes | ||
| assembly { | ||
| mstore(expectedSupportedAssets, supportedCount) | ||
| mstore(expectedUnsupportedAssets, unsupportedCount) | ||
| mstore(supportedBalances, supportedCount) | ||
| mstore(unsupportedBalances, unsupportedCount) | ||
| for (uint256 i = 0; i < unsupportedCount; i++) { | ||
| uint256 currentBalance = unsupportedTokens[i].balanceOf(address(this)) - | ||
| unsupportedAssetBalances[address(unsupportedTokens[i])]; | ||
| unsupportedReceivedAmounts[i] = currentBalance > unsupportedBalancesBefore[i] | ||
| ? currentBalance - unsupportedBalancesBefore[i] | ||
| : 0; | ||
| } | ||
|
|
||
| // Update balances for unsupported tokens | ||
| // These tokens will stay in the contract until `swapAndTransferRewards` is called | ||
| _setAssetBalances(expectedUnsupportedAssets, unsupportedBalances); | ||
| // Update balances for unsupported tokens with events | ||
| _setAssetBalances(unsupportedTokens, unsupportedReceivedAmounts); | ||
|
|
||
| // Transfer all supported assets to `LiquidToken` | ||
| uint256[] memory netTransferredAmounts = _transferRewards(expectedSupportedAssets, supportedBalances); | ||
| // Transfer supported assets to LiquidToken | ||
| uint256[] memory netTransferredAmounts = _transferRewards(supportedTokens, supportedReceivedAmounts); | ||
|
|
||
| emit RewardsClaimed( | ||
| claim.rootIndex, | ||
| earner, | ||
| expectedSupportedAssets, | ||
| supportedTokens, | ||
| netTransferredAmounts, | ||
| expectedUnsupportedAssets, | ||
| unsupportedBalances | ||
| unsupportedTokens, | ||
| unsupportedReceivedAmounts | ||
| ); | ||
| } | ||
|
|
||
| /// @dev Called by `setClaimerFor` and `_processClaim` | ||
| /// @dev Get unique tokens from claim to avoid O(n²) complexity | ||
| function _getUniqueTokensFromClaim( | ||
| IRewardsCoordinatorTypes.RewardsMerkleClaim calldata claim | ||
| ) internal view returns (IERC20[] memory) { | ||
| // Fixed: Use arrays instead of mapping for uniqueness check | ||
| IERC20[] memory tempTokens = new IERC20[](claim.tokenLeaves.length); | ||
| uint256 uniqueCount = 0; | ||
|
|
||
| for (uint256 i = 0; i < claim.tokenLeaves.length; i++) { | ||
| IERC20 currentToken = claim.tokenLeaves[i].token; | ||
| bool isDuplicate = false; | ||
|
|
||
| // Check if token already exists in our temp array | ||
| for (uint256 j = 0; j < uniqueCount; j++) { | ||
| if (tempTokens[j] == currentToken) { | ||
| isDuplicate = true; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| if (!isDuplicate) { | ||
| tempTokens[uniqueCount] = currentToken; | ||
| uniqueCount++; | ||
| } | ||
| } | ||
|
|
||
| return _resizeTokenArray(tempTokens, uniqueCount); | ||
| } | ||
|
|
||
| /// @dev Helper to check if token is supported | ||
| function _isTokenSupported(IERC20 token, IERC20[] memory supportedTokens) internal pure returns (bool) { | ||
| for (uint256 i = 0; i < supportedTokens.length; i++) { | ||
| if (supportedTokens[i] == token) { | ||
| return true; | ||
| } | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| /// @dev Resize token array without assembly | ||
| function _resizeTokenArray(IERC20[] memory arr, uint256 newSize) internal pure returns (IERC20[] memory) { | ||
| IERC20[] memory resized = new IERC20[](newSize); | ||
| for (uint256 i = 0; i < newSize; i++) { | ||
| resized[i] = arr[i]; | ||
| } | ||
| return resized; | ||
| } | ||
|
|
||
| /// @dev Resize uint array without assembly | ||
| function _resizeUintArray(uint256[] memory arr, uint256 newSize) internal pure returns (uint256[] memory) { | ||
| uint256[] memory resized = new uint256[](newSize); | ||
| for (uint256 i = 0; i < newSize; i++) { | ||
| resized[i] = arr[i]; | ||
| } | ||
| return resized; | ||
| } | ||
|
|
||
| /// @dev Called by `setClaimerFor` and `_processClaim` - Fixed with EnumerableSet | ||
| function _verifyAndUpdateClaimerFor(address earner) internal returns (bool) { | ||
| if (address(earner) == address(0)) revert ZeroAddress(); | ||
|
|
||
|
|
@@ -222,66 +295,68 @@ contract RewardsManager is | |
| if (isClaimerOnEl) { | ||
| if (!isClaimerFor[earner]) { | ||
| isClaimerFor[earner] = true; | ||
| claimerFor.push(earner); | ||
|
|
||
| _claimerForSet.add(earner); | ||
| emit ClaimerForAdded(earner); | ||
| } | ||
| } else { | ||
| if (isClaimerFor[earner]) { | ||
| isClaimerFor[earner] = false; | ||
| _removeClaimerFor(earner); | ||
| _claimerForSet.remove(earner); | ||
| emit ClaimerForRemoved(earner); | ||
| } | ||
| } | ||
|
|
||
| return isClaimerOnEl; | ||
| } | ||
|
|
||
| /// @dev Called by `_verifyAndUpdateClaimerFor` | ||
| function _removeClaimerFor(address earner) private { | ||
| for (uint i = 0; i < claimerFor.length; i++) { | ||
| if (claimerFor[i] == earner) { | ||
| claimerFor[i] = claimerFor[claimerFor.length - 1]; | ||
| claimerFor.pop(); | ||
| emit ClaimerForRemoved(earner); | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// @dev Called by `_processClaim` | ||
| /// @dev Called by `_processClaim` - Fixed with event emissions | ||
| function _setAssetBalances(IERC20[] memory assets, uint256[] memory amounts) internal { | ||
| if (assets.length != amounts.length) revert ArrayLengthMismatch(); | ||
|
|
||
| for (uint256 i = 0; i < assets.length; i++) { | ||
| if (unsupportedAssetBalances[address(assets[i])] == 0 && amounts[i] > 0) { | ||
| unsupportedAssets.push(address(assets[i])); | ||
| address assetAddr = address(assets[i]); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the comment above is accepted, do we still need to make this additive? We might as well reset it every time anyway? |
||
| uint256 oldBalance = unsupportedAssetBalances[assetAddr]; | ||
| uint256 newBalance = oldBalance + amounts[i]; | ||
|
|
||
| if (unsupportedAssetBalances[assetAddr] == 0 && amounts[i] > 0) { | ||
| unsupportedAssets.push(assetAddr); | ||
| } | ||
|
|
||
| unsupportedAssetBalances[assetAddr] = newBalance; | ||
|
|
||
| if (amounts[i] > 0) { | ||
| emit UnsupportedAssetBalanceUpdated(assetAddr, oldBalance, newBalance); | ||
| } | ||
| unsupportedAssetBalances[address(assets[i])] = amounts[i]; | ||
| } | ||
| } | ||
|
|
||
| /// @dev Called by `_processClaim` | ||
| function _transferRewards(IERC20[] memory assets, uint256[] memory amounts) internal returns (uint256[] memory) { | ||
| if (assets.length != amounts.length) revert ArrayLengthMismatch(); | ||
| /// @dev Called by `_processClaim` - Fixed to use actual received amounts | ||
| function _transferRewards( | ||
| IERC20[] memory assets, | ||
| uint256[] memory receivedAmounts | ||
| ) internal returns (uint256[] memory) { | ||
| if (assets.length != receivedAmounts.length) revert ArrayLengthMismatch(); | ||
|
|
||
| // Transfer to `LiquidToken` and calculate actual net amounts received | ||
| uint256[] memory netTransferredAmounts = new uint256[](assets.length); | ||
|
|
||
| for (uint256 i = 0; i < assets.length; i++) { | ||
| uint256 liquidTokenBalanceBefore = assets[i].balanceOf(address(liquidToken)); | ||
| uint256 rewardsManagerBalanceBefore = amounts[i]; | ||
| if (receivedAmounts[i] > 0) { | ||
| uint256 liquidTokenBalanceBefore = assets[i].balanceOf(address(liquidToken)); | ||
|
|
||
| // Transfer the actual received amount, not the stored balance | ||
| assets[i].safeTransfer(address(liquidToken), receivedAmounts[i]); | ||
|
|
||
| if (rewardsManagerBalanceBefore > 0) { | ||
| assets[i].safeTransfer(address(liquidToken), rewardsManagerBalanceBefore); | ||
| uint256 liquidTokenBalanceAfter = assets[i].balanceOf(address(liquidToken)); | ||
| netTransferredAmounts[i] = liquidTokenBalanceAfter - liquidTokenBalanceBefore; | ||
| } else { | ||
| netTransferredAmounts[i] = 0; | ||
| } | ||
| } | ||
|
|
||
| // Credit `LiquidToken` asset balances with the actual net amounts recieved | ||
| liquidToken.creditAssetBalances(assets, netTransferredAmounts); | ||
| // Credit LiquidToken asset balances with the actual net amounts received | ||
| if (assets.length > 0) { | ||
| liquidToken.creditAssetBalances(assets, netTransferredAmounts); | ||
| } | ||
|
|
||
| return netTransferredAmounts; | ||
| } | ||
|
|
@@ -290,4 +365,4 @@ contract RewardsManager is | |
| function _balanceAsset(IERC20 asset) internal view returns (uint256) { | ||
| return unsupportedAssetBalances[address(asset)]; | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, I was thinking that we don't need to do this since we are happy to report the entire actual balance as the rewards. This is because we are only concerned the impact on the LAT. So in using the entire balance, we prevent the transfer loss issue, reduce computation of checking "balance before" and in any edge case of additional tokens above the merkle claim, we are able to use it in the LAT.
This pattern also becomes of actual use in the case of rebasing token being unsupported. So in case we do
swapAndTransferRewardswe can maintain the pattern of full token balanceNote on this was added to the struct definition in the interface
Any thoughts?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this pattern differs from "balance before/after" in LTM/LT contracts because, here we are not concerned with specifying any token amount to transfer, by default the goal is for full token balance to be transferred