sherlock/deposit-dos

.gas-snapshot

@@ -461,94 +461,95 @@ StakedEXATest:invariantNoDuplicatedReward() (runs: 10, calls: 5000, reverts: 0)
 StakedEXATest:invariantRewardsUpOnly() (runs: 10, calls: 5000, reverts: 0)
 StakedEXATest:invariantShareValueIsOne() (runs: 10, calls: 5000, reverts: 0)
 StakedEXATest:testAlreadyListedError() (gas: 43906)
-StakedEXATest:testAvgIndex(uint256[3],uint256[2]) (runs: 256, μ: 1214747, ~: 1249353)
-StakedEXATest:testAvgStartTime(uint256[3],uint256[2]) (runs: 256, μ: 1201153, ~: 1235759)
-StakedEXATest:testBalanceOfDeposit(uint80) (runs: 256, μ: 355470, ~: 362517)
+StakedEXATest:testAvgIndex(uint256[3],uint256[2]) (runs: 256, μ: 1245440, ~: 1279967)
+StakedEXATest:testAvgStartTime(uint256[3],uint256[2]) (runs: 256, μ: 1231846, ~: 1266373)
+StakedEXATest:testBalanceOfDeposit(uint80) (runs: 256, μ: 343979, ~: 350703)
 StakedEXATest:testBalanceOfWithdraw(uint256) (runs: 256, μ: 60470, ~: 60477)
 StakedEXATest:testCanChangeRewardsDurationWhenDisabled() (gas: 168917)
-StakedEXATest:testClaimAfterHarvest() (gas: 840919)
-StakedEXATest:testClaimAndUnstake() (gas: 1541315)
-StakedEXATest:testClaimAndWithdrawAfterRefTime() (gas: 1091631)
-StakedEXATest:testClaimBeforeFirstHarvest() (gas: 539932)
-StakedEXATest:testDepositClaimsRewardsToReceiver() (gas: 1104891)
-StakedEXATest:testDepositEvent(uint256) (runs: 256, μ: 364421, ~: 364120)
-StakedEXATest:testDepositShouldClaim(uint256[2],uint32) (runs: 256, μ: 820656, ~: 742277)
-StakedEXATest:testDepositToAnotherWithAllowance() (gas: 409123)
+StakedEXATest:testClaimAfterHarvest() (gas: 854328)
+StakedEXATest:testClaimAndUnstake() (gas: 1579851)
+StakedEXATest:testClaimAndWithdrawAfterRefTime() (gas: 1079794)
+StakedEXATest:testClaimBeforeFirstHarvest() (gas: 528095)
+StakedEXATest:testDepositClaimsRewardsToReceiver() (gas: 1112015)
+StakedEXATest:testDepositEvent(uint256) (runs: 256, μ: 352586, ~: 352307)
+StakedEXATest:testDepositShouldClaim(uint256[2],uint32) (runs: 256, μ: 837056, ~: 758406)
+StakedEXATest:testDepositToAnotherWithAllowance() (gas: 397286)
 StakedEXATest:testDepositToAnotherWithoutAllowanceShouldFail() (gas: 122812)
-StakedEXATest:testDepositWithdrawAvgStartTimeAndIndex(uint256[3],uint256,uint256[5]) (runs: 256, μ: 1748412, ~: 1809494)
-StakedEXATest:testEarnedWithTime(uint256) (runs: 256, μ: 35439, ~: 35711)
+StakedEXATest:testDepositWithdrawAvgStartTimeAndIndex(uint256[3],uint256,uint256[5]) (runs: 256, μ: 1787759, ~: 1841958)
+StakedEXATest:testEarnedWithTime(uint256) (runs: 256, μ: 35425, ~: 35711)
 StakedEXATest:testEmergencyAdminCanPauseNotUnpause() (gas: 159017)
