@0x/contracts-staking: Fix stake accounting.

2019-09-25 15:29:22 -04:00
parent 9e3331d018
commit 5b77e2c8ac
4 changed files with 50 additions and 29 deletions
--- a/contracts/staking/contracts/src/stake/MixinStake.sol
+++ b/contracts/staking/contracts/src/stake/MixinStake.sol
@@ -187,10 +187,9 @@ contract MixinStake is
        // Sanity check the pool we're delegating to exists.
        _assertStakingPoolExists(poolId);

-        _withdrawAndSyncDelegatorRewards(
-            poolId,
-            staker
-        );
+        // Cache amount delegated to pool by staker.
+        IStructs.StoredBalance memory initDelegatedStakeToPoolByOwner =
+            _loadUnsyncedBalance(_delegatedStakeToPoolByOwner[staker][poolId]);

        // Increment how much stake the staker has delegated to the input pool.
        _increaseNextBalance(
@@ -200,6 +199,12 @@ contract MixinStake is

        // Increment how much stake has been delegated to pool.
        _increaseNextBalance(_delegatedStakeByPoolId[poolId], amount);
+
+        _withdrawAndSyncDelegatorRewards(
+            poolId,
+            staker,
+            initDelegatedStakeToPoolByOwner
+        );
    }

    /// @dev Un-Delegates a owners stake from a staking pool.
@@ -216,10 +221,9 @@ contract MixinStake is
        // sanity check the pool we're undelegating from exists
        _assertStakingPoolExists(poolId);

-        _withdrawAndSyncDelegatorRewards(
-            poolId,
-            staker
-        );
+        // Cache amount delegated to pool by staker.
+        IStructs.StoredBalance memory initDelegatedStakeToPoolByOwner =
+            _loadUnsyncedBalance(_delegatedStakeToPoolByOwner[staker][poolId]);

        // decrement how much stake the staker has delegated to the input pool
        _decreaseNextBalance(
@@ -229,6 +233,12 @@ contract MixinStake is

        // decrement how much stake has been delegated to pool
        _decreaseNextBalance(_delegatedStakeByPoolId[poolId], amount);
+
+        _withdrawAndSyncDelegatorRewards(
+            poolId,
+            staker,
+            initDelegatedStakeToPoolByOwner
+        );
    }

    /// @dev Returns a storage pointer to a user's stake in a given status.
--- a/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol
+++ b/contracts/staking/contracts/src/staking_pools/MixinStakingPoolRewards.sol
@@ -46,7 +46,8 @@ contract MixinStakingPoolRewards is

        _withdrawAndSyncDelegatorRewards(
            poolId,
-            member
+            member,
+            _loadUnsyncedBalance(_delegatedStakeToPoolByOwner[member][poolId])
        );

        // Update stored balance with synchronized version; this prevents
@@ -103,6 +104,7 @@ contract MixinStakingPoolRewards is
        return _computeDelegatorReward(
            poolId,
            member,
+            _loadUnsyncedBalance(_delegatedStakeToPoolByOwner[member][poolId]),
            unfinalizedMembersReward,
            unfinalizedMembersStake
        );
@@ -112,9 +114,12 @@ contract MixinStakingPoolRewards is
    ///      withdrawing rewards and adding/removing dependencies on cumulative rewards.
    /// @param poolId Unique id of pool.
    /// @param member of the pool.
