From 8209c14cb8106ca69553ecdcfa2437b0a14ef5e6 Mon Sep 17 00:00:00 2001 From: Ray Huang Date: Tue, 18 Feb 2025 17:57:59 +0800 Subject: [PATCH 1/9] fix L-1 --- src/core/Farm.sol | 8 ++++++++ src/core/FarmManager.sol | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/src/core/Farm.sol b/src/core/Farm.sol index 740253d..7f9705c 100644 --- a/src/core/Farm.sol +++ b/src/core/Farm.sol @@ -91,6 +91,14 @@ contract Farm is IFarm, Initializable { _; } + /** + * @notice Farm contract constructor + * @dev Inherit Beacon proxy upgrade pattern and disable initializers in the constructor + */ + constructor() { + _disableInitializers(); + } + /// @inheritdoc IFarm function initialize( address _underlyingAsset, diff --git a/src/core/FarmManager.sol b/src/core/FarmManager.sol index 360ab26..aec00d0 100644 --- a/src/core/FarmManager.sol +++ b/src/core/FarmManager.sol @@ -85,6 +85,14 @@ contract FarmManager is IFarmManager, OwnableUpgradeable, PausableUpgradeable, U /// @inheritdoc UUPSUpgradeable function _authorizeUpgrade(address newImplementation) internal override onlyOwner { } + /** + * @notice Farm Manager contract constructor + * @dev Inherit UUPS upgrade pattern and disable initializers in the constructor + */ + constructor() { + _disableInitializers(); + } + /** * @notice Initializes the FarmManager contract * @param _farmBeacon The address of the farm beacon From 08e9d7ad6820eea354fdf7c261e4b2f924d16624 Mon Sep 17 00:00:00 2001 From: Ray Huang Date: Tue, 18 Feb 2025 18:06:55 +0800 Subject: [PATCH 2/9] fix L-2 --- src/core/Farm.sol | 5 ++++- src/core/interfaces/IFarm.sol | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/core/Farm.sol b/src/core/Farm.sol index 7f9705c..cffc829 100644 --- a/src/core/Farm.sol +++ b/src/core/Farm.sol @@ -875,10 +875,13 @@ contract Farm is IFarm, Initializable { * @notice Check farm config is valid * @param _farmConfig The farm config */ - function _checkFarmConfig(FarmConfig memory _farmConfig) internal pure { + function _checkFarmConfig(FarmConfig memory _farmConfig) internal view { if (_farmConfig.rewardEndTime < _farmConfig.rewardStartTime) { revert InvalidConfigRewardTime(_farmConfig.rewardStartTime, _farmConfig.rewardEndTime); } + if (_farmConfig.rewardEndTime <= _lastUpdateTime) { + revert InvalidRewardEndTime(_farmConfig.rewardEndTime, _lastUpdateTime); + } if (_farmConfig.depositEndTime < _farmConfig.depositStartTime) { revert InvalidConfigDepositTime(_farmConfig.depositStartTime, _farmConfig.depositEndTime); } diff --git a/src/core/interfaces/IFarm.sol b/src/core/interfaces/IFarm.sol index 7d39dd3..533347c 100644 --- a/src/core/interfaces/IFarm.sol +++ b/src/core/interfaces/IFarm.sol @@ -96,6 +96,7 @@ interface IFarm { error ForceClaimNotEnabled(); error WithdrawNotEnabled(); error DelayTimeIsNotZero(); + error InvalidRewardEndTime(uint256 rewardEndTime, uint256 lastUpdateTime); event FarmConfigUpdated(FarmConfig farmConfig); event Deposit(uint256 indexed amount, address depositor, address receiver); From d9252726f210134e1d36121a11023b371ad2e219 Mon Sep 17 00:00:00 2001 From: Ray Huang Date: Tue, 18 Feb 2025 18:13:26 +0800 Subject: [PATCH 3/9] fix L-3 --- src/core/FarmManager.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/FarmManager.sol b/src/core/FarmManager.sol index aec00d0..d1600a5 100644 --- a/src/core/FarmManager.sol +++ b/src/core/FarmManager.sol @@ -829,7 +829,7 @@ contract FarmManager is IFarmManager, OwnableUpgradeable, PausableUpgradeable, U } /** - * @notice Checks if the total amount is correct + * @notice Checks if the msg.value is greater than or equal to the total amount * @param amountArr The amount array * @param msgValue The message value */ @@ -839,7 +839,7 @@ contract FarmManager is IFarmManager, OwnableUpgradeable, PausableUpgradeable, U totalAmount += amountArr[i]; } - if (msgValue != totalAmount) revert InvalidAmount(msgValue, totalAmount); + if (msgValue < totalAmount) revert InvalidAmount(msgValue, totalAmount); } /** From 90d3d399069c2b66bed9d8e91fa3bf7850299d65 Mon Sep 17 00:00:00 2001 From: Ray Huang Date: Wed, 19 Feb 2025 18:13:16 +0800 Subject: [PATCH 4/9] fix [M-1] --- src/core/Farm.sol | 19 +++++++++++++++++-- src/core/FarmManager.sol | 15 +++------------ src/core/interfaces/IFarm.sol | 22 +++++++++++++++++++++- src/core/interfaces/IFarmManager.sol | 2 -- test/FarmManager-ERC20-fuzz.t.sol | 1 - test/FarmManager-NativeAsset-fuzz.t.sol | 1 - test/FarmManager.t.sol | 6 ++---- test/utils/BaseTest.sol | 1 - 8 files changed, 43 insertions(+), 24 deletions(-) diff --git a/src/core/Farm.sol b/src/core/Farm.sol index cffc829..8cc6bd8 100644 --- a/src/core/Farm.sol +++ b/src/core/Farm.sol @@ -337,6 +337,21 @@ contract Farm is IFarm, Initializable { return _isClaimable(); } + /// @inheritdoc IFarm + function getClaimId( + uint256 amount, + address owner, + address receiver, + uint256 claimableTime, + uint256 nonce + ) + external + pure + returns (bytes32) + { + return _calcClaimId(amount, owner, receiver, claimableTime, nonce); + } + /* --- internal functions --- */ /** @@ -836,8 +851,8 @@ contract Farm is IFarm, Initializable { internal pure { - bytes32 expectedClaimId = _calcClaimId(amount, owner, receiver, claimableTime, nonce); - if (claimId != expectedClaimId) revert InvalidClaimId(claimId, expectedClaimId); + bytes32 calcClaimId = _calcClaimId(amount, owner, receiver, claimableTime, nonce); + if (claimId != calcClaimId) revert InvalidClaimId(claimId, calcClaimId); } /** diff --git a/src/core/FarmManager.sol b/src/core/FarmManager.sol index d1600a5..d5a185c 100644 --- a/src/core/FarmManager.sol +++ b/src/core/FarmManager.sol @@ -355,19 +355,10 @@ contract FarmManager is IFarmManager, OwnableUpgradeable, PausableUpgradeable, U /// @inheritdoc IFarmManager function executeClaim(ExecuteClaimParams memory executeClaimParams) public whenNotPaused { - ( - IFarm farm, - uint256 amount, - address owner, - address receiver, - uint256 claimableTime, - uint256 nonce, - bytes32 claimId - ) = ( + (IFarm farm, uint256 amount, address owner, uint256 claimableTime, uint256 nonce, bytes32 claimId) = ( executeClaimParams.farm, executeClaimParams.amount, executeClaimParams.owner, - executeClaimParams.receiver, executeClaimParams.claimableTime, executeClaimParams.nonce, executeClaimParams.claimId @@ -375,8 +366,8 @@ contract FarmManager is IFarmManager, OwnableUpgradeable, PausableUpgradeable, U _checkFarmIsValid(farm); - farm.executeClaim(amount, owner, receiver, claimableTime, nonce, claimId); - emit ClaimExecuted(farm, amount, owner, receiver, claimableTime, nonce, claimId); + farm.executeClaim(amount, owner, msg.sender, claimableTime, nonce, claimId); + emit ClaimExecuted(farm, amount, owner, msg.sender, claimableTime, nonce, claimId); } /// @inheritdoc IFarmManager diff --git a/src/core/interfaces/IFarm.sol b/src/core/interfaces/IFarm.sol index 533347c..64abe63 100644 --- a/src/core/interfaces/IFarm.sol +++ b/src/core/interfaces/IFarm.sol @@ -76,7 +76,7 @@ interface IFarm { error InvalidClaimTime(uint256 currentTime, uint256 claimStartTime, uint256 claimEndTime); error ClaimIsNotReady(uint256 claimableTime, uint256 currentTime); error AlreadyClaimed(); - error InvalidClaimId(bytes32 claimId, bytes32 expectedClaimId); + error InvalidClaimId(bytes32 claimId, bytes32 calcClaimId); error ZeroPendingRewards(); error RequestClaimFirst(); error InvalidStatusToRequestClaim(ClaimStatus status); @@ -345,6 +345,26 @@ interface IFarm { */ function getNonce(address addr) external view returns (uint256); + /** + * @notice Get calculated claim id + * @param amount The amount of the reward token + * @param owner The address of the owner + * @param receiver The address of the receiver + * @param claimableTime The claimable time + * @param nonce The nonce + * @return claimId The claim id + */ + function getClaimId( + uint256 amount, + address owner, + address receiver, + uint256 claimableTime, + uint256 nonce + ) + external + pure + returns (bytes32); + /** * @notice The claim function is open * @return isClaimable True if the claim function is open diff --git a/src/core/interfaces/IFarmManager.sol b/src/core/interfaces/IFarmManager.sol index 8342b3d..769479c 100644 --- a/src/core/interfaces/IFarmManager.sol +++ b/src/core/interfaces/IFarmManager.sol @@ -94,7 +94,6 @@ struct RequestClaimParams { * @param farm The farm to execute claim * @param amount The amount to claim * @param owner The owner of the claim - * @param receiver The receiver of the claim * @param claimableTime The claimable time of the claim * @param nonce The nonce of the claim * @param claimId The id of the claim @@ -103,7 +102,6 @@ struct ExecuteClaimParams { IFarm farm; uint256 amount; address owner; - address receiver; uint256 claimableTime; uint256 nonce; bytes32 claimId; diff --git a/test/FarmManager-ERC20-fuzz.t.sol b/test/FarmManager-ERC20-fuzz.t.sol index 43bab99..679705f 100644 --- a/test/FarmManager-ERC20-fuzz.t.sol +++ b/test/FarmManager-ERC20-fuzz.t.sol @@ -115,7 +115,6 @@ contract FarmManagerERC20FuzzTest is Test, DeployBase { farm: farm, amount: claimAmt, owner: user, - receiver: user, claimableTime: claimableTime, nonce: nonce, claimId: claimId diff --git a/test/FarmManager-NativeAsset-fuzz.t.sol b/test/FarmManager-NativeAsset-fuzz.t.sol index ea44269..9076151 100644 --- a/test/FarmManager-NativeAsset-fuzz.t.sol +++ b/test/FarmManager-NativeAsset-fuzz.t.sol @@ -114,7 +114,6 @@ contract FarmManagerNativeAssetFuzzTest is Test, DeployBase { farm: farm, amount: claimAmt, owner: user, - receiver: user, claimableTime: claimableTime, nonce: nonce, claimId: claimId diff --git a/test/FarmManager.t.sol b/test/FarmManager.t.sol index 3de2690..d183077 100644 --- a/test/FarmManager.t.sol +++ b/test/FarmManager.t.sol @@ -189,7 +189,6 @@ contract FarmManagerTest is BaseTest { farm: farm, amount: claimAmount, owner: user1, - receiver: user1, claimableTime: claimableTime, nonce: nonce, claimId: claimId @@ -199,13 +198,13 @@ contract FarmManagerTest is BaseTest { // try to claim other user's reward, should revert vm.startPrank(user2); - vm.expectRevert(); + bytes32 fakeClaimId = keccak256(abi.encode(claimAmount, user1, user2, claimableTime, nonce)); + vm.expectRevert(abi.encodeWithSelector(IFarm.InvalidClaimId.selector, claimId, fakeClaimId)); farmManager.executeClaim( ExecuteClaimParams({ farm: farm, amount: claimAmount, owner: user1, - receiver: user1, claimableTime: claimableTime, nonce: nonce, claimId: claimId @@ -227,7 +226,6 @@ contract FarmManagerTest is BaseTest { farm: farm, amount: claimAmount, owner: user1, - receiver: user1, claimableTime: claimableTime, nonce: nonce, claimId: claimId diff --git a/test/utils/BaseTest.sol b/test/utils/BaseTest.sol index d0410da..2167228 100644 --- a/test/utils/BaseTest.sol +++ b/test/utils/BaseTest.sol @@ -102,7 +102,6 @@ abstract contract BaseTest is DeployBase { farm: farm, amount: amount, owner: user, - receiver: user, claimableTime: claimableTime, nonce: nonce, claimId: claimId From 77e3f760cf3bdb98fb4aaaef06a2973fad41b77f Mon Sep 17 00:00:00 2001 From: Ray Huang Date: Thu, 20 Feb 2025 04:33:43 +0800 Subject: [PATCH 5/9] fix [M-2] --- src/core/FarmManager.sol | 147 +++++++++++++++++++++++++++++---------- 1 file changed, 109 insertions(+), 38 deletions(-) diff --git a/src/core/FarmManager.sol b/src/core/FarmManager.sol index d5a185c..0462d4a 100644 --- a/src/core/FarmManager.sol +++ b/src/core/FarmManager.sol @@ -232,14 +232,18 @@ contract FarmManager is IFarmManager, OwnableUpgradeable, PausableUpgradeable, U payable whenNotPaused { - uint256[] memory depositAmountArr = new uint256[](depositWithProofParamsArr.length); + uint256 totalDepositAmount; for (uint256 i = 0; i < depositWithProofParamsArr.length; i++) { - depositAmountArr[i] = depositWithProofParamsArr[i].amount; + DepositWithProofParams memory depositWithProofParams = depositWithProofParamsArr[i]; + totalDepositAmount += depositWithProofParams.amount; + depositNativeAssetWithProof(depositWithProofParams); } - _checkTotalAmount(depositAmountArr, msg.value); - for (uint256 i = 0; i < depositWithProofParamsArr.length; i++) { - depositNativeAssetWithProof(depositWithProofParamsArr[i]); + _checkTotalAmount(totalDepositAmount, msg.value); + + if (msg.value > totalDepositAmount) { + // refund the remaining native asset + _refundNativeAsset(msg.value - totalDepositAmount); } } @@ -285,15 +289,18 @@ contract FarmManager is IFarmManager, OwnableUpgradeable, PausableUpgradeable, U /// @inheritdoc IFarmManager function depositNativeAssetBatch(DepositParams[] memory depositParamsArr) public payable whenNotPaused { - uint256[] memory depositAmountArr = new uint256[](depositParamsArr.length); + uint256 totalDepositAmount; for (uint256 i = 0; i < depositParamsArr.length; i++) { - depositAmountArr[i] = depositParamsArr[i].amount; + DepositParams memory depositParams = depositParamsArr[i]; + totalDepositAmount += depositParams.amount; + depositNativeAsset(depositParams); } - _checkTotalAmount(depositAmountArr, msg.value); + _checkTotalAmount(totalDepositAmount, msg.value); - for (uint256 i = 0; i < depositParamsArr.length; i++) { - depositNativeAsset(depositParamsArr[i]); + if (msg.value > totalDepositAmount) { + // refund the remaining native asset + _refundNativeAsset(msg.value - totalDepositAmount); } } @@ -415,8 +422,9 @@ contract FarmManager is IFarmManager, OwnableUpgradeable, PausableUpgradeable, U } /// @inheritdoc IFarmManager + /// @dev This function is `external` because it handles `msg.value` and may refund excess native assets. function stakePendingClaimCrossChain(StakePendingClaimCrossChainParams memory stakePendingClaimCrossChainParams) - public + external payable whenNotPaused onlyDstEidIsNotCurrentChain @@ -442,12 +450,20 @@ contract FarmManager is IFarmManager, OwnableUpgradeable, PausableUpgradeable, U farm.forceExecuteClaim(amount, msg.sender, receiver, claimableTime, nonce, claimId); - _stakeCrossChain(receiver, amount, extraOptions, msg.value); + uint256 nativeFee = _stakeCrossChain(receiver, amount, extraOptions, msg.value); + + if (msg.value > nativeFee) { + // refund the remaining native asset + _refundNativeAsset(msg.value - nativeFee); + } emit PendingClaimStaked(farm, amount, msg.sender, receiver, claimableTime, nonce, claimId); } /// @inheritdoc IFarmManager + /// @dev `msg.value` is shared across the entire transaction, so this batch function cannot simply loop over `claimAndStakeCrossChain`. + /// Doing so would cause incorrect fee handling and multiple refunds. + /// Instead, fees are accumulated and processed once at the end. function stakePendingClaimCrossChainBatch( StakePendingClaimCrossChainParams[] memory stakePendingClaimCrossChainParamsArr ) @@ -456,23 +472,44 @@ contract FarmManager is IFarmManager, OwnableUpgradeable, PausableUpgradeable, U whenNotPaused onlyDstEidIsNotCurrentChain { - uint256[] memory feeAmountArr = new uint256[](stakePendingClaimCrossChainParamsArr.length); + uint256 totalFeeAmount; for (uint256 i = 0; i < stakePendingClaimCrossChainParamsArr.length; i++) { StakePendingClaimCrossChainParams memory stakePendingClaimCrossChainParams = stakePendingClaimCrossChainParamsArr[i]; - SendParam memory sendParam = formatDepositLzSendParam( - stakePendingClaimCrossChainParams.receiver, + ( + IFarm farm, + uint256 amount, + address receiver, + uint256 claimableTime, + uint256 nonce, + bytes32 claimId, + bytes memory extraOptions + ) = ( + stakePendingClaimCrossChainParams.farm, stakePendingClaimCrossChainParams.amount, + stakePendingClaimCrossChainParams.receiver, + stakePendingClaimCrossChainParams.claimableTime, + stakePendingClaimCrossChainParams.nonce, + stakePendingClaimCrossChainParams.claimId, stakePendingClaimCrossChainParams.extraOptions ); - MessagingFee memory expectFee = rewardToken.quoteSend(sendParam, false); - feeAmountArr[i] = expectFee.nativeFee; + + _checkFarmIsValid(farm); + + farm.forceExecuteClaim(amount, msg.sender, receiver, claimableTime, nonce, claimId); + + uint256 nativeFee = _stakeCrossChain(receiver, amount, extraOptions, msg.value); + + totalFeeAmount += nativeFee; + + emit PendingClaimStaked(farm, amount, msg.sender, receiver, claimableTime, nonce, claimId); } - _checkTotalAmount(feeAmountArr, msg.value); + _checkTotalAmount(totalFeeAmount, msg.value); - for (uint256 i = 0; i < stakePendingClaimCrossChainParamsArr.length; i++) { - stakePendingClaimCrossChain(stakePendingClaimCrossChainParamsArr[i]); + if (msg.value > totalFeeAmount) { + // refund the remaining native asset + _refundNativeAsset(msg.value - totalFeeAmount); } } @@ -505,8 +542,9 @@ contract FarmManager is IFarmManager, OwnableUpgradeable, PausableUpgradeable, U } /// @inheritdoc IFarmManager + /// @dev This function is `external` because it handles `msg.value` and may refund excess native assets. function claimAndStakeCrossChain(ClaimAndStakeCrossChainParams memory claimAndStakeCrossChainParams) - public + external payable whenNotPaused onlyDstEidIsNotCurrentChain @@ -522,34 +560,53 @@ contract FarmManager is IFarmManager, OwnableUpgradeable, PausableUpgradeable, U uint256 claimAmt = farm.forceClaim(amount, msg.sender, address(this)); - _stakeCrossChain(receiver, claimAmt, extraOptions, msg.value); + uint256 nativeFee = _stakeCrossChain(receiver, claimAmt, extraOptions, msg.value); + + if (msg.value > nativeFee) { + // refund the remaining native asset + _refundNativeAsset(msg.value - nativeFee); + } emit ClaimAndStake(farm, claimAmt, msg.sender, receiver); } /// @inheritdoc IFarmManager + /// @dev `msg.value` is shared across the entire transaction, so this batch function cannot simply loop over `claimAndStakeCrossChain`. + /// Doing so would cause incorrect fee handling and multiple refunds. + /// Instead, fees are accumulated and processed once at the end. function claimAndStakeCrossChainBatch(ClaimAndStakeCrossChainParams[] memory claimAndStakeCrossChainParamsArr) public payable whenNotPaused onlyDstEidIsNotCurrentChain { - uint256[] memory feeAmountArr = new uint256[](claimAndStakeCrossChainParamsArr.length); + uint256 totalFeeAmount; for (uint256 i = 0; i < claimAndStakeCrossChainParamsArr.length; i++) { ClaimAndStakeCrossChainParams memory claimAndStakeCrossChainParams = claimAndStakeCrossChainParamsArr[i]; - SendParam memory sendParam = formatDepositLzSendParam( - claimAndStakeCrossChainParams.receiver, + + (IFarm farm, uint256 amount, address receiver, bytes memory extraOptions) = ( + claimAndStakeCrossChainParams.farm, claimAndStakeCrossChainParams.amount, + claimAndStakeCrossChainParams.receiver, claimAndStakeCrossChainParams.extraOptions ); - MessagingFee memory expectFee = rewardToken.quoteSend(sendParam, false); - feeAmountArr[i] = expectFee.nativeFee; + + _checkFarmIsValid(farm); + + uint256 claimAmt = farm.forceClaim(amount, msg.sender, address(this)); + + uint256 nativeFee = _stakeCrossChain(receiver, claimAmt, extraOptions, msg.value); + + totalFeeAmount += nativeFee; + + emit ClaimAndStake(farm, claimAmt, msg.sender, receiver); } - _checkTotalAmount(feeAmountArr, msg.value); + _checkTotalAmount(totalFeeAmount, msg.value); - for (uint256 i = 0; i < claimAndStakeCrossChainParamsArr.length; i++) { - claimAndStakeCrossChain(claimAndStakeCrossChainParamsArr[i]); + if (msg.value > totalFeeAmount) { + // refund the remaining native asset + _refundNativeAsset(msg.value - totalFeeAmount); } } @@ -802,13 +859,24 @@ contract FarmManager is IFarmManager, OwnableUpgradeable, PausableUpgradeable, U * @param amount The amount to stake * @param extraOptions The extra options * @param msgValue The message value + * @return uint256 The native fee */ - function _stakeCrossChain(address receiver, uint256 amount, bytes memory extraOptions, uint256 msgValue) internal { + function _stakeCrossChain( + address receiver, + uint256 amount, + bytes memory extraOptions, + uint256 msgValue + ) + internal + returns (uint256) + { SendParam memory sendParam = formatDepositLzSendParam(receiver, amount, extraOptions); MessagingFee memory expectFee = rewardToken.quoteSend(sendParam, false); if (msgValue < expectFee.nativeFee) revert InsufficientFee(expectFee.nativeFee, msgValue); rewardToken.send{ value: expectFee.nativeFee }(sendParam, expectFee, lzConfig.refundAddress); + + return expectFee.nativeFee; } /** @@ -821,15 +889,10 @@ contract FarmManager is IFarmManager, OwnableUpgradeable, PausableUpgradeable, U /** * @notice Checks if the msg.value is greater than or equal to the total amount - * @param amountArr The amount array + * @param totalAmount The total amount * @param msgValue The message value */ - function _checkTotalAmount(uint256[] memory amountArr, uint256 msgValue) internal pure { - uint256 totalAmount; - for (uint256 i = 0; i < amountArr.length; i++) { - totalAmount += amountArr[i]; - } - + function _checkTotalAmount(uint256 totalAmount, uint256 msgValue) internal pure { if (msgValue < totalAmount) revert InvalidAmount(msgValue, totalAmount); } @@ -856,4 +919,12 @@ contract FarmManager is IFarmManager, OwnableUpgradeable, PausableUpgradeable, U function _dstEidIsCurrentChain() internal view returns (bool) { return lzConfig.eid == dstInfo.dstEid; } + + /** + * @notice Refunds the remaining native asset + */ + function _refundNativeAsset(uint256 amount) internal { + (bool success,) = msg.sender.call{ value: amount }(""); + if (!success) revert TransferNativeAssetFailed(msg.sender, amount); + } } From 67131f28b920eeb5bf853b2455aa89de8159a4e8 Mon Sep 17 00:00:00 2001 From: Ray Huang Date: Thu, 20 Feb 2025 04:36:51 +0800 Subject: [PATCH 6/9] fix [M-3] --- src/core/FarmManager.sol | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/core/FarmManager.sol b/src/core/FarmManager.sol index 0462d4a..28a1fcb 100644 --- a/src/core/FarmManager.sol +++ b/src/core/FarmManager.sol @@ -785,18 +785,17 @@ contract FarmManager is IFarmManager, OwnableUpgradeable, PausableUpgradeable, U if (opt == LZ_COMPOSE_OPT.DEPOSIT_REWARD_TOKEN) { _requireNotPaused(); - uint256 _amountLD = OFTComposeMsgCodec.amountLD(_message); - (uint256 depositAmount, address receiver) = abi.decode(data, (uint256, address)); - if (_amountLD != depositAmount) revert InvalidReceiveAmount(_amountLD, depositAmount); - IFarm rewardFarm = dstInfo.dstRewardFarm; _checkFarmIsValid(rewardFarm); + uint256 _amountLD = OFTComposeMsgCodec.amountLD(_message); + address receiver = abi.decode(data, (address)); + // NOTE: Farm will call this.transferCallback to transfer the reward token to the farm from self - rewardToken.approve(address(this), depositAmount); - rewardFarm.depositERC20(depositAmount, address(this), receiver); + rewardToken.approve(address(this), _amountLD); + rewardFarm.depositERC20(_amountLD, address(this), receiver); - emit Deposit(rewardFarm, depositAmount, address(this), receiver); + emit Deposit(rewardFarm, _amountLD, address(this), receiver); } else { revert InvalidOpt(opt); } @@ -812,13 +811,13 @@ contract FarmManager is IFarmManager, OwnableUpgradeable, PausableUpgradeable, U view returns (SendParam memory) { - bytes memory composeMsg = abi.encode(LZ_COMPOSE_OPT.DEPOSIT_REWARD_TOKEN, abi.encode(amount, receiver)); + bytes memory composeMsg = abi.encode(LZ_COMPOSE_OPT.DEPOSIT_REWARD_TOKEN, abi.encode(receiver)); return SendParam( dstInfo.dstEid, dstInfo.dstFarmManagerBytes32, amount, - amount, + 0, /* minAmountLD */ extraOptions, composeMsg, "" // oftCmd From 9b73c3482076f4e543bdd95942939f335f0d5ea1 Mon Sep 17 00:00:00 2001 From: Ray Huang Date: Fri, 21 Feb 2025 01:07:56 +0800 Subject: [PATCH 7/9] fix [M-1] --- src/core/FarmManager.sol | 8 ++++---- src/core/interfaces/IFarmManager.sol | 4 ++-- test/FarmManager-ERC20-fuzz.t.sol | 2 +- test/FarmManager-NativeAsset-fuzz.t.sol | 2 +- test/FarmManager.t.sol | 8 ++++---- test/utils/BaseTest.sol | 2 +- test/utils/FarmTest.sol | 2 +- 7 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/core/FarmManager.sol b/src/core/FarmManager.sol index 28a1fcb..1d05f15 100644 --- a/src/core/FarmManager.sol +++ b/src/core/FarmManager.sol @@ -362,10 +362,10 @@ contract FarmManager is IFarmManager, OwnableUpgradeable, PausableUpgradeable, U /// @inheritdoc IFarmManager function executeClaim(ExecuteClaimParams memory executeClaimParams) public whenNotPaused { - (IFarm farm, uint256 amount, address owner, uint256 claimableTime, uint256 nonce, bytes32 claimId) = ( + (IFarm farm, uint256 amount, address receiver, uint256 claimableTime, uint256 nonce, bytes32 claimId) = ( executeClaimParams.farm, executeClaimParams.amount, - executeClaimParams.owner, + executeClaimParams.receiver, executeClaimParams.claimableTime, executeClaimParams.nonce, executeClaimParams.claimId @@ -373,8 +373,8 @@ contract FarmManager is IFarmManager, OwnableUpgradeable, PausableUpgradeable, U _checkFarmIsValid(farm); - farm.executeClaim(amount, owner, msg.sender, claimableTime, nonce, claimId); - emit ClaimExecuted(farm, amount, owner, msg.sender, claimableTime, nonce, claimId); + farm.executeClaim(amount, msg.sender, receiver, claimableTime, nonce, claimId); + emit ClaimExecuted(farm, amount, msg.sender, receiver, claimableTime, nonce, claimId); } /// @inheritdoc IFarmManager diff --git a/src/core/interfaces/IFarmManager.sol b/src/core/interfaces/IFarmManager.sol index 769479c..71fa191 100644 --- a/src/core/interfaces/IFarmManager.sol +++ b/src/core/interfaces/IFarmManager.sol @@ -93,7 +93,7 @@ struct RequestClaimParams { * @notice The parameters for executing claim * @param farm The farm to execute claim * @param amount The amount to claim - * @param owner The owner of the claim + * @param receiver The receiver of the claim * @param claimableTime The claimable time of the claim * @param nonce The nonce of the claim * @param claimId The id of the claim @@ -101,7 +101,7 @@ struct RequestClaimParams { struct ExecuteClaimParams { IFarm farm; uint256 amount; - address owner; + address receiver; uint256 claimableTime; uint256 nonce; bytes32 claimId; diff --git a/test/FarmManager-ERC20-fuzz.t.sol b/test/FarmManager-ERC20-fuzz.t.sol index 679705f..e42c971 100644 --- a/test/FarmManager-ERC20-fuzz.t.sol +++ b/test/FarmManager-ERC20-fuzz.t.sol @@ -114,7 +114,7 @@ contract FarmManagerERC20FuzzTest is Test, DeployBase { ExecuteClaimParams({ farm: farm, amount: claimAmt, - owner: user, + receiver: user, claimableTime: claimableTime, nonce: nonce, claimId: claimId diff --git a/test/FarmManager-NativeAsset-fuzz.t.sol b/test/FarmManager-NativeAsset-fuzz.t.sol index 9076151..98538d7 100644 --- a/test/FarmManager-NativeAsset-fuzz.t.sol +++ b/test/FarmManager-NativeAsset-fuzz.t.sol @@ -113,7 +113,7 @@ contract FarmManagerNativeAssetFuzzTest is Test, DeployBase { ExecuteClaimParams({ farm: farm, amount: claimAmt, - owner: user, + receiver: user, claimableTime: claimableTime, nonce: nonce, claimId: claimId diff --git a/test/FarmManager.t.sol b/test/FarmManager.t.sol index d183077..4475a64 100644 --- a/test/FarmManager.t.sol +++ b/test/FarmManager.t.sol @@ -188,7 +188,7 @@ contract FarmManagerTest is BaseTest { ExecuteClaimParams({ farm: farm, amount: claimAmount, - owner: user1, + receiver: user1, claimableTime: claimableTime, nonce: nonce, claimId: claimId @@ -198,13 +198,13 @@ contract FarmManagerTest is BaseTest { // try to claim other user's reward, should revert vm.startPrank(user2); - bytes32 fakeClaimId = keccak256(abi.encode(claimAmount, user1, user2, claimableTime, nonce)); + bytes32 fakeClaimId = keccak256(abi.encode(claimAmount, user2, user1, claimableTime, nonce)); vm.expectRevert(abi.encodeWithSelector(IFarm.InvalidClaimId.selector, claimId, fakeClaimId)); farmManager.executeClaim( ExecuteClaimParams({ farm: farm, amount: claimAmount, - owner: user1, + receiver: user1, claimableTime: claimableTime, nonce: nonce, claimId: claimId @@ -225,7 +225,7 @@ contract FarmManagerTest is BaseTest { ExecuteClaimParams({ farm: farm, amount: claimAmount, - owner: user1, + receiver: user1, claimableTime: claimableTime, nonce: nonce, claimId: claimId diff --git a/test/utils/BaseTest.sol b/test/utils/BaseTest.sol index 2167228..e607814 100644 --- a/test/utils/BaseTest.sol +++ b/test/utils/BaseTest.sol @@ -101,7 +101,7 @@ abstract contract BaseTest is DeployBase { ExecuteClaimParams({ farm: farm, amount: amount, - owner: user, + receiver: user, claimableTime: claimableTime, nonce: nonce, claimId: claimId diff --git a/test/utils/FarmTest.sol b/test/utils/FarmTest.sol index d9fbc1c..e1f6469 100644 --- a/test/utils/FarmTest.sol +++ b/test/utils/FarmTest.sol @@ -226,7 +226,7 @@ contract FarmTest is Test { vm.expectEmit(true, true, true, true); emit IFarmManager.ClaimExecuted( - params.farm, params.amount, user, params.owner, params.claimableTime, params.nonce, params.claimId + params.farm, params.amount, user, params.receiver, params.claimableTime, params.nonce, params.claimId ); farmManager.executeClaim(params); From c3ada5934d434bf90040586cd0f98d88d76f66aa Mon Sep 17 00:00:00 2001 From: Ray Huang Date: Fri, 21 Feb 2025 17:05:18 +0800 Subject: [PATCH 8/9] add refund in depositNativeAssetWithProof and depositNativeAsset --- src/core/FarmManager.sol | 52 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 47 insertions(+), 5 deletions(-) diff --git a/src/core/FarmManager.sol b/src/core/FarmManager.sol index 1d05f15..61bb34c 100644 --- a/src/core/FarmManager.sol +++ b/src/core/FarmManager.sol @@ -205,8 +205,9 @@ contract FarmManager is IFarmManager, OwnableUpgradeable, PausableUpgradeable, U } /// @inheritdoc IFarmManager + /// @dev This function is `external` because it handles `msg.value` and may refund excess native assets. function depositNativeAssetWithProof(DepositWithProofParams memory depositWithProofParams) - public + external payable whenNotPaused { @@ -223,20 +224,42 @@ contract FarmManager is IFarmManager, OwnableUpgradeable, PausableUpgradeable, U farm.depositNativeAssetWithProof{ value: amount }(amount, msg.sender, receiver, merkleProof); + if (msg.value > amount) { + // refund the remaining native asset + _refundNativeAsset(msg.value - amount); + } + emit DepositWithProof(farm, amount, msg.sender, receiver, merkleProof); } /// @inheritdoc IFarmManager + /// @dev `msg.value` is shared across the entire transaction, so this batch function cannot simply loop over `depositNativeAssetWithProof`. + /// Doing so would cause incorrect fee handling and multiple refunds. + /// Instead, fees are accumulated and processed once at the end. function depositNativeAssetWithProofBatch(DepositWithProofParams[] memory depositWithProofParamsArr) public payable whenNotPaused { uint256 totalDepositAmount; + for (uint256 i = 0; i < depositWithProofParamsArr.length; i++) { DepositWithProofParams memory depositWithProofParams = depositWithProofParamsArr[i]; + + (IFarm farm, uint256 amount, address receiver, bytes32[] memory merkleProof) = ( + depositWithProofParams.farm, + depositWithProofParams.amount, + depositWithProofParams.receiver, + depositWithProofParams.merkleProof + ); + + _checkFarmIsValid(farm); + + farm.depositNativeAssetWithProof{ value: amount }(amount, msg.sender, receiver, merkleProof); + totalDepositAmount += depositWithProofParams.amount; - depositNativeAssetWithProof(depositWithProofParams); + + emit DepositWithProof(farm, amount, msg.sender, receiver, merkleProof); } _checkTotalAmount(totalDepositAmount, msg.value); @@ -274,7 +297,8 @@ contract FarmManager is IFarmManager, OwnableUpgradeable, PausableUpgradeable, U } /// @inheritdoc IFarmManager - function depositNativeAsset(DepositParams memory depositParams) public payable whenNotPaused { + /// @dev This function is `external` because it handles `msg.value` and may refund excess native assets. + function depositNativeAsset(DepositParams memory depositParams) external payable whenNotPaused { (IFarm farm, uint256 amount, address receiver) = (depositParams.farm, depositParams.amount, depositParams.receiver); @@ -284,16 +308,34 @@ contract FarmManager is IFarmManager, OwnableUpgradeable, PausableUpgradeable, U farm.depositNativeAsset{ value: amount }(amount, msg.sender, receiver); + if (msg.value > amount) { + // refund the remaining native asset + _refundNativeAsset(msg.value - amount); + } + emit Deposit(farm, amount, msg.sender, receiver); } /// @inheritdoc IFarmManager + /// @dev `msg.value` is shared across the entire transaction, so this batch function cannot simply loop over `depositNativeAsset`. + /// Doing so would cause incorrect fee handling and multiple refunds. + /// Instead, fees are accumulated and processed once at the end. function depositNativeAssetBatch(DepositParams[] memory depositParamsArr) public payable whenNotPaused { uint256 totalDepositAmount; + for (uint256 i = 0; i < depositParamsArr.length; i++) { DepositParams memory depositParams = depositParamsArr[i]; + + (IFarm farm, uint256 amount, address receiver) = + (depositParams.farm, depositParams.amount, depositParams.receiver); + + _checkFarmIsValid(farm); + + farm.depositNativeAsset{ value: amount }(amount, msg.sender, receiver); + totalDepositAmount += depositParams.amount; - depositNativeAsset(depositParams); + + emit Deposit(farm, amount, msg.sender, receiver); } _checkTotalAmount(totalDepositAmount, msg.value); @@ -461,7 +503,7 @@ contract FarmManager is IFarmManager, OwnableUpgradeable, PausableUpgradeable, U } /// @inheritdoc IFarmManager - /// @dev `msg.value` is shared across the entire transaction, so this batch function cannot simply loop over `claimAndStakeCrossChain`. + /// @dev `msg.value` is shared across the entire transaction, so this batch function cannot simply loop over `stakePendingClaimCrossChain`. /// Doing so would cause incorrect fee handling and multiple refunds. /// Instead, fees are accumulated and processed once at the end. function stakePendingClaimCrossChainBatch( From de8f048e519bb0d159a3b2365e632ab4a583f552 Mon Sep 17 00:00:00 2001 From: Ray Huang Date: Fri, 21 Feb 2025 17:33:29 +0800 Subject: [PATCH 9/9] fmt --- src/core/FarmManager.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/FarmManager.sol b/src/core/FarmManager.sol index 61bb34c..1fac3c1 100644 --- a/src/core/FarmManager.sol +++ b/src/core/FarmManager.sol @@ -318,7 +318,7 @@ contract FarmManager is IFarmManager, OwnableUpgradeable, PausableUpgradeable, U /// @inheritdoc IFarmManager /// @dev `msg.value` is shared across the entire transaction, so this batch function cannot simply loop over `depositNativeAsset`. - /// Doing so would cause incorrect fee handling and multiple refunds. + /// Doing so would cause incorrect fee handling and multiple refunds. /// Instead, fees are accumulated and processed once at the end. function depositNativeAssetBatch(DepositParams[] memory depositParamsArr) public payable whenNotPaused { uint256 totalDepositAmount;