Merge pull request #2384 from vzotova/remove-sub-stakes

Method to remove unused sub-stakes in StakingEscrow

docs/source/guides/network_node/staking_guide.rst

@@ -83,6 +83,8 @@ All staking-related operations done by Staker are performed through the ``nucyph
 +----------------------+-------------------------------------------------------------------------------+
 |  ``merge``           | Merge two stakes into one                                                     |
 +----------------------+-------------------------------------------------------------------------------+
+|  ``remove-unused``   | Remove unused stake                                                           |
++----------------------+-------------------------------------------------------------------------------+
 
 **Stake Command Options**
 
@@ -488,6 +490,18 @@ This can help to decrease gas consumption in some operations. To merge two stake
     (nucypher)$ nucypher stake merge --hw-wallet
 
 
+Remove unused sub-stake
+***********************
+
+Merging or editing sub-stakes can lead to 'unused', inactive sub-stakes remaining on-chain. 
+These unused sub-stakes add unnecessary gas costs to daily operations.
+To remove unused sub-stake:
+
+.. code:: bash
+
+    (nucypher)$ nucypher stake remove-unused --hw-wallet
+
+
 Collect rewards earned by the staker
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

nucypher/blockchain/eth/actors.py

@@ -1275,6 +1275,32 @@ class Staker(NucypherTokenActor):
         receipt = self._set_snapshots(value=False)
         return receipt
 
+    @only_me
+    @save_receipt
+    def remove_unused_stake(self, stake: Stake) -> TxReceipt:
+        self._ensure_stake_exists(stake)
+
+        # Read on-chain stake and validate
+        stake.sync()
+        if not stake.status().is_child(Stake.Status.INACTIVE):
+            raise ValueError(f"Stake with index {stake.index} is still active")
+
+        receipt = self._remove_unused_stake(stake_index=stake.index)
+
+        # Update staking cache element
+        self.refresh_stakes()
+        return receipt
+
+    @only_me
+    @save_receipt
+    def _remove_unused_stake(self, stake_index: int) -> TxReceipt:
+        # TODO #1497 #1358
+        # if self.is_contract:
+        # else:
+        receipt = self.staking_agent.remove_unused_stake(staker_address=self.checksum_address,
+                                                         stake_index=stake_index)
+        return receipt
+
     def non_withdrawable_stake(self) -> NU:
         staked_amount: NuNits = self.staking_agent.non_withdrawable_stake(staker_address=self.checksum_address)
         return NU.from_nunits(staked_amount)

nucypher/blockchain/eth/agents.py

@@ -711,6 +711,13 @@ class StakingEscrowAgent(EthereumContractAgent):
         # TODO: Handle SnapshotSet event (see #1193)
         return receipt
 