-StakedEXATest:testFinishDistributionEmitEvent() (gas: 402683)
-StakedEXATest:testFinishDistributionLetsClaimUnclaimed() (gas: 1571740)
-StakedEXATest:testFinishDistributionStopsEmission() (gas: 1551371)
-StakedEXATest:testFinishDistributionThatAlreadyFinished() (gas: 436103)
+StakedEXATest:testFinishDistributionEmitEvent() (gas: 389903)
+StakedEXATest:testFinishDistributionLetsClaimUnclaimed() (gas: 1585008)
+StakedEXATest:testFinishDistributionStopsEmission() (gas: 1564661)
+StakedEXATest:testFinishDistributionThatAlreadyFinished() (gas: 424266)
 StakedEXATest:testFinishDistributionTransfersRemainingToSavings() (gas: 112201)
 StakedEXATest:testGrantRevokeEmergencyAdmin() (gas: 107216)
 StakedEXATest:testGrantRevokePauser() (gas: 107191)
 StakedEXATest:testHandlerClaim(uint8) (runs: 256, μ: 302881, ~: 302881)
-StakedEXATest:testHandlerDeposit(uint80) (runs: 256, μ: 799902, ~: 826633)
-StakedEXATest:testHandlerHarvest(uint64) (runs: 256, μ: 343793, ~: 342644)
-StakedEXATest:testHandlerNotifyRewardAmount(uint64) (runs: 256, μ: 127922, ~: 123298)
-StakedEXATest:testHandlerSetDuration(uint32) (runs: 256, μ: 144043, ~: 164563)
-StakedEXATest:testHandlerSetMarket() (gas: 157950)
-StakedEXATest:testHandlerWithdraw(uint256) (runs: 256, μ: 69973, ~: 69980)
-StakedEXATest:testHarvest() (gas: 197966)
-StakedEXATest:testHarvestAmountWithReducedAllowance() (gas: 215386)
-StakedEXATest:testHarvestEffectOnRewardData() (gas: 190945)
-StakedEXATest:testHarvestEmitsRewardAmountNotified() (gas: 188943)
-StakedEXATest:testHarvestZero() (gas: 236746)
+StakedEXATest:testHandlerDeposit(uint80) (runs: 256, μ: 812693, ~: 823471)
+StakedEXATest:testHandlerHarvest(uint64) (runs: 256, μ: 331418, ~: 329863)
+StakedEXATest:testHandlerNotifyRewardAmount(uint64) (runs: 256, μ: 127990, ~: 123343)
+StakedEXATest:testHandlerSetDuration(uint32) (runs: 256, μ: 143713, ~: 164563)
+StakedEXATest:testHandlerSetMarket() (gas: 125012)
+StakedEXATest:testHandlerWithdraw(uint256) (runs: 256, μ: 69996, ~: 70003)
+StakedEXATest:testHarvest() (gas: 185186)
+StakedEXATest:testHarvestAmountWithReducedAllowance() (gas: 203043)
+StakedEXATest:testHarvestEffectOnRewardData() (gas: 178188)
+StakedEXATest:testHarvestEmitsRewardAmountNotified() (gas: 176163)
+StakedEXATest:testHarvestFailDoesntDoSDeposits() (gas: 360609)
+StakedEXATest:testHarvestZero() (gas: 255215)
 StakedEXATest:testInitialValues() (gas: 89269)
-StakedEXATest:testInsufficientBalanceError(uint256) (runs: 256, μ: 64186, ~: 64331)
-StakedEXATest:testMaxRewardsGasConsumption() (gas: 138555912)
-StakedEXATest:testMultipleClaimsVsOne() (gas: 26103669)
-StakedEXATest:testMultipleHarvests() (gas: 462059)
-StakedEXATest:testNoRewardsAfterPeriod(uint256) (runs: 256, μ: 1569710, ~: 1576305)
+StakedEXATest:testInsufficientBalanceError(uint256) (runs: 256, μ: 64204, ~: 64331)
+StakedEXATest:testMaxRewardsGasConsumption() (gas: 138544075)
+StakedEXATest:testMultipleClaimsVsOne() (gas: 25452063)
+StakedEXATest:testMultipleHarvests() (gas: 436499)
+StakedEXATest:testNoRewardsAfterPeriod(uint256) (runs: 256, μ: 1581535, ~: 1587973)
 StakedEXATest:testNotPausingRoleError() (gas: 39481)
