From 2383ca1ba53db8b7ef1c63de1ecb1771e936fe91 Mon Sep 17 00:00:00 2001 From: DrZoltanFazekas Date: Tue, 19 Nov 2024 19:57:53 +0100 Subject: [PATCH] Fix issue 9 and other minor improvements --- README.md | 6 +- src/BaseDelegation.sol | 16 +- src/Deposit.sol | 298 +++------------------------------ src/LiquidDelegationV2.sol | 22 +-- src/NonLiquidDelegationV2.sol | 34 ++-- state.sh | 6 +- test/LiquidDelegation.t.sol | 77 ++++----- test/NonLiquidDelegation.t.sol | 134 ++++++++++++++- unstake.sh | 2 +- 9 files changed, 236 insertions(+), 359 deletions(-) diff --git a/README.md b/README.md index f5a874f..c661fc8 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ The delegation contract is used by delegators to stake and unstake ZIL with the `LiquidDelegation` is the initial implementation of the liquid staking variant of the delegation contract that creates a `NonRebasingLST` contract when it is initialized. `LiquidDelegationV2` implements all other features. `NonLiquidDelegation` is the initial implementation of the non-liquid staking variant of the delegation contract that allows delegators to withdraw rewards. -The delegation contract shall be deployed and upgraded by the account with the private key that was used to run the validator node and was used to generate its BLS keypair and peer id. Make sure the `PRIVATE_KEY` environment variable is set accordingly. +Before running the deployment script, set the `PRIVATE_KEY` environment variable to the private key of the contract owner. Note that only the contract owner will be able to upgrade the contract, change the commission and activate the node as a validator. To deploy `LiquidDelegation` run ```bash @@ -60,7 +60,7 @@ The output will look like this: Now or at a later time you can set the commission on the rewards the validator earns to e.g. 10% as follows: ```bash -forge script script/commission_Delegation.s.sol --rpc-url http://localhost:4201 --broadcast --legacy --sig "run(address payable, uint16, bool)" 0x7A0b7e6D24eDe78260c9ddBD98e828B0e11A8EA2 1000 false +forge script script/commission_Delegation.s.sol --rpc-url http://localhost:4201 --broadcast --legacy --sig "run(address payable, string, bool)" 0x7A0b7e6D24eDe78260c9ddBD98e828B0e11A8EA2 1000 false ``` The output will contain the following information: @@ -77,7 +77,7 @@ cast call 0x7A0b7e6D24eDe78260c9ddBD98e828B0e11A8EA2 "DENOMINATOR()(uint256)" -- Once the validator is activated and starts earning rewards, commissions are transferred automatically to the validator node's account. Commissions of a non-liquid staking validator are deducted when delegators withdraw rewards. In case of the liquid staking variant, commissions are deducted each time delegators stake, unstake or claim what they unstaked, or when the node requests the outstanding commissions that haven't been transferred yet. To collect them, run ```bash -forge script script/commission_Delegation.s.sol --rpc-url http://localhost:4201 --broadcast --legacy --sig "run(address payable, string, bool)" 0x7A0b7e6D24eDe78260c9ddBD98e828B0e11A8EA2 10001 true +forge script script/commission_Delegation.s.sol --rpc-url http://localhost:4201 --broadcast --legacy --sig "run(address payable, string, bool)" 0x7A0b7e6D24eDe78260c9ddBD98e828B0e11A8EA2 same true ``` using `same` for the second argument to leave the commission percentage unchanged and `true` for the third argument. Replacing the second argument with `same` and the third argument with `false` only displays the current commission rate. diff --git a/src/BaseDelegation.sol b/src/BaseDelegation.sol index a35f570..c15afef 100644 --- a/src/BaseDelegation.sol +++ b/src/BaseDelegation.sol @@ -10,9 +10,7 @@ import "src/Delegation.sol"; library WithdrawalQueue { - //TODO: value set for testing purposes, make it equal to 2 * 7 * 24 * 60 * 60 - // if governance changes the unbonding period, update the value and upgrade the contract - uint256 public constant UNBONDING_PERIOD = 30; + address public constant DEPOSIT_CONTRACT = 0x000000000000000000005a494C4445504F534954; struct Item { uint256 blockNumber; @@ -25,8 +23,16 @@ library WithdrawalQueue { mapping(uint256 => Item) items; } + function unbondingPeriod() view internal returns(uint256) { + (bool success, bytes memory data) = DEPOSIT_CONTRACT.staticcall( + abi.encodeWithSignature("withdrawalPeriod()") + ); + require(success, "unbonding period unknown"); + return abi.decode(data, (uint256)); + } + function queue(Fifo storage fifo, uint256 amount) internal { - fifo.items[fifo.last] = Item(block.number + UNBONDING_PERIOD, amount); + fifo.items[fifo.last] = Item(block.number + unbondingPeriod(), amount); fifo.last++; } @@ -216,7 +222,7 @@ abstract contract BaseDelegation is Delegation, PausableUpgradeable, Ownable2Ste if (!_isActivated()) return address(this).balance; (bool success, bytes memory data) = DEPOSIT_CONTRACT.staticcall( - abi.encodeWithSignature("getStake(bytes)", $.blsPubKey) + abi.encodeWithSignature("getFutureStake(bytes)", $.blsPubKey) ); require(success, "could not retrieve staked amount"); return abi.decode(data, (uint256)); diff --git a/src/Deposit.sol b/src/Deposit.sol index fc6ebd6..6dd543b 100644 --- a/src/Deposit.sol +++ b/src/Deposit.sol @@ -158,10 +158,10 @@ contract Deposit { // or withdrawals were made. uint64 latestComputedEpoch; - uint256 public minimumStake; - uint256 public maximumStakers; + uint256 public immutable minimumStake; + uint256 public immutable maximumStakers; - uint64 public blocksPerEpoch; + uint64 public immutable blocksPerEpoch; modifier onlyControlAddress(bytes calldata blsPubKey) { require(blsPubKey.length == 48); @@ -314,6 +314,22 @@ contract Deposit { return committee().stakers[blsPubKey].balance; } + function getFutureStake( + bytes calldata blsPubKey + ) public view returns (uint256) { + require(blsPubKey.length == 48); + + // if `latestComputedEpoch > currentEpoch()` + // then `latestComputedEpoch` determines the future committee we need + // otherwise there are no committee changes after `currentEpoch()` + // i.e. `latestComputedEpoch` determines the most recent committee + Committee storage latestCommittee = _committee[latestComputedEpoch % 3]; + + // We don't need to check if `blsPubKey` is in `stakerKeys` here. If the `blsPubKey` is not a staker, the + // balance will default to zero. + return latestCommittee.stakers[blsPubKey].balance; + } + function getRewardAddress( bytes calldata blsPubKey ) public view returns (address) { @@ -415,7 +431,7 @@ contract Deposit { signature, pubkey ); - // mocked to make the tests work + //TODO: don't remove the next line return true; uint inputLength = input.length; bytes memory output = new bytes(32); @@ -515,7 +531,7 @@ contract Deposit { futureCommittee.stakers[stakerKey].index != 0, "staker does not exist" ); - //TODO: why is this required? to prevent unstaking from initialStakers? + //TODO: keep the next line commented out //require(futureCommittee.stakerKeys.length > 1, "too few stakers"); require( futureCommittee.stakers[stakerKey].balance >= amount, @@ -586,12 +602,10 @@ contract Deposit { _withdraw(count); } - function withdrawalPeriod() public pure returns (uint256) { - // 2 weeks - //return 2 * 7 * 24 * 60 * 60; - //TODO: return 14 days; - // modified to 30 seconds to make the tests faster - return 30; + function withdrawalPeriod() public view returns (uint256) { + // shorter unbonding period for testing deposit withdrawals + if (block.chainid == 33469) return 5 minutes; + return 2 weeks; } function _withdraw(uint256 count) internal { @@ -621,265 +635,3 @@ contract Deposit { require(sent, "failed to send"); } } - -/* the code below based on an outdated version of deposit.sol -struct Staker { - // The index of this staker's `blsPubKey` in the `_stakerKeys` array, plus 1. 0 is used for non-existing entries. - uint256 keyIndex; - // Invariant: `balance >= minimumStake` - uint256 balance; - address rewardAddress; - bytes peerId; -} - -contract Deposit { - bytes[] _stakerKeys; - mapping(bytes => Staker) _stakersMap; - uint256 public totalStake; - - uint256 public _minimumStake; - uint256 public _maximumStakers; - - constructor(uint256 minimumStake, uint256 maximumStakers) { - _minimumStake = minimumStake; - _maximumStakers = maximumStakers; - } - - function leaderFromRandomness( - uint256 randomness - ) private view returns (bytes memory) { - // Get a random number in the inclusive range of 0 to (totalStake - 1) - uint256 position = randomness % totalStake; - uint256 cummulative_stake = 0; - - for (uint256 i = 0; i < _stakerKeys.length; i++) { - bytes storage stakerKey = _stakerKeys[i]; - Staker storage staker = _stakersMap[stakerKey]; - - cummulative_stake += staker.balance; - - if (position < cummulative_stake) { - return stakerKey; - } - } - - revert("Unable to select next leader"); - } - - function leader() public view returns (bytes memory) { - return leaderFromRandomness(uint256(block.prevrandao)); - } - - function leaderAtView( - uint256 viewNumber - ) public view returns (bytes memory) { - uint256 randomness = uint256( - keccak256(bytes.concat(bytes32(viewNumber))) - ); - return leaderFromRandomness(randomness); - } - - // Temporary function to manually remove a staker. Can be called by the reward address of any staker with more than - // 10% stake. Will be removed later in development. - function tempRemoveStaker(bytes calldata blsPubKey) public { - require(blsPubKey.length == 48); - - // Inefficient, but its fine because this is temporary. - for (uint256 i = 0; i < _stakerKeys.length; i++) { - bytes storage stakerKey = _stakerKeys[i]; - Staker storage staker = _stakersMap[stakerKey]; - - // Check if the call is authorised. - if ( - msg.sender == staker.rewardAddress && - staker.balance > (totalStake / 10) - ) { - // The call is authorised, so we can delete the specified staker. - Staker storage stakerToDelete = _stakersMap[blsPubKey]; - - // Delete this staker's key from `_stakerKeys`. Swap the last element in the array into the deleted position. - bytes storage swappedStakerKey = _stakerKeys[ - _stakerKeys.length - 1 - ]; - Staker storage swappedStaker = _stakersMap[swappedStakerKey]; - _stakerKeys[stakerToDelete.keyIndex - 1] = swappedStakerKey; - swappedStaker.keyIndex = stakerToDelete.keyIndex; - - // The last element is now the element we want to delete. - _stakerKeys.pop(); - - // Reduce the total stake, but don't refund to the removed staker - totalStake -= stakerToDelete.balance; - - // Delete the staker from `_stakersMap` too. - delete _stakersMap[blsPubKey]; - - return; - } - } - revert( - "call must come from a reward address corresponding to a staker with more than 10% stake" - ); - } - - // keep in-sync with zilliqa/src/precompiles.rs - function _popVerify( - bytes memory pubkey, - bytes memory signature - ) private view returns (bool) { - bytes memory input = abi.encodeWithSelector( - hex"bfd24965", // bytes4(keccak256("popVerify(bytes,bytes)")) - signature, - pubkey - ); - // mocked to make the tests work - return true; - uint inputLength = input.length; - bytes memory output = new bytes(32); - bool success; - assembly { - success := staticcall( - gas(), - 0x5a494c80, // "ZIL\x80" - add(input, 0x20), - inputLength, - add(output, 0x20), - 32 - ) - } - require(success, "popVerify"); - bool result = abi.decode(output, (bool)); - return result; - } - - function deposit( - bytes calldata blsPubKey, - bytes calldata peerId, - bytes calldata signature, - address rewardAddress - ) public payable { - require(blsPubKey.length == 48); - require(peerId.length == 38); - require(signature.length == 96); - - require(_stakerKeys.length < _maximumStakers, "too many stakers"); - - // Verify signature as a proof-of-possession of the private key. - bool pop = _popVerify(blsPubKey, signature); - require(pop, "rogue key check"); - - uint256 keyIndex = _stakersMap[blsPubKey].keyIndex; - if (keyIndex == 0) { - // The staker will be at index `_stakerKeys.length`. We also need to add 1 to avoid the 0 sentinel value. - _stakersMap[blsPubKey].keyIndex = _stakerKeys.length + 1; - _stakerKeys.push(blsPubKey); - } - - _stakersMap[blsPubKey].balance += msg.value; - totalStake += msg.value; - - if (_stakersMap[blsPubKey].balance < _minimumStake) { - revert("stake less than minimum stake"); - } - - _stakersMap[blsPubKey].rewardAddress = rewardAddress; - _stakersMap[blsPubKey].peerId = peerId; - } - - // temporary function to test liquid staking - function tempIncreaseDeposit(bytes calldata blsPubKey) public payable { - Staker storage staker = _stakersMap[blsPubKey]; - require(staker.keyIndex != 0, "unknown staker"); - require(staker.rewardAddress == msg.sender, "invalid sender"); - staker.balance += msg.value; - totalStake += msg.value; - } - - // temporary function to test liquid staking - function tempDecreaseDeposit( - bytes calldata blsPubKey, - uint256 amount - ) public { - Staker storage staker = _stakersMap[blsPubKey]; - require(staker.keyIndex != 0, "unknown staker"); - require(staker.rewardAddress == msg.sender, "invalid sender"); - staker.balance -= amount; - require( - staker.balance == 0 || staker.balance >= _minimumStake, - "stake too low" - ); - totalStake -= amount; - (bool success, ) = msg.sender.call{value: amount}(""); - require(success, "withdrawal failed"); - } - - function setStake( - bytes calldata blsPubKey, - bytes calldata peerId, - address rewardAddress, - uint256 amount - ) public { - require(msg.sender == address(0)); - require(blsPubKey.length == 48); - require(peerId.length == 38); - - if (amount < _minimumStake) { - revert("stake less than minimum stake"); - } - - totalStake -= _stakersMap[blsPubKey].balance; - _stakersMap[blsPubKey].balance = amount; - totalStake += amount; - _stakersMap[blsPubKey].rewardAddress = rewardAddress; - _stakersMap[blsPubKey].peerId = peerId; - uint256 keyIndex = _stakersMap[blsPubKey].keyIndex; - if (keyIndex == 0) { - // The staker will be at index `_stakerKeys.length`. We also need to add 1 to avoid the 0 sentinel value. - _stakersMap[blsPubKey].keyIndex = _stakerKeys.length + 1; - _stakerKeys.push(blsPubKey); - } - } - - function getStake(bytes calldata blsPubKey) public view returns (uint256) { - require(blsPubKey.length == 48); - - return _stakersMap[blsPubKey].balance; - } - - function getRewardAddress( - bytes calldata blsPubKey - ) public view returns (address) { - require(blsPubKey.length == 48); - if (_stakersMap[blsPubKey].rewardAddress == address(0)) { - revert("not staked"); - } - return _stakersMap[blsPubKey].rewardAddress; - } - - function getStakers() public view returns (bytes[] memory) { - return _stakerKeys; - } - - function getStakersData() - public - view - returns (bytes[] memory stakerKeys, Staker[] memory stakers) - { - stakerKeys = _stakerKeys; - stakers = new Staker[](stakerKeys.length); - for (uint256 i = 0; i < stakerKeys.length; i++) { - stakers[i] = _stakersMap[stakerKeys[i]]; - } - } - - function getPeerId( - bytes calldata blsPubKey - ) public view returns (bytes memory) { - require(blsPubKey.length == 48); - if (_stakersMap[blsPubKey].rewardAddress == address(0)) { - revert("not staked"); - } - return _stakersMap[blsPubKey].peerId; - } -} -*/ \ No newline at end of file diff --git a/src/LiquidDelegationV2.sol b/src/LiquidDelegationV2.sol index 6e6fd7e..5181c66 100644 --- a/src/LiquidDelegationV2.sol +++ b/src/LiquidDelegationV2.sol @@ -96,26 +96,11 @@ contract LiquidDelegationV2 is BaseDelegation, ILiquidDelegation { $.taxedRewards -= msg.value; } uint256 depositedStake = getStake(); - if ( - // if the validator hasn't deposited or - NonRebasingLST($.lst).totalSupply() == 0 || - // the deposit hasn't become effective yet - depositedStake + $.taxedRewards == 0 - //TODO: The delay between staking and the deposit topup taking effect - // is a more general issue and does not only concern the initial - // deposit. It falsifies the price which does not take the stake - // into account that has just been delegated and does yet not appear - // in the depositedStake. It may help to add msg.value to depositedStake - // and taxedRewards here to get the correct price and amount of LST to be - // minted, but getPrice() will not take the pending stakings and unstakings - // into account. unless the deposit contract includes them in the value returned - // by getStake(). The solution proposed is to implement a getPendingStake() - // function is the deposit contract which returns the total of the initial - // deposit, all topups and unstaked amounts into account regardless of in - // which epoch they become effective. - ) + if (NonRebasingLST($.lst).totalSupply() == 0) + // if the validator hasn't deposited yet, the formula for calculating the shares would divide by zero, therefore shares = msg.value; else + // otherwise depositedStake is greater than zero even if the deposit hasn't been activated yet shares = NonRebasingLST($.lst).totalSupply() * msg.value / (depositedStake + $.taxedRewards); NonRebasingLST($.lst).mint(_msgSender(), shares); _increaseDeposit(msg.value); @@ -194,6 +179,7 @@ contract LiquidDelegationV2 is BaseDelegation, ILiquidDelegation { taxRewards(); } + // this function was only made public for testing purposes function getTaxedRewards() public view returns(uint256) { LiquidDelegationStorage storage $ = _getLiquidDelegationStorage(); return $.taxedRewards; diff --git a/src/NonLiquidDelegationV2.sol b/src/NonLiquidDelegationV2.sol index 18138c6..1c93ab8 100644 --- a/src/NonLiquidDelegationV2.sol +++ b/src/NonLiquidDelegationV2.sol @@ -3,8 +3,10 @@ pragma solidity ^0.8.26; import "src/BaseDelegation.sol"; import "src/NonLiquidDelegation.sol"; +import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol"; contract NonLiquidDelegationV2 is BaseDelegation, INonLiquidDelegation { + using SafeCast for int256; struct Staking { //TODO: just for testing purposes, can be removed @@ -183,7 +185,7 @@ contract NonLiquidDelegationV2 is BaseDelegation, INonLiquidDelegation { uint256 newRewards; // no rewards before the first staker is added if ($.stakings.length > 0) { value += int256($.stakings[$.stakings.length - 1].total); - newRewards = _uint256(int256(getRewards()) - $.totalRewards); + newRewards = (int256(getRewards()) - $.totalRewards).toUint256(); } $.totalRewards = int256(getRewards()); //$.stakings.push(Staking(uint256(amount), uint256(value), newRewards)); @@ -240,6 +242,9 @@ contract NonLiquidDelegationV2 is BaseDelegation, INonLiquidDelegation { (uint256 result, uint64 i, uint64 index) = additionalSteps == type(uint64).max ? _rewards() : _rewards(additionalSteps); + // the caller has not delegated any stake + if (index == 0) + return 0; uint256 taxedRewards = taxRewards(result); $.allWithdrawnRewards[_msgSender()] += taxedRewards; $.firstStakingIndex[_msgSender()] = i; @@ -260,15 +265,6 @@ contract NonLiquidDelegationV2 is BaseDelegation, INonLiquidDelegation { return _rewards(type(uint64).max); } - //TODO: The contract assumes that delegators start contributing to the rewards earned by the validator - // from the point when they delegate their stake. However, this is not true since the delegated - // stake is only added to the validator's deposit in the epoch after next. Smilarly, the contract - // assumes that unstaking has immediate effect on the validator's deposit i.e. the delegator's - // contribution to the rewards earned by the validator are descreased accordingly. This is also - // not correct and it could be misused by delegators to maximize their share of the rewards at - // the expense of other delegators by staking in the first and unstaking in the last block of an - // epoch and thereby receiving a share of the rewards for 3599 additional blocks during which the - // validator was actually not earning more rewards due to the stake of the delegator. function _rewards(uint64 additionalSteps) internal view returns(uint256 result, uint64 i, uint64 index) { NonLiquidDelegationStorage storage $ = _getNonLiquidDelegationStorage(); uint64 firstIndex; @@ -290,10 +286,18 @@ contract NonLiquidDelegationV2 is BaseDelegation, INonLiquidDelegation { return (result, i, index); } // all rewards recorded in the stakings were taken into account - if (index == $.stakings.length) + if (index == $.stakings.length) { + // ensure that the next time the function is called the last index + // representing the rewards accrued since the last staking are not + // included in the result any more - however, what if there have + // been no stakings i.e. the last index remains the same, but there + // have been additional rewards - how can we determine the amount of + // rewards added since we called _withdrawRewards() last time? + index++; // the last step is to add the rewards accrued since the last staking if (total > 0) - result += _uint256(int256(getRewards()) - $.totalRewards) * amount / total; + result += (int256(getRewards()) - $.totalRewards).toUint256() * amount / total; + } } // ensure that the next time the function is called the initial value of i refers // to the last amount and total among the stakingIndices of the staker that already @@ -304,12 +308,6 @@ contract NonLiquidDelegationV2 is BaseDelegation, INonLiquidDelegation { function collectCommission() public override {} - function _uint256(int256 num) internal pure returns(uint256 res) { - // underflow is intentional as a way of handling - // assignments of a negative int256 to an uint256 - res = num < 0 ? 0 - uint256(num) : uint256(num); - } - function supportsInterface(bytes4 interfaceId) public view override returns (bool) { return interfaceId == type(INonLiquidDelegation).interfaceId || super.supportsInterface(interfaceId); } diff --git a/state.sh b/state.sh index 5286377..c9c5ff7 100755 --- a/state.sh +++ b/state.sh @@ -52,7 +52,11 @@ commissionNumerator=$(cast call $1 "getCommissionNumerator()(uint256)" --block $ denominator=$(cast call $1 "DENOMINATOR()(uint256)" --block $block_num --rpc-url http://localhost:4201 | sed 's/\[[^]]*\]//g') if [[ "$variant" == "ILiquidDelegation" ]]; then totalSupply=$(cast call $lst "totalSupply()(uint256)" --block $block_num --rpc-url http://localhost:4201 | sed 's/\[[^]]*\]//g') - price=$(bc -l <<< "scale=36; ($stake+$rewardsBeforeUnstaking-($rewardsBeforeUnstaking-$taxedRewardsBeforeUnstaking)*$commissionNumerator/$denominator)/$totalSupply") + if [[ $totalSupply -ne 0 ]]; then + price=$(bc -l <<< "scale=36; ($stake+$rewardsBeforeUnstaking-($rewardsBeforeUnstaking-$taxedRewardsBeforeUnstaking)*$commissionNumerator/$denominator)/$totalSupply") + else + price=$(bc -l <<< "scale=36; 1/1") + fi price0=$(cast call $1 "getPrice()(uint256)" --block $block_num --rpc-url http://localhost:4201 | sed 's/\[[^]]*\]//g') echo LST supply: $(cast to-unit $totalSupply ether) ZIL echo LST price: $price \~ $(cast to-unit $price0 ether) diff --git a/test/LiquidDelegation.t.sol b/test/LiquidDelegation.t.sol index 856ce5b..147f8ce 100644 --- a/test/LiquidDelegation.t.sol +++ b/test/LiquidDelegation.t.sol @@ -20,6 +20,7 @@ contract LiquidDelegationTest is Test { address staker = 0xd819fFcE7A58b1E835c25617Db7b46a00888B013; function setUp() public { + vm.chainId(33469); uint256 deployerPrivateKey = vm.envUint("PRIVATE_KEY"); owner = vm.addr(deployerPrivateKey); //console.log("Signer is %s", owner); @@ -205,7 +206,7 @@ contract LiquidDelegationTest is Test { lst.totalSupply() ); - uint256 ownerZILBefore; + uint256[2] memory ownerZIL = [uint256(0), 0]; uint256 ownerZILAfter; uint256 loggedAmount; uint256 loggedShares; @@ -233,7 +234,7 @@ contract LiquidDelegationTest is Test { abi.encode(lst.totalSupply() * delegatedAmount / (delegation.getStake() + delegation.getRewards())) ); - ownerZILBefore = delegation.owner().balance; + ownerZIL[0] = delegation.owner().balance; delegation.stake{ value: delegatedAmount @@ -242,7 +243,7 @@ contract LiquidDelegationTest is Test { // wait 2 epochs for the change to the deposit to take affect vm.roll(block.number + Deposit(delegation.DEPOSIT_CONTRACT()).blocksPerEpoch() * 2); - ownerZILAfter = delegation.owner().balance; + ownerZIL[1] = delegation.owner().balance; entries = vm.getRecordedLogs(); for (uint256 i = 0; i < entries.length; i++) { @@ -256,9 +257,9 @@ contract LiquidDelegationTest is Test { totalShares += loggedShares; Console.log("Owner commission after staking: %s.%s%s ZIL", - ownerZILAfter - ownerZILBefore + ownerZIL[1] - ownerZIL[0] ); - assertEq(rewardsDelta * delegation.getCommissionNumerator() / delegation.DENOMINATOR(), ownerZILAfter - ownerZILBefore, "commission mismatch after staking"); + assertEq(rewardsDelta * delegation.getCommissionNumerator() / delegation.DENOMINATOR(), ownerZIL[1] - ownerZIL[0], "commission mismatch after staking"); Console.log("Deposit after staking: %s.%s%s ZIL", delegation.getStake() @@ -317,8 +318,8 @@ contract LiquidDelegationTest is Test { abi.encode(lst.balanceOf(staker)) ); - uint256 stakerLSTBefore = lst.balanceOf(staker); - ownerZILBefore = delegation.owner().balance; + uint256[2] memory stakerLST = [lst.balanceOf(staker), 0]; + ownerZIL[0] = delegation.owner().balance; uint256 shares = initialDeposit ? lst.balanceOf(staker) : lst.balanceOf(staker) - depositAmount; assertEq(totalShares, shares, "staked shares balance mismatch"); @@ -330,8 +331,10 @@ contract LiquidDelegationTest is Test { // wait 2 epochs for the change to the deposit to take affect vm.roll(block.number + Deposit(delegation.DEPOSIT_CONTRACT()).blocksPerEpoch() * 2); - uint256 stakerLSTAfter = lst.balanceOf(staker); - ownerZILAfter = delegation.owner().balance; + stakerLST[1] = lst.balanceOf(staker); + ownerZIL[1] = delegation.owner().balance; + + assertEq((rewardsBeforeUnstaking - taxedRewardsAfterStaking) * delegation.getCommissionNumerator() / delegation.DENOMINATOR(), ownerZIL[1] - ownerZIL[0], "commission mismatch after unstaking"); entries = vm.getRecordedLogs(); @@ -344,14 +347,12 @@ contract LiquidDelegationTest is Test { } assertEq(totalShares * lstPrice / 10**18 / 1 ether, loggedAmount, "unstaked amount mismatch"); assertEq(shares, loggedShares, "unstaked shares mismatch"); - assertEq(shares, stakerLSTBefore - stakerLSTAfter, "shares balance mismatch"); + assertEq(shares, stakerLST[0] - stakerLST[1], "shares balance mismatch"); Console.log("Owner commission after unstaking: %s.%s%s ZIL", - ownerZILAfter - ownerZILBefore + ownerZIL[1] - ownerZIL[0] ); - assertEq((rewardsBeforeUnstaking - taxedRewardsAfterStaking) * delegation.getCommissionNumerator() / delegation.DENOMINATOR(), ownerZILAfter - ownerZILBefore, "commission mismatch after unstaking"); - Console.log("Deposit after unstaking: %s.%s%s ZIL", delegation.getStake() ); @@ -401,13 +402,13 @@ contract LiquidDelegationTest is Test { "" ); - uint256 stakerZILBefore = staker.balance; - ownerZILBefore = delegation.owner().balance; + uint256[2] memory stakerZIL = [staker.balance, 0]; + ownerZIL[0] = delegation.owner().balance; delegation.claim(); - uint256 stakerZILAfter = staker.balance; - ownerZILAfter = delegation.owner().balance; + stakerZIL[1] = staker.balance; + ownerZIL[1] = delegation.owner().balance; entries = vm.getRecordedLogs(); @@ -417,12 +418,12 @@ contract LiquidDelegationTest is Test { } } assertEq(loggedAmount, unstakedAmount, "unstaked vs claimed amount mismatch"); - assertEq(loggedAmount, stakerZILAfter - stakerZILBefore, "claimed amount vs staker balance mismatch"); + assertEq(loggedAmount, stakerZIL[1] - stakerZIL[0], "claimed amount vs staker balance mismatch"); Console.log("Owner commission after claiming: %s.%s%s ZIL", - ownerZILAfter - ownerZILBefore + ownerZIL[1] - ownerZIL[0] ); - assertEq((rewardsAfterUnstaking - taxedRewardsAfterUnstaking) * delegation.getCommissionNumerator() / delegation.DENOMINATOR(), ownerZILAfter - ownerZILBefore, "commission mismatch after claiming"); + assertEq((rewardsAfterUnstaking - taxedRewardsAfterUnstaking) * delegation.getCommissionNumerator() / delegation.DENOMINATOR(), ownerZIL[1] - ownerZIL[0], "commission mismatch after claiming"); Console.log("Deposit after claiming: %s.%s%s ZIL", delegation.getStake() @@ -469,7 +470,7 @@ contract LiquidDelegationTest is Test { 1, // numberOfDelegations 0, // rewardsAccruedAfterEach taxedRewardsAfterStaking, // rewardsBeforeUnstaking - WithdrawalQueue.UNBONDING_PERIOD, // after unstaking wait blocksUntil claiming + WithdrawalQueue.unbondingPeriod(), // after unstaking wait blocksUntil claiming true // initialDeposit using funds held by the node, otherwise delegated by a staker ); } @@ -523,7 +524,7 @@ contract LiquidDelegationTest is Test { console.log("validator balance: %s", owner.balance); //vm.roll(block.number + Deposit(delegation.DEPOSIT_CONTRACT()).withdrawalPeriod()); //TODO: once https://github.com/Zilliqa/zq2/issues/1761 is fixed, uncomment the previous line and comment out the next one - vm.warp(block.timestamp + Deposit(delegation.DEPOSIT_CONTRACT()).withdrawalPeriod()); // skip(WithdrawalQueue.UNBONDING_PERIOD); + vm.warp(block.timestamp + Deposit(delegation.DEPOSIT_CONTRACT()).withdrawalPeriod()); // skip(WithdrawalQueue.unbondingPeriod()); Deposit(delegation.DEPOSIT_CONTRACT()).withdraw(); console.log("validator withdrew again"); console.log("validator balance: %s", owner.balance); @@ -548,7 +549,7 @@ contract LiquidDelegationTest is Test { 1, // numberOfDelegations 0, // rewardsAccruedAfterEach taxedRewardsAfterStaking, // rewardsBeforeUnstaking - WithdrawalQueue.UNBONDING_PERIOD, // after unstaking wait blocksUntil claiming + WithdrawalQueue.unbondingPeriod(), // after unstaking wait blocksUntil claiming true // initialDeposit using funds held by the node, otherwise delegated by a staker ); } @@ -571,7 +572,7 @@ contract LiquidDelegationTest is Test { 1, // numberOfDelegations 0, // rewardsAccruedAfterEach taxedRewardsAfterStaking + 365 * 24 * 51_000 ether * depositAmount / totalDeposit, // rewardsBeforeUnstaking - WithdrawalQueue.UNBONDING_PERIOD, // after unstaking wait blocksUntil claiming + WithdrawalQueue.unbondingPeriod(), // after unstaking wait blocksUntil claiming true // initialDeposit using funds held by the node, otherwise delegated by a staker ); } @@ -594,7 +595,7 @@ contract LiquidDelegationTest is Test { 1, // numberOfDelegations 0, // rewardsAccruedAfterEach taxedRewardsAfterStaking + 365 * 24 * 51_000 ether * depositAmount / totalDeposit, // rewardsBeforeUnstaking - WithdrawalQueue.UNBONDING_PERIOD, // after unstaking wait blocksUntil claiming + WithdrawalQueue.unbondingPeriod(), // after unstaking wait blocksUntil claiming true // initialDeposit using funds held by the node, otherwise delegated by a staker ); } @@ -617,7 +618,7 @@ contract LiquidDelegationTest is Test { 1, // numberOfDelegations 0, // rewardsAccruedAfterEach taxedRewardsAfterStaking + 365 * 24 * 51_000 ether * depositAmount / totalDeposit, // rewardsBeforeUnstaking - WithdrawalQueue.UNBONDING_PERIOD, // after unstaking wait blocksUntil claiming + WithdrawalQueue.unbondingPeriod(), // after unstaking wait blocksUntil claiming true // initialDeposit using funds held by the node, otherwise delegated by a staker ); } @@ -640,7 +641,7 @@ contract LiquidDelegationTest is Test { 1, // numberOfDelegations 0, // rewardsAccruedAfterEach taxedRewardsAfterStaking + 365 * 24 * 51_000 ether * depositAmount / totalDeposit, // rewardsBeforeUnstaking - WithdrawalQueue.UNBONDING_PERIOD, // after unstaking wait blocksUntil claiming + WithdrawalQueue.unbondingPeriod(), // after unstaking wait blocksUntil claiming true // initialDeposit using funds held by the node, otherwise delegated by a staker ); } @@ -663,7 +664,7 @@ contract LiquidDelegationTest is Test { 1, // numberOfDelegations 0, // rewardsAccruedAfterEach taxedRewardsAfterStaking + 365 * 24 * 51_000 ether * depositAmount / totalDeposit, // rewardsBeforeUnstaking - WithdrawalQueue.UNBONDING_PERIOD, // after unstaking wait blocksUntil claiming + WithdrawalQueue.unbondingPeriod(), // after unstaking wait blocksUntil claiming true // initialDeposit using funds held by the node, otherwise delegated by a staker ); } @@ -686,7 +687,7 @@ contract LiquidDelegationTest is Test { 1, // numberOfDelegations 0, // rewardsAccruedAfterEach taxedRewardsAfterStaking + 365 * 24 * 51_000 ether * depositAmount / totalDeposit, // rewardsBeforeUnstaking - WithdrawalQueue.UNBONDING_PERIOD, // after unstaking wait blocksUntil claiming + WithdrawalQueue.unbondingPeriod(), // after unstaking wait blocksUntil claiming false // initialDeposit using funds held by the node, otherwise delegated by a staker ); } @@ -709,7 +710,7 @@ contract LiquidDelegationTest is Test { 1, // numberOfDelegations 0, // rewardsAccruedAfterEach taxedRewardsAfterStaking + 365 * 24 * 51_000 ether * depositAmount / totalDeposit, // rewardsBeforeUnstaking - WithdrawalQueue.UNBONDING_PERIOD, // after unstaking wait blocksUntil claiming + WithdrawalQueue.unbondingPeriod(), // after unstaking wait blocksUntil claiming false // initialDeposit using funds held by the node, otherwise delegated by a staker ); } @@ -732,7 +733,7 @@ contract LiquidDelegationTest is Test { 1, // numberOfDelegations 0, // rewardsAccruedAfterEach taxedRewardsAfterStaking + 365 * 24 * 51_000 ether * depositAmount / totalDeposit, // rewardsBeforeUnstaking - WithdrawalQueue.UNBONDING_PERIOD, // after unstaking wait blocksUntil claiming + WithdrawalQueue.unbondingPeriod(), // after unstaking wait blocksUntil claiming false // initialDeposit using funds held by the node, otherwise delegated by a staker ); } @@ -755,7 +756,7 @@ contract LiquidDelegationTest is Test { 1, // numberOfDelegations 0, // rewardsAccruedAfterEach taxedRewardsAfterStaking + 365 * 24 * 51_000 ether * depositAmount / totalDeposit, // rewardsBeforeUnstaking - WithdrawalQueue.UNBONDING_PERIOD, // after unstaking wait blocksUntil claiming + WithdrawalQueue.unbondingPeriod(), // after unstaking wait blocksUntil claiming true // initialDeposit using funds held by the node, otherwise delegated by a staker ); } @@ -778,7 +779,7 @@ contract LiquidDelegationTest is Test { 1, // numberOfDelegations 0, // rewardsAccruedAfterEach taxedRewardsAfterStaking + 365 * 24 * 51_000 ether * depositAmount / totalDeposit, // rewardsBeforeUnstaking - WithdrawalQueue.UNBONDING_PERIOD, // after unstaking wait blocksUntil claiming + WithdrawalQueue.unbondingPeriod(), // after unstaking wait blocksUntil claiming false // initialDeposit using funds held by the node, otherwise delegated by a staker ); } @@ -801,7 +802,7 @@ contract LiquidDelegationTest is Test { 1, // numberOfDelegations 0, // rewardsAccruedAfterEach taxedRewardsAfterStaking + 365 * 24 * 51_000 ether * depositAmount / totalDeposit, // rewardsBeforeUnstaking - WithdrawalQueue.UNBONDING_PERIOD, // after unstaking wait blocksUntil claiming + WithdrawalQueue.unbondingPeriod(), // after unstaking wait blocksUntil claiming false // initialDeposit using funds held by the node, otherwise delegated by a staker ); } @@ -824,7 +825,7 @@ contract LiquidDelegationTest is Test { 1, // numberOfDelegations 0, // rewardsAccruedAfterEach taxedRewardsAfterStaking + 365 * 24 * 51_000 ether * depositAmount / totalDeposit, // rewardsBeforeUnstaking - WithdrawalQueue.UNBONDING_PERIOD, // after unstaking wait blocksUntil claiming + WithdrawalQueue.unbondingPeriod(), // after unstaking wait blocksUntil claiming false // initialDeposit using funds held by the node, otherwise delegated by a staker ); } @@ -849,7 +850,7 @@ contract LiquidDelegationTest is Test { // (numberOfDelegations - 1) * rewardsAccruedAfterEach <= rewardsBeforeUnstaking 5 * 51_000 ether / uint256(3600) * depositAmount / totalDeposit, // rewardsAccruedAfterEach taxedRewardsAfterStaking + 51_000 ether / uint256(60) * depositAmount / totalDeposit, // rewardsBeforeUnstaking - WithdrawalQueue.UNBONDING_PERIOD, // after unstaking wait blocksUntil claiming + WithdrawalQueue.unbondingPeriod(), // after unstaking wait blocksUntil claiming true // initialDeposit using funds held by the node, otherwise delegated by a staker ); } @@ -872,7 +873,7 @@ contract LiquidDelegationTest is Test { 1, // numberOfDelegations 0, // rewardsAccruedAfterEach taxedRewardsAfterStaking + 51_000 ether / uint256(60) * depositAmount / totalDeposit, // rewardsBeforeUnstaking - WithdrawalQueue.UNBONDING_PERIOD, // after unstaking wait blocksUntil claiming + WithdrawalQueue.unbondingPeriod(), // after unstaking wait blocksUntil claiming true // initialDeposit using funds held by the node, otherwise delegated by a staker ); } @@ -920,7 +921,7 @@ contract LiquidDelegationTest is Test { 1, // numberOfDelegations 0, // rewardsAccruedAfterEach rewardsBeforeUnstaking, - WithdrawalQueue.UNBONDING_PERIOD, // blocksUntil claiming + WithdrawalQueue.unbondingPeriod(), // blocksUntil claiming true // initialDeposit ); // Replace the values below in the same order with the values output by the STATE script diff --git a/test/NonLiquidDelegation.t.sol b/test/NonLiquidDelegation.t.sol index dab9614..ee0a185 100644 --- a/test/NonLiquidDelegation.t.sol +++ b/test/NonLiquidDelegation.t.sol @@ -24,6 +24,7 @@ contract NonLiquidDelegationTest is Test { ]; function setUp() public { + vm.chainId(33469); uint256 deployerPrivateKey = vm.envUint("PRIVATE_KEY"); owner = vm.addr(deployerPrivateKey); //console.log("Signer is %s", owner); @@ -338,6 +339,31 @@ contract NonLiquidDelegationTest is Test { //Console.log("staker balance: %s.%s%s", staker[i-1].balance); vm.stopPrank(); } + + // if we try to withdraw again immediately (in the same block), + // the amount withdrawn must equal zero + //* + for (uint256 i = 1; i <= staker.length; i++) { + vm.startPrank(staker[i-1]); + if (steps == 123_456_789) + snapshot("staker %s withdrawing all, remaining rewards:", i, 0); + else + snapshot("staker %s withdrawing 1+%s times", i, steps); + Console.log("rewards accrued until last staking: %s.%s%s", delegation.getTotalRewards()); + Console.log("delegation contract balance: %s.%s%s", address(delegation).balance); + //Console.log("staker balance: %s.%s%s", staker[i-1].balance); + Console.log("staker rewards: %s.%s%s", delegation.rewards()); + if (steps == 123_456_789) + Console.log("staker withdrew: %s.%s%s", delegation.withdrawAllRewards()); + else + //TODO: add a test that withdraws a fixed amount < delegation.rewards(step) + Console.log("staker withdrew: %s.%s%s", delegation.withdrawRewards(delegation.rewards(steps), steps)); + Console.log("rewards accrued until last staking: %s.%s%s", delegation.getTotalRewards()); + Console.log("delegation contract balance: %s.%s%s", address(delegation).balance); + //Console.log("staker balance: %s.%s%s", staker[i-1].balance); + vm.stopPrank(); + } + //*/ } function test_withdrawAllRewards_OwnDeposit() public { @@ -568,9 +594,9 @@ contract NonLiquidDelegationTest is Test { //Console.log("staker balance: %s.%s%s", staker[i-1].balance); vm.stopPrank(); - vm.roll(block.number + WithdrawalQueue.UNBONDING_PERIOD); + vm.roll(block.number + WithdrawalQueue.unbondingPeriod()); //TODO: remove the next line once https://github.com/Zilliqa/zq2/issues/1761 is fixed - vm.warp(block.timestamp + WithdrawalQueue.UNBONDING_PERIOD); + vm.warp(block.timestamp + WithdrawalQueue.unbondingPeriod()); i = 1; @@ -592,4 +618,108 @@ contract NonLiquidDelegationTest is Test { vm.stopPrank(); } + function test_rewardsAfterWithdrawalLessThanBeforeWithdrawal() public { + uint256 i; + uint256 x; + + // otherwise snapshot() doesn't find the staker and reverts + staker[0] = owner; + deposit(10_000_000 ether, true); + + // wait 2 epochs for the change to the deposit to take affect + vm.roll(block.number + Deposit(delegation.DEPOSIT_CONTRACT()).blocksPerEpoch() * 2); + + for (i = 0; i < 4; i++) { + vm.deal(staker[i], 100_000 ether); + } + + delegation = NonLiquidDelegationV2(proxy); + + // rewards accrued so far + vm.deal(address(delegation), 50_000 ether); + x = 50; + i = 2; + vm.startPrank(staker[i-1]); + vm.recordLogs(); + vm.expectEmit( + true, + false, + false, + false, + address(delegation) + ); + emit Delegation.Staked( + staker[i-1], + x * 1 ether, + "" + ); + delegation.stake{value: x * 1 ether}(); + + // wait 2 epochs for the change to the deposit to take affect + vm.roll(block.number + Deposit(delegation.DEPOSIT_CONTRACT()).blocksPerEpoch() * 2); + vm.stopPrank(); + + vm.deal(address(delegation), address(delegation).balance + 10_000 ether); + vm.startPrank(staker[i-1]); + snapshot("staker %s withdrawing all, remaining rewards:", i, 0); + console.log("-----------------------------------------------"); + + Console.log("contract balance: %s.%s%s", address(delegation).balance); + Console.log("staker balance: %s.%s%s", staker[i-1].balance); + uint256 rewards = delegation.rewards(); + Console.log("staker rewards: %s.%s%s", rewards); + + ( + uint64[] memory stakingIndices, + uint64 firstStakingIndex, + uint256 allWithdrawnRewards, + uint64 lastWithdrawnRewardIndex + ) = delegation.getStakingData(); + Console.log("stakingIndices = [ %s]", stakingIndices); + console.log("firstStakingIndex = %s allWithdrawnRewards = %s lastWithdrawnRewardIndex = %s", uint(firstStakingIndex), allWithdrawnRewards, uint(lastWithdrawnRewardIndex)); + + vm.recordLogs(); + vm.expectEmit( + true, + true, + true, + true, + address(delegation) + ); + emit NonLiquidDelegationV2.RewardPaid( + staker[i-1], + rewards + ); + rewards = delegation.withdrawAllRewards(); + + ( + stakingIndices, + firstStakingIndex, + allWithdrawnRewards, + lastWithdrawnRewardIndex + ) = delegation.getStakingData(); + Console.log("stakingIndices = [ %s]", stakingIndices); + console.log("firstStakingIndex = %s allWithdrawnRewards = %s lastWithdrawnRewardIndex = %s", uint(firstStakingIndex), allWithdrawnRewards, uint(lastWithdrawnRewardIndex)); + + Console.log("contract balance: %s.%s%s", address(delegation).balance); + Console.log("staker balance: %s.%s%s", staker[i-1].balance); + Console.log("staker rewards: %s.%s%s", delegation.rewards()); + Console.log("staker should have received: %s.%s%s", rewards); + vm.stopPrank(); + } + + function test_withdrawAllRewardsThenNoMoreStakings_DelegatedDeposit() public { + run( + abi.encode([uint256(0x20), 5, 1, 2, 3, 1, 2]), //bytes -> uint256[] memory stakerIndicesBeforeWithdrawals, + abi.encode([int256(0x20), 5, 50, 50, 25, 35, -35]), //bytes -> int256[] memory relativeAmountsBeforeWithdrawals, + abi.encode([uint256(0x20), 0]), //bytes -> uint256[] memory stakerIndicesAfterWithdrawals, + abi.encode([int256(0x20), 0]), //bytes -> int256[] memory relativeAmountsAfterWithdrawals, + 123_456_789, //uint256 withdrawalInSteps, + 10_000_000 ether, //uint256 depositAmount, + 50_000 ether, //uint256 rewardsBeforeStaking, + 10_000 ether, //uint256 rewardsAccruedAfterEach, + false //bool initialDeposit + ); + } + } \ No newline at end of file diff --git a/unstake.sh b/unstake.sh index 95cbb35..13f0513 100755 --- a/unstake.sh +++ b/unstake.sh @@ -1,7 +1,7 @@ #!/bin/bash if [ $# -lt 2 ]; then - echo "Provide the delegation contract address, a staker private key and optionally the number of shares as arguments." + echo "Provide the delegation contract address, a staker private key and optionally how much to unstake as arguments." exit 1 fi