From 7c7d803c8274fb5bec63bd50d8545c0d1df0c0d8 Mon Sep 17 00:00:00 2001 From: Denis Ermolin Date: Thu, 22 Apr 2021 19:14:00 +0700 Subject: [PATCH 01/12] chg: combine status and deactiovationEpoch --- contracts/root/predicates/ERC20Predicate.sol | 4 +- contracts/root/predicates/IPredicate.sol | 4 +- .../staking/slashing/SlashingManager.sol | 2 +- .../staking/stakeManager/StakeManager.sol | 67 +++++++++++-------- .../stakeManager/StakeManagerExtension.sol | 6 +- .../stakeManager/StakeManagerStorage.sol | 14 +++- 6 files changed, 57 insertions(+), 40 deletions(-) diff --git a/contracts/root/predicates/ERC20Predicate.sol b/contracts/root/predicates/ERC20Predicate.sol index c1fd8cffa..fc68b38b7 100644 --- a/contracts/root/predicates/ERC20Predicate.sol +++ b/contracts/root/predicates/ERC20Predicate.sol @@ -9,9 +9,7 @@ import {SafeMath} from "openzeppelin-solidity/contracts/math/SafeMath.sol"; import {IErcPredicate} from "./IPredicate.sol"; import {Registry} from "../../common/Registry.sol"; -import { - WithdrawManagerHeader -} from "../withdrawManager/WithdrawManagerStorage.sol"; +import {WithdrawManagerHeader} from "../withdrawManager/WithdrawManagerStorage.sol"; contract ERC20Predicate is IErcPredicate { using RLPReader for bytes; diff --git a/contracts/root/predicates/IPredicate.sol b/contracts/root/predicates/IPredicate.sol index e84e2a1c7..9b50f8f66 100644 --- a/contracts/root/predicates/IPredicate.sol +++ b/contracts/root/predicates/IPredicate.sol @@ -7,9 +7,7 @@ import {RLPEncode} from "../../common/lib/RLPEncode.sol"; import {IWithdrawManager} from "../withdrawManager/IWithdrawManager.sol"; import {IDepositManager} from "../depositManager/IDepositManager.sol"; -import { - ExitsDataStructure -} from "../withdrawManager/WithdrawManagerStorage.sol"; +import {ExitsDataStructure} from "../withdrawManager/WithdrawManagerStorage.sol"; import {ChainIdMixin} from "../../common/mixin/ChainIdMixin.sol"; interface IPredicate { diff --git a/contracts/staking/slashing/SlashingManager.sol b/contracts/staking/slashing/SlashingManager.sol index a76428cab..fefc735ad 100644 --- a/contracts/staking/slashing/SlashingManager.sol +++ b/contracts/staking/slashing/SlashingManager.sol @@ -105,7 +105,7 @@ contract SlashingManager is ISlashingManager, Ownable { lastAdd = signer; uint256 amount; uint256 delegatedAmount; - (amount,,,,,,,,,,,delegatedAmount,) = stakeManager.validators(validatorId); + (amount,,,,,,,,,,,delegatedAmount,,) = stakeManager.validators(validatorId); // add delegation power amount = amount.add(delegatedAmount); diff --git a/contracts/staking/stakeManager/StakeManager.sol b/contracts/staking/stakeManager/StakeManager.sol index f11909521..79569d609 100644 --- a/contracts/staking/stakeManager/StakeManager.sol +++ b/contracts/staking/stakeManager/StakeManager.sol @@ -44,7 +44,7 @@ contract StakeManager is } struct UnstakedValidatorsContext { - uint256 deactivationEpoch; + uint256 signedStakePower; uint256[] deactivatedValidators; uint256 validatorIndex; } @@ -158,7 +158,8 @@ contract StakeManager is function validatorReward(uint256 validatorId) public view returns (uint256) { uint256 _validatorReward; - if (validators[validatorId].deactivationEpoch == 0) { + (, uint256 deactivationEpoch) = _readStatus(validatorId); + if (deactivationEpoch == 0) { (_validatorReward, ) = _evaluateValidatorAndDelegationReward(validatorId); } return validators[validatorId].reward.add(_validatorReward).sub(INITIALIZED_AMOUNT); @@ -177,11 +178,12 @@ contract StakeManager is } function isValidator(uint256 validatorId) public view returns (bool) { + (Status status, uint256 deactivationEpoch) = _readStatus(validatorId); return _isValidator( - validators[validatorId].status, + status, validators[validatorId].amount, - validators[validatorId].deactivationEpoch, + deactivationEpoch, currentEpoch ); } @@ -411,10 +413,10 @@ contract StakeManager is function unstake(uint256 validatorId) external onlyStaker(validatorId) { require(validatorAuction[validatorId].amount == 0); - Status status = validators[validatorId].status; + (Status status, uint256 deactivationEpoch) = _readStatus(validatorId); require( validators[validatorId].activationEpoch > 0 && - validators[validatorId].deactivationEpoch == 0 && + deactivationEpoch == 0 && (status == Status.Active || status == Status.Locked) ); @@ -457,12 +459,11 @@ contract StakeManager is } function unstakeClaim(uint256 validatorId) public onlyStaker(validatorId) { - uint256 deactivationEpoch = validators[validatorId].deactivationEpoch; + (Status status, uint256 deactivationEpoch) = _readStatus(validatorId); // can only claim stake back after WITHDRAWAL_DELAY require( deactivationEpoch > 0 && - deactivationEpoch.add(WITHDRAWAL_DELAY) <= currentEpoch && - validators[validatorId].status != Status.Unstaked + deactivationEpoch.add(WITHDRAWAL_DELAY) <= currentEpoch && status != Status.Unstaked ); uint256 amount = validators[validatorId].amount; @@ -479,7 +480,7 @@ contract StakeManager is validators[validatorId].signer = address(0); signerToValidator[validators[validatorId].signer] = INCORRECT_VALIDATOR_ID; - validators[validatorId].status = Status.Unstaked; + _writeStatus(validatorId, Status.Unstaked, deactivationEpoch); _transferToken(msg.sender, amount); logger.logUnstaked(msg.sender, validatorId, amount, newTotalStaked); @@ -490,7 +491,8 @@ contract StakeManager is uint256 amount, bool stakeRewards ) public onlyWhenUnlocked onlyStaker(validatorId) { - require(validators[validatorId].deactivationEpoch == 0, "No restaking"); + (, uint256 deactivationEpoch) = _readStatus(validatorId); + require(deactivationEpoch == 0, "No restaking"); if (amount > 0) { _transferTokenFrom(msg.sender, address(this), amount); @@ -577,7 +579,6 @@ contract StakeManager is uint256[3][] calldata sigs ) external onlyRootChain returns (uint256) { uint256 _currentEpoch = currentEpoch; - uint256 signedStakePower; address lastAdd; uint256 totalStakers = validatorState.stakerCount; @@ -605,15 +606,14 @@ contract StakeManager is uint256 validatorId = signerToValidator[signer]; uint256 amount = validators[validatorId].amount; - Status status = validators[validatorId].status; - unstakeCtx.deactivationEpoch = validators[validatorId].deactivationEpoch; + (Status status, uint256 deactivationEpoch) = _readStatus(validatorId); - if (_isValidator(status, amount, unstakeCtx.deactivationEpoch, _currentEpoch)) { + if (_isValidator(status, amount, deactivationEpoch, _currentEpoch)) { lastAdd = signer; - signedStakePower = signedStakePower.add(validators[validatorId].delegatedAmount).add(amount); + unstakeCtx.signedStakePower = unstakeCtx.signedStakePower.add(validators[validatorId].delegatedAmount).add(amount); - if (unstakeCtx.deactivationEpoch != 0) { + if (deactivationEpoch != 0) { // this validator not a part of signers list anymore unstakeCtx.deactivatedValidators[unstakeCtx.validatorIndex] = validatorId; unstakeCtx.validatorIndex++; @@ -636,7 +636,7 @@ contract StakeManager is _increaseRewardAndAssertConsensus( blockInterval, proposer, - signedStakePower, + unstakeCtx.signedStakePower, stateRoot, unsignedCtx.unsignedValidators, unsignedCtx.unsignedValidatorIndex, @@ -713,8 +713,10 @@ contract StakeManager is } function unjail(uint256 validatorId) public onlyStaker(validatorId) { - require(validators[validatorId].status == Status.Locked, "Not jailed"); - require(validators[validatorId].deactivationEpoch == 0, "Already unstaking"); + (Status status, uint256 deactivationEpoch) = _readStatus(validatorId); + + require(status == Status.Locked, "Not jailed"); + require(deactivationEpoch == 0, "Already unstaking"); uint256 _currentEpoch = currentEpoch; require(validators[validatorId].jailTime <= _currentEpoch, "Incomplete jail period"); @@ -730,7 +732,7 @@ contract StakeManager is // undo timeline so that validator is normal validator updateTimeline(int256(amount.add(validators[validatorId].delegatedAmount)), 1, 0); - validators[validatorId].status = Status.Active; + _writeStatus(validatorId, Status.Active, deactivationEpoch); address signer = validators[validatorId].signer; logger.logUnjailed(validatorId, signer); @@ -762,11 +764,12 @@ contract StakeManager is function updateValidatorDelegation(bool delegation) external { uint256 validatorId = signerToValidator[msg.sender]; + (Status status, uint256 deactivationEpoch) = _readStatus(validatorId); require( _isValidator( - validators[validatorId].status, + status, validators[validatorId].amount, - validators[validatorId].deactivationEpoch, + deactivationEpoch, currentEpoch ), "not validator" @@ -1047,7 +1050,9 @@ contract StakeManager is uint256 _currentEpoch = currentEpoch; validators[validatorId].jailTime = _currentEpoch.add(jailCheckpoints); - validators[validatorId].status = Status.Locked; + + (, uint256 deactivationEpoch) = _readStatus(validatorId); + _writeStatus(validatorId, Status.Locked, deactivationEpoch); logger.logJailed(validatorId, _currentEpoch, validators[validatorId].signer); return validators[validatorId].amount.add(validators[validatorId].delegatedAmount); } @@ -1065,25 +1070,28 @@ contract StakeManager is uint256 newTotalStaked = totalStaked.add(amount); totalStaked = newTotalStaked; - + // TODO initialize only required fields validators[validatorId] = Validator({ reward: INITIALIZED_AMOUNT, amount: amount, activationEpoch: _currentEpoch, - deactivationEpoch: 0, + deactivationEpoch_deprecated: 0, jailTime: 0, signer: signer, contractAddress: acceptDelegation ? validatorShareFactory.create(validatorId, address(_logger), registry) : address(0x0), - status: Status.Active, + status_deprecated: Status.Active, commissionRate: 0, lastCommissionUpdate: 0, delegatorsReward: INITIALIZED_AMOUNT, delegatedAmount: 0, - initialRewardPerStake: rewardPerStake + initialRewardPerStake: rewardPerStake, + status: 0 }); + _writeStatus(validatorId, Status.Active, 0); + latestSignerUpdateEpoch[validatorId] = _currentEpoch; NFTContract.mint(user, validatorId); @@ -1107,7 +1115,8 @@ contract StakeManager is uint256 amount = validators[validatorId].amount; address validator = ownerOf(validatorId); - validators[validatorId].deactivationEpoch = exitEpoch; + (Status status, ) = _readStatus(validatorId); + _writeStatus(validatorId, status, exitEpoch); // unbond all delegators in future int256 delegationAmount = int256(validators[validatorId].delegatedAmount); diff --git a/contracts/staking/stakeManager/StakeManagerExtension.sol b/contracts/staking/stakeManager/StakeManagerExtension.sol index c74dbcca4..34a7c917d 100644 --- a/contracts/staking/stakeManager/StakeManagerExtension.sol +++ b/contracts/staking/stakeManager/StakeManagerExtension.sol @@ -25,8 +25,9 @@ contract StakeManagerExtension is StakeManagerStorage, Initializable, StakeManag ) external { uint256 currentValidatorAmount = validators[validatorId].amount; + (, uint256 deactivationEpoch) = _readStatus(validatorId); require( - validators[validatorId].deactivationEpoch == 0 && currentValidatorAmount != 0, + deactivationEpoch == 0 && currentValidatorAmount != 0, "Invalid validator for an auction" ); uint256 senderValidatorId = signerToValidator[msg.sender]; @@ -104,8 +105,9 @@ contract StakeManagerExtension is StakeManagerStorage, Initializable, StakeManag perceivedStake = perceivedStake.add(validators[validatorId].delegatedAmount); + (, uint256 deactivationEpoch) = _readStatus(validatorId); // validator is last auctioner - if (perceivedStake >= auctionAmount && validators[validatorId].deactivationEpoch == 0) { + if (perceivedStake >= auctionAmount && deactivationEpoch == 0) { require(token.transfer(auctionUser, auctionAmount), "Bid return failed"); //cleanup auction data auction.startEpoch = _currentEpoch; diff --git a/contracts/staking/stakeManager/StakeManagerStorage.sol b/contracts/staking/stakeManager/StakeManagerStorage.sol index 6b448a838..ed221812b 100644 --- a/contracts/staking/stakeManager/StakeManagerStorage.sol +++ b/contracts/staking/stakeManager/StakeManagerStorage.sol @@ -34,16 +34,17 @@ contract StakeManagerStorage is GovernanceLockable, RootChainable { uint256 amount; uint256 reward; uint256 activationEpoch; - uint256 deactivationEpoch; + uint256 deactivationEpoch_deprecated; uint256 jailTime; address signer; address contractAddress; - Status status; + Status status_deprecated; uint256 commissionRate; uint256 lastCommissionUpdate; uint256 delegatorsReward; uint256 delegatedAmount; uint256 initialRewardPerStake; + uint256 status; } uint256 constant MAX_COMMISION_RATE = 100; @@ -93,4 +94,13 @@ contract StakeManagerStorage is GovernanceLockable, RootChainable { mapping(uint256 => uint256) public latestSignerUpdateEpoch; uint256 public totalHeimdallFee; + + function _readStatus(uint256 validatorId) internal view returns(Status status, uint256 deactivationEpoch) { + uint256 combinedStatus = validators[validatorId].status; + return (Status(combinedStatus >> 240), uint256(uint240(combinedStatus))); + } + + function _writeStatus(uint256 validatorId, Status status, uint256 deactivationEpoch) internal { + validators[validatorId].status = (uint256(status) << 240) | deactivationEpoch; + } } From 886082a108257a677f0a9682f8e645d9ba064844 Mon Sep 17 00:00:00 2001 From: Denis Ermolin Date: Fri, 23 Apr 2021 13:11:16 +0700 Subject: [PATCH 02/12] chg: eliminate few more storage reads in normal circumstances --- .../staking/slashing/SlashingManager.sol | 2 +- .../staking/stakeManager/StakeManager.sol | 109 +++++++++--------- .../stakeManager/StakeManagerExtension.sol | 45 ++++---- .../stakeManager/StakeManagerStorage.sol | 16 +-- .../StakeManagerStorageExtension.sol | 18 +++ .../staking/validatorShare/ValidatorShare.sol | 36 +++--- 6 files changed, 121 insertions(+), 105 deletions(-) diff --git a/contracts/staking/slashing/SlashingManager.sol b/contracts/staking/slashing/SlashingManager.sol index fefc735ad..a76428cab 100644 --- a/contracts/staking/slashing/SlashingManager.sol +++ b/contracts/staking/slashing/SlashingManager.sol @@ -105,7 +105,7 @@ contract SlashingManager is ISlashingManager, Ownable { lastAdd = signer; uint256 amount; uint256 delegatedAmount; - (amount,,,,,,,,,,,delegatedAmount,,) = stakeManager.validators(validatorId); + (amount,,,,,,,,,,,delegatedAmount,) = stakeManager.validators(validatorId); // add delegation power amount = amount.add(delegatedAmount); diff --git a/contracts/staking/stakeManager/StakeManager.sol b/contracts/staking/stakeManager/StakeManager.sol index 79569d609..d48ebb423 100644 --- a/contracts/staking/stakeManager/StakeManager.sol +++ b/contracts/staking/stakeManager/StakeManager.sol @@ -148,7 +148,7 @@ contract StakeManager is } function delegatedAmount(uint256 validatorId) public view returns (uint256) { - return validators[validatorId].delegatedAmount; + return signerState[validators[validatorId].signer].totalAmount.sub(validators[validatorId].amount); } function delegatorsReward(uint256 validatorId) public view returns (uint256) { @@ -158,7 +158,7 @@ contract StakeManager is function validatorReward(uint256 validatorId) public view returns (uint256) { uint256 _validatorReward; - (, uint256 deactivationEpoch) = _readStatus(validatorId); + (, uint256 deactivationEpoch) = _readStatus(validators[validatorId].signer); if (deactivationEpoch == 0) { (_validatorReward, ) = _evaluateValidatorAndDelegationReward(validatorId); } @@ -178,11 +178,10 @@ contract StakeManager is } function isValidator(uint256 validatorId) public view returns (bool) { - (Status status, uint256 deactivationEpoch) = _readStatus(validatorId); + (Status status, uint256 deactivationEpoch) = _readStatus(validators[validatorId].signer); return _isValidator( status, - validators[validatorId].amount, deactivationEpoch, currentEpoch ); @@ -413,7 +412,7 @@ contract StakeManager is function unstake(uint256 validatorId) external onlyStaker(validatorId) { require(validatorAuction[validatorId].amount == 0); - (Status status, uint256 deactivationEpoch) = _readStatus(validatorId); + (Status status, uint256 deactivationEpoch) = _readStatus(validators[validatorId].signer); require( validators[validatorId].activationEpoch > 0 && deactivationEpoch == 0 && @@ -459,7 +458,9 @@ contract StakeManager is } function unstakeClaim(uint256 validatorId) public onlyStaker(validatorId) { - (Status status, uint256 deactivationEpoch) = _readStatus(validatorId); + address signer = validators[validatorId].signer; + + (Status status, uint256 deactivationEpoch) = _readStatus(signer); // can only claim stake back after WITHDRAWAL_DELAY require( deactivationEpoch > 0 && @@ -479,8 +480,9 @@ contract StakeManager is validators[validatorId].jailTime = 0; validators[validatorId].signer = address(0); - signerToValidator[validators[validatorId].signer] = INCORRECT_VALIDATOR_ID; - _writeStatus(validatorId, Status.Unstaked, deactivationEpoch); + signerToValidator[signer] = INCORRECT_VALIDATOR_ID; + _writeStatus(signer, Status.Unstaked, deactivationEpoch); + signerState[signer].totalAmount = signerState[signer].totalAmount.sub(amount); _transferToken(msg.sender, amount); logger.logUnstaked(msg.sender, validatorId, amount, newTotalStaked); @@ -491,7 +493,7 @@ contract StakeManager is uint256 amount, bool stakeRewards ) public onlyWhenUnlocked onlyStaker(validatorId) { - (, uint256 deactivationEpoch) = _readStatus(validatorId); + (, uint256 deactivationEpoch) = _readStatus(validators[validatorId].signer); require(deactivationEpoch == 0, "No restaking"); if (amount > 0) { @@ -546,11 +548,13 @@ contract StakeManager is } function increaseValidatorDelegatedAmount(uint256 validatorId, uint256 amount) private { - validators[validatorId].delegatedAmount = validators[validatorId].delegatedAmount.add(amount); + address signer = validators[validatorId].signer; + signerState[signer].totalAmount = signerState[signer].totalAmount.add(amount); } function decreaseValidatorDelegatedAmount(uint256 validatorId, uint256 amount) public onlyDelegation(validatorId) { - validators[validatorId].delegatedAmount = validators[validatorId].delegatedAmount.sub(amount); + address signer = validators[validatorId].signer; + signerState[signer].totalAmount = signerState[signer].totalAmount.sub(amount); } function updateSigner(uint256 validatorId, bytes memory signerPubkey) public onlyStaker(validatorId) { @@ -565,6 +569,10 @@ contract StakeManager is signerToValidator[currentSigner] = INCORRECT_VALIDATOR_ID; signerToValidator[signer] = validatorId; validators[validatorId].signer = signer; + + signerState[signer] = signerState[currentSigner]; + delete signerState[currentSigner]; + _updateSigner(currentSigner, signer); // reset update time to current time @@ -604,18 +612,16 @@ contract StakeManager is break; } - uint256 validatorId = signerToValidator[signer]; - uint256 amount = validators[validatorId].amount; - (Status status, uint256 deactivationEpoch) = _readStatus(validatorId); + (Status status, uint256 deactivationEpoch) = _readStatus(signer); - if (_isValidator(status, amount, deactivationEpoch, _currentEpoch)) { + if (_isValidator(status, deactivationEpoch, _currentEpoch)) { lastAdd = signer; - unstakeCtx.signedStakePower = unstakeCtx.signedStakePower.add(validators[validatorId].delegatedAmount).add(amount); + unstakeCtx.signedStakePower = unstakeCtx.signedStakePower.add(signerState[signer].totalAmount); if (deactivationEpoch != 0) { // this validator not a part of signers list anymore - unstakeCtx.deactivatedValidators[unstakeCtx.validatorIndex] = validatorId; + unstakeCtx.deactivatedValidators[unstakeCtx.validatorIndex] = signerToValidator[signer]; unstakeCtx.validatorIndex++; } else { unsignedCtx = _fillUnsignedValidators(unsignedCtx, signer); @@ -623,7 +629,7 @@ contract StakeManager is } else if (status == Status.Locked) { // TODO fix double unsignedValidators appearance // make sure that jailed validator doesn't get his rewards too - unsignedCtx.unsignedValidators[unsignedCtx.unsignedValidatorIndex] = validatorId; + unsignedCtx.unsignedValidators[unsignedCtx.unsignedValidatorIndex] = signerToValidator[signer]; unsignedCtx.unsignedValidatorIndex++; unsignedCtx.validatorIndex++; } @@ -689,7 +695,7 @@ contract StakeManager is uint256 delSlashedAmount = IValidatorShare(delegationContract).slash( validators[validatorId].amount, - validators[validatorId].delegatedAmount, + signerState[validators[validatorId].signer].totalAmount, _amount ); _amount = _amount.sub(delSlashedAmount); @@ -713,7 +719,9 @@ contract StakeManager is } function unjail(uint256 validatorId) public onlyStaker(validatorId) { - (Status status, uint256 deactivationEpoch) = _readStatus(validatorId); + address signer = validators[validatorId].signer; + + (Status status, uint256 deactivationEpoch) = _readStatus(signer); require(status == Status.Locked, "Not jailed"); require(deactivationEpoch == 0, "Already unstaking"); @@ -730,11 +738,10 @@ contract StakeManager is } // undo timeline so that validator is normal validator - updateTimeline(int256(amount.add(validators[validatorId].delegatedAmount)), 1, 0); + updateTimeline(int256(signerState[signer].totalAmount), 1, 0); - _writeStatus(validatorId, Status.Active, deactivationEpoch); - - address signer = validators[validatorId].signer; + _writeStatus(signer, Status.Active, deactivationEpoch); + logger.logUnjailed(validatorId, signer); } @@ -762,13 +769,13 @@ contract StakeManager is } } + // TODO should be callable by the validator NFT owner instead function updateValidatorDelegation(bool delegation) external { uint256 validatorId = signerToValidator[msg.sender]; - (Status status, uint256 deactivationEpoch) = _readStatus(validatorId); + (Status status, uint256 deactivationEpoch) = _readStatus(msg.sender); require( _isValidator( status, - validators[validatorId].amount, deactivationEpoch, currentEpoch ), @@ -794,11 +801,10 @@ contract StakeManager is function _isValidator( Status status, - uint256 amount, uint256 deactivationEpoch, uint256 _currentEpoch ) private pure returns (bool) { - return (amount > 0 && (deactivationEpoch == 0 || deactivationEpoch > _currentEpoch) && status == Status.Active); + return (deactivationEpoch == 0 || deactivationEpoch > _currentEpoch) && status == Status.Active; } function _fillUnsignedValidators(UnsignedValidatorsContext memory context, address signer) @@ -930,16 +936,15 @@ contract StakeManager is // attempt to save gas in case if rewards were updated previosuly if (initialRewardPerStake < currentRewardPerStake) { uint256 validatorsStake = validators[validatorId].amount; - uint256 delegatedAmount = validators[validatorId].delegatedAmount; - if (delegatedAmount > 0) { - uint256 combinedStakePower = validatorsStake.add(delegatedAmount); + uint256 totalAmount = signerState[validators[validatorId].signer].totalAmount; + if (totalAmount > validatorsStake) { _increaseValidatorRewardWithDelegation( validatorId, validatorsStake, - delegatedAmount, + totalAmount, _getEligibleValidatorReward( validatorId, - combinedStakePower, + totalAmount, currentRewardPerStake, initialRewardPerStake ) @@ -985,10 +990,9 @@ contract StakeManager is function _increaseValidatorRewardWithDelegation( uint256 validatorId, uint256 validatorsStake, - uint256 delegatedAmount, + uint256 combinedStakePower, uint256 reward ) private { - uint256 combinedStakePower = delegatedAmount.add(validatorsStake); (uint256 validatorReward, uint256 delegatorsReward) = _getValidatorAndDelegationReward(validatorId, validatorsStake, reward, combinedStakePower); @@ -1031,7 +1035,7 @@ contract StakeManager is returns (uint256 validatorReward, uint256 delegatorsReward) { uint256 validatorsStake = validators[validatorId].amount; - uint256 combinedStakePower = validatorsStake.add(validators[validatorId].delegatedAmount); + uint256 combinedStakePower = signerState[validators[validatorId].signer].totalAmount; uint256 eligibleReward = rewardPerStake - validators[validatorId].initialRewardPerStake; return _getValidatorAndDelegationReward( @@ -1050,11 +1054,13 @@ contract StakeManager is uint256 _currentEpoch = currentEpoch; validators[validatorId].jailTime = _currentEpoch.add(jailCheckpoints); + + address signer = validators[validatorId].signer; + (, uint256 deactivationEpoch) = _readStatus(signer); + _writeStatus(signer, Status.Locked, deactivationEpoch); - (, uint256 deactivationEpoch) = _readStatus(validatorId); - _writeStatus(validatorId, Status.Locked, deactivationEpoch); - logger.logJailed(validatorId, _currentEpoch, validators[validatorId].signer); - return validators[validatorId].amount.add(validators[validatorId].delegatedAmount); + logger.logJailed(validatorId, _currentEpoch, signer); + return signerState[signer].totalAmount; } function _stakeFor( @@ -1081,16 +1087,16 @@ contract StakeManager is contractAddress: acceptDelegation ? validatorShareFactory.create(validatorId, address(_logger), registry) : address(0x0), - status_deprecated: Status.Active, + status_deprecated: Status_deprecated.Active, commissionRate: 0, lastCommissionUpdate: 0, delegatorsReward: INITIALIZED_AMOUNT, - delegatedAmount: 0, - initialRewardPerStake: rewardPerStake, - status: 0 + delegatedAmount_deprecated: 0, + initialRewardPerStake: rewardPerStake }); - _writeStatus(validatorId, Status.Active, 0); + _writeStatus(signer, Status.Active, 0); + signerState[signer].totalAmount = amount; latestSignerUpdateEpoch[validatorId] = _currentEpoch; NFTContract.mint(user, validatorId); @@ -1115,23 +1121,22 @@ contract StakeManager is uint256 amount = validators[validatorId].amount; address validator = ownerOf(validatorId); - (Status status, ) = _readStatus(validatorId); - _writeStatus(validatorId, status, exitEpoch); - - // unbond all delegators in future - int256 delegationAmount = int256(validators[validatorId].delegatedAmount); + address signer = validators[validatorId].signer; + (Status status, ) = _readStatus(signer); + _writeStatus(signer, status, exitEpoch); + // disabled delegation address delegationContract = validators[validatorId].contractAddress; if (delegationContract != address(0)) { IValidatorShare(delegationContract).lock(); } - _removeSigner(validators[validatorId].signer); + _removeSigner(signer); _liquidateRewards(validatorId, validator); uint256 targetEpoch = exitEpoch <= currentEpoch ? 0 : exitEpoch; - updateTimeline(-(int256(amount) + delegationAmount), -1, targetEpoch); + updateTimeline(-int256(signerState[signer].totalAmount), -1, targetEpoch); logger.logUnstakeInit(validator, validatorId, exitEpoch, amount); } diff --git a/contracts/staking/stakeManager/StakeManagerExtension.sol b/contracts/staking/stakeManager/StakeManagerExtension.sol index 34a7c917d..fa34b8586 100644 --- a/contracts/staking/stakeManager/StakeManagerExtension.sol +++ b/contracts/staking/stakeManager/StakeManagerExtension.sol @@ -24,13 +24,16 @@ contract StakeManagerExtension is StakeManagerStorage, Initializable, StakeManag bytes calldata _signerPubkey ) external { uint256 currentValidatorAmount = validators[validatorId].amount; + address signer = validators[validatorId].signer; - (, uint256 deactivationEpoch) = _readStatus(validatorId); + // re-use variable, dirty. It's deactivationEpoch + (, uint256 senderValidatorId) = _readStatus(signer); require( - deactivationEpoch == 0 && currentValidatorAmount != 0, + senderValidatorId == 0 && currentValidatorAmount != 0, "Invalid validator for an auction" ); - uint256 senderValidatorId = signerToValidator[msg.sender]; + + senderValidatorId = signerToValidator[msg.sender]; // make sure that signer wasn't used already require( NFTContract.balanceOf(msg.sender) == 0 && // existing validators can't bid @@ -54,8 +57,7 @@ contract StakeManagerExtension is StakeManagerStorage, Initializable, StakeManag "Invalid auction period" ); - uint256 perceivedStake = currentValidatorAmount; - perceivedStake = perceivedStake.add(validators[validatorId].delegatedAmount); + uint256 perceivedStake = signerState[signer].totalAmount; Auction storage auction = validatorAuction[validatorId]; uint256 currentAuctionAmount = auction.amount; @@ -99,13 +101,12 @@ contract StakeManagerExtension is StakeManagerStorage, Initializable, StakeManag ); require(auction.user != address(0x0), "Invalid auction"); + address signer = validators[validatorId].signer; uint256 validatorAmount = validators[validatorId].amount; - uint256 perceivedStake = validatorAmount; + uint256 perceivedStake = signerState[signer].totalAmount; uint256 auctionAmount = auction.amount; - perceivedStake = perceivedStake.add(validators[validatorId].delegatedAmount); - - (, uint256 deactivationEpoch) = _readStatus(validatorId); + (, uint256 deactivationEpoch) = _readStatus(signer); // validator is last auctioner if (perceivedStake >= auctionAmount && deactivationEpoch == 0) { require(token.transfer(auctionUser, auctionAmount), "Bid return failed"); @@ -128,19 +129,19 @@ contract StakeManagerExtension is StakeManagerStorage, Initializable, StakeManag } function migrateValidatorsData(uint256 validatorIdFrom, uint256 validatorIdTo) external { - for (uint256 i = validatorIdFrom; i < validatorIdTo; ++i) { - ValidatorShare contractAddress = ValidatorShare(validators[i].contractAddress); - if (contractAddress != ValidatorShare(0)) { - // move validator rewards out from ValidatorShare contract - validators[i].reward = contractAddress.validatorRewards_deprecated().add(INITIALIZED_AMOUNT); - validators[i].delegatedAmount = contractAddress.activeAmount(); - validators[i].commissionRate = contractAddress.commissionRate_deprecated(); - } else { - validators[i].reward = validators[i].reward.add(INITIALIZED_AMOUNT); - } - - validators[i].delegatorsReward = INITIALIZED_AMOUNT; - } + // for (uint256 i = validatorIdFrom; i < validatorIdTo; ++i) { + // ValidatorShare contractAddress = ValidatorShare(validators[i].contractAddress); + // if (contractAddress != ValidatorShare(0)) { + // // move validator rewards out from ValidatorShare contract + // validators[i].reward = contractAddress.validatorRewards_deprecated().add(INITIALIZED_AMOUNT); + // validators[i].delegatedAmount = contractAddress.activeAmount(); + // validators[i].commissionRate = contractAddress.commissionRate_deprecated(); + // } else { + // validators[i].reward = validators[i].reward.add(INITIALIZED_AMOUNT); + // } + + // validators[i].delegatorsReward = INITIALIZED_AMOUNT; + // } } function updateCheckpointRewardParams( diff --git a/contracts/staking/stakeManager/StakeManagerStorage.sol b/contracts/staking/stakeManager/StakeManagerStorage.sol index ed221812b..ede1b8383 100644 --- a/contracts/staking/stakeManager/StakeManagerStorage.sol +++ b/contracts/staking/stakeManager/StakeManagerStorage.sol @@ -10,7 +10,7 @@ import {StakingNFT} from "./StakingNFT.sol"; import {ValidatorShareFactory} from "../validatorShare/ValidatorShareFactory.sol"; contract StakeManagerStorage is GovernanceLockable, RootChainable { - enum Status {Inactive, Active, Locked, Unstaked} + enum Status_deprecated {Inactive, Active, Locked, Unstaked} struct Auction { uint256 amount; @@ -38,13 +38,12 @@ contract StakeManagerStorage is GovernanceLockable, RootChainable { uint256 jailTime; address signer; address contractAddress; - Status status_deprecated; + Status_deprecated status_deprecated; uint256 commissionRate; uint256 lastCommissionUpdate; uint256 delegatorsReward; - uint256 delegatedAmount; + uint256 delegatedAmount_deprecated; uint256 initialRewardPerStake; - uint256 status; } uint256 constant MAX_COMMISION_RATE = 100; @@ -94,13 +93,4 @@ contract StakeManagerStorage is GovernanceLockable, RootChainable { mapping(uint256 => uint256) public latestSignerUpdateEpoch; uint256 public totalHeimdallFee; - - function _readStatus(uint256 validatorId) internal view returns(Status status, uint256 deactivationEpoch) { - uint256 combinedStatus = validators[validatorId].status; - return (Status(combinedStatus >> 240), uint256(uint240(combinedStatus))); - } - - function _writeStatus(uint256 validatorId, Status status, uint256 deactivationEpoch) internal { - validators[validatorId].status = (uint256(status) << 240) | deactivationEpoch; - } } diff --git a/contracts/staking/stakeManager/StakeManagerStorageExtension.sol b/contracts/staking/stakeManager/StakeManagerStorageExtension.sol index dca5cd233..5c620c7c9 100644 --- a/contracts/staking/stakeManager/StakeManagerStorageExtension.sol +++ b/contracts/staking/stakeManager/StakeManagerStorageExtension.sol @@ -1,6 +1,13 @@ pragma solidity 0.5.17; contract StakeManagerStorageExtension { + enum Status {Inactive, Active, Locked, Unstaked} + + struct Signer { + uint256 status; + uint256 totalAmount; + } + address public eventsHub; uint256 public rewardPerStake; address public extensionCode; @@ -14,4 +21,15 @@ contract StakeManagerStorageExtension { uint256 public maxRewardedCheckpoints; // increase / decrease value for faster or slower checkpoints, 0 - 100% uint256 public checkpointRewardDelta; + + mapping(address => Signer) signerState; // TODO change to signer => Signer mapping + + function _readStatus(address signer) internal view returns(Status status, uint256 deactivationEpoch) { + uint256 combinedStatus = signerState[signer].status; + return (Status(combinedStatus >> 240), uint256(uint240(combinedStatus))); + } + + function _writeStatus(address signer, Status status, uint256 deactivationEpoch) internal { + signerState[signer].status = (uint256(status) << 240) | deactivationEpoch; + } } diff --git a/contracts/staking/validatorShare/ValidatorShare.sol b/contracts/staking/validatorShare/ValidatorShare.sol index f9c10e460..978fd976f 100644 --- a/contracts/staking/validatorShare/ValidatorShare.sol +++ b/contracts/staking/validatorShare/ValidatorShare.sol @@ -194,25 +194,27 @@ contract ValidatorShare is IValidatorShare, ERC20NonTradable, OwnableLockable, I function slash( uint256 validatorStake, - uint256 delegatedAmount, + uint256 totalAmount, uint256 totalAmountToSlash ) external onlyOwner returns (uint256) { - uint256 _withdrawPool = withdrawPool; - uint256 delegationAmount = delegatedAmount.add(_withdrawPool); - if (delegationAmount == 0) { - return 0; - } - // total amount to be slashed from delegation pool (active + inactive) - uint256 _amountToSlash = delegationAmount.mul(totalAmountToSlash).div(validatorStake.add(delegationAmount)); - uint256 _amountToSlashWithdrawalPool = _withdrawPool.mul(_amountToSlash).div(delegationAmount); - - // slash inactive pool - uint256 stakeSlashed = _amountToSlash.sub(_amountToSlashWithdrawalPool); - stakeManager.decreaseValidatorDelegatedAmount(validatorId, stakeSlashed); - activeAmount = activeAmount.sub(stakeSlashed); - - withdrawPool = withdrawPool.sub(_amountToSlashWithdrawalPool); - return _amountToSlash; + // uint256 _withdrawPool = withdrawPool; + // uint256 delegationAmount = totalAmount.sub(validatorStake).add(_withdrawPool); + // if (delegationAmount == 0) { + // return 0; + // } + + // // total amount to be slashed from delegation pool (active + inactive) + // uint256 _amountToSlash = delegationAmount.mul(totalAmountToSlash).div(totalAmount); + // uint256 _amountToSlashWithdrawalPool = _withdrawPool.mul(_amountToSlash).div(delegationAmount); + + // // slash inactive pool + // uint256 stakeSlashed = _amountToSlash.sub(_amountToSlashWithdrawalPool); + // stakeManager.decreaseValidatorDelegatedAmount(validatorId, stakeSlashed); + // activeAmount = activeAmount.sub(stakeSlashed); + + // withdrawPool = withdrawPool.sub(_amountToSlashWithdrawalPool); + // return _amountToSlash; + return 0; } function updateDelegation(bool _delegation) external onlyOwner { From b8fdf6c903bc9211941f5ea8eb22f7ffa1ebbe01 Mon Sep 17 00:00:00 2001 From: Denis Ermolin Date: Fri, 23 Apr 2021 14:44:05 +0700 Subject: [PATCH 03/12] fix: do not remove signer from unstaked validator record Due to the usage of signer for getting total staked balance --- contracts/staking/stakeManager/StakeManager.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/staking/stakeManager/StakeManager.sol b/contracts/staking/stakeManager/StakeManager.sol index d48ebb423..74dc0bca6 100644 --- a/contracts/staking/stakeManager/StakeManager.sol +++ b/contracts/staking/stakeManager/StakeManager.sol @@ -478,7 +478,6 @@ contract StakeManager is validators[validatorId].amount = 0; validators[validatorId].jailTime = 0; - validators[validatorId].signer = address(0); signerToValidator[signer] = INCORRECT_VALIDATOR_ID; _writeStatus(signer, Status.Unstaked, deactivationEpoch); From 5567e04d0b453a27703da8ff3d88ee3bef7834d5 Mon Sep 17 00:00:00 2001 From: Denis Ermolin Date: Fri, 23 Apr 2021 14:44:27 +0700 Subject: [PATCH 04/12] fix: increase total staked amount during validator restake --- contracts/staking/stakeManager/StakeManager.sol | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/contracts/staking/stakeManager/StakeManager.sol b/contracts/staking/stakeManager/StakeManager.sol index 74dc0bca6..6e5357890 100644 --- a/contracts/staking/stakeManager/StakeManager.sol +++ b/contracts/staking/stakeManager/StakeManager.sol @@ -492,7 +492,8 @@ contract StakeManager is uint256 amount, bool stakeRewards ) public onlyWhenUnlocked onlyStaker(validatorId) { - (, uint256 deactivationEpoch) = _readStatus(validators[validatorId].signer); + address signer = validators[validatorId].signer; + (, uint256 deactivationEpoch) = _readStatus(signer); require(deactivationEpoch == 0, "No restaking"); if (amount > 0) { @@ -509,6 +510,7 @@ contract StakeManager is uint256 newTotalStaked = totalStaked.add(amount); totalStaked = newTotalStaked; validators[validatorId].amount = validators[validatorId].amount.add(amount); + signerState[signer].totalAmount = signerState[signer].totalAmount.add(amount); updateTimeline(int256(amount), 0, 0); From 5a3165108ec1640dd6b2abbcc30ebf44e69baf04 Mon Sep 17 00:00:00 2001 From: Denis Ermolin Date: Fri, 23 Apr 2021 14:44:44 +0700 Subject: [PATCH 05/12] fix: test --- .../StakeManagerStorageExtension.sol | 2 +- .../staking/stakeManager/StakeManager.test.js | 21 +++++++------------ 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/contracts/staking/stakeManager/StakeManagerStorageExtension.sol b/contracts/staking/stakeManager/StakeManagerStorageExtension.sol index 5c620c7c9..ce6ae8723 100644 --- a/contracts/staking/stakeManager/StakeManagerStorageExtension.sol +++ b/contracts/staking/stakeManager/StakeManagerStorageExtension.sol @@ -22,7 +22,7 @@ contract StakeManagerStorageExtension { // increase / decrease value for faster or slower checkpoints, 0 - 100% uint256 public checkpointRewardDelta; - mapping(address => Signer) signerState; // TODO change to signer => Signer mapping + mapping(address => Signer) public signerState; function _readStatus(address signer) internal view returns(Status status, uint256 deactivationEpoch) { uint256 combinedStatus = signerState[signer].status; diff --git a/test/units/staking/stakeManager/StakeManager.test.js b/test/units/staking/stakeManager/StakeManager.test.js index 963597e84..81ba556a8 100644 --- a/test/units/staking/stakeManager/StakeManager.test.js +++ b/test/units/staking/stakeManager/StakeManager.test.js @@ -1547,13 +1547,6 @@ contract('StakeManager', async function(accounts) { from: validatorUser }), 'fee too small') }) - - it('when fee overflows', async function() { - const overflowFee = new BN(2).pow(new BN(256)) - await expectRevert.unspecified(this.stakeManager.topUpForFee(validatorUser, overflowFee, { - from: validatorUser - })) - }) }) }) @@ -2104,6 +2097,7 @@ contract('StakeManager', async function(accounts) { prepareToTest() testConfirmAuctionBidForNewValidator() }) + describe('when 1000 dynasties has passed', function() { prepareToTest() before(async function() { @@ -2113,6 +2107,7 @@ contract('StakeManager', async function(accounts) { testConfirmAuctionBidForNewValidator() }) + describe('when validator has more stake then last bid', function() { prepareToTest() before(async function() { @@ -2378,7 +2373,7 @@ contract('StakeManager', async function(accounts) { await this.stakeManager.unstakeClaim(aliceId, { from: initialStakers[1].getChecksumAddressString() }) }) - it('Should migrate', async function() { + it('should migrate', async function() { await this.stakeManager.migrateDelegation(aliceId, bobId, migrationAmount, { from: delegator }) }) }) @@ -2401,7 +2396,7 @@ contract('StakeManager', async function(accounts) { }) describe('Chad delegates to Alice', async function() { - it('Should delegate', async function() { + it('should delegate', async function() { this.receipt = await buyVoucher(aliceContract, delegationAmount, delegator) }) @@ -2489,13 +2484,13 @@ contract('StakeManager', async function(accounts) { }) it('Alice active amount must be updated', async function() { - const validator = await this.stakeManager.validators(aliceId) - assertBigNumberEquality(validator.delegatedAmount, delegationAmountBN.sub(migrationAmountBN)) + const delegatedAmount = await this.stakeManager.delegatedAmount(aliceId) + assertBigNumberEquality(delegatedAmount, delegationAmountBN.sub(migrationAmountBN)) }) it('Bob active amount must be updated', async function() { - const validator = await this.stakeManager.validators(bobId) - assertBigNumberEquality(validator.delegatedAmount, migrationAmount) + const delegatedAmount = await this.stakeManager.delegatedAmount(bobId) + assertBigNumberEquality(delegatedAmount, migrationAmount) }) }) }) From 344dd9d9f5046d9169dd6f727754fb14c1a015ed Mon Sep 17 00:00:00 2001 From: Denis Ermolin Date: Fri, 23 Apr 2021 14:47:24 +0700 Subject: [PATCH 06/12] misc: return back code --- .../staking/validatorShare/ValidatorShare.sol | 35 +++++++++---------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/contracts/staking/validatorShare/ValidatorShare.sol b/contracts/staking/validatorShare/ValidatorShare.sol index 978fd976f..3200c2f3d 100644 --- a/contracts/staking/validatorShare/ValidatorShare.sol +++ b/contracts/staking/validatorShare/ValidatorShare.sol @@ -197,24 +197,23 @@ contract ValidatorShare is IValidatorShare, ERC20NonTradable, OwnableLockable, I uint256 totalAmount, uint256 totalAmountToSlash ) external onlyOwner returns (uint256) { - // uint256 _withdrawPool = withdrawPool; - // uint256 delegationAmount = totalAmount.sub(validatorStake).add(_withdrawPool); - // if (delegationAmount == 0) { - // return 0; - // } - - // // total amount to be slashed from delegation pool (active + inactive) - // uint256 _amountToSlash = delegationAmount.mul(totalAmountToSlash).div(totalAmount); - // uint256 _amountToSlashWithdrawalPool = _withdrawPool.mul(_amountToSlash).div(delegationAmount); - - // // slash inactive pool - // uint256 stakeSlashed = _amountToSlash.sub(_amountToSlashWithdrawalPool); - // stakeManager.decreaseValidatorDelegatedAmount(validatorId, stakeSlashed); - // activeAmount = activeAmount.sub(stakeSlashed); - - // withdrawPool = withdrawPool.sub(_amountToSlashWithdrawalPool); - // return _amountToSlash; - return 0; + uint256 _withdrawPool = withdrawPool; + uint256 delegationAmount = totalAmount.sub(validatorStake).add(_withdrawPool); + if (delegationAmount == 0) { + return 0; + } + + // total amount to be slashed from delegation pool (active + inactive) + uint256 _amountToSlash = delegationAmount.mul(totalAmountToSlash).div(totalAmount); + uint256 _amountToSlashWithdrawalPool = _withdrawPool.mul(_amountToSlash).div(delegationAmount); + + // slash inactive pool + uint256 stakeSlashed = _amountToSlash.sub(_amountToSlashWithdrawalPool); + stakeManager.decreaseValidatorDelegatedAmount(validatorId, stakeSlashed); + activeAmount = activeAmount.sub(stakeSlashed); + + withdrawPool = withdrawPool.sub(_amountToSlashWithdrawalPool); + return _amountToSlash; } function updateDelegation(bool _delegation) external onlyOwner { From 6c0773978e084d68e74cff24670f13c840d26723 Mon Sep 17 00:00:00 2001 From: Denis Ermolin Date: Fri, 23 Apr 2021 14:53:51 +0700 Subject: [PATCH 07/12] chg: migration of data code --- .../stakeManager/StakeManagerExtension.sol | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/contracts/staking/stakeManager/StakeManagerExtension.sol b/contracts/staking/stakeManager/StakeManagerExtension.sol index fa34b8586..96bff5ba6 100644 --- a/contracts/staking/stakeManager/StakeManagerExtension.sol +++ b/contracts/staking/stakeManager/StakeManagerExtension.sol @@ -129,19 +129,12 @@ contract StakeManagerExtension is StakeManagerStorage, Initializable, StakeManag } function migrateValidatorsData(uint256 validatorIdFrom, uint256 validatorIdTo) external { - // for (uint256 i = validatorIdFrom; i < validatorIdTo; ++i) { - // ValidatorShare contractAddress = ValidatorShare(validators[i].contractAddress); - // if (contractAddress != ValidatorShare(0)) { - // // move validator rewards out from ValidatorShare contract - // validators[i].reward = contractAddress.validatorRewards_deprecated().add(INITIALIZED_AMOUNT); - // validators[i].delegatedAmount = contractAddress.activeAmount(); - // validators[i].commissionRate = contractAddress.commissionRate_deprecated(); - // } else { - // validators[i].reward = validators[i].reward.add(INITIALIZED_AMOUNT); - // } - - // validators[i].delegatorsReward = INITIALIZED_AMOUNT; - // } + for (uint256 i = validatorIdFrom; i < validatorIdTo; ++i) { + address signer = validators[i].signer; + + signerState[signer].totalAmount = validators[i].amount.add(validators[i].delegatedAmount_deprecated).sub(INITIALIZED_AMOUNT); + _writeStatus(signer, Status(uint256(validators[i].status_deprecated)), validators[i].deactivationEpoch_deprecated); + } } function updateCheckpointRewardParams( From 9e9bd938075afc33ea5446e66ffbfe7fd91cbd9f Mon Sep 17 00:00:00 2001 From: Denis Ermolin Date: Fri, 23 Apr 2021 15:06:36 +0700 Subject: [PATCH 08/12] new: withdraw rewards to different address --- .../staking/validatorShare/ValidatorShare.sol | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/contracts/staking/validatorShare/ValidatorShare.sol b/contracts/staking/validatorShare/ValidatorShare.sol index 3200c2f3d..a915911aa 100644 --- a/contracts/staking/validatorShare/ValidatorShare.sol +++ b/contracts/staking/validatorShare/ValidatorShare.sol @@ -110,7 +110,7 @@ contract ValidatorShare is IValidatorShare, ERC20NonTradable, OwnableLockable, I */ function buyVoucher(uint256 _amount, uint256 _minSharesToMint) public returns(uint256 amountToDeposit) { - _withdrawAndTransferReward(msg.sender); + _withdrawAndTransferReward(msg.sender, msg.sender); amountToDeposit = _buyShares(_amount, _minSharesToMint, msg.sender); require(stakeManager.delegationDeposit(validatorId, amountToDeposit, msg.sender), "deposit failed"); @@ -159,12 +159,17 @@ contract ValidatorShare is IValidatorShare, ERC20NonTradable, OwnableLockable, I } function withdrawRewards() public { - uint256 rewards = _withdrawAndTransferReward(msg.sender); + uint256 rewards = _withdrawAndTransferReward(msg.sender, msg.sender); + require(rewards >= minAmount, "Too small rewards amount"); + } + + function withdrawRewardsTo(address to) public { + uint256 rewards = _withdrawAndTransferReward(msg.sender, to); require(rewards >= minAmount, "Too small rewards amount"); } function migrateOut(address user, uint256 amount) external onlyOwner { - _withdrawAndTransferReward(user); + _withdrawAndTransferReward(user, user); (uint256 totalStaked, uint256 rate) = getTotalStake(user); require(totalStaked >= amount, "Migrating too much"); @@ -181,7 +186,7 @@ contract ValidatorShare is IValidatorShare, ERC20NonTradable, OwnableLockable, I } function migrateIn(address user, uint256 amount) external onlyOwner { - _withdrawAndTransferReward(user); + _withdrawAndTransferReward(user, user); _buyShares(amount, 0, user); } @@ -282,7 +287,7 @@ contract ValidatorShare is IValidatorShare, ERC20NonTradable, OwnableLockable, I uint256 shares = claimAmount.mul(precision).div(rate); require(shares <= maximumSharesToBurn, "too much slippage"); - _withdrawAndTransferReward(msg.sender); + _withdrawAndTransferReward(msg.sender, msg.sender); _burn(msg.sender, shares); stakeManager.updateValidatorState(validatorId, -int256(claimAmount)); @@ -359,11 +364,11 @@ contract ValidatorShare is IValidatorShare, ERC20NonTradable, OwnableLockable, I return liquidRewards; } - function _withdrawAndTransferReward(address user) private returns (uint256) { - uint256 liquidRewards = _withdrawReward(user); + function _withdrawAndTransferReward(address from, address to) private returns (uint256) { + uint256 liquidRewards = _withdrawReward(from); if (liquidRewards != 0) { - require(stakeManager.transferFunds(validatorId, liquidRewards, user), "Insufficent rewards"); - stakingLogger.logDelegatorClaimRewards(validatorId, user, liquidRewards); + require(stakeManager.transferFunds(validatorId, liquidRewards, to), "Insufficent rewards"); + stakingLogger.logDelegatorClaimRewards(validatorId, from, liquidRewards); } return liquidRewards; } @@ -402,9 +407,9 @@ contract ValidatorShare is IValidatorShare, ERC20NonTradable, OwnableLockable, I uint256 value ) internal { // get rewards for recipient - _withdrawAndTransferReward(to); + _withdrawAndTransferReward(to, to); // convert rewards to shares - _withdrawAndTransferReward(from); + _withdrawAndTransferReward(from, from); // move shares to recipient super._transfer(from, to, value); } From 2fb60891d3424d361ddc709ef8af89bc4c6c0356 Mon Sep 17 00:00:00 2001 From: Denis Ermolin Date: Fri, 23 Apr 2021 17:52:41 +0700 Subject: [PATCH 09/12] fix: deduce total amount during slashing --- contracts/staking/stakeManager/StakeManager.sol | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contracts/staking/stakeManager/StakeManager.sol b/contracts/staking/stakeManager/StakeManager.sol index 6e5357890..4585f9298 100644 --- a/contracts/staking/stakeManager/StakeManager.sol +++ b/contracts/staking/stakeManager/StakeManager.sol @@ -691,6 +691,9 @@ contract StakeManager is uint256 _amount = slashData[1].toUint(); totalAmount = totalAmount.add(_amount); + address signer = validators[validatorId].signer; + signerState[signer].totalAmount = signerState[signer].totalAmount.sub(_amount); + address delegationContract = validators[validatorId].contractAddress; if (delegationContract != address(0x0)) { uint256 delSlashedAmount = From 20c93aeeaaf2b396eb13feb776f3de881252b900 Mon Sep 17 00:00:00 2001 From: Denis Ermolin Date: Fri, 23 Apr 2021 19:28:08 +0700 Subject: [PATCH 10/12] fix: slashing --- contracts/staking/slashing/SlashingManager.sol | 7 ++----- contracts/staking/stakeManager/StakeManager.sol | 11 ++++++++--- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/contracts/staking/slashing/SlashingManager.sol b/contracts/staking/slashing/SlashingManager.sol index a76428cab..ecb3f91b0 100644 --- a/contracts/staking/slashing/SlashingManager.sol +++ b/contracts/staking/slashing/SlashingManager.sol @@ -103,13 +103,10 @@ contract SlashingManager is ISlashingManager, Ownable { break; } else if (stakeManager.isValidator(validatorId) && signer > lastAdd) { lastAdd = signer; - uint256 amount; - uint256 delegatedAmount; - (amount,,,,,,,,,,,delegatedAmount,) = stakeManager.validators(validatorId); + (, uint256 totalAmount) = stakeManager.signerState(signer); // add delegation power - amount = amount.add(delegatedAmount); - _stakePower = _stakePower.add(amount); + _stakePower = _stakePower.add(totalAmount); } } return (_stakePower >= stakeManager.currentValidatorSetTotalStake().mul(2).div(3).add(1)); diff --git a/contracts/staking/stakeManager/StakeManager.sol b/contracts/staking/stakeManager/StakeManager.sol index 4585f9298..b55f5ace1 100644 --- a/contracts/staking/stakeManager/StakeManager.sol +++ b/contracts/staking/stakeManager/StakeManager.sol @@ -681,8 +681,11 @@ contract StakeManager is uint256 jailedAmount; uint256 totalAmount; uint256 i; + uint256 delSlashedAmount; for (; i < slashingInfoList.length; i++) { + uint256 delSlashedAmount = 0; + RLPReader.RLPItem[] memory slashData = slashingInfoList[i].toList(); uint256 validatorId = slashData[0].toUint(); @@ -692,19 +695,21 @@ contract StakeManager is totalAmount = totalAmount.add(_amount); address signer = validators[validatorId].signer; - signerState[signer].totalAmount = signerState[signer].totalAmount.sub(_amount); + uint256 totalAmount = signerState[signer].totalAmount; address delegationContract = validators[validatorId].contractAddress; if (delegationContract != address(0x0)) { - uint256 delSlashedAmount = + delSlashedAmount = IValidatorShare(delegationContract).slash( validators[validatorId].amount, - signerState[validators[validatorId].signer].totalAmount, + totalAmount, _amount ); _amount = _amount.sub(delSlashedAmount); } + signerState[signer].totalAmount = totalAmount.sub(_amount.add(delSlashedAmount)); + uint256 validatorStakeSlashed = validators[validatorId].amount.sub(_amount); validators[validatorId].amount = validatorStakeSlashed; From 0ef64578fe71ebd547e30f96a9f7d8a93b1e5a23 Mon Sep 17 00:00:00 2001 From: Denis Ermolin Date: Mon, 26 Apr 2021 17:33:01 +0700 Subject: [PATCH 11/12] fix: slashing tests --- test/units/staking/SlashingManager.test.js | 11 ++++++++--- test/units/staking/ValidatorShare.test.js | 2 +- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/test/units/staking/SlashingManager.test.js b/test/units/staking/SlashingManager.test.js index f69d245ea..c86494966 100644 --- a/test/units/staking/SlashingManager.test.js +++ b/test/units/staking/SlashingManager.test.js @@ -149,8 +149,10 @@ contract('Slashing:validator', async function(accounts) { }) it('validator must be unstaked within current epoch', async function() { - const validatorData = await stakeManager.validators(slashedValidatorId) - assertBigNumberEquality(validatorData.deactivationEpoch, await stakeManager.currentEpoch()) + const state = await stakeManager.signerState(validatorAddr) + // mask out deactivation epoch + const deactivationEpoch = state.status.and(web3.utils.toBN('1766847064778384329583297500742918515827483896875618958121606201292619775')) + assertBigNumberEquality(deactivationEpoch, await stakeManager.currentEpoch()) }) it('validator must not be jailed', async function() { @@ -174,6 +176,7 @@ contract('Slashing:validator', async function(accounts) { }) }) }) + contract('Slashing:delegation', async function(accounts) { let stakeToken let stakeManager @@ -229,10 +232,12 @@ contract('Slashing:delegation', async function(accounts) { logs[0].event.should.equal('Slashed') assertBigNumberEquality(logs[0].args.amount, web3.utils.toWei('200')) const validator1 = await stakeManager.validators(1) + validator = await stakeManager.validators(2) + const state = await stakeManager.signerState(wallets[1].getAddressString()) assertBigNumberEquality(validator1.amount, web3.utils.toWei('900')) - assertBigNumberEquality(validator.delegatedAmount, web3.utils.toWei('230')) + assertBigNumberEquality(state.totalAmount.sub(validator.amount), web3.utils.toWei('230')) }) }) }) diff --git a/test/units/staking/ValidatorShare.test.js b/test/units/staking/ValidatorShare.test.js index 0e2be563a..b123b602b 100644 --- a/test/units/staking/ValidatorShare.test.js +++ b/test/units/staking/ValidatorShare.test.js @@ -104,7 +104,6 @@ contract('ValidatorShare', async function() { this.stakeToken = await TestToken.new('MATIC', 'MATIC') await this.stakeManager.setStakingToken(this.stakeToken.address) - await this.stakeToken.mint(this.stakeManager.address, toWei('10000000')) this.validatorId = '8' @@ -647,6 +646,7 @@ contract('ValidatorShare', async function() { await buyVoucher(this.validatorContract, this.stakeAmount, this.user) }) + before('slash', async function() { await slash.call(this, [{ validator: this.validatorId, amount: this.stakeAmount }], [this.validatorUser], this.validatorUser) }) From 8c0e2674c844c21c27cf66c8c250c2b20dc2beb3 Mon Sep 17 00:00:00 2001 From: Denis Ermolin Date: Mon, 26 Apr 2021 17:44:57 +0700 Subject: [PATCH 12/12] fix: migration test --- test/units/staking/stakeManager/StakeManager.test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/units/staking/stakeManager/StakeManager.test.js b/test/units/staking/stakeManager/StakeManager.test.js index 81ba556a8..1ce7cc60d 100644 --- a/test/units/staking/stakeManager/StakeManager.test.js +++ b/test/units/staking/stakeManager/StakeManager.test.js @@ -2426,7 +2426,8 @@ contract('StakeManager', async function(accounts) { it('Active amount must be updated', async function() { const validator = await this.stakeManager.validators(aliceId) - assertBigNumberEquality(validator.delegatedAmount, delegationAmountBN) + const state = await this.stakeManager.signerState(initialStakers[1].getChecksumAddressString()) + assertBigNumberEquality(state.totalAmount.sub(validator.amount), delegationAmountBN) }) })