-StakedEXATest:testNotifyRewardAmount(uint256,uint256) (runs: 256, μ: 129996, ~: 129940)
-StakedEXATest:testNotifyRewardWithUnderlyingAsset() (gas: 497663)
-StakedEXATest:testOnlyAdminEnableReward() (gas: 1199166)
+StakedEXATest:testNotifyRewardAmount(uint256,uint256) (runs: 256, μ: 130002, ~: 129940)
+StakedEXATest:testNotifyRewardWithUnderlyingAsset() (gas: 485826)
+StakedEXATest:testOnlyAdminEnableReward() (gas: 1199122)
 StakedEXATest:testOnlyAdminFinishDistribution() (gas: 191108)
 StakedEXATest:testOnlyAdminNotifyRewardAmount() (gas: 203519)
 StakedEXATest:testOnlyAdminSetProvider() (gas: 143746)
 StakedEXATest:testOnlyAdminSetProviderRatio() (gas: 143405)
 StakedEXATest:testOnlyAdminSetRewardsDuration() (gas: 153065)
 StakedEXATest:testOnlyAdminSetSavings() (gas: 141208)
-StakedEXATest:testPausable() (gas: 1154949)
-StakedEXATest:testPausableClaim() (gas: 639521)
-StakedEXATest:testPausableHarvest() (gas: 344626)
+StakedEXATest:testPausable() (gas: 1168239)
+StakedEXATest:testPausableClaim() (gas: 627684)
+StakedEXATest:testPausableHarvest() (gas: 331824)
 StakedEXATest:testPauserCanPauseUnpause() (gas: 157877)
 StakedEXATest:testPenaltyGrowthRange() (gas: 67211)
 StakedEXATest:testPenaltyThresholdRange() (gas: 37134)
-StakedEXATest:testPermitAndDeposit() (gas: 373108)
-StakedEXATest:testRemoveDepositAllowance() (gas: 501636)
-StakedEXATest:testResetDepositAfterRefTime(uint256) (runs: 256, μ: 1020731, ~: 1020409)
-StakedEXATest:testRewardAmountNotifiedEvent(uint256) (runs: 256, μ: 104709, ~: 105499)
+StakedEXATest:testPermitAndDeposit() (gas: 361271)
+StakedEXATest:testRemoveDepositAllowance() (gas: 489799)
+StakedEXATest:testResetDepositAfterRefTime(uint256) (runs: 256, μ: 1027901, ~: 1027624)
+StakedEXATest:testRewardAmountNotifiedEvent(uint256) (runs: 256, μ: 104656, ~: 105511)
 StakedEXATest:testRewardNotListedError() (gas: 1109565)
-StakedEXATest:testRewardPaidEvent(uint256,uint256) (runs: 256, μ: 812237, ~: 754793)
-StakedEXATest:testRewardsAmounts(uint256) (runs: 256, μ: 1577106, ~: 1576695)
+StakedEXATest:testRewardPaidEvent(uint256,uint256) (runs: 256, μ: 800409, ~: 742956)
+StakedEXATest:testRewardsAmounts(uint256) (runs: 256, μ: 1588776, ~: 1588391)
 StakedEXATest:testRewardsDurationSetEvent(uint40) (runs: 256, μ: 52073, ~: 52056)
-StakedEXATest:testSetDuration(uint256,uint40) (runs: 256, μ: 58976, ~: 59226)
+StakedEXATest:testSetDuration(uint256,uint40) (runs: 256, μ: 58991, ~: 59226)
 StakedEXATest:testSetMarketAddressZero() (gas: 37124)
-StakedEXATest:testSetMarketOnlyAdmin() (gas: 1300351)
+StakedEXATest:testSetMarketOnlyAdmin() (gas: 1272665)
 StakedEXATest:testSetMaxRewardsTokensExceeded() (gas: 104946678)
 StakedEXATest:testSetMinTime() (gas: 81938)
 StakedEXATest:testSetPenaltyGrowth() (gas: 82148)
 StakedEXATest:testSetPenaltyThreshold() (gas: 81925)
 StakedEXATest:testSetProviderRatioOverOneError() (gas: 37179)
 StakedEXATest:testSetProviderZeroAddressError() (gas: 37131)
 StakedEXATest:testSetSavingsZeroAddressError() (gas: 37240)
