From 7d47d3bc769ceb5a7de84c9e39f58a18d7c9de1f Mon Sep 17 00:00:00 2001 From: vzotova Date: Mon, 15 Mar 2021 17:56:26 +0300 Subject: [PATCH 1/3] Refinements for pool staking contract --- newsfragments/2596.feature.rst | 1 + .../PoolingStakingContractV2.sol | 65 ++++++++++----- .../test_pooling_contract_v2.py | 82 +++++++++++++------ 3 files changed, 105 insertions(+), 43 deletions(-) create mode 100644 newsfragments/2596.feature.rst diff --git a/newsfragments/2596.feature.rst b/newsfragments/2596.feature.rst new file mode 100644 index 000000000..f2f93d6e6 --- /dev/null +++ b/newsfragments/2596.feature.rst @@ -0,0 +1 @@ +Refinements for pool staking contract diff --git a/nucypher/blockchain/eth/sol/source/contracts/staking_contracts/PoolingStakingContractV2.sol b/nucypher/blockchain/eth/sol/source/contracts/staking_contracts/PoolingStakingContractV2.sol index 6cbee6848..57252960b 100644 --- a/nucypher/blockchain/eth/sol/source/contracts/staking_contracts/PoolingStakingContractV2.sol +++ b/nucypher/blockchain/eth/sol/source/contracts/staking_contracts/PoolingStakingContractV2.sol @@ -25,7 +25,8 @@ contract PoolingStakingContractV2 is InitializableStakingContract, Ownable { uint256 depositedTokens ); event ETHWithdrawn(address indexed sender, uint256 value); - event DepositSet(address indexed sender, bool value); + event DepositIsEnabledSet(address indexed sender, bool value); + event WorkerOwnerSet(address indexed sender, address indexed workerOwner); struct Delegator { uint256 depositedTokens; @@ -34,7 +35,7 @@ contract PoolingStakingContractV2 is InitializableStakingContract, Ownable { } /// Defines base fraction and precision of worker fraction. Value 100 defines 1 worker fraction is 1% of reward - uint256 public constant BASIS_FRACTION = 100; + uint256 public constant BASIS_FRACTION = 10000; StakingEscrow public escrow; address public workerOwner; @@ -60,13 +61,14 @@ contract PoolingStakingContractV2 is InitializableStakingContract, Ownable { uint256 _workerFraction, StakingInterfaceRouter _router, address _workerOwner - ) public initializer { + ) external initializer { require(_workerOwner != address(0) && _workerFraction <= BASIS_FRACTION); InitializableStakingContract.initialize(_router); _transferOwnership(msg.sender); escrow = _router.target().escrow(); workerFraction = _workerFraction; workerOwner = _workerOwner; + emit WorkerOwnerSet(msg.sender, _workerOwner); } /** @@ -74,7 +76,7 @@ contract PoolingStakingContractV2 is InitializableStakingContract, Ownable { */ function enableDeposit() external onlyOwner { depositIsEnabled = true; - emit DepositSet(msg.sender, depositIsEnabled); + emit DepositIsEnabledSet(msg.sender, depositIsEnabled); } /** @@ -82,13 +84,22 @@ contract PoolingStakingContractV2 is InitializableStakingContract, Ownable { */ function disableDeposit() external onlyOwner { depositIsEnabled = false; - emit DepositSet(msg.sender, depositIsEnabled); + emit DepositIsEnabledSet(msg.sender, depositIsEnabled); + } + + /** + * @notice Set worker owner address + */ + function setWorkerOwner(address _workerOwner) external onlyOwner { + workerOwner = _workerOwner; + emit WorkerOwnerSet(msg.sender, _workerOwner); } /** * @notice Calculate worker's fraction depending on deposited tokens + * Override to implement dynamic worker fraction. */ - function getWorkerFraction() public view returns (uint256) { + function getWorkerFraction() public view virtual returns (uint256) { return workerFraction; } @@ -109,10 +120,14 @@ contract PoolingStakingContractV2 is InitializableStakingContract, Ownable { /** * @notice Get available reward for all delegators and owner */ - function getAvailableReward() public view returns (uint256) { + function getAvailableDelegatorReward() public view returns (uint256) { + // locked + unlocked tokens in StakingEscrow contract which belong to pool uint256 stakedTokens = escrow.getAllTokens(address(this)); + // tokens which directly belong to pool uint256 freeTokens = token.balanceOf(address(this)); + // tokens in excess of the initially deposited uint256 reward = stakedTokens.add(freeTokens).sub(totalDepositedTokens); + // check how many of reward tokens belong directly to pool if (reward > freeTokens) { return freeTokens; } @@ -120,26 +135,32 @@ contract PoolingStakingContractV2 is InitializableStakingContract, Ownable { } /** - * @notice Get cumulative reward + * @notice Get cumulative reward. + * Available and withdrawn reward together to use in delegator/owner reward calculations */ function getCumulativeReward() public view returns (uint256) { - return getAvailableReward().add(totalWithdrawnReward); + return getAvailableDelegatorReward().add(totalWithdrawnReward); } /** * @notice Get available reward in tokens for worker node owner */ function getAvailableWorkerReward() public view returns (uint256) { + // total current and historical reward uint256 reward = getCumulativeReward(); + // calculate total reward for worker including historical reward uint256 maxAllowableReward; + // usual case if (totalDepositedTokens != 0) { uint256 fraction = getWorkerFraction(); maxAllowableReward = reward.mul(fraction).div(BASIS_FRACTION); + // special case when there are no delegators } else { maxAllowableReward = reward; } + // check that worker has any new reward if (maxAllowableReward > workerWithdrawnReward) { return maxAllowableReward - workerWithdrawnReward; } @@ -149,26 +170,32 @@ contract PoolingStakingContractV2 is InitializableStakingContract, Ownable { /** * @notice Get available reward in tokens for delegator */ - function getAvailableReward(address _delegator) + function getAvailableDelegatorReward(address _delegator) public view returns (uint256) { + // special case when there are no delegators if (totalDepositedTokens == 0) { return 0; } + // total current and historical reward uint256 reward = getCumulativeReward(); Delegator storage delegator = delegators[_delegator]; uint256 fraction = getWorkerFraction(); + + // calculate total reward for delegator including historical reward + // excluding worker share uint256 maxAllowableReward = reward.mul(delegator.depositedTokens).mul(BASIS_FRACTION - fraction).div( totalDepositedTokens.mul(BASIS_FRACTION) ); - return - maxAllowableReward > delegator.withdrawnReward - ? maxAllowableReward - delegator.withdrawnReward - : 0; + // check that worker has any new reward + if (maxAllowableReward > delegator.withdrawnReward) { + return maxAllowableReward - delegator.withdrawnReward; + } + return 0; } /** @@ -202,7 +229,7 @@ contract PoolingStakingContractV2 is InitializableStakingContract, Ownable { require(_value <= balance, "Not enough tokens in the contract"); Delegator storage delegator = delegators[msg.sender]; - uint256 availableReward = getAvailableReward(msg.sender); + uint256 availableReward = getAvailableDelegatorReward(msg.sender); require( _value <= availableReward, "Requested amount of tokens exceeded allowed portion"); delegator.withdrawnReward = delegator.withdrawnReward.add(_value); @@ -219,11 +246,11 @@ contract PoolingStakingContractV2 is InitializableStakingContract, Ownable { uint256 balance = token.balanceOf(address(this)); Delegator storage delegator = delegators[msg.sender]; - uint256 availableReward = getAvailableReward(msg.sender); + uint256 availableReward = getAvailableDelegatorReward(msg.sender); uint256 value = availableReward.add(delegator.depositedTokens); require(value <= balance, "Not enough tokens in the contract"); - // TODO remove double reading + // TODO remove double reading: availableReward and availableWorkerReward use same calls to external contracts uint256 availableWorkerReward = getAvailableWorkerReward(); // potentially could be less then due reward @@ -252,8 +279,8 @@ contract PoolingStakingContractV2 is InitializableStakingContract, Ownable { totalWithdrawnETH = totalWithdrawnETH.sub(delegator.withdrawnETH); delegator.withdrawnETH = 0; if (availableETH > 0) { - msg.sender.sendValue(availableETH); emit ETHWithdrawn(msg.sender, availableETH); + msg.sender.sendValue(availableETH); } } @@ -284,8 +311,8 @@ contract PoolingStakingContractV2 is InitializableStakingContract, Ownable { delegator.withdrawnETH = delegator.withdrawnETH.add(availableETH); totalWithdrawnETH = totalWithdrawnETH.add(availableETH); - msg.sender.sendValue(availableETH); emit ETHWithdrawn(msg.sender, availableETH); + msg.sender.sendValue(availableETH); } /** diff --git a/tests/contracts/main/staking_contracts/test_pooling_contract_v2.py b/tests/contracts/main/staking_contracts/test_pooling_contract_v2.py index f877f42e9..1296e884d 100644 --- a/tests/contracts/main/staking_contracts/test_pooling_contract_v2.py +++ b/tests/contracts/main/staking_contracts/test_pooling_contract_v2.py @@ -24,8 +24,8 @@ from web3.contract import Contract from nucypher.blockchain.eth.token import NU -WORKER_FRACTION = 10 -BASIS_FRACTION = 100 +WORKER_FRACTION = 1000 +BASIS_FRACTION = 10000 @pytest.fixture() @@ -56,6 +56,7 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, delegators = testerchain.client.accounts[3:6] deposit_log = pooling_contract.events.TokensDeposited.createFilter(fromBlock='latest') withdraw_log = pooling_contract.events.TokensWithdrawn.createFilter(fromBlock='latest') + workers_log = pooling_contract.events.WorkerOwnerSet.createFilter(fromBlock=0) assert pooling_contract.functions.owner().call() == owner assert pooling_contract.functions.workerOwner().call() == worker_owner @@ -65,7 +66,7 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, assert pooling_contract.functions.totalWithdrawnReward().call() == 0 assert token.functions.balanceOf(pooling_contract.address).call() == 0 assert pooling_contract.functions.getAvailableWorkerReward().call() == 0 - assert pooling_contract.functions.getAvailableReward().call() == 0 + assert pooling_contract.functions.getAvailableDelegatorReward().call() == 0 # Give some tokens to delegators for index, delegator in enumerate(delegators): @@ -76,17 +77,17 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, # Delegators deposit tokens to the pooling contract total_deposited_tokens = 0 tokens_supply = 0 - assert pooling_contract.functions.getAvailableReward().call() == 0 + assert pooling_contract.functions.getAvailableDelegatorReward().call() == 0 for index, delegator in enumerate(delegators): assert pooling_contract.functions.delegators(delegator).call() == [0, 0, 0] - assert pooling_contract.functions.getAvailableReward(delegator).call() == 0 + assert pooling_contract.functions.getAvailableDelegatorReward(delegator).call() == 0 tokens = token.functions.balanceOf(delegator).call() // 2 tx = token.functions.approve(pooling_contract.address, tokens).transact({'from': delegator}) testerchain.wait_for_receipt(tx) tx = pooling_contract.functions.depositTokens(tokens).transact({'from': delegator}) testerchain.wait_for_receipt(tx) assert pooling_contract.functions.delegators(delegator).call() == [tokens, 0, 0] - assert pooling_contract.functions.getAvailableReward(delegator).call() == 0 + assert pooling_contract.functions.getAvailableDelegatorReward(delegator).call() == 0 total_deposited_tokens += tokens tokens_supply += tokens assert pooling_contract.functions.totalDepositedTokens().call() == total_deposited_tokens @@ -103,10 +104,10 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, assert pooling_contract.functions.workerWithdrawnReward().call() == 0 assert pooling_contract.functions.totalWithdrawnReward().call() == 0 assert pooling_contract.functions.getAvailableWorkerReward().call() == 0 - assert pooling_contract.functions.getAvailableReward().call() == 0 + assert pooling_contract.functions.getAvailableDelegatorReward().call() == 0 # Disable deposit - log = pooling_contract.events.DepositSet.createFilter(fromBlock='latest') + log = pooling_contract.events.DepositIsEnabledSet.createFilter(fromBlock='latest') with pytest.raises((TransactionFailed, ValueError)): tx = pooling_contract.functions.disableDeposit().transact({'from': delegators[0]}) testerchain.wait_for_receipt(tx) @@ -174,7 +175,7 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, assert token.functions.balanceOf(pooling_contract.address).call() == 0 # Give some tokens as a reward - assert pooling_contract.functions.getAvailableReward().call() == 0 + assert pooling_contract.functions.getAvailableDelegatorReward().call() == 0 reward = token_economics.minimum_allowed_locked tx = token.functions.approve(escrow.address, reward).transact() testerchain.wait_for_receipt(tx) @@ -187,10 +188,10 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, testerchain.wait_for_receipt(tx) withdrawn_stake = reward + stake - assert pooling_contract.functions.getAvailableReward().call() == 0 + assert pooling_contract.functions.getAvailableDelegatorReward().call() == 0 tx = pooling_contract_interface.functions.withdrawAsStaker(withdrawn_stake).transact({'from': owner}) testerchain.wait_for_receipt(tx) - assert pooling_contract.functions.getAvailableReward().call() == reward + assert pooling_contract.functions.getAvailableDelegatorReward().call() == reward worker_reward = reward * WORKER_FRACTION // BASIS_FRACTION assert pooling_contract.functions.getAvailableWorkerReward().call() == worker_reward assert pooling_contract.functions.totalDepositedTokens().call() == total_deposited_tokens @@ -211,11 +212,11 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, testerchain.wait_for_receipt(tx) portion = max_portion // 2 - assert pooling_contract.functions.getAvailableReward(delegator).call() == max_portion + assert pooling_contract.functions.getAvailableDelegatorReward(delegator).call() == max_portion tx = pooling_contract.functions.withdrawTokens(portion).transact({'from': delegator}) testerchain.wait_for_receipt(tx) assert pooling_contract.functions.delegators(delegator).call() == [deposited_tokens, portion, 0] - assert pooling_contract.functions.getAvailableReward(delegator).call() == max_portion - portion + assert pooling_contract.functions.getAvailableDelegatorReward(delegator).call() == max_portion - portion tokens_supply -= portion total_withdrawn_tokens += portion available_reward -= portion @@ -233,7 +234,7 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, # Node owner withdraws tokens assert pooling_contract.functions.getAvailableWorkerReward().call() == worker_reward - assert pooling_contract.functions.getAvailableReward().call() == available_reward + assert pooling_contract.functions.getAvailableDelegatorReward().call() == available_reward # Only node owner can call this method with pytest.raises((TransactionFailed, ValueError)): @@ -255,7 +256,7 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, assert token.functions.balanceOf(owner).call() == 0 assert token.functions.balanceOf(worker_owner).call() == worker_reward assert pooling_contract.functions.totalWithdrawnReward().call() == total_withdrawn_tokens - assert pooling_contract.functions.getAvailableReward().call() == available_reward - worker_reward + assert pooling_contract.functions.getAvailableDelegatorReward().call() == available_reward - worker_reward events = withdraw_log.get_all_entries() assert len(events) == len(delegators) + 1 @@ -281,7 +282,7 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, max_portion = reward * deposited_tokens * (BASIS_FRACTION - WORKER_FRACTION) // \ (previous_total_deposited_tokens * BASIS_FRACTION) supposed_portion = max_portion // 2 - reward_portion = pooling_contract.functions.getAvailableReward(delegator).call() + reward_portion = pooling_contract.functions.getAvailableDelegatorReward(delegator).call() # could be some rounding errors assert abs(supposed_portion - reward_portion) <= 10 @@ -296,7 +297,7 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, assert token.functions.balanceOf(pooling_contract.address).call() == tokens_supply assert token.functions.balanceOf(delegator).call() == previous_portion + new_portion assert token.functions.balanceOf(worker_owner).call() == worker_reward - assert pooling_contract.functions.getAvailableReward(delegator).call() == 0 + assert pooling_contract.functions.getAvailableDelegatorReward(delegator).call() == 0 withdraw_to_decrease = withdrawn_worker_reward * deposited_tokens // previous_total_deposited_tokens total_withdrawn_tokens -= withdraw_to_decrease @@ -321,7 +322,7 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, max_portion = reward * deposited_tokens * (BASIS_FRACTION - WORKER_FRACTION) // \ (previous_total_deposited_tokens * BASIS_FRACTION) supposed_portion = max_portion // 2 - reward_portion = pooling_contract.functions.getAvailableReward(delegator).call() + reward_portion = pooling_contract.functions.getAvailableDelegatorReward(delegator).call() # could be some rounding errors assert abs(supposed_portion - reward_portion) <= 10 @@ -340,8 +341,8 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, delegator = delegators[1] deposited_tokens = pooling_contract.functions.delegators(delegator).call()[0] withdrawn_tokens = pooling_contract.functions.delegators(delegator).call()[1] - reward_portion = pooling_contract.functions.getAvailableReward(delegator).call() - other_reward_portion = pooling_contract.functions.getAvailableReward(delegators[2]).call() + reward_portion = pooling_contract.functions.getAvailableDelegatorReward(delegator).call() + other_reward_portion = pooling_contract.functions.getAvailableDelegatorReward(delegators[2]).call() new_portion = deposited_tokens + reward_portion previous_portion = token.functions.balanceOf(delegator).call() @@ -356,7 +357,7 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, assert token.functions.balanceOf(pooling_contract.address).call() == tokens_supply assert token.functions.balanceOf(delegator).call() == previous_portion + new_portion assert token.functions.balanceOf(worker_owner).call() == worker_reward + new_worker_transfer - assert pooling_contract.functions.getAvailableReward(delegator).call() == 0 + assert pooling_contract.functions.getAvailableDelegatorReward(delegator).call() == 0 withdraw_to_decrease = withdrawn_worker_reward * deposited_tokens // previous_total_deposited_tokens total_withdrawn_tokens -= withdraw_to_decrease @@ -382,12 +383,12 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, assert pooling_contract.functions.getAvailableWorkerReward().call() == new_worker_reward # Check others rewards - assert abs(pooling_contract.functions.getAvailableReward(delegators[2]).call() - other_reward_portion) <= 10 + assert abs(pooling_contract.functions.getAvailableDelegatorReward(delegators[2]).call() - other_reward_portion) <= 10 # Withdraw last portion for last delegator delegator = delegators[2] deposited_tokens = pooling_contract.functions.delegators(delegator).call()[0] - reward_portion = pooling_contract.functions.getAvailableReward(delegator).call() + reward_portion = pooling_contract.functions.getAvailableDelegatorReward(delegator).call() new_portion = deposited_tokens + reward_portion previous_portion = token.functions.balanceOf(delegator).call() @@ -398,7 +399,7 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, assert token.functions.balanceOf(pooling_contract.address).call() <= 1 assert token.functions.balanceOf(delegator).call() == previous_portion + new_portion assert token.functions.balanceOf(worker_owner).call() == worker_reward + new_worker_transfer + new_worker_reward - assert pooling_contract.functions.getAvailableReward().call() <= 1 + assert pooling_contract.functions.getAvailableDelegatorReward().call() <= 1 events = withdraw_log.get_all_entries() assert len(events) == len(delegators) + 6 @@ -412,6 +413,39 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, assert event_args['value'] == new_portion assert event_args['depositedTokens'] == 0 + # Change worker owner + + # Prepare reward + tx = token.functions.transfer(pooling_contract.address, new_reward).transact({'from': creator}) + testerchain.wait_for_receipt(tx) + + # Only pool owner can change value + new_worker_owner = delegators[0] + with pytest.raises((TransactionFailed, ValueError)): + tx = pooling_contract.functions.setWorkerOwner(new_worker_owner).transact({'from': worker_owner}) + testerchain.wait_for_receipt(tx) + + tx = pooling_contract.functions.setWorkerOwner(new_worker_owner).transact({'from': owner}) + testerchain.wait_for_receipt(tx) + + # Now old worker owner can't withdraw reward + with pytest.raises((TransactionFailed, ValueError)): + tx = pooling_contract.functions.withdrawWorkerReward().transact({'from': worker_owner}) + testerchain.wait_for_receipt(tx) + + # But new can + tx = pooling_contract.functions.withdrawWorkerReward().transact({'from': new_worker_owner}) + testerchain.wait_for_receipt(tx) + + events = workers_log.get_all_entries() + assert len(events) == 2 + event_args = events[0]['args'] + assert event_args['sender'] == owner + assert event_args['workerOwner'] == worker_owner + event_args = events[1]['args'] + assert event_args['sender'] == owner + assert event_args['workerOwner'] == new_worker_owner + def test_fee(testerchain, token_economics, token, policy_manager, pooling_contract, pooling_contract_interface): creator = testerchain.client.accounts[0] From 7dfe82fb6efbead00854198206f05545193d4009 Mon Sep 17 00:00:00 2001 From: vzotova Date: Wed, 17 Mar 2021 18:58:53 +0300 Subject: [PATCH 2/3] Change lifecycle for pool: new rules for deposit() and withdrawAll() --- .../PoolingStakingContractV2.sol | 39 +++--- .../test_pooling_contract_v2.py | 132 +++++++++++------- 2 files changed, 103 insertions(+), 68 deletions(-) diff --git a/nucypher/blockchain/eth/sol/source/contracts/staking_contracts/PoolingStakingContractV2.sol b/nucypher/blockchain/eth/sol/source/contracts/staking_contracts/PoolingStakingContractV2.sol index 57252960b..d478d087d 100644 --- a/nucypher/blockchain/eth/sol/source/contracts/staking_contracts/PoolingStakingContractV2.sol +++ b/nucypher/blockchain/eth/sol/source/contracts/staking_contracts/PoolingStakingContractV2.sol @@ -25,7 +25,6 @@ contract PoolingStakingContractV2 is InitializableStakingContract, Ownable { uint256 depositedTokens ); event ETHWithdrawn(address indexed sender, uint256 value); - event DepositIsEnabledSet(address indexed sender, bool value); event WorkerOwnerSet(address indexed sender, address indexed workerOwner); struct Delegator { @@ -34,7 +33,7 @@ contract PoolingStakingContractV2 is InitializableStakingContract, Ownable { uint256 withdrawnETH; } - /// Defines base fraction and precision of worker fraction. Value 100 defines 1 worker fraction is 1% of reward + /// Defines base fraction and precision of worker fraction. Value 10000 defines 100 worker fraction is 1% of reward uint256 public constant BASIS_FRACTION = 10000; StakingEscrow public escrow; @@ -48,12 +47,12 @@ contract PoolingStakingContractV2 is InitializableStakingContract, Ownable { uint256 public workerWithdrawnReward; mapping(address => Delegator) public delegators; - bool public depositIsEnabled = true; /** * @notice Initialize function for using with OpenZeppelin proxy * @param _workerFraction Share of token reward that worker node owner will get. - * Use value up to BASIS_FRACTION, if _workerFraction = BASIS_FRACTION -> means 100% reward as commission + * Use value up to BASIS_FRACTION (10000), if _workerFraction = BASIS_FRACTION -> means 100% reward as commission. + * For example, 100 worker fraction is 1% of reward * @param _router StakingInterfaceRouter address * @param _workerOwner Owner of worker node, only this address can withdraw worker commission */ @@ -72,19 +71,22 @@ contract PoolingStakingContractV2 is InitializableStakingContract, Ownable { } /** - * @notice Enabled deposit + * @notice withdrawAll() is allowed */ - function enableDeposit() external onlyOwner { - depositIsEnabled = true; - emit DepositIsEnabledSet(msg.sender, depositIsEnabled); + function isWithdrawAllAllowed() public view returns (bool) { + // no tokens in StakingEscrow contract which belong to pool + return escrow.getAllTokens(address(this)) == 0; } /** - * @notice Disable deposit + * @notice deposit() is allowed */ - function disableDeposit() external onlyOwner { - depositIsEnabled = false; - emit DepositIsEnabledSet(msg.sender, depositIsEnabled); + function isDepositAllowed() public view returns (bool) { + // tokens which directly belong to pool + uint256 freeTokens = token.balanceOf(address(this)); + + // no sub-stakes and no earned reward + return isWithdrawAllAllowed() && freeTokens == totalDepositedTokens; } /** @@ -108,7 +110,7 @@ contract PoolingStakingContractV2 is InitializableStakingContract, Ownable { * @param _value Amount of tokens to transfer */ function depositTokens(uint256 _value) external { - require(depositIsEnabled, "Deposit must be enabled"); + require(isDepositAllowed(), "Deposit must be enabled"); require(_value > 0, "Value must be not empty"); totalDepositedTokens = totalDepositedTokens.add(_value); Delegator storage delegator = delegators[msg.sender]; @@ -120,7 +122,7 @@ contract PoolingStakingContractV2 is InitializableStakingContract, Ownable { /** * @notice Get available reward for all delegators and owner */ - function getAvailableDelegatorReward() public view returns (uint256) { + function getAvailableReward() public view returns (uint256) { // locked + unlocked tokens in StakingEscrow contract which belong to pool uint256 stakedTokens = escrow.getAllTokens(address(this)); // tokens which directly belong to pool @@ -139,7 +141,7 @@ contract PoolingStakingContractV2 is InitializableStakingContract, Ownable { * Available and withdrawn reward together to use in delegator/owner reward calculations */ function getCumulativeReward() public view returns (uint256) { - return getAvailableDelegatorReward().add(totalWithdrawnReward); + return getAvailableReward().add(totalWithdrawnReward); } /** @@ -170,11 +172,7 @@ contract PoolingStakingContractV2 is InitializableStakingContract, Ownable { /** * @notice Get available reward in tokens for delegator */ - function getAvailableDelegatorReward(address _delegator) - public - view - returns (uint256) - { + function getAvailableDelegatorReward(address _delegator) public view returns (uint256) { // special case when there are no delegators if (totalDepositedTokens == 0) { return 0; @@ -243,6 +241,7 @@ contract PoolingStakingContractV2 is InitializableStakingContract, Ownable { * @notice Withdraw reward, deposit and fee to delegator */ function withdrawAll() public { + require(isWithdrawAllAllowed(), "Withdraw deposit and reward must be enabled"); uint256 balance = token.balanceOf(address(this)); Delegator storage delegator = delegators[msg.sender]; diff --git a/tests/contracts/main/staking_contracts/test_pooling_contract_v2.py b/tests/contracts/main/staking_contracts/test_pooling_contract_v2.py index 1296e884d..fd382ac35 100644 --- a/tests/contracts/main/staking_contracts/test_pooling_contract_v2.py +++ b/tests/contracts/main/staking_contracts/test_pooling_contract_v2.py @@ -66,7 +66,9 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, assert pooling_contract.functions.totalWithdrawnReward().call() == 0 assert token.functions.balanceOf(pooling_contract.address).call() == 0 assert pooling_contract.functions.getAvailableWorkerReward().call() == 0 - assert pooling_contract.functions.getAvailableDelegatorReward().call() == 0 + assert pooling_contract.functions.getAvailableReward().call() == 0 + assert pooling_contract.functions.isDepositAllowed().call() + assert pooling_contract.functions.isWithdrawAllAllowed().call() # Give some tokens to delegators for index, delegator in enumerate(delegators): @@ -77,7 +79,7 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, # Delegators deposit tokens to the pooling contract total_deposited_tokens = 0 tokens_supply = 0 - assert pooling_contract.functions.getAvailableDelegatorReward().call() == 0 + assert pooling_contract.functions.getAvailableReward().call() == 0 for index, delegator in enumerate(delegators): assert pooling_contract.functions.delegators(delegator).call() == [0, 0, 0] assert pooling_contract.functions.getAvailableDelegatorReward(delegator).call() == 0 @@ -104,43 +106,23 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, assert pooling_contract.functions.workerWithdrawnReward().call() == 0 assert pooling_contract.functions.totalWithdrawnReward().call() == 0 assert pooling_contract.functions.getAvailableWorkerReward().call() == 0 - assert pooling_contract.functions.getAvailableDelegatorReward().call() == 0 - - # Disable deposit - log = pooling_contract.events.DepositIsEnabledSet.createFilter(fromBlock='latest') - with pytest.raises((TransactionFailed, ValueError)): - tx = pooling_contract.functions.disableDeposit().transact({'from': delegators[0]}) - testerchain.wait_for_receipt(tx) - tx = pooling_contract.functions.disableDeposit().transact({'from': owner}) - testerchain.wait_for_receipt(tx) - events = log.get_all_entries() - assert len(events) == 1 - event_args = events[-1]['args'] - assert event_args['sender'] == owner - assert not event_args['value'] + assert pooling_contract.functions.getAvailableReward().call() == 0 + assert pooling_contract.functions.isDepositAllowed().call() + assert pooling_contract.functions.isWithdrawAllAllowed().call() + # Delegator can cancel deposit before stake will be created delegator = delegators[0] tokens = token.functions.balanceOf(delegator).call() + tx = pooling_contract.functions.withdrawAll().transact({'from': delegator}) + testerchain.wait_for_receipt(tx) + assert token.functions.balanceOf(delegator).call() == 2 * tokens + assert pooling_contract.functions.delegators(delegator).call() == [0, 0, 0] + # Return back tx = token.functions.approve(pooling_contract.address, tokens).transact({'from': delegator}) testerchain.wait_for_receipt(tx) - with pytest.raises((TransactionFailed, ValueError)): - tx = pooling_contract.functions.depositTokens(tokens).transact({'from': delegator}) - testerchain.wait_for_receipt(tx) - tx = token.functions.approve(pooling_contract.address, 0).transact({'from': delegator}) + tx = pooling_contract.functions.depositTokens(tokens).transact({'from': delegator}) testerchain.wait_for_receipt(tx) - # Enable deposit - with pytest.raises((TransactionFailed, ValueError)): - tx = pooling_contract.functions.enableDeposit().transact({'from': delegators[0]}) - testerchain.wait_for_receipt(tx) - tx = pooling_contract.functions.enableDeposit().transact({'from': owner}) - testerchain.wait_for_receipt(tx) - events = log.get_all_entries() - assert len(events) == 2 - event_args = events[-1]['args'] - assert event_args['sender'] == owner - assert event_args['value'] - # Delegators deposit tokens to the pooling contract again for index, delegator in enumerate(delegators): tokens = token.functions.balanceOf(delegator).call() @@ -155,7 +137,7 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, assert token.functions.balanceOf(pooling_contract.address).call() == tokens_supply events = deposit_log.get_all_entries() - assert len(events) == len(delegators) + index + 1 + assert len(events) == len(delegators) + index + 2 event_args = events[-1]['args'] assert event_args['sender'] == delegator assert event_args['value'] == tokens @@ -173,31 +155,52 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, testerchain.wait_for_receipt(tx) assert pooling_contract.functions.totalDepositedTokens().call() == total_deposited_tokens assert token.functions.balanceOf(pooling_contract.address).call() == 0 + assert not pooling_contract.functions.isDepositAllowed().call() + assert not pooling_contract.functions.isWithdrawAllAllowed().call() + + # Can't deposit after stake was created + tokens = token_economics.minimum_allowed_locked + tx = token.functions.approve(pooling_contract.address, tokens).transact({'from': creator}) + testerchain.wait_for_receipt(tx) + with pytest.raises((TransactionFailed, ValueError)): + tx = pooling_contract.functions.depositTokens(tokens).transact({'from': creator}) + testerchain.wait_for_receipt(tx) + tx = token.functions.approve(pooling_contract.address, 0).transact({'from': creator}) + testerchain.wait_for_receipt(tx) # Give some tokens as a reward - assert pooling_contract.functions.getAvailableDelegatorReward().call() == 0 + assert pooling_contract.functions.getAvailableReward().call() == 0 reward = token_economics.minimum_allowed_locked tx = token.functions.approve(escrow.address, reward).transact() testerchain.wait_for_receipt(tx) tx = escrow.functions.deposit(pooling_contract.address, reward, 0).transact() testerchain.wait_for_receipt(tx) + assert not pooling_contract.functions.isDepositAllowed().call() + assert not pooling_contract.functions.isWithdrawAllAllowed().call() # Only owner can withdraw tokens from the staking escrow with pytest.raises((TransactionFailed, ValueError)): tx = pooling_contract_interface.functions.withdrawAsStaker(reward).transact({'from': delegators[0]}) testerchain.wait_for_receipt(tx) - withdrawn_stake = reward + stake - assert pooling_contract.functions.getAvailableDelegatorReward().call() == 0 - tx = pooling_contract_interface.functions.withdrawAsStaker(withdrawn_stake).transact({'from': owner}) + assert pooling_contract.functions.getAvailableReward().call() == 0 + tx = pooling_contract_interface.functions.withdrawAsStaker(reward).transact({'from': owner}) testerchain.wait_for_receipt(tx) - assert pooling_contract.functions.getAvailableDelegatorReward().call() == reward + assert pooling_contract.functions.getAvailableReward().call() == reward worker_reward = reward * WORKER_FRACTION // BASIS_FRACTION assert pooling_contract.functions.getAvailableWorkerReward().call() == worker_reward assert pooling_contract.functions.totalDepositedTokens().call() == total_deposited_tokens - assert token.functions.balanceOf(pooling_contract.address).call() == withdrawn_stake - tokens_supply = withdrawn_stake + assert token.functions.balanceOf(pooling_contract.address).call() == reward + tokens_supply = reward total_withdrawn_tokens = 0 + assert not pooling_contract.functions.isDepositAllowed().call() + assert not pooling_contract.functions.isWithdrawAllAllowed().call() + + # Can't withdraw initial deposit before stake will be unlocked + for delegator in delegators: + with pytest.raises((TransactionFailed, ValueError)): + tx = pooling_contract.functions.withdrawAll().transact({'from': delegator}) + testerchain.wait_for_receipt(tx) # Each delegator can withdraw some portion of tokens available_reward = reward @@ -226,7 +229,7 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, assert pooling_contract.functions.totalWithdrawnReward().call() == total_withdrawn_tokens events = withdraw_log.get_all_entries() - assert len(events) == index + 1 + assert len(events) == index + 2 event_args = events[-1]['args'] assert event_args['sender'] == delegator assert event_args['value'] == portion @@ -234,7 +237,7 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, # Node owner withdraws tokens assert pooling_contract.functions.getAvailableWorkerReward().call() == worker_reward - assert pooling_contract.functions.getAvailableDelegatorReward().call() == available_reward + assert pooling_contract.functions.getAvailableReward().call() == available_reward # Only node owner can call this method with pytest.raises((TransactionFailed, ValueError)): @@ -256,10 +259,10 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, assert token.functions.balanceOf(owner).call() == 0 assert token.functions.balanceOf(worker_owner).call() == worker_reward assert pooling_contract.functions.totalWithdrawnReward().call() == total_withdrawn_tokens - assert pooling_contract.functions.getAvailableDelegatorReward().call() == available_reward - worker_reward + assert pooling_contract.functions.getAvailableReward().call() == available_reward - worker_reward events = withdraw_log.get_all_entries() - assert len(events) == len(delegators) + 1 + assert len(events) == len(delegators) + 2 event_args = events[-1]['args'] assert event_args['sender'] == worker_owner assert event_args['value'] == worker_reward @@ -270,6 +273,16 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, tx = pooling_contract.functions.withdrawWorkerReward().transact({'from': worker_owner}) testerchain.wait_for_receipt(tx) + # Withdraw stake + tx = pooling_contract_interface.functions.withdrawAsStaker(stake - 1).transact({'from': owner}) + testerchain.wait_for_receipt(tx) + assert not pooling_contract.functions.isWithdrawAllAllowed().call() + tx = pooling_contract_interface.functions.withdrawAsStaker(1).transact({'from': owner}) + testerchain.wait_for_receipt(tx) + tokens_supply += stake + assert not pooling_contract.functions.isDepositAllowed().call() + assert pooling_contract.functions.isWithdrawAllAllowed().call() + # Each delegator can withdraw rest of reward and deposit previous_total_deposited_tokens = total_deposited_tokens withdrawn_worker_reward = worker_reward @@ -307,12 +320,15 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, assert abs(pooling_contract.functions.workerWithdrawnReward().call() - withdrawn_worker_reward) <= 1 events = withdraw_log.get_all_entries() - assert len(events) == len(delegators) + 2 + assert len(events) == len(delegators) + 3 event_args = events[-1]['args'] assert event_args['sender'] == delegator assert event_args['value'] == new_portion assert event_args['depositedTokens'] == 0 + assert not pooling_contract.functions.isDepositAllowed().call() + assert pooling_contract.functions.isWithdrawAllAllowed().call() + # Check worker's reward, still zero assert pooling_contract.functions.getAvailableWorkerReward().call() == 0 @@ -367,7 +383,7 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, assert abs(pooling_contract.functions.workerWithdrawnReward().call() - withdrawn_worker_reward) <= 1 events = withdraw_log.get_all_entries() - assert len(events) == len(delegators) + 4 + assert len(events) == len(delegators) + 5 event_args = events[-2]['args'] assert event_args['sender'] == worker_owner assert event_args['value'] == new_worker_transfer @@ -385,6 +401,9 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, # Check others rewards assert abs(pooling_contract.functions.getAvailableDelegatorReward(delegators[2]).call() - other_reward_portion) <= 10 + assert not pooling_contract.functions.isDepositAllowed().call() + assert pooling_contract.functions.isWithdrawAllAllowed().call() + # Withdraw last portion for last delegator delegator = delegators[2] deposited_tokens = pooling_contract.functions.delegators(delegator).call()[0] @@ -399,10 +418,10 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, assert token.functions.balanceOf(pooling_contract.address).call() <= 1 assert token.functions.balanceOf(delegator).call() == previous_portion + new_portion assert token.functions.balanceOf(worker_owner).call() == worker_reward + new_worker_transfer + new_worker_reward - assert pooling_contract.functions.getAvailableDelegatorReward().call() <= 1 + assert pooling_contract.functions.getAvailableReward().call() <= 1 events = withdraw_log.get_all_entries() - assert len(events) == len(delegators) + 6 + assert len(events) == len(delegators) + 7 event_args = events[-2]['args'] assert event_args['sender'] == worker_owner assert event_args['value'] == new_worker_reward @@ -413,11 +432,18 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, assert event_args['value'] == new_portion assert event_args['depositedTokens'] == 0 + assert not pooling_contract.functions.isDepositAllowed().call() + assert pooling_contract.functions.isWithdrawAllAllowed().call() + + # # Change worker owner + # # Prepare reward tx = token.functions.transfer(pooling_contract.address, new_reward).transact({'from': creator}) testerchain.wait_for_receipt(tx) + assert not pooling_contract.functions.isDepositAllowed().call() + assert pooling_contract.functions.isWithdrawAllAllowed().call() # Only pool owner can change value new_worker_owner = delegators[0] @@ -446,6 +472,16 @@ def test_staking(testerchain, token_economics, token, escrow, pooling_contract, assert event_args['sender'] == owner assert event_args['workerOwner'] == new_worker_owner + # There are no reward or locked tokens, deposit is allowed again + assert pooling_contract.functions.isDepositAllowed().call() + assert pooling_contract.functions.isWithdrawAllAllowed().call() + delegator = delegators[0] + tokens = token_economics.minimum_allowed_locked + tx = token.functions.approve(pooling_contract.address, tokens).transact({'from': creator}) + testerchain.wait_for_receipt(tx) + tx = pooling_contract.functions.depositTokens(tokens).transact({'from': creator}) + testerchain.wait_for_receipt(tx) + def test_fee(testerchain, token_economics, token, policy_manager, pooling_contract, pooling_contract_interface): creator = testerchain.client.accounts[0] From 62576f12e7f54c83a5487806b416886a74bb6df3 Mon Sep 17 00:00:00 2001 From: vzotova Date: Fri, 26 Mar 2021 16:08:30 +0300 Subject: [PATCH 3/3] Apply RFCs from #2596 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: David Núñez Co-authored-by: Derek Pierre --- .../staking_contracts/PoolingStakingContractV2.sol | 11 +++++++---- .../staking_contracts/test_pooling_contract_v2.py | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/nucypher/blockchain/eth/sol/source/contracts/staking_contracts/PoolingStakingContractV2.sol b/nucypher/blockchain/eth/sol/source/contracts/staking_contracts/PoolingStakingContractV2.sol index d478d087d..4c3eab936 100644 --- a/nucypher/blockchain/eth/sol/source/contracts/staking_contracts/PoolingStakingContractV2.sol +++ b/nucypher/blockchain/eth/sol/source/contracts/staking_contracts/PoolingStakingContractV2.sol @@ -33,7 +33,10 @@ contract PoolingStakingContractV2 is InitializableStakingContract, Ownable { uint256 withdrawnETH; } - /// Defines base fraction and precision of worker fraction. Value 10000 defines 100 worker fraction is 1% of reward + /** + * Defines base fraction and precision of worker fraction. + * E.g., for a value of 10000, a worker fraction of 100 represents 1% of reward (100/10000) + */ uint256 public constant BASIS_FRACTION = 10000; StakingEscrow public escrow; @@ -253,7 +256,7 @@ contract PoolingStakingContractV2 is InitializableStakingContract, Ownable { uint256 availableWorkerReward = getAvailableWorkerReward(); // potentially could be less then due reward - uint256 availableETH = getAvailableETH(msg.sender); + uint256 availableETH = getAvailableDelegatorETH(msg.sender); // prevent losing reward for worker after calculations uint256 workerReward = availableWorkerReward.mul(delegator.depositedTokens).div(totalDepositedTokens); @@ -286,7 +289,7 @@ contract PoolingStakingContractV2 is InitializableStakingContract, Ownable { /** * @notice Get available ether for delegator */ - function getAvailableETH(address _delegator) public view returns (uint256) { + function getAvailableDelegatorETH(address _delegator) public view returns (uint256) { Delegator storage delegator = delegators[_delegator]; uint256 balance = address(this).balance; // ETH balance + already withdrawn @@ -305,7 +308,7 @@ contract PoolingStakingContractV2 is InitializableStakingContract, Ownable { */ function withdrawETH() public override { Delegator storage delegator = delegators[msg.sender]; - uint256 availableETH = getAvailableETH(msg.sender); + uint256 availableETH = getAvailableDelegatorETH(msg.sender); require(availableETH > 0, "There is no available ETH to withdraw"); delegator.withdrawnETH = delegator.withdrawnETH.add(availableETH); diff --git a/tests/contracts/main/staking_contracts/test_pooling_contract_v2.py b/tests/contracts/main/staking_contracts/test_pooling_contract_v2.py index fd382ac35..a54f47a73 100644 --- a/tests/contracts/main/staking_contracts/test_pooling_contract_v2.py +++ b/tests/contracts/main/staking_contracts/test_pooling_contract_v2.py @@ -533,7 +533,7 @@ def test_fee(testerchain, token_economics, token, policy_manager, pooling_contra deposited_tokens = pooling_contract.functions.delegators(delegator).call()[0] max_portion = value * deposited_tokens // total_deposited_tokens balance = testerchain.client.get_balance(delegator) - assert pooling_contract.functions.getAvailableETH(delegator).call() == max_portion + assert pooling_contract.functions.getAvailableDelegatorETH(delegator).call() == max_portion tx = pooling_contract.functions.withdrawETH().transact({'from': delegator, 'gas_price': 0}) testerchain.wait_for_receipt(tx)