[H10] A service provider can deceive its delegators

eth-contracts/contracts/ServiceProviderFactory.sol

@@ -17,13 +17,17 @@ contract ServiceProviderFactory is InitializableV2 {
     string private constant ERROR_ONLY_GOVERNANCE = (
         "ServiceProviderFactory: Only callable by Governance contract"
     );
+    string private constant ERROR_ONLY_SP_GOVERNANCE = (
+        "ServiceProviderFactory: Only callable by Service Provider or Governance"
+    );
 
     address private stakingAddress;
     address private delegateManagerAddress;
     address private governanceAddress;
     address private serviceTypeManagerAddress;
     address private claimsManagerAddress;
     uint256 private decreaseStakeLockupDuration;
+    uint256 private deployerCutLockupDuration;
 
     /// @dev - Stores following entities
     ///        1) Directly staked amount by SP, not including delegators
@@ -47,8 +51,11 @@ contract ServiceProviderFactory is InitializableV2 {
         uint256 lockupExpiryBlock;
     }
 
-    /// @dev - Mapping of service provider address to details
-    mapping(address => ServiceProviderDetails) private spDetails;
+    /// @dev - Data structure for time delay during deployer cut update
+    struct UpdateDeployerCutRequest {
+        uint256 newDeployerCut;
+        uint256 lockupExpiryBlock;
+    }
 
     /// @dev - Struct maintaining information about sp
     /// @dev - blocknumber is block.number when endpoint registered
@@ -59,6 +66,9 @@ contract ServiceProviderFactory is InitializableV2 {
         address delegateOwnerWallet;
     }
 
+    /// @dev - Mapping of service provider address to details
+    mapping(address => ServiceProviderDetails) private spDetails;
+
     /// @dev - Uniquely assigned serviceProvider ID, incremented for each service type
     /// @notice - Keeps track of the total number of services registered regardless of
     ///           whether some have been deregistered since
@@ -82,6 +92,9 @@ contract ServiceProviderFactory is InitializableV2 {
     /// @dev - Mapping of service provider -> decrease stake request
     mapping(address => DecreaseStakeRequest) private decreaseStakeRequests;
 
+    /// @dev - Mapping of service provider -> update deployer cut requests
+    mapping(address => UpdateDeployerCutRequest) private updateDeployerCutRequests;
+
     event RegisteredServiceProvider(
       uint256 indexed _spID,
       bytes32 indexed _serviceType,
@@ -124,6 +137,7 @@ contract ServiceProviderFactory is InitializableV2 {
     );
 
     event DecreaseStakeLockupDurationUpdated(uint256 indexed _lockupDuration);
+    event UpdateDeployerCutLockupDurationUpdated(uint256 indexed _lockupDuration);
     event GovernanceAddressUpdated(address indexed _newGovernanceAddress);
     event StakingAddressUpdated(address indexed _newStakingAddress);
     event ClaimsManagerAddressUpdated(address indexed _newClaimsManagerAddress);
@@ -140,11 +154,15 @@ contract ServiceProviderFactory is InitializableV2 {
      */
     function initialize (
         address _governanceAddress,
-        uint256 _decreaseStakeLockupDuration
+        address _claimsManagerAddress,
+        uint256 _decreaseStakeLockupDuration,
+        uint256 _deployerCutLockupDuration
     ) public initializer
     {
-        decreaseStakeLockupDuration = _decreaseStakeLockupDuration;
         _updateGovernanceAddress(_governanceAddress);
+        claimsManagerAddress = _claimsManagerAddress;
+        _updateDecreaseStakeLockupDuration(_decreaseStakeLockupDuration);
+        _updateDeployerCutLockupDuration(_deployerCutLockupDuration);
         InitializableV2.initialize();
     }
 
@@ -575,6 +593,86 @@ contract ServiceProviderFactory is InitializableV2 {
         return spId;
     }
 
+    /**
+     * @notice Update the deployer cut for a given service provider
+     * @param _serviceProvider - address of service provider
+     * @param _cut - new value for deployer cut
+     */
+    function requestUpdateDeployerCut(address _serviceProvider, uint256 _cut) external
+    {
+        _requireIsInitialized();
+
+        require(
+            msg.sender == _serviceProvider || msg.sender == governanceAddress,
+            ERROR_ONLY_SP_GOVERNANCE
+        );
+
+        require(
+            (updateDeployerCutRequests[_serviceProvider].lockupExpiryBlock == 0) &&
+            (updateDeployerCutRequests[_serviceProvider].newDeployerCut == 0),
+            "ServiceProviderFactory: Update deployer cut operation pending"
+        );
+
+        require(
+            _cut <= DEPLOYER_CUT_BASE,
+            "ServiceProviderFactory: Service Provider cut cannot exceed base value"
+        );
+
+        updateDeployerCutRequests[_serviceProvider] = UpdateDeployerCutRequest({
+            lockupExpiryBlock: block.number + deployerCutLockupDuration,
+            newDeployerCut: _cut
+        });
+    }
+
+    /**
+     * @notice Cancel a pending request to update deployer cut
+     * @param _serviceProvider - address of service provider
+     */
+    function cancelUpdateDeployerCut(address _serviceProvider) external
+    {
+        _requireIsInitialized();
+        _requirePendingDeployerCutOperation(_serviceProvider);
+
+        require(
+            msg.sender == _serviceProvider || msg.sender == governanceAddress,
+            ERROR_ONLY_SP_GOVERNANCE
+        );
+
+        // Zero out request information
+        delete updateDeployerCutRequests[_serviceProvider];
+    }
+
+    /**
+     * @notice Evalue request to update service provider cut of claims
+     * @notice Update service provider cut as % of delegate claim, divided by the deployerCutBase.
+     * @dev SPs will interact with this value as a percent, value translation done client side
+       @dev A value of 5 dictates a 5% cut, with ( 5 / 100 ) * delegateReward going to an SP from each delegator each round.
+     */
+    function updateDeployerCut(address _serviceProvider) external
+    {
+        _requireIsInitialized();
+        _requirePendingDeployerCutOperation(_serviceProvider);
+
+        require(
+            msg.sender == _serviceProvider || msg.sender == governanceAddress,
+            ERROR_ONLY_SP_GOVERNANCE
+        );
+
+        require(
+            updateDeployerCutRequests[_serviceProvider].lockupExpiryBlock <= block.number,
+            "ServiceProviderFactory: Lockup must be expired"
+        );
+
+        spDetails[_serviceProvider].deployerCut = (
+            updateDeployerCutRequests[_serviceProvider].newDeployerCut
+        );
+
+        // Zero out request information
+        delete updateDeployerCutRequests[_serviceProvider];
+
+        emit ServiceProviderCutUpdated(_serviceProvider, spDetails[_serviceProvider].deployerCut);
+    }
+
     /**
      * @notice Update service provider balance
      * @dev Called by DelegateManager by functions modifying entire stake like claim and slash
@@ -599,43 +697,30 @@ contract ServiceProviderFactory is InitializableV2 {
         _updateServiceProviderBoundStatus(_serviceProvider);
     }
 
-    /**
-     * @notice Update service provider cut of claims
-     * @notice Update service provider cut as % of delegate claim, divided by the deployerCutBase.
-     * @dev SPs will interact with this value as a percent, value translation done client side
-       @dev A value of 5 dictates a 5% cut, with ( 5 / 100 ) * delegateReward going to an SP from each delegator each round.
-     * @param _serviceProvider - address of service provider
-     * @param _cut - new deployer cut value
-     */
-    function updateServiceProviderCut(
-        address _serviceProvider,
-        uint256 _cut
-    ) external
-    {
+    /// @notice Update service provider lockup duration
+    function updateDecreaseStakeLockupDuration(uint256 _duration) external {
         _requireIsInitialized();
 
         require(
-            msg.sender == _serviceProvider,
-            "ServiceProviderFactory: Service Provider cut update operation restricted to deployer");
+            msg.sender == governanceAddress,
+            ERROR_ONLY_GOVERNANCE
+        );
 
-        require(
-            _cut <= DEPLOYER_CUT_BASE,
-            "ServiceProviderFactory: Service Provider cut cannot exceed base value");
-        spDetails[_serviceProvider].deployerCut = _cut;
-        emit ServiceProviderCutUpdated(_serviceProvider, _cut);
+        _updateDecreaseStakeLockupDuration(_duration);
+        emit DecreaseStakeLockupDurationUpdated(_duration);
     }
 
     /// @notice Update service provider lockup duration
-    function updateDecreaseStakeLockupDuration(uint256 _duration) external {
+    function updateDeployerCutLockupDuration(uint256 _duration) external {
         _requireIsInitialized();
 
         require(
             msg.sender == governanceAddress,
             ERROR_ONLY_GOVERNANCE
         );
 
-        decreaseStakeLockupDuration = _duration;
-        emit DecreaseStakeLockupDurationUpdated(_duration);
+        _updateDeployerCutLockupDuration(_duration);
+        emit UpdateDeployerCutLockupDurationUpdated(_duration);
     }
 
     /// @notice Get denominator for deployer cut calculations
@@ -647,6 +732,15 @@ contract ServiceProviderFactory is InitializableV2 {
         return DEPLOYER_CUT_BASE;
     }
 
+    /// @notice Get current deployer cut update lockup duration
+    function getDeployerCutLockupDuration()
+    external view returns (uint256)
+    {
+        _requireIsInitialized();
+
+        return deployerCutLockupDuration;
+    }
+
     /// @notice Get total number of service providers for a given serviceType
     function getTotalServiceTypeProviders(bytes32 _serviceType)
     external view returns (uint256)
@@ -736,6 +830,21 @@ contract ServiceProviderFactory is InitializableV2 {
         );
     }
 
+    /**
+     * @notice Get information about pending decrease stake requests for service provider
+     * @param _serviceProvider - address of service provider
+     */
+    function getPendingUpdateDeployerCutRequest(address _serviceProvider)
+    external view returns (uint256 newDeployerCut, uint256 lockupExpiryBlock)
+    {
+        _requireIsInitialized();
+
+        return (
+            updateDeployerCutRequests[_serviceProvider].newDeployerCut,
+            updateDeployerCutRequests[_serviceProvider].lockupExpiryBlock
+        );
+    }
+
     /// @notice Get current unstake lockup duration
     function getDecreaseStakeLockupDuration()
     external view returns (uint256)
@@ -892,6 +1001,33 @@ contract ServiceProviderFactory is InitializableV2 {
         governanceAddress = _governanceAddress;
     }
 
+    /**
+     * @notice Set the deployer cut lockup duration
+     * @param _duration - incoming duration
+     */
+    function _updateDeployerCutLockupDuration(uint256 _duration) internal
+    {
+        require(
+            ClaimsManager(claimsManagerAddress).getFundingRoundBlockDiff() < _duration,
+            "ServiceProviderFactory: Incoming duration must be greater than funding round block diff"
+        );
+        deployerCutLockupDuration = _duration;
+    }
+
+    /**
+     * @notice Set the decrease stake lockup duration
+     * @param _duration - incoming duration
+     */
+    function _updateDecreaseStakeLockupDuration(uint256 _duration) internal
+    {
+        Governance governance = Governance(governanceAddress);
+        require(
+            _duration > governance.getVotingPeriod() + governance.getExecutionDelay(),
+            "ServiceProviderFactory: decreaseStakeLockupDuration duration must be greater than governance votingPeriod + executionDelay"
+        );
+        decreaseStakeLockupDuration = _duration;
+    }
+
     /**
      * @notice Compare a given amount input against valid min and max bounds for service provider
      * @param _serviceProvider - address of service provider
@@ -936,6 +1072,12 @@ contract ServiceProviderFactory is InitializableV2 {
     }
 
     // ========================================= Private Functions =========================================
+    function _requirePendingDeployerCutOperation (address _serviceProvider) private view {
+        require(
+            (updateDeployerCutRequests[_serviceProvider].lockupExpiryBlock != 0),
+            "ServiceProviderFactory: No update deployer cut operation pending"
+        );
+    }
 
     function _requireStakingAddressIsSet() private view {
         require(

eth-contracts/migrations/7_versioning_service_migration.js

@@ -35,6 +35,10 @@ const dpTypeMax = _lib.audToWei(2000000)
 // - 1/13 block/s * 604800 s/wk ~= 46523 block/wk
 const decreaseStakeLockupDuration = 46523
 
+// modifying deployer cut = 8 days in blocks
+// - 1/13 block/s * 691200 s/8 days ~= 53169 block/wk
+const deployerCutLockupDuration = 53169
+
 module.exports = (deployer, network, accounts) => {
   deployer.then(async () => {
     const config = contractConfig[network]
@@ -106,8 +110,13 @@ module.exports = (deployer, network, accounts) => {
     const serviceProviderFactory0 = await deployer.deploy(ServiceProviderFactory, { from: proxyDeployerAddress })
     const serviceProviderFactoryCalldata = _lib.encodeCall(
       'initialize',
-      ['address', 'uint256'],
-      [process.env.governanceAddress, decreaseStakeLockupDuration]
+      ['address', 'address', 'uint256', 'uint256'],
+      [
+        process.env.governanceAddress,
+        process.env.claimsManagerAddress,
+        decreaseStakeLockupDuration,
+        deployerCutLockupDuration
+      ]
     )
     const serviceProviderFactoryProxy = await deployer.deploy(
       AudiusAdminUpgradeabilityProxy,