+    /// @param unsyncedStake The member's unsynced delegated balance at the
+    ///        beginning of this transaction.
    function _withdrawAndSyncDelegatorRewards(
        bytes32 poolId,
-        address member
+        address member,
+        IStructs.StoredBalance memory unsyncedStake
    )
        internal
    {
@@ -125,6 +130,7 @@ contract MixinStakingPoolRewards is
        uint256 balance = _computeDelegatorReward(
            poolId,
            member,
+            unsyncedStake,
            // No unfinalized values because we ensured the pool is already
            // finalized.
            0,
@@ -247,12 +253,14 @@ contract MixinStakingPoolRewards is
    /// @dev Computes the reward balance in ETH of a specific member of a pool.
    /// @param poolId Unique id of pool.
    /// @param member of the pool.
+    /// @param unsyncedStake Unsynced delegated stake to pool by staker
    /// @param unfinalizedMembersReward Unfinalized total members reward (if any).
    /// @param unfinalizedMembersStake Unfinalized total members stake (if any).
    /// @return reward Balance in WETH.
    function _computeDelegatorReward(
        bytes32 poolId,
        address member,
+        IStructs.StoredBalance memory unsyncedStake,
        uint256 unfinalizedMembersReward,
        uint256 unfinalizedMembersStake
    )
@@ -267,9 +275,6 @@ contract MixinStakingPoolRewards is
            return 0;
        }

-        IStructs.StoredBalance memory unsyncedStake =
-            _loadUnsyncedBalance(_delegatedStakeToPoolByOwner[member][poolId]);
-
        // There can be no rewards if the last epoch when stake was synced is
        // equal to the current epoch, because all prior rewards, including
        // rewards finalized this epoch have been claimed.
--- a/contracts/staking/contracts/test/TestDelegatorRewards.sol
+++ b/contracts/staking/contracts/test/TestDelegatorRewards.sol
@@ -86,7 +86,7 @@ contract TestDelegatorRewards is
        _poolById[poolId].operator = operatorAddress;
        _setOperatorShare(poolId, operatorReward, membersReward);
        _initGenesisCumulativeRewards(poolId);
-        
+
        _syncPoolRewards(
            poolId,
            operatorReward + membersReward,
@@ -110,6 +110,7 @@ contract TestDelegatorRewards is
        external
    {
        _initGenesisCumulativeRewards(poolId);
+        IStructs.StoredBalance memory initialStake = _delegatedStakeToPoolByOwner[delegator][poolId];
        IStructs.StoredBalance storage _stake = _delegatedStakeToPoolByOwner[delegator][poolId];
        _stake.isInitialized = true;
        _stake.currentEpochBalance += uint96(stake);
@@ -117,7 +118,8 @@ contract TestDelegatorRewards is
        _stake.currentEpoch = uint32(currentEpoch);
        _withdrawAndSyncDelegatorRewards(
            poolId,
-            delegator
+            delegator,
+            initialStake
        );
    }

@@ -132,6 +134,7 @@ contract TestDelegatorRewards is
        external
    {
        _initGenesisCumulativeRewards(poolId);
+        IStructs.StoredBalance memory initialStake = _delegatedStakeToPoolByOwner[delegator][poolId];
        IStructs.StoredBalance storage _stake = _delegatedStakeToPoolByOwner[delegator][poolId];
        if (_stake.currentEpoch < currentEpoch) {
            _stake.currentEpochBalance = _stake.nextEpochBalance;
@@ -141,7 +144,8 @@ contract TestDelegatorRewards is
        _stake.currentEpoch = uint32(currentEpoch);
        _withdrawAndSyncDelegatorRewards(
            poolId,
-            delegator
+            delegator,
+            initialStake
        );
    }

@@ -156,6 +160,7 @@ contract TestDelegatorRewards is
        external
    {
        _initGenesisCumulativeRewards(poolId);
+        IStructs.StoredBalance memory initialStake = _delegatedStakeToPoolByOwner[delegator][poolId];
        IStructs.StoredBalance storage _stake = _delegatedStakeToPoolByOwner[delegator][poolId];
        if (_stake.currentEpoch < currentEpoch) {
            _stake.currentEpochBalance = _stake.nextEpochBalance;
@@ -165,7 +170,8 @@ contract TestDelegatorRewards is
        _stake.currentEpoch = uint32(currentEpoch);
        _withdrawAndSyncDelegatorRewards(
            poolId,
-            delegator
+            delegator,
+            initialStake
        );
    }

--- a/contracts/staking/test/cumulative_reward_tracking_test.ts
+++ b/contracts/staking/test/cumulative_reward_tracking_test.ts
@@ -39,7 +39,7 @@ blockchainTests.resets('Cumulative Reward Tracking', env => {
            await simulation.runTestAsync(
                [TestAction.Finalize],
                [TestAction.CreatePool],
-                [{ event: 'SetCumulativeReward', epoch: 1 }, { event: 'SetMostRecentCumulativeReward', epoch: 1 }],
+                [{ event: 'SetCumulativeReward', epoch: 1 }],
            );
        });
        it('delegating in the same epoch pool is created', async () => {
@@ -86,7 +86,7 @@ blockchainTests.resets('Cumulative Reward Tracking', env => {
                    // Unsets the CR for epoch 0
                    TestAction.Delegate,
                ],
-                [{ event: 'SetCumulativeReward', epoch: 1 }, { event: 'SetMostRecentCumulativeReward', epoch: 1 }],
+                [{ event: 'SetCumulativeReward', epoch: 1 }],
            );
        });
        it('re-delegating in a new epoch', async () => {
@@ -105,7 +105,7 @@ blockchainTests.resets('Cumulative Reward Tracking', env => {
                    // Sets MRCR to epoch 1
                    TestAction.Delegate,
                ],
-                [{ event: 'SetCumulativeReward', epoch: 1 }, { event: 'SetMostRecentCumulativeReward', epoch: 1 }],
+                [{ event: 'SetCumulativeReward', epoch: 1 }],
            );
        });
        it('delegating in epoch 1 then again in epoch 2', async () => {
@@ -128,7 +128,7 @@ blockchainTests.resets('Cumulative Reward Tracking', env => {
                    // Clears CR for epoch 1
                    TestAction.Delegate,
                ],
-                [{ event: 'SetCumulativeReward', epoch: 2 }, { event: 'SetMostRecentCumulativeReward', epoch: 2 }],
+                [{ event: 'SetCumulativeReward', epoch: 2 } ],
            );
        });
        it('delegate in epoch 1 then undelegate in epoch 2', async () => {
@@ -152,7 +152,7 @@ blockchainTests.resets('Cumulative Reward Tracking', env => {
                    // Clear CR for epoch 1
                    TestAction.Undelegate,
                ],
-                [{ event: 'SetCumulativeReward', epoch: 2 }, { event: 'SetMostRecentCumulativeReward', epoch: 2 }],
+                [{ event: 'SetCumulativeReward', epoch: 2 } ],
            );
        });
        it('delegate in epoch 0 and epoch 1, then undelegate half in epoch 2', async () => {
@@ -181,7 +181,7 @@ blockchainTests.resets('Cumulative Reward Tracking', env => {
                    // Clears CR for epoch 1
                    TestAction.Undelegate,
                ],
-                [{ event: 'SetCumulativeReward', epoch: 2 }, { event: 'SetMostRecentCumulativeReward', epoch: 2 }],
+                [{ event: 'SetCumulativeReward', epoch: 2 } ],
            );
        });
        it('delegate in epoch 1 and 2 then again in 3', async () => {
@@ -210,7 +210,7 @@ blockchainTests.resets('Cumulative Reward Tracking', env => {
                    // Clears CR for epoch 1
                    TestAction.Delegate,
                ],
-                [{ event: 'SetCumulativeReward', epoch: 2 }, { event: 'SetMostRecentCumulativeReward', epoch: 2 }],
+                [{ event: 'SetCumulativeReward', epoch: 2 } ],
            );
        });
        it('delegate in epoch 0, earn reward in epoch 1', async () => {
@@ -233,7 +233,7 @@ blockchainTests.resets('Cumulative Reward Tracking', env => {
                    // Sets MRCR to epoch 2
                    TestAction.Finalize,
                ],
-                [{ event: 'SetCumulativeReward', epoch: 2 }, { event: 'SetMostRecentCumulativeReward', epoch: 2 }],
+                [{ event: 'SetCumulativeReward', epoch: 2 } ],
            );
        });
        it('delegate in epoch 0, epoch 2, earn reward in epoch 3, then delegate', async () => {
@@ -341,7 +341,7 @@ blockchainTests.resets('Cumulative Reward Tracking', env => {
                    // Clears CR for epoch 1
                    TestAction.Delegate,
                ],
-                [{ event: 'SetCumulativeReward', epoch: 4 }, { event: 'SetMostRecentCumulativeReward', epoch: 4 }],
+                [{ event: 'SetCumulativeReward', epoch: 4 } ],
            );
        });
        it('earn reward in epoch 1 with no stake, then delegate', async () => {
@@ -362,7 +362,7 @@ blockchainTests.resets('Cumulative Reward Tracking', env => {
                    // Creates CR for epoch 2
                    TestAction.Delegate,
                ],
-                [{ event: 'SetCumulativeReward', epoch: 1 }, { event: 'SetMostRecentCumulativeReward', epoch: 1 }],
+                [{ event: 'SetCumulativeReward', epoch: 1 }],
            );
        });
        it('delegate in epoch 1, 3, then delegate in epoch 4', async () => {
@@ -397,7 +397,7 @@ blockchainTests.resets('Cumulative Reward Tracking', env => {
                    // Creates CR for epoch 5
                    TestAction.Delegate,
                ],
-                [{ event: 'SetCumulativeReward', epoch: 4 }, { event: 'SetMostRecentCumulativeReward', epoch: 4 }],
+                [{ event: 'SetCumulativeReward', epoch: 4 } ],
            );
        });
        it('delegate in epoch 1, then epoch 3', async () => {
@@ -425,7 +425,7 @@ blockchainTests.resets('Cumulative Reward Tracking', env => {
                    // Clears CR for epoch 2
                    TestAction.Delegate,
                ],
-                [{ event: 'SetCumulativeReward', epoch: 3 }, { event: 'SetMostRecentCumulativeReward', epoch: 3 }],
+                [{ event: 'SetCumulativeReward', epoch: 3 } ],
            );
        });
    });