+    @contract_api(TRANSACTION)
+    def remove_unused_stake(self, staker_address: ChecksumAddress, stake_index: int) -> TxReceipt:
+        contract_function: ContractFunction = self.contract.functions.removeUnusedSubStake(stake_index)
+        receipt: TxReceipt = self.blockchain.send_transaction(contract_function=contract_function,
+                                                              sender_address=staker_address)
+        return receipt
+
     @contract_api(CONTRACT_CALL)
     def staking_parameters(self) -> StakingEscrowParameters:
         parameter_signatures = (

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

@@ -45,7 +45,7 @@ interface WorkLockInterface {
 /**
 * @notice Contract holds and locks stakers tokens.
 * Each staker that locks their tokens will receive some compensation
-* @dev |v5.4.4|
+* @dev |v5.5.1|
 */
 contract StakingEscrow is Issuer, IERC900History {
 
@@ -1048,6 +1048,30 @@ contract StakingEscrow is Issuer, IERC900History {
         }
     }
 
+    /**
+    * @notice Remove unused sub-stake to decrease gas cost for several methods
+    */
+    function removeUnusedSubStake(uint16 _index) external onlyStaker {
+        StakerInfo storage info = stakerInfo[msg.sender];
+
+        uint256 lastIndex = info.subStakes.length - 1;
+        SubStakeInfo storage subStake = info.subStakes[_index];
+        require(subStake.lastPeriod != 0 &&
+                (info.currentCommittedPeriod == 0 ||
+                subStake.lastPeriod < info.currentCommittedPeriod) &&
+                (info.nextCommittedPeriod == 0 ||
+                subStake.lastPeriod < info.nextCommittedPeriod));
+
+        if (_index != lastIndex) {
+            SubStakeInfo storage lastSubStake = info.subStakes[lastIndex];
+            subStake.firstPeriod = lastSubStake.firstPeriod;
+            subStake.lastPeriod = lastSubStake.lastPeriod;
+            subStake.periods = lastSubStake.periods;
+            subStake.lockedValue = lastSubStake.lockedValue;
+        }
+        info.subStakes.pop();
+    }
+
     /**
     * @notice Withdraw available amount of tokens to staker
     * @param _value Amount of tokens to withdraw

nucypher/cli/commands/stake.py

@@ -73,7 +73,7 @@ from nucypher.cli.literature import (
     INSUFFICIENT_BALANCE_TO_CREATE, PROMPT_STAKE_CREATE_VALUE, PROMPT_STAKE_CREATE_LOCK_PERIODS,
     ONLY_DISPLAYING_MERGEABLE_STAKES_NOTE, CONFIRM_MERGE, SUCCESSFUL_STAKES_MERGE, SUCCESSFUL_ENABLE_SNAPSHOTS,
     SUCCESSFUL_DISABLE_SNAPSHOTS, CONFIRM_ENABLE_SNAPSHOTS,
-    CONFIRM_STAKE_USE_UNLOCKED)
+    CONFIRM_STAKE_USE_UNLOCKED, CONFIRM_REMOVE_SUBSTAKE, SUCCESSFUL_STAKE_REMOVAL)
 from nucypher.cli.options import (
     group_options,
     option_config_file,
@@ -1068,6 +1068,61 @@ def merge(general_config: GroupGeneralConfig,
     paint_stakes(emitter=emitter, staker=STAKEHOLDER)
 
 
+@stake.command()
+@group_transacting_staker_options
+@option_config_file
+@option_force
+@group_general_config
+@click.option('--index', help="Index of unused stake to remove", type=click.INT)
+def remove_unused(general_config: GroupGeneralConfig,
+                  transacting_staker_options: TransactingStakerOptions,
+                  config_file, force, index):
+    """Remove unused stake."""
+
+    # Setup
+    emitter = setup_emitter(general_config)
+    STAKEHOLDER = transacting_staker_options.create_character(emitter, config_file)
+    action_period = STAKEHOLDER.staking_agent.get_current_period()
+    blockchain = transacting_staker_options.get_blockchain()
+
+    client_account, staking_address = select_client_account_for_staking(
+        emitter=emitter,
+        stakeholder=STAKEHOLDER,
+        staking_address=transacting_staker_options.staker_options.staking_address,
+        individual_allocation=STAKEHOLDER.individual_allocation,
+        force=force)
+
+    # Handle stake update and selection
+    if index is not None:  # 0 is valid.
+        current_stake = STAKEHOLDER.stakes[index]
+    else:
+        current_stake = select_stake(staker=STAKEHOLDER, emitter=emitter, stakes_status=Stake.Status.INACTIVE)
+
+    if not force:
+        click.confirm(CONFIRM_REMOVE_SUBSTAKE.format(stake_index=current_stake.index), abort=True)
+
+    # Authenticate
+    password = get_password(stakeholder=STAKEHOLDER,
+                            blockchain=blockchain,
+                            client_account=client_account,
+                            hw_wallet=transacting_staker_options.hw_wallet)
+    STAKEHOLDER.assimilate(password=password)
+
+    # Non-interactive: Consistency check to prevent the above agreement from going stale.
+    last_second_current_period = STAKEHOLDER.staking_agent.get_current_period()
+    if action_period != last_second_current_period:
+        emitter.echo(PERIOD_ADVANCED_WARNING, color='red')
+        raise click.Abort
+
+    # Execute
+    receipt = STAKEHOLDER.remove_unused_stake(stake=current_stake)
+
+    # Report
+    emitter.echo(SUCCESSFUL_STAKE_REMOVAL, color='green', verbosity=1)
+    paint_receipt_summary(emitter=emitter, receipt=receipt, chain_name=blockchain.client.chain_name)
+    paint_stakes(emitter=emitter, staker=STAKEHOLDER)
+
+
 @stake.command('collect-reward')
 @group_transacting_staker_options
 @option_config_file

nucypher/cli/literature.py

@@ -271,6 +271,10 @@ CONFIRM_MERGE = "Publish merging of {stake_index_1} and {stake_index_2} stakes?"
 
 SUCCESSFUL_STAKES_MERGE = 'Successfully Merged Stakes'
 
+CONFIRM_REMOVE_SUBSTAKE = "Publish removal of {stake_index} stake?"
+
+SUCCESSFUL_STAKE_REMOVAL = 'Successfully Removed Stake'
+
 #
 # Rewards
 #

tests/acceptance/blockchain/actors/test_staker.py

@@ -199,7 +199,7 @@ def test_staker_increases_stake(staker, token_economics):
     assert stake.value == origin_stake.value + balance
 
 
-def test_staker_merges_stakes(agency, staker, token_economics):
+def test_staker_merges_stakes(agency, staker):
     stake_index_1 = 0
     stake_index_2 = 3
     origin_stake_1 = staker.stakes[stake_index_1]
@@ -224,6 +224,19 @@ def test_staker_merges_stakes(agency, staker, token_economics):
         staker.merge_stakes(stake_1=staker.stakes[1], stake_2=stake)
 
 
+def test_remove_unused_stake(agency, staker):
+    stake_index = 3
+    staker.refresh_stakes()
+    original_stakes = list(staker.stakes)
+    unused_stake = original_stakes[stake_index]
+    assert unused_stake.final_locked_period == 1
+
+    staker.remove_unused_stake(stake=unused_stake)
+
+    stakes = staker.stakes
+    assert stakes == original_stakes[:-1]
+
+
 def test_staker_manages_restaking(testerchain, test_registry, staker):
     # Enable Restaking
     receipt = staker.enable_restaking()

tests/acceptance/blockchain/agents/test_staking_escrow_agent.py

@@ -138,7 +138,6 @@ def test_get_swarm(agency, blockchain_ursulas):
     assert is_address(staker_addr)
 
 
-
 @pytest.mark.usefixtures("blockchain_ursulas")
 def test_sample_stakers(agency):
     _token_agent, staking_agent, _policy_agent = agency
@@ -264,16 +263,6 @@ def test_deposit_and_increase(agency, testerchain, test_registry, token_economic
     assert staking_agent.get_locked_tokens(staker_account, 1) == locked_tokens + amount
 
 
-def test_disable_restaking(agency, testerchain, test_registry):
-    staking_agent = ContractAgency.get_agent(StakingEscrowAgent, registry=test_registry)
-    staker_account, worker_account, *other = testerchain.unassigned_accounts
-
-    assert staking_agent.is_restaking(staker_account)
-    receipt = staking_agent.set_restaking(staker_account, value=False)
-    assert receipt['status'] == 1
-    assert not staking_agent.is_restaking(staker_account)
-
-
 def test_lock_restaking(agency, testerchain, test_registry):
     staker_account, worker_account, *other = testerchain.unassigned_accounts
     staking_agent = ContractAgency.get_agent(StakingEscrowAgent, registry=test_registry)
@@ -438,6 +427,33 @@ def test_merge(agency, testerchain, test_registry, token_economics):
     assert staking_agent.get_locked_tokens(staker_account, 0) == current_locked_tokens
 
 
+def test_remove_unused_stake(agency, testerchain, test_registry):
+    staking_agent = ContractAgency.get_agent(StakingEscrowAgent, registry=test_registry)
+    staker_account = testerchain.unassigned_accounts[0]
+
+    testerchain.time_travel(periods=1)
+    staking_agent.mint(staker_address=staker_account)
+    current_period = staking_agent.get_current_period()
+    original_stakes = list(staking_agent.get_all_stakes(staker_address=staker_account))
+    assert original_stakes[2].last_period == current_period - 1
+
+    current_locked_tokens = staking_agent.get_locked_tokens(staker_account, 0)
+    next_locked_tokens = staking_agent.get_locked_tokens(staker_account, 1)
+
+    receipt = staking_agent.remove_unused_stake(staker_address=staker_account, stake_index=2)
+    assert receipt['status'] == 1
+
+    # Ensure stake was extended by one period.
+    stakes = list(staking_agent.get_all_stakes(staker_address=staker_account))
+    assert len(stakes) == len(original_stakes) - 1
+    assert stakes[0] == original_stakes[0]
+    assert stakes[1] == original_stakes[1]
+    assert stakes[2] == original_stakes[4]
+    assert stakes[3] == original_stakes[3]
+    assert staking_agent.get_locked_tokens(staker_account, 1) == next_locked_tokens
+    assert staking_agent.get_locked_tokens(staker_account, 0) == current_locked_tokens
+
+
 def test_batch_deposit(testerchain,
                        agency,
                        token_economics,
@@ -448,7 +464,6 @@ def test_batch_deposit(testerchain,
 
     amount = token_economics.minimum_allowed_locked
     lock_periods = token_economics.minimum_locked_periods
-    current_period = staking_agent.get_current_period()
 
     stakers = [get_random_checksum_address() for _ in range(4)]
 

tests/acceptance/cli/ursula/test_stakeholder_and_ursula.py

@@ -286,6 +286,33 @@ def test_merge_stakes(click_runner,
     assert stakes[selection_2].last_period == 1
 
 
+def test_remove_unused(click_runner,
+                       stakeholder_configuration_file_location,
+                       token_economics,
+                       testerchain,
+                       agency_local_registry,
+                       manual_staker,
+                       stake_value):
+
+    staking_agent = ContractAgency.get_agent(StakingEscrowAgent, registry=agency_local_registry)
+    original_stakes = list(staking_agent.get_all_stakes(staker_address=manual_staker))
+
+    selection = 2
+    assert original_stakes[selection].last_period == 1
+
+    stake_args = ('stake', 'remove-unused',
+                  '--config-file', stakeholder_configuration_file_location,
+                  '--staking-address', manual_staker,
+                  '--index', selection,
+                  '--force')
+    user_input = f'0\n' + f'{INSECURE_DEVELOPMENT_PASSWORD}\n' + YES_ENTER
+    result = click_runner.invoke(nucypher_cli, stake_args, input=user_input, catch_exceptions=False)
+    assert result.exit_code == 0
+
+    stakes = list(staking_agent.get_all_stakes(staker_address=manual_staker))
+    assert len(stakes) == len(original_stakes) - 1
+
+
 def test_stake_bond_worker(click_runner,
                            testerchain,
                            agency_local_registry,

tests/contracts/main/staking_escrow/test_staking_escrow_additional.py

@@ -1467,3 +1467,130 @@ def test_snapshots(testerchain, token, escrow_contract):
     assert last_balance_staker1 + deposit_staker2 == escrow.functions.totalStakedAt(now).call()
     assert balance_staker1 == escrow.functions.totalStakedForAt(staker1, now - 1).call()
     assert balance_staker1 + deposit_staker2 == escrow.functions.totalStakedAt(now - 1).call()
+
+
+def test_remove_unused_sub_stakes(testerchain, token, escrow_contract, token_economics):
+    escrow = escrow_contract(token_economics.maximum_allowed_locked, disable_reward=True)
+    creator = testerchain.client.accounts[0]
+    staker = testerchain.client.accounts[1]
+
+    # GIVe staker some tokens
+    stake = 10 * token_economics.minimum_allowed_locked
+    tx = token.functions.transfer(staker, stake).transact({'from': creator})
+    testerchain.wait_for_receipt(tx)
+    tx = token.functions.approve(escrow.address, stake).transact({'from': staker})
+    testerchain.wait_for_receipt(tx)
+
+    # Prepare sub-stakes
+    initial_period = escrow.functions.getCurrentPeriod().call()
+    sub_stake = token_economics.minimum_allowed_locked
+    duration = token_economics.minimum_locked_periods
+    for i in range(3):
+        tx = escrow.functions.deposit(staker, sub_stake, duration).transact({'from': staker})
+        testerchain.wait_for_receipt(tx)
+    for i in range(2):
+        tx = escrow.functions.deposit(staker, sub_stake, duration + 1).transact({'from': staker})
+        testerchain.wait_for_receipt(tx)
+        testerchain.time_travel(hours=1)
+    assert escrow.functions.getLockedTokens(staker, 1).call() == 5 * sub_stake
+
+    tx = escrow.functions.mergeStake(1, 0).transact({'from': staker})
+    testerchain.wait_for_receipt(tx)
+    tx = escrow.functions.mergeStake(1, 2).transact({'from': staker})
+    testerchain.wait_for_receipt(tx)
+    tx = escrow.functions.mergeStake(3, 4).transact({'from': staker})
+    testerchain.wait_for_receipt(tx)
+    assert escrow.functions.getLockedTokens(staker, 1).call() == 5 * sub_stake
+    assert escrow.functions.getSubStakesLength(staker).call() == 5
+    assert escrow.functions.getSubStakeInfo(staker, 0).call() == [initial_period + 1, 1, 0, sub_stake]
+    assert escrow.functions.getSubStakeInfo(staker, 1).call() == [initial_period + 1, 0, duration, 3 * sub_stake]
+    assert escrow.functions.getSubStakeInfo(staker, 2).call() == [initial_period + 1, 1, 0, sub_stake]
+    assert escrow.functions.getSubStakeInfo(staker, 3).call() == [initial_period + 1, initial_period + 1, 0, sub_stake]
+    assert escrow.functions.getSubStakeInfo(staker, 4).call() == [initial_period + 2, 0, duration + 1, 2 * sub_stake]
+
+    # Can't remove active sub-stakes
+    with pytest.raises((TransactionFailed, ValueError)):
+        tx = escrow.functions.removeUnusedSubStake(1).transact({'from': staker})
+        testerchain.wait_for_receipt(tx)
+    with pytest.raises((TransactionFailed, ValueError)):
+        tx = escrow.functions.removeUnusedSubStake(4).transact({'from': staker})
+        testerchain.wait_for_receipt(tx)
+    with pytest.raises((TransactionFailed, ValueError)):
+        tx = escrow.functions.removeUnusedSubStake(5).transact({'from': staker})
+        testerchain.wait_for_receipt(tx)
+
+    # Remove first unused sub-stake
+    tx = escrow.functions.removeUnusedSubStake(0).transact({'from': staker})
+    testerchain.wait_for_receipt(tx)
+    assert escrow.functions.getLockedTokens(staker, 1).call() == 5 * sub_stake
+    assert escrow.functions.getSubStakesLength(staker).call() == 4
+    assert escrow.functions.getSubStakeInfo(staker, 0).call() == [initial_period + 2, 0, duration + 1, 2 * sub_stake]
+    assert escrow.functions.getSubStakeInfo(staker, 1).call() == [initial_period + 1, 0, duration, 3 * sub_stake]
+    assert escrow.functions.getSubStakeInfo(staker, 2).call() == [initial_period + 1, 1, 0, sub_stake]
+    assert escrow.functions.getSubStakeInfo(staker, 3).call() == [initial_period + 1, initial_period + 1, 0, sub_stake]
+
+    with pytest.raises((TransactionFailed, ValueError)):
+        tx = escrow.functions.removeUnusedSubStake(0).transact({'from': staker})
+        testerchain.wait_for_receipt(tx)
+
+    # Remove unused sub-stake in the middle
+    tx = escrow.functions.removeUnusedSubStake(2).transact({'from': staker})
+    testerchain.wait_for_receipt(tx)
+    assert escrow.functions.getLockedTokens(staker, 1).call() == 5 * sub_stake
+    assert escrow.functions.getSubStakesLength(staker).call() == 3
+    assert escrow.functions.getSubStakeInfo(staker, 0).call() == [initial_period + 2, 0, duration + 1, 2 * sub_stake]
+    assert escrow.functions.getSubStakeInfo(staker, 1).call() == [initial_period + 1, 0, duration, 3 * sub_stake]
+    assert escrow.functions.getSubStakeInfo(staker, 2).call() == [initial_period + 1, initial_period + 1, 0, sub_stake]
+
+    # Remove last sub-stake
+    tx = escrow.functions.removeUnusedSubStake(2).transact({'from': staker})
+    testerchain.wait_for_receipt(tx)
+    assert escrow.functions.getLockedTokens(staker, 1).call() == 5 * sub_stake
+    assert escrow.functions.getSubStakesLength(staker).call() == 2
+    assert escrow.functions.getSubStakeInfo(staker, 0).call() == [initial_period + 2, 0, duration + 1, 2 * sub_stake]
+    assert escrow.functions.getSubStakeInfo(staker, 1).call() == [initial_period + 1, 0, duration, 3 * sub_stake]
+
+    # Prepare other case: when sub-stake is unlocked but still active
+    tx = escrow.functions.initialize(0, creator).transact({'from': creator})
+    testerchain.wait_for_receipt(tx)
+    tx = escrow.functions.setWindDown(True).transact({'from': staker})
+    testerchain.wait_for_receipt(tx)
+    tx = escrow.functions.bondWorker(staker).transact({'from': staker})
+    testerchain.wait_for_receipt(tx)
+    for i in range(duration):
+        tx = escrow.functions.commitToNextPeriod().transact({'from': staker})
+        testerchain.wait_for_receipt(tx)
+        testerchain.time_travel(hours=1)
+
+    current_period = escrow.functions.getCurrentPeriod().call()
+    assert escrow.functions.getSubStakeInfo(staker, 0).call() == [initial_period + 2, 0, 1, 2 * sub_stake]
+    assert escrow.functions.getSubStakeInfo(staker, 1).call() == [initial_period + 1, current_period, 0, 3 * sub_stake]
+
+    # Can't remove active sub-stakes
+    with pytest.raises((TransactionFailed, ValueError)):
+        tx = escrow.functions.removeUnusedSubStake(1).transact({'from': staker})
+        testerchain.wait_for_receipt(tx)
+    tx = escrow.functions.mint().transact({'from': staker})
+    testerchain.wait_for_receipt(tx)
+    with pytest.raises((TransactionFailed, ValueError)):
+        tx = escrow.functions.removeUnusedSubStake(1).transact({'from': staker})
+        testerchain.wait_for_receipt(tx)
+    tx = escrow.functions.commitToNextPeriod().transact({'from': staker})
+    testerchain.wait_for_receipt(tx)
+    with pytest.raises((TransactionFailed, ValueError)):
+        tx = escrow.functions.removeUnusedSubStake(1).transact({'from': staker})
+        testerchain.wait_for_receipt(tx)
+
+    testerchain.time_travel(hours=1)
+    current_period = escrow.functions.getCurrentPeriod().call()
+    assert escrow.functions.getSubStakeInfo(staker, 0).call() == [initial_period + 2, current_period, 0, 2 * sub_stake]
+    assert escrow.functions.getSubStakeInfo(staker, 1).call() == [initial_period + 1, current_period - 1, 0, 3 * sub_stake]
+    with pytest.raises((TransactionFailed, ValueError)):
+        tx = escrow.functions.removeUnusedSubStake(1).transact({'from': staker})
+        testerchain.wait_for_receipt(tx)
+    tx = escrow.functions.mint().transact({'from': staker})
+    testerchain.wait_for_receipt(tx)
+    tx = escrow.functions.removeUnusedSubStake(1).transact({'from': staker})
+    testerchain.wait_for_receipt(tx)
+    assert escrow.functions.getSubStakesLength(staker).call() == 1
+    assert escrow.functions.getSubStakeInfo(staker, 0).call() == [initial_period + 2, current_period, 0, 2 * sub_stake]