Staking Security Update - implementation for SIP-0042

.gitignore

@@ -28,7 +28,7 @@ solc-select
 artifacts
 cache
 .idea/
-workspace.code-workspace
+*.code-workspace
 .env
 node_modules
 artifacts/

contracts/governance/Staking/Checkpoints.sol

@@ -35,6 +35,18 @@ contract Checkpoints is StakingStorage, SafeMath96 {
 
 	event AdminRemoved(address admin);
 
+	/// @param pauser address to grant power to pause the contract
+	/// @param added true - added, false - removed
+	event PauserAddedOrRemoved(address indexed pauser, bool indexed added);
+
+	/// @notice An event emitted when a staking is paused or unpaused
+	/// @param setPaused true - pause, false - unpause
+	event StakingPaused(bool indexed setPaused);
+
+	/// @notice An event emitted when a staking is frozen or unfrozen
+	/// @param setFrozen true - pause, false - unpause
+	event StakingFrozen(bool indexed setFrozen);
+
 	event ContractCodeHashAdded(bytes32 hash);
 
 	event ContractCodeHashRemoved(bytes32 hash);
@@ -49,7 +61,7 @@ contract Checkpoints is StakingStorage, SafeMath96 {
 	function _increaseVestingStake(uint256 lockedTS, uint96 value) internal {
 		uint32 nCheckpoints = numVestingCheckpoints[lockedTS];
 		uint96 vested = vestingCheckpoints[lockedTS][nCheckpoints - 1].stake;
-		uint96 newVest = add96(vested, value, "Staking::_increaseVestingStake: vested amount overflow");
+		uint96 newVest = add96(vested, value, "vested overflow");
 		_writeVestingCheckpoint(lockedTS, nCheckpoints, newVest);
 	}
 
@@ -61,7 +73,7 @@ contract Checkpoints is StakingStorage, SafeMath96 {
 	function _decreaseVestingStake(uint256 lockedTS, uint96 value) internal {
 		uint32 nCheckpoints = numVestingCheckpoints[lockedTS];
 		uint96 vested = vestingCheckpoints[lockedTS][nCheckpoints - 1].stake;
-		uint96 newVest = sub96(vested, value, "Staking::_decreaseVestingStake: vested amount underflow");
+		uint96 newVest = sub96(vested, value, "vested underflow");
 		_writeVestingCheckpoint(lockedTS, nCheckpoints, newVest);
 	}
 
@@ -76,7 +88,7 @@ contract Checkpoints is StakingStorage, SafeMath96 {
 		uint32 nCheckpoints,
 		uint96 newVest
 	) internal {
-		uint32 blockNumber = safe32(block.number, "Staking::_writeVestingCheckpoint: block number exceeds 32 bits");
+		uint32 blockNumber = safe32(block.number, "block num > 32 bits");
 
 		if (nCheckpoints > 0 && vestingCheckpoints[lockedTS][nCheckpoints - 1].fromBlock == blockNumber) {
 			vestingCheckpoints[lockedTS][nCheckpoints - 1].stake = newVest;
@@ -99,7 +111,7 @@ contract Checkpoints is StakingStorage, SafeMath96 {
 	) internal {
 		uint32 nCheckpoints = numUserStakingCheckpoints[account][lockedTS];
 		uint96 staked = userStakingCheckpoints[account][lockedTS][nCheckpoints - 1].stake;
-		uint96 newStake = add96(staked, value, "Staking::_increaseUserStake: staked amount overflow");
+		uint96 newStake = add96(staked, value, "staked overflow");
 		_writeUserCheckpoint(account, lockedTS, nCheckpoints, newStake);
 	}
 
@@ -116,7 +128,7 @@ contract Checkpoints is StakingStorage, SafeMath96 {
 	) internal {
 		uint32 nCheckpoints = numUserStakingCheckpoints[account][lockedTS];
 		uint96 staked = userStakingCheckpoints[account][lockedTS][nCheckpoints - 1].stake;
-		uint96 newStake = sub96(staked, value, "Staking::_decreaseUserStake: staked amount underflow");
+		uint96 newStake = sub96(staked, value, "staked underflow");
 		_writeUserCheckpoint(account, lockedTS, nCheckpoints, newStake);
 	}
 
@@ -133,7 +145,7 @@ contract Checkpoints is StakingStorage, SafeMath96 {
 		uint32 nCheckpoints,
 		uint96 newStake
 	) internal {
-		uint32 blockNumber = safe32(block.number, "Staking::_writeStakingCheckpoint: block number exceeds 32 bits");
+		uint32 blockNumber = safe32(block.number, "block number > 32 bits");
 
 		if (nCheckpoints > 0 && userStakingCheckpoints[account][lockedTS][nCheckpoints - 1].fromBlock == blockNumber) {
 			userStakingCheckpoints[account][lockedTS][nCheckpoints - 1].stake = newStake;
@@ -156,7 +168,7 @@ contract Checkpoints is StakingStorage, SafeMath96 {
 	) internal {
 		uint32 nCheckpoints = numDelegateStakingCheckpoints[delegatee][lockedTS];
 		uint96 staked = delegateStakingCheckpoints[delegatee][lockedTS][nCheckpoints - 1].stake;
-		uint96 newStake = add96(staked, value, "Staking::_increaseDelegateStake: staked amount overflow");
+		uint96 newStake = add96(staked, value, "staked overflow");
 		_writeDelegateCheckpoint(delegatee, lockedTS, nCheckpoints, newStake);
 	}
 
@@ -181,7 +193,7 @@ contract Checkpoints is StakingStorage, SafeMath96 {
 		//		(no delegation to another address).
 		// @dev It can be greater than 0, but inconsistent after 3 transactions
 		if (staked > value) {
-			newStake = sub96(staked, value, "Staking::_decreaseDelegateStake: staked amount underflow");
+			newStake = sub96(staked, value, "staked underflow");
 		}
 		_writeDelegateCheckpoint(delegatee, lockedTS, nCheckpoints, newStake);
 	}
@@ -199,7 +211,7 @@ contract Checkpoints is StakingStorage, SafeMath96 {
 		uint32 nCheckpoints,
 		uint96 newStake
 	) internal {
-		uint32 blockNumber = safe32(block.number, "Staking::_writeStakingCheckpoint: block number exceeds 32 bits");
+		uint32 blockNumber = safe32(block.number, "block numb > 32 bits");
 		uint96 oldStake = delegateStakingCheckpoints[delegatee][lockedTS][nCheckpoints - 1].stake;
 
 		if (nCheckpoints > 0 && delegateStakingCheckpoints[delegatee][lockedTS][nCheckpoints - 1].fromBlock == blockNumber) {
@@ -219,7 +231,7 @@ contract Checkpoints is StakingStorage, SafeMath96 {
 	function _increaseDailyStake(uint256 lockedTS, uint96 value) internal {
 		uint32 nCheckpoints = numTotalStakingCheckpoints[lockedTS];
 		uint96 staked = totalStakingCheckpoints[lockedTS][nCheckpoints - 1].stake;
-		uint96 newStake = add96(staked, value, "Staking::_increaseDailyStake: staked amount overflow");
+		uint96 newStake = add96(staked, value, "staked overflow");
 		_writeStakingCheckpoint(lockedTS, nCheckpoints, newStake);
 	}
 
@@ -231,7 +243,7 @@ contract Checkpoints is StakingStorage, SafeMath96 {
 	function _decreaseDailyStake(uint256 lockedTS, uint96 value) internal {
 		uint32 nCheckpoints = numTotalStakingCheckpoints[lockedTS];
 		uint96 staked = totalStakingCheckpoints[lockedTS][nCheckpoints - 1].stake;
-		uint96 newStake = sub96(staked, value, "Staking::_decreaseDailyStake: staked amount underflow");
+		uint96 newStake = sub96(staked, value, "staked underflow");
 		_writeStakingCheckpoint(lockedTS, nCheckpoints, newStake);
 	}
 
@@ -246,7 +258,7 @@ contract Checkpoints is StakingStorage, SafeMath96 {
 		uint32 nCheckpoints,
 		uint96 newStake
 	) internal {
-		uint32 blockNumber = safe32(block.number, "Staking::_writeStakingCheckpoint: block number exceeds 32 bits");
+		uint32 blockNumber = safe32(block.number, "block num > 32 bits");
 
 		if (nCheckpoints > 0 && totalStakingCheckpoints[lockedTS][nCheckpoints - 1].fromBlock == blockNumber) {
 			totalStakingCheckpoints[lockedTS][nCheckpoints - 1].stake = newStake;

contracts/governance/Staking/Staking.sol

@@ -38,7 +38,7 @@ contract Staking is IStaking, WeightedStaking, ApprovalReceiver {
 		uint256 until,
 		address stakeFor,
 		address delegatee
-	) external {
+	) external whenNotPaused {
 		_stake(msg.sender, amount, until, stakeFor, delegatee, false);
 	}
 
@@ -58,7 +58,7 @@ contract Staking is IStaking, WeightedStaking, ApprovalReceiver {
 		uint256 until,
 		address stakeFor,
 		address delegatee
-	) public onlyThisContract {
+	) public onlyThisContract whenNotPaused {
 		_stake(sender, amount, until, stakeFor, delegatee, false);
 	}
 
@@ -133,9 +133,9 @@ contract Staking is IStaking, WeightedStaking, ApprovalReceiver {
 	 * @param previousLock The old unlocking timestamp.
 	 * @param until The new unlocking timestamp in seconds.
 	 * */
-	function extendStakingDuration(uint256 previousLock, uint256 until) public {
+	function extendStakingDuration(uint256 previousLock, uint256 until) public whenNotPaused {
 		until = timestampToLockDate(until);
-		require(previousLock <= until, "cannot reduce the staking duration");
+		require(previousLock <= until, "cannot reduce staking duration");
 
 		/// @dev Do not exceed the max duration, no overflow possible.
 		uint256 latest = timestampToLockDate(block.timestamp + MAX_DURATION);
@@ -144,7 +144,7 @@ contract Staking is IStaking, WeightedStaking, ApprovalReceiver {
 		/// @dev Update checkpoints.
 		/// @dev TODO James: Can reading stake at block.number -1 cause trouble with multiple tx in a block?
 		uint96 amount = _getPriorUserStakeByDate(msg.sender, previousLock, block.number - 1);
-		require(amount > 0, "nothing staked until the previous lock date");
+		require(amount > 0, "no stakes till the prev lock date");
 		_decreaseUserStake(msg.sender, previousLock, amount);
 		_increaseUserStake(msg.sender, until, amount);
 
@@ -216,7 +216,7 @@ contract Staking is IStaking, WeightedStaking, ApprovalReceiver {
 		uint256 intervalLength,
 		address stakeFor,
 		address delegatee
-	) public {
+	) public whenNotPaused {
 		/**
 		 * @dev Stake them until lock dates according to the vesting schedule.
 		 * Note: because staking is only possible in periods of 2 weeks,
@@ -251,7 +251,7 @@ contract Staking is IStaking, WeightedStaking, ApprovalReceiver {
 		uint96 amount,
 		uint256 until,
 		address receiver
-	) public {
+	) public whenNotFrozen {
 		_withdraw(amount, until, receiver, false);
 		// @dev withdraws tokens for lock date 2 weeks later than given lock date if sender is a contract
 		//		we need to check block.timestamp here
@@ -269,7 +269,7 @@ contract Staking is IStaking, WeightedStaking, ApprovalReceiver {
 		uint96 amount,
 		uint256 until,
 		address receiver
-	) public {
+	) public whenNotFrozen {
 		require(vestingWhitelist[msg.sender], "unauthorized");
 
 		_withdraw(amount, until, receiver, true);
@@ -335,7 +335,7 @@ contract Staking is IStaking, WeightedStaking, ApprovalReceiver {
 
 			/// @dev punishedAmount can be 0 if block.timestamp are very close to 'until'
 			if (punishedAmount > 0) {
-				require(address(feeSharing) != address(0), "Staking::withdraw: FeeSharing address wasn't set");
+				require(address(feeSharing) != address(0), "FeeSharing address wasn't set");
 				/// @dev Move punished amount to fee sharing.
 				/// @dev Approve transfer here and let feeSharing do transfer and write checkpoint.
 				SOVToken.approve(address(feeSharing), punishedAmount);
@@ -345,7 +345,7 @@ contract Staking is IStaking, WeightedStaking, ApprovalReceiver {
 
 		/// @dev transferFrom
 		bool success = SOVToken.transfer(receiver, amount);
-		require(success, "Staking::withdraw: Token transfer failed");
+		require(success, "Token transfer failed");
 
 		emit StakingWithdrawn(msg.sender, amount, until, receiver, isGovernance);
 	}
@@ -397,7 +397,7 @@ contract Staking is IStaking, WeightedStaking, ApprovalReceiver {
 	 * @param until The date until which the tokens were staked.
 	 * */
 	function _validateWithdrawParams(uint96 amount, uint256 until) internal view {
-		require(amount > 0, "Staking::withdraw: amount of tokens to be withdrawn needs to be bigger than 0");
+		require(amount > 0, "Amount of tokens to withdraw must be > 0");
 		uint96 balance = _getPriorUserStakeByDate(msg.sender, until, block.number - 1);
 		require(amount <= balance, "Staking::withdraw: not enough balance");
 	}
@@ -429,7 +429,7 @@ contract Staking is IStaking, WeightedStaking, ApprovalReceiver {
 	 * @param delegatee The address to delegate votes to.
 	 * @param lockDate the date if the position to delegate.
 	 * */
-	function delegate(address delegatee, uint256 lockDate) public {
+	function delegate(address delegatee, uint256 lockDate) public whenNotPaused {
 		_delegate(msg.sender, delegatee, lockDate);
 		// @dev delegates tokens for lock date 2 weeks later than given lock date
 		//		if message sender is a contract
@@ -474,7 +474,7 @@ contract Staking is IStaking, WeightedStaking, ApprovalReceiver {
 		uint8 v,
 		bytes32 r,
 		bytes32 s
-	) public {
+	) public whenNotPaused {
 		/**
 		 * @dev The DOMAIN_SEPARATOR is a hash that uniquely identifies a
 		 * smart contract. It is built from a string denoting it as an
@@ -641,7 +641,7 @@ contract Staking is IStaking, WeightedStaking, ApprovalReceiver {
 	function setWeightScaling(uint96 _weightScaling) public onlyOwner {
 		require(
 			MIN_WEIGHT_SCALING <= _weightScaling && _weightScaling <= MAX_WEIGHT_SCALING,
-			"weight scaling doesn't belong to range [1, 9]"
+			"wrong weight scaling" /* scaling doesn't belong to range [1, 9] */
 		);
 		weightScaling = _weightScaling;
 	}

contracts/governance/Staking/StakingStorage.sol

@@ -134,6 +134,15 @@ contract StakingStorage is Ownable {
 	/// @dev numVestingCheckpoints[date] is a number.
 	mapping(uint256 => uint32) public numVestingCheckpoints;
 
-	///@notice the vesting registry contract
+	///@notice vesting registry contract
 	VestingRegistryLogic public vestingRegistryLogic;
+
+	/// @dev user => flag whether user has pauser role.
+	mapping(address => bool) public pausers;
+
+	/// @dev Staking contract is paused
+	bool paused;
+
+	/// @dev Staking contract is frozen
+	bool frozen;
 }