Zokyo/test improv

.solcover.js

@@ -0,0 +1,22 @@
+module.exports = {
+    skipFiles: [
+        'Migrations.sol',
+        'test/BlockRewardMock.sol',
+        'test/ConsensusMock.sol',
+        'test/EternalStorageProxyMock.sol',
+        'test/ProxyStorageMock.sol',
+        'test/VotingMock.sol',
+    ],
+    // need for dependencies
+    copyNodeModules: true,
+    copyPackages: [
+        'openzeppelin-solidity'
+    ],
+    dir: '.',
+    providerOptions: {
+        total_accounts: 110,
+        default_balance_ether: 100000000,
+        gasPrice: '0x1'
+    },
+    norpc: false
+};

.solhint.json

@@ -0,0 +1,22 @@
+{
+  "extends": "solhint:default",
+  "rules": {
+    "avoid-throw": "off",
+    "avoid-suicide": "error",
+    "avoid-sha3": "warn",
+    "indent": ["warn", 4],
+    "compiler-fixed": "off",
+    "not-rely-on-time": "off",
+    "quotes": ["error", "double"],
+    "no-empty-blocks": "off",
+    "no-complex-fallback": "off",
+    "two-lines-top-level-separator": "off",
+    "code-complexity": 8,
+    "avoid-call-value": "off",
+    "no-simple-event-func-name": "off",
+    "avoid-low-level-calls": "off",
+    "no-inline-assembly": "off",
+    "max-line-length": 170,
+    "bracket-align": "off"
+  }
+}

.soliumrc.json

@@ -0,0 +1,44 @@
+{
+  "extends": "solium:recommended",
+  "plugins": [
+    "security"
+  ],
+  "rules": {
+    "quotes": [
+      "error",
+      "double"
+    ],
+    "indentation": [
+      "error",
+      4
+    ],
+    "max-len": ["error", 79],
+    "lbrace": "off",
+    "linebreak-style": ["error", "unix"],
+    "no-constant": ["error"],
+    "no-empty-blocks": "off",
+    "uppercase": "off",
+    "visibility-first": "error",
+    "security/enforce-explicit-visibility": ["error"],
+    "security/no-block-members": ["warning"],
+    "security/no-inline-assembly": ["warning"],
+    "blank-lines": "off",
+    "imports-on-top": "error",
+    "array-declarations": "warning",
+    "operator-whitespace": "warning",
+    "conditionals-whitespace": "warning",
+    "semicolon-whitespace": "warning",
+    "function-whitespace": "warning",
+    "mixedcase": "warning",
+    "no-unused-vars": "warning",
+    "pragma-on-top": "error",
+    "function-order": "warning",
+    "emit": "error",
+    "value-in-payable": "error",
+    "error-reason": "warning",
+    "no-experimental": "warning",
+    "deprecated-suicide": "error",
+    "whitespace": "warning",
+    "arg-overflow": "error"
+  }
+}

contracts/BlockReward.sol

