Extracted getLockedTokensInPast method, some optimization

nucypher/blockchain/eth/sol/source/contracts/MinersEscrow.sol

@@ -210,29 +210,56 @@ contract MinersEscrow is Issuer {
         return getLastPeriodOfSubStake(subStake, startPeriod);
     }
 
+
     /**
     * @notice Get the value of locked tokens for a miner in a specified period
     * @dev Information may be incorrect for mined or unconfirmed surpassed period
-    * @param _miner Miner
+    * @param _info Miner structure
-    * @param _periods Amount of periods that will be added to the current period
+    * @param _currentPeriod Current period
+    * @param _period Next period
     **/
-    function getLockedTokens(address _miner, int16 _periods)
+    function getLockedTokens(MinerInfo storage _info, uint16 _currentPeriod, uint16 _period)
-        public view returns (uint256 lockedValue)
+        internal view returns (uint256 lockedValue)
     {
-        uint16 startPeriod = getCurrentPeriod();
+        uint16 startPeriod = getStartPeriod(_info, _currentPeriod);
-        uint16 nextPeriod = startPeriod.addSigned16(_periods);
+        for (uint256 i = 0; i < _info.subStakes.length; i++) {
-        MinerInfo storage info = minerInfo[_miner];
+            SubStakeInfo storage subStake = _info.subStakes[i];
-        startPeriod = getStartPeriod(info, startPeriod);
+            if (subStake.firstPeriod <= _period &&
-
+                getLastPeriodOfSubStake(subStake, startPeriod) >= _period) {
-        for (uint256 i = 0; i < info.subStakes.length; i++) {
-            SubStakeInfo storage subStake = info.subStakes[i];
-            if (subStake.firstPeriod <= nextPeriod &&
-                getLastPeriodOfSubStake(subStake, startPeriod) >= nextPeriod) {
                 lockedValue = lockedValue.add(subStake.lockedValue);
             }
         }
     }
 
+    /**
+    * @notice Get the value of locked tokens for a miner in a future period
+    * @param _miner Miner
+    * @param _periods Amount of periods that will be added to the current period
+    **/
+    function getLockedTokens(address _miner, uint16 _periods)
+        public view returns (uint256 lockedValue)
+    {
+        MinerInfo storage info = minerInfo[_miner];
+        uint16 currentPeriod = getCurrentPeriod();
+        uint16 nextPeriod = currentPeriod.add16(_periods);
+        return getLockedTokens(info, currentPeriod, nextPeriod);
+    }
+
+    /**
+    * @notice Get the value of locked tokens for a miner in a previous period
+    * @dev Information may be incorrect for mined or unconfirmed surpassed period
+    * @param _miner Miner
+    * @param _periods Amount of periods that will be subtracted from the current period
+    **/
+    function getLockedTokensInPast(address _miner, uint16 _periods)
+        public view returns (uint256 lockedValue)
+    {
+        MinerInfo storage info = minerInfo[_miner];
+        uint16 currentPeriod = getCurrentPeriod();
+        uint16 previousPeriod = currentPeriod.sub16(_periods);
+        return getLockedTokens(info, currentPeriod, previousPeriod);
+    }
+
     /**
     * @notice Get the value of locked tokens for a miner in the current period
     * @param _miner Miner
@@ -265,6 +292,7 @@ contract MinersEscrow is Issuer {
     {
         require(_periods > 0);
         uint16 currentPeriod = getCurrentPeriod();
+        uint16 nextPeriod = currentPeriod.add16(_periods);
         for (uint256 i = 0; i < miners.length; i++) {
             address miner = miners[i];
             MinerInfo storage info = minerInfo[miner];
@@ -272,7 +300,7 @@ contract MinersEscrow is Issuer {
                 info.confirmedPeriod2 != currentPeriod) {
                 continue;
             }
-            lockedTokens = lockedTokens.add(getLockedTokens(miner, int16(_periods)));
+            lockedTokens = lockedTokens.add(getLockedTokens(info, currentPeriod, nextPeriod));
         }
     }
 
@@ -393,12 +421,13 @@ contract MinersEscrow is Issuer {
         uint16 lastActivePeriod = getLastActivePeriod(_miner);
         mint(_miner);
 
-        uint256 lockedTokens = getLockedTokens(_miner, 1);
+        uint16 currentPeriod = getCurrentPeriod();
+        uint16 nextPeriod = currentPeriod.add16(1);
         MinerInfo storage info = minerInfo[_miner];
+        uint256 lockedTokens = getLockedTokens(info, currentPeriod, nextPeriod);
         require(_value.add(lockedTokens) <= info.value &&
             _value.add(lockedTokens) <= maxAllowableLockedTokens);
 
-        uint16 nextPeriod = getCurrentPeriod().add16(1);
         if (info.confirmedPeriod1 != nextPeriod && info.confirmedPeriod2 != nextPeriod) {
             saveSubStake(info, nextPeriod, 0, _periods, _value);
         } else {
@@ -487,11 +516,13 @@ contract MinersEscrow is Issuer {
     * @param _value Amount of tokens to withdraw
     **/
     function withdraw(uint256 _value) public onlyMiner {
+        uint16 currentPeriod = getCurrentPeriod();
+        uint16 nextPeriod = currentPeriod.add16(1);
         MinerInfo storage info = minerInfo[msg.sender];
         // the max locked tokens in most cases will be in the current period
         // but when the miner stakes more then we should use the next period
-        uint256 lockedTokens = Math.max(getLockedTokens(msg.sender, 1),
+        uint256 lockedTokens = Math.max(getLockedTokens(info, currentPeriod, nextPeriod),
-            getLockedTokens(msg.sender, 0));
+            getLockedTokens(info, currentPeriod, currentPeriod));
         require(_value <= token.balanceOf(address(this)) &&
             _value <= info.value.sub(lockedTokens));
         info.value -= _value;
@@ -561,7 +592,7 @@ contract MinersEscrow is Issuer {
         mint(msg.sender);
         MinerInfo storage info = minerInfo[msg.sender];
         uint16 currentPeriod = getCurrentPeriod();
-        uint16 nextPeriod = currentPeriod + 1;
+        uint16 nextPeriod = currentPeriod.add16(1);
 
         // the period has already been confirmed
         if (info.confirmedPeriod1 == nextPeriod ||
@@ -569,7 +600,7 @@ contract MinersEscrow is Issuer {
             return;
         }
 
-        uint256 lockedTokens = getLockedTokens(msg.sender, 1);
+        uint256 lockedTokens = getLockedTokens(info, currentPeriod, nextPeriod);
         confirmActivity(msg.sender, lockedTokens, 0, lastActivePeriod);
     }
 
@@ -696,6 +727,7 @@ contract MinersEscrow is Issuer {
     {
         require(_periods > 0 && _points.length > 0);
         uint16 currentPeriod = getCurrentPeriod();
+        uint16 nextPeriod = currentPeriod.add16(_periods);
         result = new address[](_points.length);
 
         uint256 pointIndex = 0;
@@ -713,7 +745,7 @@ contract MinersEscrow is Issuer {
                 continue;
             }
             if (addMoreTokens) {
-                sumOfLockedTokens = sumOfLockedTokens.add(getLockedTokens(currentMiner, int16(_periods)));
+                sumOfLockedTokens = sumOfLockedTokens.add(getLockedTokens(info, currentPeriod, nextPeriod));
             }
             if (sumOfLockedTokens > point) {
                 result[pointIndex] = currentMiner;

tests/blockchain/eth/contracts/contracts/MinersEscrowTestSet.sol

@@ -151,4 +151,4 @@ contract ChallengeOverseerForMinersEscrowMock {
     {
         escrow.slashMiner(_miner, _penalty, _investigator, _reward);
     }
-}
+}

tests/blockchain/eth/contracts/integration/test_intercontract_integration.py

@@ -589,7 +589,7 @@ def test_all(testerchain, token, escrow, policy_manager, overseer, user_escrow_p
     # Slash part of the free amount of tokens
     period = escrow.functions.getCurrentPeriod().call()
     tokens_amount = escrow.functions.minerInfo(ursula1).call()[VALUE_FIELD]
-    previous_lock = escrow.functions.getLockedTokens(ursula1, -1).call()
+    previous_lock = escrow.functions.getLockedTokensInPast(ursula1, 1).call()
     lock = escrow.functions.getLockedTokens(ursula1).call()
     next_lock = escrow.functions.getLockedTokens(ursula1, 1).call()
     total_previous_lock = escrow.functions.lockedPerPeriod(period - 1).call()
@@ -598,7 +598,7 @@ def test_all(testerchain, token, escrow, policy_manager, overseer, user_escrow_p
     tx = overseer.functions.slashMiner(ursula1, 100, alice1, 10).transact()
     testerchain.wait_for_receipt(tx)
     assert tokens_amount - 100 == escrow.functions.minerInfo(ursula1).call()[VALUE_FIELD]
-    assert previous_lock == escrow.functions.getLockedTokens(ursula1, -1).call()
+    assert previous_lock == escrow.functions.getLockedTokensInPast(ursula1, 1).call()
     assert lock == escrow.functions.getLockedTokens(ursula1).call()
     assert next_lock == escrow.functions.getLockedTokens(ursula1, 1).call()
     assert total_previous_lock == escrow.functions.lockedPerPeriod(period - 1).call()
@@ -609,13 +609,13 @@ def test_all(testerchain, token, escrow, policy_manager, overseer, user_escrow_p
     # Slash part of the one sub stake
     tokens_amount = escrow.functions.minerInfo(ursula2).call()[VALUE_FIELD]
     unlocked_amount = tokens_amount - escrow.functions.getLockedTokens(ursula2).call()
-    previous_lock = escrow.functions.getLockedTokens(ursula2, -1).call()
+    previous_lock = escrow.functions.getLockedTokensInPast(ursula2, 1).call()
     lock = escrow.functions.getLockedTokens(ursula2).call()
     next_lock = escrow.functions.getLockedTokens(ursula2, 1).call()
     tx = overseer.functions.slashMiner(ursula2, unlocked_amount + 100, alice1, 20).transact()
     testerchain.wait_for_receipt(tx)
     assert lock - 100 == escrow.functions.minerInfo(ursula2).call()[VALUE_FIELD]
-    assert previous_lock == escrow.functions.getLockedTokens(ursula2, -1).call()
+    assert previous_lock == escrow.functions.getLockedTokensInPast(ursula2, 1).call()
     assert lock - 100 == escrow.functions.getLockedTokens(ursula2).call()
     assert next_lock - 100 == escrow.functions.getLockedTokens(ursula2, 1).call()
     assert total_previous_lock == escrow.functions.lockedPerPeriod(period - 1).call()
@@ -626,14 +626,14 @@ def test_all(testerchain, token, escrow, policy_manager, overseer, user_escrow_p
     # Slash two sub stakes
     tokens_amount = escrow.functions.minerInfo(user_escrow_1.address).call()[VALUE_FIELD]
     unlocked_amount = tokens_amount - escrow.functions.getLockedTokens(user_escrow_1.address).call()
-    previous_lock = escrow.functions.getLockedTokens(user_escrow_1.address, -1).call()
+    previous_lock = escrow.functions.getLockedTokensInPast(user_escrow_1.address, 1).call()
     lock = escrow.functions.getLockedTokens(user_escrow_1.address).call()
     next_lock = escrow.functions.getLockedTokens(user_escrow_1.address, 1).call()
     alice2_balance = token.functions.balanceOf(alice2).call()
     tx = overseer.functions.slashMiner(user_escrow_1.address, unlocked_amount + 600, alice2, 60).transact()
     testerchain.wait_for_receipt(tx)
     assert lock - 600 == escrow.functions.minerInfo(user_escrow_1.address).call()[VALUE_FIELD]
-    assert previous_lock == escrow.functions.getLockedTokens(user_escrow_1.address, -1).call()
+    assert previous_lock == escrow.functions.getLockedTokensInPast(user_escrow_1.address, 1).call()
     assert lock - 600 == escrow.functions.getLockedTokens(user_escrow_1.address).call()
     assert next_lock - 600 == escrow.functions.getLockedTokens(user_escrow_1.address, 1).call()
     assert total_previous_lock == escrow.functions.lockedPerPeriod(period - 1).call()

tests/blockchain/eth/contracts/main/miners_escrow/test_mining.py

@@ -402,7 +402,7 @@ def test_slashing(testerchain, token, escrow_contract):
     testerchain.wait_for_receipt(tx)
     testerchain.time_travel(hours=1)
     period += 1
-    assert 90 == escrow.functions.getLockedTokens(ursula, -1).call()
+    assert 90 == escrow.functions.getLockedTokensInPast(ursula, 1).call()
     assert 190 == escrow.functions.getLockedTokens(ursula).call()
     assert 100 == escrow.functions.getLockedTokens(ursula, 4).call()
     assert 190 == escrow.functions.minerInfo(ursula).call()[VALUE_FIELD]
@@ -415,7 +415,7 @@ def test_slashing(testerchain, token, escrow_contract):
     tx = overseer.functions.slashMiner(ursula, 10, investigator, 0).transact()
     testerchain.wait_for_receipt(tx)
     assert 180 == escrow.functions.minerInfo(ursula).call()[VALUE_FIELD]
-    assert 90 == escrow.functions.getLockedTokens(ursula, -1).call()
+    assert 90 == escrow.functions.getLockedTokensInPast(ursula, 1).call()
     assert 180 == escrow.functions.getLockedTokens(ursula).call()
     assert 100 == escrow.functions.getLockedTokens(ursula, 4).call()
     assert 20 == token.functions.balanceOf(investigator).call()
@@ -436,7 +436,7 @@ def test_slashing(testerchain, token, escrow_contract):
     testerchain.wait_for_receipt(tx)
     testerchain.time_travel(hours=2)
     period += 2
-    assert 290 == escrow.functions.getLockedTokens(ursula, -1).call()
+    assert 290 == escrow.functions.getLockedTokensInPast(ursula, 1).call()
     assert 290 == escrow.functions.getLockedTokens(ursula).call()
     assert 180 == escrow.functions.getLockedTokens(ursula, 2).call()
     deposit = escrow.functions.minerInfo(ursula).call()[VALUE_FIELD]  # Some reward is already mined
@@ -448,7 +448,7 @@ def test_slashing(testerchain, token, escrow_contract):
     tx = overseer.functions.slashMiner(ursula, deposit - 290, investigator, 0).transact()
     testerchain.wait_for_receipt(tx)
     assert 290 == escrow.functions.minerInfo(ursula).call()[VALUE_FIELD]
-    assert 290 == escrow.functions.getLockedTokens(ursula, -1).call()
+    assert 290 == escrow.functions.getLockedTokensInPast(ursula, 1).call()
     assert 290 == escrow.functions.getLockedTokens(ursula).call()
     assert 180 == escrow.functions.getLockedTokens(ursula, 2).call()
     assert 20 == token.functions.balanceOf(investigator).call()
@@ -468,7 +468,7 @@ def test_slashing(testerchain, token, escrow_contract):
     tx = overseer.functions.slashMiner(ursula, 20, investigator, 0).transact()
     testerchain.wait_for_receipt(tx)
     assert 270 == escrow.functions.minerInfo(ursula).call()[VALUE_FIELD]
-    assert 290 == escrow.functions.getLockedTokens(ursula, -1).call()
+    assert 290 == escrow.functions.getLockedTokensInPast(ursula, 1).call()
     assert 270 == escrow.functions.getLockedTokens(ursula).call()
     assert 180 == escrow.functions.getLockedTokens(ursula, 2).call()
     assert 20 == token.functions.balanceOf(investigator).call()
@@ -489,7 +489,7 @@ def test_slashing(testerchain, token, escrow_contract):
     tx = overseer.functions.slashMiner(ursula, 100, investigator, 0).transact()
     testerchain.wait_for_receipt(tx)
     assert 170 == escrow.functions.minerInfo(ursula).call()[VALUE_FIELD]
-    assert 290 == escrow.functions.getLockedTokens(ursula, -1).call()
+    assert 290 == escrow.functions.getLockedTokensInPast(ursula, 1).call()
     assert 170 == escrow.functions.getLockedTokens(ursula).call()
     assert 170 == escrow.functions.getLockedTokens(ursula, 2).call()
     assert 20 == token.functions.balanceOf(investigator).call()
@@ -618,16 +618,16 @@ def test_slashing(testerchain, token, escrow_contract):
     tx = escrow.functions.deposit(100, 2).transact({'from': ursula2})
     testerchain.wait_for_receipt(tx)
     testerchain.time_travel(hours=2)
-    assert 100 == escrow.functions.getLockedTokens(ursula2, -2).call()
+    assert 100 == escrow.functions.getLockedTokensInPast(ursula2, 2).call()
-    assert 200 == escrow.functions.getLockedTokens(ursula2, -1).call()
+    assert 200 == escrow.functions.getLockedTokensInPast(ursula2, 1).call()
     assert 200 == escrow.functions.getLockedTokens(ursula2).call()
     assert 200 == escrow.functions.getLockedTokens(ursula2, 1).call()
 
     # Slash one sub stake to set the last period of this sub stake to the previous period
     tx = overseer.functions.slashMiner(ursula2, 100, investigator, 0).transact()
     testerchain.wait_for_receipt(tx)
-    assert 100 == escrow.functions.getLockedTokens(ursula2, -2).call()
+    assert 100 == escrow.functions.getLockedTokensInPast(ursula2, 2).call()
-    assert 200 == escrow.functions.getLockedTokens(ursula2, -1).call()
+    assert 200 == escrow.functions.getLockedTokensInPast(ursula2, 1).call()
     assert 100 == escrow.functions.getLockedTokens(ursula2).call()
     assert 100 == escrow.functions.getLockedTokens(ursula2, 1).call()
 
@@ -643,8 +643,8 @@ def test_slashing(testerchain, token, escrow_contract):
     # and check that the second sub stake will not combine with the slashed amount of the first one
     tx = overseer.functions.slashMiner(ursula2, 50, investigator, 0).transact()
     testerchain.wait_for_receipt(tx)
-    assert 100 == escrow.functions.getLockedTokens(ursula2, -2).call()
+    assert 100 == escrow.functions.getLockedTokensInPast(ursula2, 2).call()
-    assert 200 == escrow.functions.getLockedTokens(ursula2, -1).call()
+    assert 200 == escrow.functions.getLockedTokensInPast(ursula2, 1).call()
     assert 50 == escrow.functions.getLockedTokens(ursula2).call()
     assert 50 == escrow.functions.getLockedTokens(ursula2, 1).call()