[C02] Rewards pay out incorrectly when the service provider has a pending “decrease stake” request

eth-contracts/contracts/ClaimsManager.sol

@@ -227,7 +227,7 @@ contract ClaimsManager is InitializableV2 {
  function processClaim(
  address _claimer,
  uint _totalLockedForSP
- ) external
+ ) external returns (uint mintedRewards)
  {
  _requireIsInitialized();
  require(
@@ -262,7 +262,7 @@ contract ClaimsManager is InitializableV2 {
  // Total rewards can be zero if all stake is currently locked up
  if (!withinBounds || rewardsForClaimer == 0) {
  stakingContract.updateClaimHistory(0, _claimer);
- return;
+ return 0;
  }
 
  // ERC20Mintable always returns true
@@ -286,6 +286,8 @@ contract ClaimsManager is InitializableV2 {
  totalStakedAtFundBlockForClaimer,
  newTotal
  );
+
+ return rewardsForClaimer;
  }
 
  /**

eth-contracts/contracts/DelegateManager.sol

@@ -338,18 +338,19 @@ contract DelegateManager is InitializableV2 {
  (
  uint totalBalanceInStaking,
  uint totalBalanceInSPFactory,
- uint totalBalanceOutsideStaking
+ uint totalActiveFunds,
+ uint spLockedStake,
+ uint totalRewards
  ) = _validateClaimRewards(spFactory);
 
  // No-op if balance is already equivalent
  // This case can occur if no rewards due to bound violation or all stake is locked
- if (totalBalanceInStaking == totalBalanceOutsideStaking) {
+ if (totalRewards == 0) {
  return;
  }
 
  // Total rewards
  // Equal to (balance in staking) - ((balance in sp factory) + (balance in delegate manager))
- uint totalRewards = totalBalanceInStaking.sub(totalBalanceOutsideStaking);
 
  // Emit claim event
  emit Claim(msg.sender, totalRewards, totalBalanceInStaking);
@@ -359,11 +360,6 @@ contract DelegateManager is InitializableV2 {
  uint spDeployerCutRewards = 0;
  uint totalDelegatedStakeIncrease = 0;
 
- // Total valid funds used to calculate rewards distribution
- uint totalActiveFunds = (
- totalBalanceOutsideStaking.sub(spDelegateInfo[msg.sender].totalLockedUpStake)
- );
-
  // Traverse all delegates and calculate their rewards
  // As each delegate reward is calculated, increment SP cut reward accordingly
  for (uint i = 0; i < spDelegateInfo[msg.sender].delegators.length; i++) {
@@ -412,8 +408,10 @@ contract DelegateManager is InitializableV2 {
  );
 
  // Rewards directly allocated to service provider for their stake
+ // Total active funds for direct deployer reward share
+ /// totalActiveDeployerFunds = totalBalanceInSPFactory.sub(spLockedStake);
  uint spRewardShare = (
- totalBalanceInSPFactory.mul(totalRewards)
+ (totalBalanceInSPFactory.sub(spLockedStake)).mul(totalRewards)
  ).div(totalActiveFunds);
 
  spFactory.updateServiceProviderStake(
@@ -700,41 +698,56 @@ contract DelegateManager is InitializableV2 {
  * @notice Helper function for claimRewards to get balances from Staking contract
  and do validation
  * @param spFactory - reference to ServiceProviderFactory contract
- * @return (totalBalanceInStaking, totalBalanceInSPFactory, totalBalanceOutsideStaking)
+ * @return (totalBalanceInStaking, totalBalanceInSPFactory, totalActiveFunds, spLockedStake, totalRewards)
  */
  function _validateClaimRewards(ServiceProviderFactory spFactory)
- internal returns (uint totalBalanceInStaking, uint totalBalanceInSPFactory, uint totalBalanceOutsideStaking)
- {
-
+ internal returns (
+ uint totalBalanceInStaking,
+ uint totalBalanceInSPFactory,
+ uint totalActiveFunds,
+ uint spLockedStake,
+ uint totalRewards
+ )
+ {
  // Account for any pending locked up stake for the service provider
- (uint spLockedStake,) = spFactory.getPendingDecreaseStakeRequest(msg.sender);
+ (spLockedStake,) = spFactory.getPendingDecreaseStakeRequest(msg.sender);
+ uint totalLockedUpStake = spDelegateInfo[msg.sender].totalLockedUpStake.add(spLockedStake);
 
  // Process claim for msg.sender
  // Total locked parameter is equal to delegate locked up stake + service provider locked up stake
- ClaimsManager(claimsManagerAddress).processClaim(
+uint mintedRewards = ClaimsManager(claimsManagerAddress).processClaim(
  msg.sender,
-(spDelegateInfo[msg.sender].totalLockedUpStake.add(spLockedStake))
+ totalLockedUpStake
  );
 
  // Amount stored in staking contract for owner
-uint _totalBalanceInStaking = Staking(stakingAddress).totalStakedFor(msg.sender);
- require(_totalBalanceInStaking > 0, "Stake required for claim");
+totalBalanceInStaking = Staking(stakingAddress).totalStakedFor(msg.sender);
+ require(totalBalanceInStaking > 0, "Stake required for claim");
 
  // Amount in sp factory for claimer
- (uint _totalBalanceInSPFactory,,,,,) = spFactory.getServiceProviderDetails(msg.sender);
-
- // Decrease total balance by any locked up stake
- _totalBalanceInSPFactory = _totalBalanceInSPFactory.sub(spLockedStake);
-
+ (totalBalanceInSPFactory,,,,,) = spFactory.getServiceProviderDetails(msg.sender);
  // Require active stake to claim any rewards
- require(_totalBalanceInSPFactory > 0, "Service Provider stake required");
+ require(totalBalanceInSPFactory.sub(spLockedStake) > 0, "Service Provider stake required");
 
  // Amount in delegate manager staked to service provider
- uint _totalBalanceOutsideStaking = (
-_totalBalanceInSPFactory.add(spDelegateInfo[msg.sender].totalDelegatedStake)
+ uint totalBalanceOutsideStaking = (
+totalBalanceInSPFactory.add(spDelegateInfo[msg.sender].totalDelegatedStake)
  );
 
- return (_totalBalanceInStaking, _totalBalanceInSPFactory, _totalBalanceOutsideStaking);
+ totalActiveFunds = totalBalanceOutsideStaking.sub(totalLockedUpStake);
+
+ require(
+ mintedRewards == totalBalanceInStaking.sub(totalBalanceOutsideStaking),
+ "Reward amount mismatch"
+ );
+
+ return (
+ totalBalanceInStaking,
+ totalBalanceInSPFactory,
+ totalActiveFunds,
+ spLockedStake,
+ mintedRewards
+ );
  }
 
  /**

eth-contracts/contracts/test/MockDelegateManager.sol

@@ -21,7 +21,7 @@ contract MockDelegateManager is InitializableV2 {
  function testProcessClaim(
  address _claimer,
  uint _totalLockedForSP
- ) external {
+ ) external returns (uint) {
  ClaimsManager claimsManager = ClaimsManager(
  claimsManagerAddress
  );

eth-contracts/test/delegateManager.test.js

@@ -270,9 +270,10 @@ contract('DelegateManager', async (accounts) => {
  let spDetails = await serviceProviderFactory.getServiceProviderDetails(account)
  spFactoryStake = spDetails.deployerStake
  totalInStakingContract = await staking.totalStakedFor(account)
+ let spDecreaseRequest = await serviceProviderFactory.getPendingDecreaseStakeRequest(account)
 
  let delegatedStake = await delegateManager.getTotalDelegatedToServiceProvider(account)
- let lockedUpStake = await delegateManager.getTotalLockedDelegationForServiceProvider(account)
+ let lockedUpDelegatorStake = await delegateManager.getTotalLockedDelegationForServiceProvider(account)
  let delegatorInfo = {}
  let delegators = await delegateManager.getDelegatorsList(account)
  for (var i = 0; i < delegators.length; i++) {
@@ -286,16 +287,17 @@ contract('DelegateManager', async (accounts) => {
  }
  }
  let outsideStake = spFactoryStake.add(delegatedStake)
- let totalActiveStake = outsideStake.sub(lockedUpStake)
+ let totalActiveStake = outsideStake.sub(lockedUpDelegatorStake)
  let stakeDiscrepancy = totalInStakingContract.sub(outsideStake)
  let accountSummary = {
  totalInStakingContract,
  delegatedStake,
  spFactoryStake,
  delegatorInfo,
  outsideStake,
- lockedUpStake,
- totalActiveStake
+ lockedUpDelegatorStake,
+ totalActiveStake,
+ spDecreaseRequest
  }
 
  if (print) {
@@ -914,7 +916,7 @@ contract('DelegateManager', async (accounts) => {
  )
 
  let preSlashInfo = await getAccountStakeInfo(stakerAccount, false)
- let preSlashLockupStake = preSlashInfo.lockedUpStake
+ let preSlashLockupStake = preSlashInfo.lockedUpDelegatorStake
  assert.isTrue(
  preSlashLockupStake.eq(initialDelegateAmount),
  'Initial delegate amount not found')
@@ -924,7 +926,7 @@ contract('DelegateManager', async (accounts) => {
 
  let postRewardInfo = await getAccountStakeInfo(stakerAccount, false)
 
- let postSlashLockupStake = postRewardInfo.lockedUpStake
+ let postSlashLockupStake = postRewardInfo.lockedUpDelegatorStake
  assert.equal(
  postSlashLockupStake,
  0,
@@ -1483,11 +1485,12 @@ contract('DelegateManager', async (accounts) => {
  let fundsPerRound = await claimsManager.getFundsPerRound()
  let expectedIncrease = fundsPerRound.div(_lib.toBN(4))
  // Request decrease in stake corresponding to 1/2 of DEFAULT_AMOUNT
- await serviceProviderFactory.requestDecreaseStake(DEFAULT_AMOUNT.div(_lib.toBN(2)), { from: stakerAccount })
- let info = await getAccountStakeInfo(stakerAccount)
+ let decreaseStakeAmount = DEFAULT_AMOUNT.div(_lib.toBN(2))
+ await serviceProviderFactory.requestDecreaseStake(decreaseStakeAmount, { from: stakerAccount })
+ let info = await getAccountStakeInfo(stakerAccount, true)
  await claimsManager.initiateRound({ from: stakerAccount })
  await delegateManager.claimRewards({ from: stakerAccount })
- let info2 = await getAccountStakeInfo(stakerAccount)
+ let info2 = await getAccountStakeInfo(stakerAccount, true)
  let stakingDiff = (info2.totalInStakingContract).sub(info.totalInStakingContract)
  let spFactoryDiff = (info2.spFactoryStake).sub(info.spFactoryStake)
  assert.isTrue(stakingDiff.eq(expectedIncrease), 'Expected increase not found in Staking.sol')
@@ -1499,7 +1502,7 @@ contract('DelegateManager', async (accounts) => {
  let requestInfo = await serviceProviderFactory.getPendingDecreaseStakeRequest(stakerAccount)
  assert.isTrue((requestInfo.lockupExpiryBlock).gt(_lib.toBN(0)), 'Expected lockup expiry block to be set')
  assert.isTrue((requestInfo.amount).gt(_lib.toBN(0)), 'Expected amount to be set')
-
+
  // Slash
  await _lib.slash(_lib.audToWei(5), slasherAccount, governance, delegateManagerKey, guardianAddress)
 
@@ -1512,7 +1515,7 @@ contract('DelegateManager', async (accounts) => {
  let duration = await serviceProviderFactory.getDecreaseStakeLockupDuration()
  // Double decrease stake duration
  let newDuration = duration.add(duration)
-
+
  await _lib.assertRevert(
  serviceProviderFactory.updateDecreaseStakeLockupDuration(newDuration),
  "Only callable by Governance contract"