-StakedEXATest:testTotalSupplyDeposit(uint80) (runs: 256, μ: 354985, ~: 362032)
+StakedEXATest:testTotalSupplyDeposit(uint80) (runs: 256, μ: 343537, ~: 350261)
 StakedEXATest:testTotalSupplyWithdraw(uint256) (runs: 256, μ: 61961, ~: 61968)
-StakedEXATest:testUntransferable(uint80) (runs: 256, μ: 374660, ~: 382606)
-StakedEXATest:testWithdrawEvent(uint256) (runs: 256, μ: 520415, ~: 520103)
-StakedEXATest:testWithdrawSameAmountRewardsShouldEqual(uint256,uint256) (runs: 256, μ: 1074152, ~: 1143271)
-StakedEXATest:testWithdrawWithRewards(uint256) (runs: 256, μ: 879810, ~: 879498)
+StakedEXATest:testUntransferable(uint80) (runs: 256, μ: 363146, ~: 370769)
+StakedEXATest:testWithdrawEvent(uint256) (runs: 256, μ: 508580, ~: 508302)
+StakedEXATest:testWithdrawSameAmountRewardsShouldEqual(uint256,uint256) (runs: 256, μ: 1062958, ~: 1131434)
+StakedEXATest:testWithdrawWithRewards(uint256) (runs: 256, μ: 867953, ~: 867675)
 StakedEXATest:testZeroRateError() (gas: 58070)
-StakingPreviewerTest:testAllClaimable() (gas: 431329)
-StakingPreviewerTest:testAllClaimed() (gas: 633069)
-StakingPreviewerTest:testAllEarned() (gas: 316435)
-StakingPreviewerTest:testAllRewards() (gas: 485836)
-StakingPreviewerTest:testStaking() (gas: 528556)
+StakingPreviewerTest:testAllClaimable() (gas: 453659)
+StakingPreviewerTest:testAllClaimed() (gas: 655399)
+StakingPreviewerTest:testAllEarned() (gas: 338765)
+StakingPreviewerTest:testAllRewards() (gas: 508166)
+StakingPreviewerTest:testStaking() (gas: 559386)
 SwapperTest:testSwapBasic() (gas: 216831)
 SwapperTest:testSwapWithAllowance() (gas: 481530)
 SwapperTest:testSwapWithInaccurateSlippageSendsETHToAccount() (gas: 297968)

contracts/StakedEXA.sol

@@ -151,7 +151,7 @@ contract StakedEXA is
         uint256 numerator = start * balance + block.timestamp * 1e18 * amount;
         avgStart[to] = numerator == 0 ? 0 : (numerator - 1) / total + 1;
       }
