diff --git a/eth-contracts/contracts/ClaimsManager.sol b/eth-contracts/contracts/ClaimsManager.sol index 4165aa09358..689aca78747 100644 --- a/eth-contracts/contracts/ClaimsManager.sol +++ b/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; } /** diff --git a/eth-contracts/contracts/DelegateManager.sol b/eth-contracts/contracts/DelegateManager.sol index 69e910692b2..b52df5af7a0 100644 --- a/eth-contracts/contracts/DelegateManager.sol +++ b/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 + ); } /** diff --git a/eth-contracts/contracts/test/MockDelegateManager.sol b/eth-contracts/contracts/test/MockDelegateManager.sol index 5c6b51842de..55b15936e4a 100644 --- a/eth-contracts/contracts/test/MockDelegateManager.sol +++ b/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 ); diff --git a/eth-contracts/test/delegateManager.test.js b/eth-contracts/test/delegateManager.test.js index 31b20ece39b..9790a25ded5 100644 --- a/eth-contracts/test/delegateManager.test.js +++ b/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,7 +287,7 @@ 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, @@ -294,8 +295,9 @@ contract('DelegateManager', async (accounts) => { 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"