Add support for Strategist to harvest rewards

CLAUDE.md

@@ -44,6 +44,7 @@ pnpm test                      # Mainnet unit tests
 pnpm test:base                 # Base network unit tests
 pnpm test:sonic                # Sonic network unit tests
 pnpm test:coverage             # Mainnet unit tests with coverage
+pnpm test test/**/FILE_NAME.js # Running a specific test file
 ```
 
 ### Fork Tests (require `PROVIDER_URL` in `.env`)

contracts/contracts/automation/ClaimStrategyRewardsSafeModule.sol

@@ -1,16 +1,11 @@
 // SPDX-License-Identifier: BUSL-1.1
 pragma solidity ^0.8.0;
 
-import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
 import { AbstractSafeModule } from "./AbstractSafeModule.sol";
 
-import { ISafe } from "../interfaces/ISafe.sol";
 import { IStrategy } from "../interfaces/IStrategy.sol";
-import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 
 contract ClaimStrategyRewardsSafeModule is AbstractSafeModule {
-    using SafeERC20 for IERC20;
-
     mapping(address => bool) public isStrategyWhitelisted;
     address[] public strategies;
 
@@ -39,22 +34,25 @@ contract ClaimStrategyRewardsSafeModule is AbstractSafeModule {
     function claimRewards(bool silent) external onlyRole(OPERATOR_ROLE) {
         uint256 strategiesLength = strategies.length;
         for (uint256 i = 0; i < strategiesLength; i++) {
-            address strategy = strategies[i];
-
-            // Execute `collectRewardTokens` for all strategies
-            bool success = safeContract.execTransactionFromModule(
-                strategy, // To
-                0, // Value
-                abi.encodeWithSelector(IStrategy.collectRewardTokens.selector),
-                0 // Call
-            );
+            _claimRewardsFor(strategies[i], silent);
+        }
+    }
 
-            if (!success) {
-                emit ClaimRewardsFailed(strategy);
-            }
+    /**
+     * @dev Internal: collect rewards from one strategy.
+     */
+    function _claimRewardsFor(address strategy, bool silent) internal {
+        bool success = safeContract.execTransactionFromModule(
+            strategy,
+            0, // Value
+            abi.encodeWithSelector(IStrategy.collectRewardTokens.selector),
+            0 // Call
+        );
 
-            require(success || silent, "Failed to claim rewards");
+        if (!success) {
+            emit ClaimRewardsFailed(strategy);
         }
+        require(success || silent, "Failed to claim rewards");
     }
 
     /**

contracts/contracts/mocks/MockClaimableStrategy.sol

@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: BUSL-1.1
+pragma solidity ^0.8.0;
+
+import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
+
+/**
+ * @title MockClaimableStrategy
+ * @dev Mock strategy for testing ClaimStrategyRewardsSafeModule.
+ *      Holds reward tokens and transfers them to msg.sender on collectRewardTokens().
+ */
+contract MockClaimableStrategy {
+    address[] private _rewardTokenAddresses;
+    bool public shouldRevert;
+
+    function setRewardTokenAddresses(address[] memory tokens) external {
+        _rewardTokenAddresses = tokens;
+    }
+
+    function setShouldRevert(bool _shouldRevert) external {
+        shouldRevert = _shouldRevert;
+    }
+
+    function getRewardTokenAddresses()
+        external
+        view
+        returns (address[] memory)
+    {
+        return _rewardTokenAddresses;
+    }
+
+    /**
+     * @dev Transfers all held reward tokens to msg.sender (the Safe).
+     *      Reverts if shouldRevert is set, causing the Safe module exec to return false.
+     */
+    function collectRewardTokens() external {
+        require(!shouldRevert, "MockClaimableStrategy: forced revert");
+        for (uint256 i = 0; i < _rewardTokenAddresses.length; i++) {
+            uint256 balance = IERC20(_rewardTokenAddresses[i]).balanceOf(
+                address(this)
+            );
+            if (balance > 0) {
+                IERC20(_rewardTokenAddresses[i]).transfer(msg.sender, balance);
+            }
+        }
+    }
+}

contracts/contracts/strategies/BaseCurveAMOStrategy.sol

@@ -547,7 +547,7 @@ contract BaseCurveAMOStrategy is InitializableAbstractStrategy {
     function collectRewardTokens()
         external
         override
-        onlyHarvester
+        onlyHarvesterOrStrategist
         nonReentrant
     {
         // CRV rewards flow.

contracts/contracts/strategies/CurveAMOStrategy.sol

@@ -586,7 +586,7 @@ contract CurveAMOStrategy is InitializableAbstractStrategy {
     function collectRewardTokens()
         external
         override
-        onlyHarvester
+        onlyHarvesterOrStrategist
         nonReentrant
     {
         // Collect CRV rewards from inflation

contracts/contracts/strategies/algebra/StableSwapAMMStrategy.sol

@@ -561,7 +561,7 @@ contract StableSwapAMMStrategy is InitializableAbstractStrategy {
     function collectRewardTokens()
         external
         override
-        onlyHarvester
+        onlyHarvesterOrStrategist
         nonReentrant
     {
         // Collect SWPx rewards from the gauge

contracts/contracts/strategies/crosschain/CrossChainMasterStrategy.sol

@@ -182,7 +182,7 @@ contract CrossChainMasterStrategy is
     function collectRewardTokens()
         external
         override
-        onlyHarvester
+        onlyHarvesterOrStrategist
         nonReentrant
     {}
 

contracts/contracts/utils/InitializableAbstractStrategy.sol

@@ -119,16 +119,28 @@ abstract contract InitializableAbstractStrategy is Initializable, Governable {
 
     /**
      * @notice Collect accumulated reward token and send to Vault.
+     *         No-ops when the harvester address is not set.
      */
-    function collectRewardTokens() external virtual onlyHarvester nonReentrant {
+    function collectRewardTokens()
+        external
+        virtual
+        onlyHarvesterOrStrategist
+        nonReentrant
+    {
+        if (harvesterAddress == address(0)) {
+            return;
+        }
         _collectRewardTokens();
     }
 
     /**
-     * @dev Default implementation that transfers reward tokens to the Harvester
+     * @dev Default implementation that transfers reward tokens to the Harvester.
      * Implementing strategies need to add custom logic to collect the rewards.
      */
     function _collectRewardTokens() internal virtual {
+        if (harvesterAddress == address(0)) {
+            return;
+        }
         uint256 rewardTokenCount = rewardTokenAddresses.length;
         for (uint256 i = 0; i < rewardTokenCount; ++i) {
             IERC20 rewardToken = IERC20(rewardTokenAddresses[i]);
@@ -153,10 +165,14 @@ abstract contract InitializableAbstractStrategy is Initializable, Governable {
     }
 
     /**
-     * @dev Verifies that the caller is the Harvester.
+     * @dev Verifies that the caller is the Harvester or Strategist.
      */
-    modifier onlyHarvester() {
-        require(msg.sender == harvesterAddress, "Caller is not the Harvester");
+    modifier onlyHarvesterOrStrategist() {
+        require(
+            msg.sender == harvesterAddress ||
+                msg.sender == IVault(vaultAddress).strategistAddr(),
+            "Caller is not the Harvester or Strategist"
+        );
         _;
     }
 
@@ -295,7 +311,7 @@ abstract contract InitializableAbstractStrategy is Initializable, Governable {
      */
     function setHarvesterAddress(address _harvesterAddress)
         external
-        onlyGovernor
+        onlyGovernorOrStrategist
     {
         emit HarvesterAddressesUpdated(harvesterAddress, _harvesterAddress);
         harvesterAddress = _harvesterAddress;

contracts/deploy/deployActions.js

@@ -1235,6 +1235,12 @@ const deploySafeModulesForUnitTests = async () => {
     mockAutoWithdrawalVault.address,
     addresses.dead,
   ]);
+
+  await deployWithConfirmation("ClaimStrategyRewardsSafeModule", [
+    cSafeContract.address, // safe
+    cSafeContract.address, // operator
+    [], // strategies — added per-test via addStrategy
+  ]);
 };
 
 module.exports = {

contracts/deploy/mainnet/187_upgrade_curve_amo_morpho_harvester.js

@@ -0,0 +1,129 @@
+const addresses = require("../../utils/addresses");
+const { deploymentWithGovernanceProposal } = require("../../utils/deploy");
+
+module.exports = deploymentWithGovernanceProposal(
+  {
+    deployName: "187_upgrade_curve_amo_morpho_harvester",
+    forceDeploy: false,
+    reduceQueueTime: true,
+    deployerIsProposer: false,
+    proposalId: "",
+  },
+  async ({ deployWithConfirmation }) => {
+    // ── Proxy handles ────────────────────────────────────────────────────────
+    const cOUSDCurveAMOProxy = await ethers.getContract("OUSDCurveAMOProxy");
+    const cOETHCurveAMOProxy = await ethers.getContract("OETHCurveAMOProxy");
+    const cOUSDMorphoV2Proxy = await ethers.getContract(
+      "OUSDMorphoV2StrategyProxy"
+    );
+
+    // Strategy contract handles (used to call governor functions via proxy)
+    const cOUSDCurveAMO = await ethers.getContractAt(
+      "CurveAMOStrategy",
+      cOUSDCurveAMOProxy.address
+    );
+    const cOETHCurveAMO = await ethers.getContractAt(
+      "CurveAMOStrategy",
+      cOETHCurveAMOProxy.address
+    );
+    const cOUSDMorphoV2 = await ethers.getContractAt(
+      "MorphoV2Strategy",
+      cOUSDMorphoV2Proxy.address
+    );
+
+    // ── Deploy new implementations ────────────────────────────────────────────
+    // Both AMOs use CurveAMOStrategy but with different constructor args,
+    // so they get distinct artifact names.
+
+    // 1. OUSD/USDC Curve AMO
+    const dOUSDCurveAMO = await deployWithConfirmation(
+      "OUSDCurveAMOStrategy",
+      [
+        [
+          addresses.mainnet.curve.OUSD_USDC.pool, // platformAddress (Curve pool = LP token)
+          addresses.mainnet.VaultProxy, // vaultAddress
+        ],
+        addresses.mainnet.OUSDProxy, // _otoken
+        addresses.mainnet.USDC, // _hardAsset
+        addresses.mainnet.curve.OUSD_USDC.gauge, // _gauge
+        addresses.mainnet.CRVMinter, // _minter
+      ],
+      "CurveAMOStrategy", // actual contract to compile
+      true
+    );
+
+    // 2. OETH/WETH Curve AMO
+    const dOETHCurveAMO = await deployWithConfirmation(
+      "OETHCurveAMOStrategy",
+      [
+        [
+          addresses.mainnet.curve.OETH_WETH.pool, // platformAddress
+          addresses.mainnet.OETHVaultProxy, // vaultAddress
+        ],
+        addresses.mainnet.OETHProxy, // _otoken (OETH)
+        addresses.mainnet.WETH, // _hardAsset
+        addresses.mainnet.curve.OETH_WETH.gauge, // _gauge
+        addresses.mainnet.CRVMinter, // _minter
+      ],
+      "CurveAMOStrategy",
+      true
+    );
+
+    // 3. OUSD Morpho V2
+    const dOUSDMorphoV2 = await deployWithConfirmation(
+      "MorphoV2Strategy",
+      [
+        [
+          addresses.mainnet.MorphoOUSDv2Vault, // platformAddress
+          addresses.mainnet.VaultProxy, // vaultAddress
+        ],
+        addresses.mainnet.USDC, // _assetToken
+      ],
+      undefined,
+      true
+    );
+
+    // ── Governance actions ────────────────────────────────────────────────────
+    return {
+      name: "Upgrade Curve AMO and Morpho V2 strategy implementations and set CoW Harvester",
+      actions: [
+        // 1. Upgrade OUSD Curve AMO
+        {
+          contract: cOUSDCurveAMOProxy,
+          signature: "upgradeTo(address)",
+          args: [dOUSDCurveAMO.address],
+        },
+        // 2. Set Multichain Strategist on OUSD Curve AMO
+        {
+          contract: cOUSDCurveAMO,
+          signature: "setHarvesterAddress(address)",
+          args: [addresses.multichainStrategist],
+        },
+        // 3. Upgrade OETH Curve AMO
+        {
+          contract: cOETHCurveAMOProxy,
+          signature: "upgradeTo(address)",
+          args: [dOETHCurveAMO.address],
+        },
+        // 4. Set Multichain Strategist on OETH Curve AMO
+        {
+          contract: cOETHCurveAMO,
+          signature: "setHarvesterAddress(address)",
+          args: [addresses.multichainStrategist],
+        },
+        // 5. Upgrade OUSD Morpho V2
+        {
+          contract: cOUSDMorphoV2Proxy,
+          signature: "upgradeTo(address)",
+          args: [dOUSDMorphoV2.address],
+        },
+        // 6. Set CoW Harvester on OUSD Morpho V2
+        {
+          contract: cOUSDMorphoV2,
+          signature: "setHarvesterAddress(address)",
+          args: [addresses.mainnet.CoWHarvester],
+        },
+      ],
+    };
+  }
+);

contracts/test/behaviour/harvestable.js

@@ -7,10 +7,12 @@ const { impersonateAndFund } = require("../../utils/signers");
  * @param {*} context a function that returns a fixture with the additional properties:
  * - harvester: the OUSD or OETH harvester contract.
  * - strategy: the strategy to test
+ * - newBehavior: (optional, default false) set true for strategies upgraded with
+ *   the new onlyHarvester modifier that also accepts the strategist.
  * @example
     shouldBehaveLikeHarvester(() => ({
       ...fixture,
-      harvester: fixture.oethHarvester
+      harvester: fixture.oethHarvester,
       strategy: fixture.nativeStakingSSVStrategy,
     }));
  */
@@ -22,13 +24,24 @@ const shouldBehaveLikeHarvestable = (context) => {
       const harvesterSigner = await impersonateAndFund(harvester.address);
       await strategy.connect(harvesterSigner).collectRewardTokens();
     });
+
+    it("Should allow strategist to collect rewards", async () => {
+      const { newBehavior, strategist, strategy } = context();
+      if (!newBehavior) return;
+      await strategy.connect(strategist).collectRewardTokens();
+    });
+
     it("Should NOT allow rewards to be collected by non-harvester", async () => {
-      const { anna, governor, strategist, strategy } = context();
+      const { newBehavior, anna, governor, strategy } = context();
+
+      const errMsg = newBehavior
+        ? "Caller is not the Harvester or Strategist"
+        : "Caller is not the Harvester";
 
-      for (const signer of [anna, governor, strategist]) {
+      for (const signer of [anna, governor]) {
         await expect(
           strategy.connect(signer).collectRewardTokens()
-        ).to.be.revertedWith("Caller is not the Harvester");
+        ).to.be.revertedWith(errMsg);
       }
     });