@@ -85,7 +85,7 @@ contract BlockReward is EternalStorage, BlockRewardBase {
     require(benefactors.length == 1);
     require(kind[0] == 0);
 
-    uint256 blockRewardAmount = getBlockRewardAmount();
+    uint256 blockRewardAmount = getBlockRewardAmountPerValidator(benefactors[0]);
 
     (address[] memory _delegators, uint256[] memory _rewards) = IConsensus(ProxyStorage(getProxyStorage()).getConsensus()).getDelegatorsForRewardDistribution(benefactors[0], blockRewardAmount);
 
@@ -183,6 +183,20 @@ contract BlockReward is EternalStorage, BlockRewardBase {
     return uintStorage[BLOCK_REWARD_AMOUNT];
   }
 
+  function getBlockRewardAmountPerValidator(address _validator) public view returns(uint256) {
+    IConsensus consensus = IConsensus(ProxyStorage(getProxyStorage()).getConsensus());
+    uint256 stakeAmount = consensus.stakeAmount(_validator);
+    uint256 totalStakeAmount = consensus.totalStakeAmount();
+    uint256 currentValidatorsLength = consensus.currentValidatorsLength();
+    // this may arise in peculiar cases when the consensus totalStakeAmount wasn't calculated yet
+    // for example at the first blocks after the contract was deployed
+    if (totalStakeAmount == 0) {
+      return getBlockRewardAmount();
+    }
+    return getBlockRewardAmount().mul(stakeAmount).mul(currentValidatorsLength).div(totalStakeAmount);
+  }
+
+
   function getProxyStorage() public view returns(address) {
     return addressStorage[PROXY_STORAGE];
   }

contracts/Consensus.sol

@@ -71,17 +71,7 @@ contract Consensus is ConsensusUtils {
   * @param _amount the amount msg.sender wishes to withdraw from the contract
   */
   function withdraw(uint256 _amount) external {
-    require(_amount > 0);
-    require(_amount <= stakeAmount(msg.sender));
-    require(_amount <= delegatedAmount(msg.sender, msg.sender));
-
-    _delegatedAmountSub(msg.sender, msg.sender, _amount);
-    _stakeAmountSub(msg.sender, _amount);
-    if (stakeAmount(msg.sender) < getMinStake()) {
-      _pendingValidatorsRemove(msg.sender);
-    }
-
-    msg.sender.transfer(_amount);
+    _withdraw(msg.sender, _amount, msg.sender);
   }
 
   /**
@@ -90,40 +80,17 @@ contract Consensus is ConsensusUtils {
   * @param _amount the amount msg.sender wishes to withdraw from the contract
   */
   function withdraw(address _validator, uint256 _amount) external {
-    require(_validator != address(0));
-    require(_amount > 0);
-    require(_amount <= stakeAmount(_validator));
-    require(_amount <= delegatedAmount(msg.sender, _validator));
-
-    _delegatedAmountSub(msg.sender, _validator, _amount);
-    _stakeAmountSub(_validator, _amount);
-    if (stakeAmount(_validator) < getMinStake()) {
-      _pendingValidatorsRemove(_validator);
-    }
-
-    msg.sender.transfer(_amount);
+    _withdraw(msg.sender, _amount, _validator);
   }
 
   /**
   * @dev Function to be called by the block reward contract each block to handle cycles and snapshots logic
   */
   function cycle() external onlyBlockReward {
-    if (_shouldTakeSnapshot()) {
-      uint256 snapshotId = getNextSnapshotId();
-      if (snapshotId == getSnapshotsPerCycle().sub(1)) {
-        _setNextSnapshotId(0);
-      } else {
-        _setNextSnapshotId(snapshotId.add(1));
-      }
-      _setSnapshot(snapshotId, pendingValidators());
-      _setLastSnapshotTakenAtBlock(block.number);
-      delete snapshotId;
-    }
     if (_hasCycleEnded()) {
       IVoting(ProxyStorage(getProxyStorage()).getVoting()).onCycleEnd(currentValidators());
       _setCurrentCycle();
-      uint256 randomSnapshotId = _getRandom(0, getSnapshotsPerCycle() - 1);
-      address[] memory newSet = getSnapshotAddresses(randomSnapshotId);
+      address[] memory newSet = pendingValidators();
       if (newSet.length > 0) {
         _setNewValidatorSet(newSet);
       }
@@ -132,7 +99,6 @@ contract Consensus is ConsensusUtils {
         _setShouldEmitInitiateChange(true);
         emit ShouldEmitInitiateChange();
       }
-      delete randomSnapshotId;
       IBlockReward(ProxyStorage(getProxyStorage()).getBlockReward()).onCycleEnd();
     }
   }

contracts/ConsensusUtils.sol

@@ -16,8 +16,9 @@ contract ConsensusUtils is EternalStorage, ValidatorSet {
   uint256 public constant DECIMALS = 10 ** 18;
   uint256 public constant MAX_VALIDATORS = 100;
   uint256 public constant MIN_STAKE = 1e23; // 100,000
-  uint256 public constant CYCLE_DURATION_BLOCKS = 120960; // 7 days [7*24*60*60/5]
-  uint256 public constant SNAPSHOTS_PER_CYCLE = 10; // snapshot each 1008 minutes [120960/10/60*5]
+  uint256 public constant MAX_STAKE = 5e24; // 5,000,000
+  uint256 public constant CYCLE_DURATION_BLOCKS = 34560; // 48 hours [48*60*60/5]
+  uint256 public constant SNAPSHOTS_PER_CYCLE = 0; // snapshot each 288 minutes [34560/10/60*5]
   uint256 public constant DEFAULT_VALIDATOR_FEE = 1e17; // 10%
 
   /**
@@ -84,22 +85,61 @@ contract ConsensusUtils is EternalStorage, ValidatorSet {
   bytes32 internal constant WAS_PROXY_STORAGE_SET = keccak256(abi.encodePacked("wasProxyStorageSet"));
   bytes32 internal constant NEW_VALIDATOR_SET = keccak256(abi.encodePacked("newValidatorSet"));
   bytes32 internal constant SHOULD_EMIT_INITIATE_CHANGE = keccak256(abi.encodePacked("shouldEmitInitiateChange"));
+  bytes32 internal constant TOTAL_STAKE_AMOUNT = keccak256(abi.encodePacked("totalStakeAmount"));
 
   function _delegate(address _staker, uint256 _amount, address _validator) internal {
     require(_staker != address(0));
     require(_amount != 0);
     require(_validator != address(0));
 
-    // overstaking should not be possible
-    require (stakeAmount(_validator) < getMinStake());
-    require (stakeAmount(_validator).add(_amount) <= getMinStake());
-
     _delegatedAmountAdd(_staker, _validator, _amount);
     _stakeAmountAdd(_validator, _amount);
 
+    // stake amount of the validator isn't greater than the max stake
+    require(stakeAmount(_validator) <= getMaxStake());
+
+    // the validator must stake himselft the minimum stake
     if (stakeAmount(_validator) >= getMinStake() && !isPendingValidator(_validator)) {
       _pendingValidatorsAdd(_validator);
     }
+
+    // if _validator is one of the current validators
+    if (isValidator(_validator)) {
+      // the total stake needs to be adjusted for the block reward formula
+      _totalStakeAmountAdd(_amount);
+    }
+  }
+
+  function _withdraw(address _staker, uint256 _amount, address _validator) internal {
+    require(_validator != address(0));
+    require(_amount > 0);
+    require(_amount <= stakeAmount(_validator));
+    require(_amount <= delegatedAmount(_staker, _validator));
+
+    bool _isValidator = isValidator(_validator);
+
+    // if new stake amount is lesser than minStake and the validator is one of the current validators
+    if (stakeAmount(_validator).sub(_amount) < getMinStake() && _isValidator) {
+      // do not withdaw the amount until the validator is in current set
+      _pendingValidatorsRemove(_validator);
+      return;
+    }
+
+
+    _delegatedAmountSub(_staker, _validator, _amount);
+    _stakeAmountSub(_validator, _amount);
+
+    // if _validator is one of the current validators
+    if (_isValidator) {
+      // the total stake needs to be adjusted for the block reward formula
+      _totalStakeAmountSub(_amount);
+    }
+
+    // if validator is needed to be removed from pending, but not current
+    if (stakeAmount(_validator) < getMinStake()) {
+      _pendingValidatorsRemove(_validator);
+    }
+    _staker.transfer(_amount);
   }
 
   function _setSystemAddress(address _newAddress) internal {
@@ -139,10 +179,17 @@ contract ConsensusUtils is EternalStorage, ValidatorSet {
     return MIN_STAKE;
   }
 
+  /**
+  * returns maximum stake (wei) for a validator
+  */
+  function getMaxStake() public pure returns(uint256) {
+    return MAX_STAKE;
+  }
+
   /**
   * returns number of blocks per cycle (block time is 5 seconds)
   */
-  function getCycleDurationBlocks() public pure returns(uint256) {
+  function getCycleDurationBlocks() public view returns(uint256) {
     return CYCLE_DURATION_BLOCKS;
   }
 
@@ -236,6 +283,12 @@ contract ConsensusUtils is EternalStorage, ValidatorSet {
   }
 
   function _setCurrentValidators(address[] _currentValidators) internal {
+    uint256 totalStake = 0;
+    for (uint i = 0; i < _currentValidators.length; i++) {
+      uint256 stakedAmount = stakeAmount(_currentValidators[i]);
+      totalStake = totalStake + stakedAmount;
+    }
+    _setTotalStakeAmount(totalStake);
     addressArrayStorage[CURRENT_VALIDATORS] = _currentValidators;
   }
 
@@ -286,13 +339,18 @@ contract ConsensusUtils is EternalStorage, ValidatorSet {
       }
       delete addressArrayStorage[PENDING_VALIDATORS][lastIndex];
       addressArrayStorage[PENDING_VALIDATORS].length--;
+      // if the validator in on of the current validators
     }
   }
 
   function stakeAmount(address _address) public view returns(uint256) {
     return uintStorage[keccak256(abi.encodePacked("stakeAmount", _address))];
   }
 
+  function totalStakeAmount() public view returns(uint256) {
+    return uintStorage[TOTAL_STAKE_AMOUNT];
+  }
+
   function _stakeAmountAdd(address _address, uint256 _amount) internal {
     uintStorage[keccak256(abi.encodePacked("stakeAmount", _address))] = uintStorage[keccak256(abi.encodePacked("stakeAmount", _address))].add(_amount);
   }
@@ -393,20 +451,24 @@ contract ConsensusUtils is EternalStorage, ValidatorSet {
     addressArrayStorage[NEW_VALIDATOR_SET] = _newSet;
   }
 
-  function shouldEmitInitiateChange() public view returns(bool) {
-    return boolStorage[SHOULD_EMIT_INITIATE_CHANGE];
+  function _setTotalStakeAmount(uint256 _totalStake) internal {
+    uintStorage[TOTAL_STAKE_AMOUNT] = _totalStake;
   }
 
-  function _setShouldEmitInitiateChange(bool _status) internal {
-    boolStorage[SHOULD_EMIT_INITIATE_CHANGE] = _status;
+  function _totalStakeAmountAdd(uint256 _stakeAmount) internal {
+    uintStorage[TOTAL_STAKE_AMOUNT] = uintStorage[TOTAL_STAKE_AMOUNT].add(_stakeAmount);
+  }
+
+  function _totalStakeAmountSub(uint256 _stakeAmount) internal {
+    uintStorage[TOTAL_STAKE_AMOUNT] = uintStorage[TOTAL_STAKE_AMOUNT].sub(_stakeAmount);
   }
 
-  function _getBlocksToSnapshot() internal pure returns(uint256) {
-    return getCycleDurationBlocks().div(getSnapshotsPerCycle());
+  function shouldEmitInitiateChange() public view returns(bool) {
+    return boolStorage[SHOULD_EMIT_INITIATE_CHANGE];
   }
 
-  function _shouldTakeSnapshot() internal view returns(bool) {
-    return (block.number - getLastSnapshotTakenAtBlock() >= _getBlocksToSnapshot());
+  function _setShouldEmitInitiateChange(bool _status) internal {
+    boolStorage[SHOULD_EMIT_INITIATE_CHANGE] = _status;
   }
 
   function _hasCycleEnded() internal view returns(bool) {

contracts/interfaces/IConsensus.sol

@@ -9,4 +9,6 @@ interface IConsensus {
     function isValidator(address _address) external view returns(bool);
     function getDelegatorsForRewardDistribution(address _validator, uint256 _rewardAmount) external view returns(address[], uint256[]);
     function isFinalized() external view returns(bool);
+    function stakeAmount(address _address) external view returns(uint256);
+    function totalStakeAmount() external view returns(uint256);
 }

test/contracts/BlockRewardMock.sol → contracts/test/BlockRewardMock.sol

@@ -1,6 +1,6 @@
 pragma solidity ^0.4.24;
 
-import "../../contracts/BlockReward.sol";
+import "../BlockReward.sol";
 
 contract BlockRewardMock is BlockReward {
   function setSystemAddressMock(address _newAddress) public onlyOwner {
@@ -18,4 +18,8 @@ contract BlockRewardMock is BlockReward {
   function setShouldEmitRewardedOnCycleMock(bool _status) public {
     boolStorage[SHOULD_EMIT_REWARDED_ON_CYCLE] = _status;
   }
+
+  function cycleMock() public {
+    IConsensus(ProxyStorage(getProxyStorage()).getConsensus()).cycle();
+  }
 }