-      harvest();
+      try this.harvest() {} catch {} // solhint-disable-line no-empty-blocks
     } else if (to == address(0)) {
       uint256 length = rewardsTokens.length;
       for (uint256 i = 0; i < length; ++i) {
@@ -341,23 +341,17 @@ contract StakedEXA is
   /// @dev This function withdraws the maximum allowable assets from the provider's market,
   /// calculates the portion to be distributed as rewards based on `providerRatio`,
   /// deposits any remaining assets back into savings, and notifies the contract of the new reward amount.
-  function harvest() public whenNotPaused {
+  function harvest() external whenNotPaused {
     Market memMarket = market;
     address memProvider = provider;
-    uint256 assets = Math.min(
-      memMarket.convertToAssets(memMarket.allowance(memProvider, address(this))),
-      memMarket.maxWithdraw(memProvider)
-    );
-    uint256 amount = assets.mulWadDown(providerRatio);
-    IERC20 providerAsset = IERC20(address(memMarket.asset()));
-    uint256 duration = rewards[providerAsset].duration;
-    if (duration == 0 || amount < rewards[providerAsset].duration) return;
+    uint256 shares = Math.min(memMarket.allowance(memProvider, address(this)), memMarket.maxRedeem(memProvider));
+    uint256 sharesReward = shares.mulWadDown(providerRatio);
 
-    memMarket.withdraw(assets, address(this), memProvider);
-    uint256 save = assets - amount;
-    if (save != 0) memMarket.deposit(save, savings);
+    uint256 amount = memMarket.redeem(sharesReward, address(this), memProvider);
+    uint256 save = shares - sharesReward;
+    if (save != 0) memMarket.transferFrom(memProvider, savings, save);
 
-    notifyRewardAmount(providerAsset, amount, address(this));
+    notifyRewardAmount(IERC20(address(memMarket.asset())), amount, address(this));
   }
 
   /// @notice Returns all reward tokens.
@@ -464,8 +458,6 @@ contract StakedEXA is
     IERC20 providerAsset = IERC20(address(market_.asset()));
     if (rewards[providerAsset].finishAt == 0) enableReward(providerAsset);
 
-    providerAsset.approve(address(market_), type(uint256).max);
-
     emit MarketSet(market_, msg.sender);
   }
 

test/StakedEXA.t.sol

@@ -424,7 +424,7 @@ contract StakedEXATest is Test {
       provider += assets;
     }
     uint256 savings = stEXA.market().maxWithdraw(SAVINGS);
-    stEXA.harvest();
+    try stEXA.harvest() {} catch {} // solhint-disable-line no-empty-blocks
     (uint256 rDuration, , , , ) = stEXA.rewards(asset);
     if (rDuration != 0 && assets.mulWadDown(providerRatio) >= rDuration) {
       assertEq(stEXA.market().balanceOf(PROVIDER), 0, "assets left");
@@ -485,7 +485,6 @@ contract StakedEXATest is Test {
     }
     (, uint40 finishAt, , , ) = stEXA.rewards(IERC20(address(market.asset())));
     assertGt(finishAt, 0);
-    assertEq(stEXA.market().asset().allowance(address(stEXA), address(stEXA.market())), type(uint256).max);
   }
 
   function testInitialValues() external view {
@@ -1731,7 +1730,7 @@ contract StakedEXATest is Test {
 
     uint256 claimableAmount = claimable(rA, address(this));
     uint256 balanceBefore = rA.balanceOf(address(this));
-    
+
     stEXA.approve(BOB, 1);
 
     exa.mint(BOB, assets);
@@ -1741,7 +1740,6 @@ contract StakedEXATest is Test {
     vm.stopPrank();
 
     assertEq(claimableAmount, rA.balanceOf(address(this)) - balanceBefore);
-
   }
 
   function testDepositToAnotherWithoutAllowanceShouldFail() external {
@@ -1755,7 +1753,7 @@ contract StakedEXATest is Test {
   function testDepositToAnotherWithAllowance() external {
     uint256 assets = 1_000e18;
     exa.mint(address(this), assets);
-    
+
     vm.prank(BOB);
     stEXA.approve(address(this), 1);
 
@@ -1765,7 +1763,7 @@ contract StakedEXATest is Test {
   function testRemoveDepositAllowance() external {
     uint256 assets = 1_000e18;
     exa.mint(address(this), assets * 2);
-    
+
     vm.prank(BOB);
     stEXA.approve(address(this), 1);
 
@@ -1778,6 +1776,13 @@ contract StakedEXATest is Test {
     stEXA.deposit(assets, BOB);
   }
 
+  function testHarvestFailDoesntDoSDeposits() external {
+    stEXA.setRewardsDuration(providerAsset, 0);
+    uint256 assets = 1_000e18;
+    exa.mint(address(this), assets);
+    stEXA.deposit(assets, address(this));
+  }
+
   function minMaxWithdrawAllowance() internal view returns (uint256) {
     return Math.min(market.convertToAssets(market.allowance(PROVIDER, address(stEXA))), market.maxWithdraw(PROVIDER));
   }