Various fixes and formatting chores (#885)

* fix: clean up solium linting errors

* fix: make various contracts natspec compliant

* fix: this.balance deprecated; convert to address(this).balance

* fix: contract.call deprecated and switch to gasleft()

* fix: ignore empty block rule project-wide

* fix: add ignore cases for the rest of the linting warnings

.soliumrc.json

@@ -3,6 +3,7 @@
   "plugins": ["security"],
   "rules": {
     "quotes": ["error", "double"],
+    "no-empty-blocks": "off",
     "indentation": ["error", 2],
     "arg-overflow": ["warning", 3],
     "security/enforce-explicit-visibility": ["error"],

contracts/Bounty.sol

@@ -43,7 +43,7 @@ contract Bounty is PullPayment, Destructible {
     require(researcher != 0);
     // Check Target contract invariants
     require(!target.checkInvariant());
-    asyncSend(researcher, this.balance);
+    asyncSend(researcher, address(this).balance);
     claimed = true;
   }
 

contracts/DayLimit.sol

@@ -61,6 +61,7 @@ contract DayLimit {
    * @return uint256 of today's index.
    */
   function today() private view returns (uint256) {
+    // solium-disable-next-line security/no-block-members
     return block.timestamp / 1 days;
   }
 

contracts/ECRecovery.sol

@@ -47,6 +47,7 @@ library ECRecovery {
     if (v != 27 && v != 28) {
       return (address(0));
     } else {
+      // solium-disable-next-line arg-overflow
       return ecrecover(hash, v, r, s);
     }
   }

contracts/LimitBalance.sol

@@ -23,7 +23,7 @@ contract LimitBalance {
    * @dev Checks if limit was reached. Case true, it throws.
    */
   modifier limitedPayable() {
-    require(this.balance <= limit);
+    require(address(this).balance <= limit);
     _;
 
   }

contracts/ReentrancyGuard.sol

@@ -12,7 +12,7 @@ contract ReentrancyGuard {
   /**
    * @dev We use a single lock for the whole contract.
    */
-  bool private reentrancy_lock = false;
+  bool private reentrancyLock = false;
 
   /**
    * @dev Prevents a contract from calling itself, directly or indirectly.
@@ -23,10 +23,10 @@ contract ReentrancyGuard {
    * wrapper marked as `nonReentrant`.
    */
   modifier nonReentrant() {
-    require(!reentrancy_lock);
-    reentrancy_lock = true;
+    require(!reentrancyLock);
+    reentrancyLock = true;
     _;
-    reentrancy_lock = false;
+    reentrancyLock = false;
   }
 
 }

contracts/crowdsale/Crowdsale.sol

@@ -82,7 +82,12 @@ contract Crowdsale {
     weiRaised = weiRaised.add(weiAmount);
 
     _processPurchase(_beneficiary, tokens);
-    emit TokenPurchase(msg.sender, _beneficiary, weiAmount, tokens);
+    emit TokenPurchase(
+      msg.sender,
+      _beneficiary,
+      weiAmount,
+      tokens
+    );
 
     _updatePurchasingState(_beneficiary, weiAmount);
 

contracts/crowdsale/distribution/utils/RefundVault.sol

@@ -44,7 +44,7 @@ contract RefundVault is Ownable {
     require(state == State.Active);
     state = State.Closed;
     emit Closed();
-    wallet.transfer(this.balance);
+    wallet.transfer(address(this).balance);
   }
 
   function enableRefunds() onlyOwner public {

contracts/crowdsale/price/IncreasingPriceCrowdsale.sol

@@ -34,6 +34,7 @@ contract IncreasingPriceCrowdsale is TimedCrowdsale {
    * @return The number of tokens a buyer gets per wei at a given time
    */
   function getCurrentRate() public view returns (uint256) {
+    // solium-disable-next-line security/no-block-members
     uint256 elapsedTime = block.timestamp.sub(openingTime);
     uint256 timeRange = closingTime.sub(openingTime);
     uint256 rateRange = initialRate.sub(finalRate);

contracts/crowdsale/validation/TimedCrowdsale.sol

@@ -18,6 +18,7 @@ contract TimedCrowdsale is Crowdsale {
    * @dev Reverts if not in crowdsale time range.
    */
   modifier onlyWhileOpen {
+    // solium-disable-next-line security/no-block-members
     require(block.timestamp >= openingTime && block.timestamp <= closingTime);
     _;
   }
@@ -28,6 +29,7 @@ contract TimedCrowdsale is Crowdsale {
    * @param _closingTime Crowdsale closing time
    */
   function TimedCrowdsale(uint256 _openingTime, uint256 _closingTime) public {
+    // solium-disable-next-line security/no-block-members
     require(_openingTime >= block.timestamp);
     require(_closingTime >= _openingTime);
 
@@ -40,6 +42,7 @@ contract TimedCrowdsale is Crowdsale {
    * @return Whether crowdsale period has elapsed
    */
   function hasClosed() public view returns (bool) {
+    // solium-disable-next-line security/no-block-members
     return block.timestamp > closingTime;
   }
 

contracts/examples/SimpleSavingsWallet.sol

@@ -25,7 +25,7 @@ contract SimpleSavingsWallet is Heritable {
    * @dev wallet can receive funds.
    */
   function () public payable {
-    emit Received(msg.sender, msg.value, this.balance);
+    emit Received(msg.sender, msg.value, address(this).balance);
   }
 
   /**
@@ -35,6 +35,6 @@ contract SimpleSavingsWallet is Heritable {
     require(payee != 0 && payee != address(this));
     require(amount > 0);
     payee.transfer(amount);
-    emit Sent(payee, amount, this.balance);
+    emit Sent(payee, amount, address(this).balance);
   }
 }

contracts/mocks/ERC223TokenMock.sol

@@ -21,6 +21,7 @@ contract ERC223TokenMock is BasicToken {
   {
     transfer(_to, _value);
     bool isContract = false;
+    // solium-disable-next-line security/no-inline-assembly
     assembly {
       isContract := not(iszero(extcodesize(_to)))
     }

contracts/mocks/ERC721ReceiverMock.sol

@@ -14,9 +14,21 @@ contract ERC721ReceiverMock is ERC721Receiver {
     reverts = _reverts;
   }
 
-  function onERC721Received(address _address, uint256 _tokenId, bytes _data) public returns(bytes4) {
+  function onERC721Received(
+    address _address,
+    uint256 _tokenId,
+    bytes _data
+  )
+    public
+    returns(bytes4)
+  {
     require(!reverts);
-    emit Received(_address, _tokenId, _data, msg.gas);
+    emit Received(
+      _address,
+      _tokenId,
+      _data,
+      gasleft() // msg.gas was deprecated in solidityv0.4.21
+    );
     return retval;
   }
 }

contracts/mocks/MessageHelper.sol

@@ -15,6 +15,7 @@ contract MessageHelper {
   }
 
   function call(address to, bytes data) public returns (bool) {
+    // solium-disable-next-line security/no-low-level-calls
     if (to.call(data))
       return true;
     else

contracts/mocks/ReentrancyAttack.sol

@@ -4,6 +4,7 @@ pragma solidity ^0.4.21;
 contract ReentrancyAttack {
 
   function callSender(bytes4 data) public {
+    // solium-disable-next-line security/no-low-level-calls
     require(msg.sender.call(data));
   }
 

contracts/mocks/ReentrancyMock.sol

@@ -27,7 +27,8 @@ contract ReentrancyMock is ReentrancyGuard {
     bytes4 func = bytes4(keccak256("countThisRecursive(uint256)"));
     if (n > 0) {
       count();
-      bool result = this.call(func, n - 1);
+      // solium-disable-next-line security/no-low-level-calls
+      bool result = address(this).call(func, n - 1);
       require(result == true);
     }
   }

contracts/ownership/HasNoEther.sol

@@ -36,6 +36,7 @@ contract HasNoEther is Ownable {
    * @dev Transfer all Ether held by the contract to the owner.
    */
   function reclaimEther() external onlyOwner {
-    assert(owner.send(this.balance));
+    // solium-disable-next-line security/no-send
+    assert(owner.send(address(this).balance));
   }
 }

contracts/ownership/Heritable.sol

@@ -81,6 +81,7 @@ contract Heritable is Ownable {
   function proclaimDeath() public onlyHeir {
     require(ownerLives());
     emit OwnerProclaimedDead(owner, heir_, timeOfDeath_);
+    // solium-disable-next-line security/no-block-members
     timeOfDeath_ = block.timestamp;
   }
 
@@ -97,6 +98,7 @@ contract Heritable is Ownable {
    */
   function claimHeirOwnership() public onlyHeir {
     require(!ownerLives());
+    // solium-disable-next-line security/no-block-members
     require(block.timestamp >= timeOfDeath_ + heartbeatTimeout_);
     emit OwnershipTransferred(owner, heir_);
     emit HeirOwnershipClaimed(owner, heir_);

contracts/payment/PullPayment.sol

@@ -23,7 +23,7 @@ contract PullPayment {
     uint256 payment = payments[payee];
 
     require(payment != 0);
-    require(this.balance >= payment);
+    require(address(this).balance >= payment);
 
     totalPayments = totalPayments.sub(payment);
     payments[payee] = 0;

contracts/payment/SplitPayment.sol

@@ -42,11 +42,11 @@ contract SplitPayment {
 
     require(shares[payee] > 0);
 
-    uint256 totalReceived = this.balance.add(totalReleased);
+    uint256 totalReceived = address(this).balance.add(totalReleased);
     uint256 payment = totalReceived.mul(shares[payee]).div(totalShares).sub(released[payee]);
 
     require(payment != 0);
-    require(this.balance >= payment);
+    require(address(this).balance >= payment);
 
     released[payee] = released[payee].add(payment);
     totalReleased = totalReleased.add(payment);

contracts/token/ERC20/TokenTimelock.sol

@@ -21,6 +21,7 @@ contract TokenTimelock {
   uint256 public releaseTime;
 
   function TokenTimelock(ERC20Basic _token, address _beneficiary, uint256 _releaseTime) public {
+    // solium-disable-next-line security/no-block-members
     require(_releaseTime > block.timestamp);
     token = _token;
     beneficiary = _beneficiary;
@@ -31,6 +32,7 @@ contract TokenTimelock {
    * @notice Transfers tokens held by timelock to beneficiary.
    */
   function release() public {
+    // solium-disable-next-line security/no-block-members
     require(block.timestamp >= releaseTime);
 
     uint256 amount = token.balanceOf(this);

contracts/token/ERC20/TokenVesting.sol

+/* solium-disable security/no-block-members */
+
 pragma solidity ^0.4.21;
 
 import "./ERC20Basic.sol";

contracts/token/ERC721/ERC721BasicToken.sol

@@ -167,6 +167,7 @@ contract ERC721BasicToken is ERC721Basic {
     public
     canTransfer(_tokenId)
   {
+    // solium-disable-next-line arg-overflow
     safeTransferFrom(_from, _to, _tokenId, "");
   }
 
@@ -192,6 +193,7 @@ contract ERC721BasicToken is ERC721Basic {
     canTransfer(_tokenId)
   {
     transferFrom(_from, _to, _tokenId);
+    // solium-disable-next-line arg-overflow
     require(checkAndCallSafeTransfer(_from, _to, _tokenId, _data));
   }
 

contracts/token/ERC827/ERC827.sol

@@ -5,16 +5,15 @@ import "../ERC20/ERC20.sol";
 
 
 /**
-   @title ERC827 interface, an extension of ERC20 token standard
-
-   Interface of a ERC827 token, following the ERC20 standard with extra
-   methods to transfer value and data and execute calls in transfers and
-   approvals.
+ * @title ERC827 interface, an extension of ERC20 token standard
+ *
+ * @dev Interface of a ERC827 token, following the ERC20 standard with extra
+ * @dev methods to transfer value and data and execute calls in transfers and
+ * @dev approvals.
  */
 contract ERC827 is ERC20 {
-
-  function approve( address _spender, uint256 _value, bytes _data ) public returns (bool);
-  function transfer( address _to, uint256 _value, bytes _data ) public returns (bool);
+  function approve(address _spender, uint256 _value, bytes _data) public returns (bool);
+  function transfer(address _to, uint256 _value, bytes _data) public returns (bool);
   function transferFrom(
     address _from,
     address _to,
@@ -23,5 +22,4 @@ contract ERC827 is ERC20 {
   )
     public
     returns (bool);
-
 }

contracts/token/ERC827/ERC827Token.sol

+/* solium-disable security/no-low-level-calls */
+
 pragma solidity ^0.4.21;
 
 import "./ERC827.sol";
@@ -5,31 +7,32 @@ import "../ERC20/StandardToken.sol";
 
 
 /**
-   @title ERC827, an extension of ERC20 token standard
-
-   Implementation the ERC827, following the ERC20 standard with extra
-   methods to transfer value and data and execute calls in transfers and
-   approvals.
-   Uses OpenZeppelin StandardToken.
+ * @title ERC827, an extension of ERC20 token standard
+ *
+ * @dev Implementation the ERC827, following the ERC20 standard with extra
+ * @dev methods to transfer value and data and execute calls in transfers and
+ * @dev approvals.
+ *
+ * @dev Uses OpenZeppelin StandardToken.
  */
 contract ERC827Token is ERC827, StandardToken {
 
   /**
-     @dev Addition to ERC20 token methods. It allows to
-     approve the transfer of value and execute a call with the sent data.
-
-     Beware that changing an allowance with this method brings the risk that
-     someone may use both the old and the new allowance by unfortunate
-     transaction ordering. One possible solution to mitigate this race condition
-     is to first reduce the spender's allowance to 0 and set the desired value
-     afterwards:
-     https://github.com/ethereum/EIPs/issues/20#issuecomment-263524729
-
-     @param _spender The address that will spend the funds.
-     @param _value The amount of tokens to be spent.
-     @param _data ABI-encoded contract call to call `_to` address.
-
-     @return true if the call function was executed successfully
+   * @dev Addition to ERC20 token methods. It allows to
+   * @dev approve the transfer of value and execute a call with the sent data.
+   *
+   * @dev Beware that changing an allowance with this method brings the risk that
+   * @dev someone may use both the old and the new allowance by unfortunate
+   * @dev transaction ordering. One possible solution to mitigate this race condition
+   * @dev is to first reduce the spender's allowance to 0 and set the desired value
+   * @dev afterwards:
+   * @dev https://github.com/ethereum/EIPs/issues/20#issuecomment-263524729
+   *
+   * @param _spender The address that will spend the funds.
+   * @param _value The amount of tokens to be spent.
+   * @param _data ABI-encoded contract call to call `_to` address.
+   *
+   * @return true if the call function was executed successfully
    */
   function approve(address _spender, uint256 _value, bytes _data) public returns (bool) {
     require(_spender != address(this));
@@ -42,14 +45,14 @@ contract ERC827Token is ERC827, StandardToken {
   }
 
   /**
-     @dev Addition to ERC20 token methods. Transfer tokens to a specified
-     address and execute a call with the sent data on the same transaction
-
-     @param _to address The address which you want to transfer to
-     @param _value uint256 the amout of tokens to be transfered
-     @param _data ABI-encoded contract call to call `_to` address.
-
-     @return true if the call function was executed successfully
+   * @dev Addition to ERC20 token methods. Transfer tokens to a specified
+   * @dev address and execute a call with the sent data on the same transaction
+   *
+   * @param _to address The address which you want to transfer to
+   * @param _value uint256 the amout of tokens to be transfered
+   * @param _data ABI-encoded contract call to call `_to` address.
+   *
+   * @return true if the call function was executed successfully
    */
   function transfer(address _to, uint256 _value, bytes _data) public returns (bool) {
     require(_to != address(this));
@@ -61,15 +64,15 @@ contract ERC827Token is ERC827, StandardToken {
   }
 
   /**
-     @dev Addition to ERC20 token methods. Transfer tokens from one address to
-     another and make a contract call on the same transaction
-
-     @param _from The address which you want to send tokens from
-     @param _to The address which you want to transfer to
-     @param _value The amout of tokens to be transferred
-     @param _data ABI-encoded contract call to call `_to` address.
-
-     @return true if the call function was executed successfully
+   * @dev Addition to ERC20 token methods. Transfer tokens from one address to
+   * @dev another and make a contract call on the same transaction
+   *
+   * @param _from The address which you want to send tokens from
+   * @param _to The address which you want to transfer to
+   * @param _value The amout of tokens to be transferred
+   * @param _data ABI-encoded contract call to call `_to` address.
+   *
+   * @return true if the call function was executed successfully
    */
   function transferFrom(
     address _from,
@@ -89,12 +92,13 @@ contract ERC827Token is ERC827, StandardToken {
 
   /**
    * @dev Addition to StandardToken methods. Increase the amount of tokens that
-   * an owner allowed to a spender and execute a call with the sent data.
+   * @dev an owner allowed to a spender and execute a call with the sent data.
+   *
+   * @dev approve should be called when allowed[_spender] == 0. To increment
+   * @dev allowed value is better to use this function to avoid 2 calls (and wait until
+   * @dev the first transaction is mined)
+   * @dev From MonolithDAO Token.sol
    *
-   * approve should be called when allowed[_spender] == 0. To increment
-   * allowed value is better to use this function to avoid 2 calls (and wait until
-   * the first transaction is mined)
-   * From MonolithDAO Token.sol
    * @param _spender The address which will spend the funds.
    * @param _addedValue The amount of tokens to increase the allowance by.
    * @param _data ABI-encoded contract call to call `_spender` address.
@@ -111,12 +115,13 @@ contract ERC827Token is ERC827, StandardToken {
 
   /**
    * @dev Addition to StandardToken methods. Decrease the amount of tokens that
-   * an owner allowed to a spender and execute a call with the sent data.
+   * @dev an owner allowed to a spender and execute a call with the sent data.
+   *
+   * @dev approve should be called when allowed[_spender] == 0. To decrement
+   * @dev allowed value is better to use this function to avoid 2 calls (and wait until
+   * @dev the first transaction is mined)
+   * @dev From MonolithDAO Token.sol
    *
-   * approve should be called when allowed[_spender] == 0. To decrement
-   * allowed value is better to use this function to avoid 2 calls (and wait until
-   * the first transaction is mined)
-   * From MonolithDAO Token.sol
    * @param _spender The address which will spend the funds.
    * @param _subtractedValue The amount of tokens to decrease the allowance by.
    * @param _data ABI-encoded contract call to call `_spender` address.