Эх сурвалжийг харах

converted if() throw convention to require()/assert()/revert()

Peter Murray 8 жил өмнө
parent
commit
18581f138f

+ 8 - 14
contracts/Bounty.sol

@@ -19,13 +19,11 @@ contract Bounty is PullPayment, Destructible {
    * @dev Fallback function allowing the contract to recieve funds, if they haven't already been claimed.
    */
   function() payable {
-    if (claimed) {
-      throw;
-    }
+    require(!claimed);
   }
 
   /**
-   * @dev Create and deploy the target contract (extension of Target contract), and sets the 
+   * @dev Create and deploy the target contract (extension of Target contract), and sets the
    * msg.sender as a researcher
    * @return A target contract
    */
@@ -48,13 +46,9 @@ contract Bounty is PullPayment, Destructible {
    */
   function claim(Target target) {
     address researcher = researchers[target];
-    if (researcher == 0) {
-      throw;
-    }
+    require(researcher != 0);
     // Check Target contract invariants
-    if (target.checkInvariant()) {
-      throw;
-    }
+    require(!target.checkInvariant());
     asyncSend(researcher, this.balance);
     claimed = true;
   }
@@ -69,10 +63,10 @@ contract Bounty is PullPayment, Destructible {
 contract Target {
 
    /**
-    * @dev Checks all values a contract assumes to be true all the time. If this function returns 
-    * false, the contract is broken in some way and is in an inconsistent state. 
-    * In order to win the bounty, security researchers will try to cause this broken state. 
-    * @return True if all invariant values are correct, false otherwise. 
+    * @dev Checks all values a contract assumes to be true all the time. If this function returns
+    * false, the contract is broken in some way and is in an inconsistent state.
+    * In order to win the bounty, security researchers will try to cause this broken state.
+    * @return True if all invariant values are correct, false otherwise.
     */
   function checkInvariant() returns(bool);
 }

+ 1 - 3
contracts/DayLimit.sol

@@ -67,9 +67,7 @@ contract DayLimit {
    * @dev Simple modifier for daily limit.
    */
   modifier limitedDaily(uint256 _value) {
-    if (!underLimit(_value)) {
-      throw;
-    }
+    require(underLimit(_value));
     _;
   }
 }

+ 2 - 4
contracts/LimitBalance.sol

@@ -12,7 +12,7 @@ contract LimitBalance {
   uint256 public limit;
 
   /**
-   * @dev Constructor that sets the passed value as a limit. 
+   * @dev Constructor that sets the passed value as a limit.
    * @param _limit uint256 to represent the limit.
    */
   function LimitBalance(uint256 _limit) {
@@ -23,9 +23,7 @@ contract LimitBalance {
    * @dev Checks if limit was reached. Case true, it throws.
    */
   modifier limitedPayable() {
-    if (this.balance > limit) {
-      throw;
-    }
+    require(this.balance <= limit);
     _;
 
   }

+ 17 - 19
contracts/MultisigWallet.sol

@@ -25,19 +25,19 @@ contract MultisigWallet is Multisig, Shareable, DayLimit {
    * @param _owners A list of owners.
    * @param _required The amount required for a transaction to be approved.
    */
-  function MultisigWallet(address[] _owners, uint256 _required, uint256 _daylimit)       
-    Shareable(_owners, _required)        
+  function MultisigWallet(address[] _owners, uint256 _required, uint256 _daylimit)
+    Shareable(_owners, _required)
     DayLimit(_daylimit) { }
 
-  /** 
-   * @dev destroys the contract sending everything to `_to`. 
+  /**
+   * @dev destroys the contract sending everything to `_to`.
    */
   function destroy(address _to) onlymanyowners(keccak256(msg.data)) external {
     selfdestruct(_to);
   }
 
-  /** 
-   * @dev Fallback function, receives value and emits a deposit event. 
+  /**
+   * @dev Fallback function, receives value and emits a deposit event.
    */
   function() payable {
     // just being sent some cash?
@@ -46,10 +46,10 @@ contract MultisigWallet is Multisig, Shareable, DayLimit {
   }
 
   /**
-   * @dev Outside-visible transaction entry point. Executes transaction immediately if below daily 
-   * spending limit. If not, goes into multisig process. We provide a hash on return to allow the 
-   * sender to provide shortcuts for the other confirmations (allowing them to avoid replicating 
-   * the _to, _value, and _data arguments). They still get the option of using them if they want, 
+   * @dev Outside-visible transaction entry point. Executes transaction immediately if below daily
+   * spending limit. If not, goes into multisig process. We provide a hash on return to allow the
+   * sender to provide shortcuts for the other confirmations (allowing them to avoid replicating
+   * the _to, _value, and _data arguments). They still get the option of using them if they want,
    * anyways.
    * @param _to The receiver address
    * @param _value The value to send
@@ -61,7 +61,7 @@ contract MultisigWallet is Multisig, Shareable, DayLimit {
       SingleTransact(msg.sender, _value, _to, _data);
       // yes - just execute the call.
       if (!_to.call.value(_value)(_data)) {
-        throw;
+        revert();
       }
       return 0;
     }
@@ -76,30 +76,28 @@ contract MultisigWallet is Multisig, Shareable, DayLimit {
   }
 
   /**
-   * @dev Confirm a transaction by providing just the hash. We use the previous transactions map, 
+   * @dev Confirm a transaction by providing just the hash. We use the previous transactions map,
    * txs, in order to determine the body of the transaction from the hash provided.
    * @param _h The transaction hash to approve.
    */
   function confirm(bytes32 _h) onlymanyowners(_h) returns (bool) {
     if (txs[_h].to != 0) {
-      if (!txs[_h].to.call.value(txs[_h].value)(txs[_h].data)) {
-        throw;
-      }
+      assert(txs[_h].to.call.value(txs[_h].value)(txs[_h].data));
       MultiTransact(msg.sender, _h, txs[_h].value, txs[_h].to, txs[_h].data);
       delete txs[_h];
       return true;
     }
   }
 
-  /** 
-   * @dev Updates the daily limit value. 
+  /**
+   * @dev Updates the daily limit value.
    * @param _newLimit  uint256 to represent the new limit.
    */
   function setDailyLimit(uint256 _newLimit) onlymanyowners(keccak256(msg.data)) external {
     _setDailyLimit(_newLimit);
   }
 
-  /** 
+  /**
    * @dev Resets the value spent to enable more spending
    */
   function resetSpentToday() onlymanyowners(keccak256(msg.data)) external {
@@ -108,7 +106,7 @@ contract MultisigWallet is Multisig, Shareable, DayLimit {
 
 
   // INTERNAL METHODS
-  /** 
+  /**
    * @dev Clears the list of transactions pending approval.
    */
   function clearPending() internal {

+ 5 - 8
contracts/ReentrancyGuard.sol

@@ -9,7 +9,7 @@ pragma solidity ^0.4.11;
 contract ReentrancyGuard {
 
   /**
-   * @dev We use a single lock for the whole contract. 
+   * @dev We use a single lock for the whole contract.
    */
   bool private rentrancy_lock = false;
 
@@ -22,13 +22,10 @@ contract ReentrancyGuard {
    * wrapper marked as `nonReentrant`.
    */
   modifier nonReentrant() {
-    if(rentrancy_lock == false) {
-      rentrancy_lock = true;
-      _;
-      rentrancy_lock = false;
-    } else {
-      throw;
-    }
+    require(!rentrancy_lock);
+    rentrancy_lock = true;
+    _;
+    rentrancy_lock = false;
   }
 
 }

+ 2 - 2
contracts/lifecycle/Pausable.sol

@@ -19,7 +19,7 @@ contract Pausable is Ownable {
    * @dev modifier to allow actions only when the contract IS paused
    */
   modifier whenNotPaused() {
-    if (paused) throw;
+    require(!paused);
     _;
   }
 
@@ -27,7 +27,7 @@ contract Pausable is Ownable {
    * @dev modifier to allow actions only when the contract IS NOT paused
    */
   modifier whenPaused {
-    if (!paused) throw;
+    require(paused);
     _;
   }
 

+ 5 - 7
contracts/ownership/Claimable.sol

@@ -6,25 +6,23 @@ import './Ownable.sol';
 
 /**
  * @title Claimable
- * @dev Extension for the Ownable contract, where the ownership needs to be claimed. 
+ * @dev Extension for the Ownable contract, where the ownership needs to be claimed.
  * This allows the new owner to accept the transfer.
  */
 contract Claimable is Ownable {
   address public pendingOwner;
 
   /**
-   * @dev Modifier throws if called by any account other than the pendingOwner. 
+   * @dev Modifier throws if called by any account other than the pendingOwner.
    */
   modifier onlyPendingOwner() {
-    if (msg.sender != pendingOwner) {
-      throw;
-    }
+    require(msg.sender == pendingOwner);
     _;
   }
 
   /**
-   * @dev Allows the current owner to set the pendingOwner address. 
-   * @param newOwner The address to transfer ownership to. 
+   * @dev Allows the current owner to set the pendingOwner address.
+   * @param newOwner The address to transfer ownership to.
    */
   function transferOwnership(address newOwner) onlyOwner {
     pendingOwner = newOwner;

+ 7 - 9
contracts/ownership/DelayedClaimable.sol

@@ -15,26 +15,24 @@ contract DelayedClaimable is Claimable {
   uint256 public start;
 
   /**
-   * @dev Used to specify the time period during which a pending 
-   * owner can claim ownership. 
+   * @dev Used to specify the time period during which a pending
+   * owner can claim ownership.
    * @param _start The earliest time ownership can be claimed.
-   * @param _end The latest time ownership can be claimed. 
+   * @param _end The latest time ownership can be claimed.
    */
   function setLimits(uint256 _start, uint256 _end) onlyOwner {
-    if (_start > _end)
-        throw;
+    require(_start <= _end);
     end = _end;
     start = _start;
   }
 
 
   /**
-   * @dev Allows the pendingOwner address to finalize the transfer, as long as it is called within 
-   * the specified start and end time. 
+   * @dev Allows the pendingOwner address to finalize the transfer, as long as it is called within
+   * the specified start and end time.
    */
   function claimOwnership() onlyPendingOwner {
-    if ((block.number > end) || (block.number < start))
-        throw;
+    require((block.number <= end) && (block.number >= start));
     owner = pendingOwner;
     pendingOwner = 0x0;
     end = 0;

+ 6 - 10
contracts/ownership/HasNoEther.sol

@@ -2,7 +2,7 @@ pragma solidity ^0.4.11;
 
 import "./Ownable.sol";
 
-/** 
+/**
  * @title Contracts that should not own Ether
  * @author Remco Bloemen <remco@2π.com>
  * @dev This tries to block incoming ether to prevent accidental loss of Ether. Should Ether end up
@@ -16,15 +16,13 @@ contract HasNoEther is Ownable {
 
   /**
   * @dev Constructor that rejects incoming Ether
-  * @dev The `payable` flag is added so we can access `msg.value` without compiler warning. If we 
-  * leave out payable, then Solidity will allow inheriting contracts to implement a payable 
-  * constructor. By doing it this way we prevent a payable constructor from working. Alternatively 
+  * @dev The `payable` flag is added so we can access `msg.value` without compiler warning. If we
+  * leave out payable, then Solidity will allow inheriting contracts to implement a payable
+  * constructor. By doing it this way we prevent a payable constructor from working. Alternatively
   * we could use assembly to access msg.value.
   */
   function HasNoEther() payable {
-    if(msg.value > 0) {
-      throw;
-    }
+    require(msg.value == 0);
   }
 
   /**
@@ -37,8 +35,6 @@ contract HasNoEther is Ownable {
    * @dev Transfer all Ether held by the contract to the owner.
    */
   function reclaimEther() external onlyOwner {
-    if(!owner.send(this.balance)) {
-      throw;
-    }
+    assert(owner.send(this.balance));
   }
 }

+ 3 - 3
contracts/ownership/HasNoTokens.sol

@@ -3,7 +3,7 @@ pragma solidity ^0.4.11;
 import "./Ownable.sol";
 import "../token/ERC20Basic.sol";
 
-/** 
+/**
  * @title Contracts that should not own Tokens
  * @author Remco Bloemen <remco@2π.com>
  * @dev This blocks incoming ERC23 tokens to prevent accidental loss of tokens.
@@ -12,14 +12,14 @@ import "../token/ERC20Basic.sol";
  */
 contract HasNoTokens is Ownable {
 
- /** 
+ /**
   * @dev Reject all ERC23 compatible tokens
   * @param from_ address The address that is transferring the tokens
   * @param value_ uint256 the amount of the specified token
   * @param data_ Bytes The data passed from the caller.
   */
   function tokenFallback(address from_, uint256 value_, bytes data_) external {
-    throw;
+    revert();
   }
 
   /**

+ 6 - 8
contracts/ownership/Ownable.sol

@@ -3,14 +3,14 @@ pragma solidity ^0.4.11;
 
 /**
  * @title Ownable
- * @dev The Ownable contract has an owner address, and provides basic authorization control 
- * functions, this simplifies the implementation of "user permissions". 
+ * @dev The Ownable contract has an owner address, and provides basic authorization control
+ * functions, this simplifies the implementation of "user permissions".
  */
 contract Ownable {
   address public owner;
 
 
-  /** 
+  /**
    * @dev The Ownable constructor sets the original `owner` of the contract to the sender
    * account.
    */
@@ -20,19 +20,17 @@ contract Ownable {
 
 
   /**
-   * @dev Throws if called by any account other than the owner. 
+   * @dev Throws if called by any account other than the owner.
    */
   modifier onlyOwner() {
-    if (msg.sender != owner) {
-      throw;
-    }
+    require(msg.sender == owner);
     _;
   }
 
 
   /**
    * @dev Allows the current owner to transfer control of the contract to a newOwner.
-   * @param newOwner The address to transfer ownership to. 
+   * @param newOwner The address to transfer ownership to.
    */
   function transferOwnership(address newOwner) onlyOwner {
     if (newOwner != address(0)) {

+ 9 - 15
contracts/ownership/Shareable.sol

@@ -3,7 +3,7 @@ pragma solidity ^0.4.11;
 
 /**
  * @title Shareable
- * @dev inheritable "property" contract that enables methods to be protected by requiring the 
+ * @dev inheritable "property" contract that enables methods to be protected by requiring the
  * acquiescence of either a single, or, crucially, each of a number of, designated owners.
  * @dev Usage: use modifiers onlyowner (just own owned) or onlymanyowners(hash), whereby the same hash must be provided by some number (specified in constructor) of the set of owners (specified in the constructor) before the interior is executed.
  */
@@ -36,14 +36,12 @@ contract Shareable {
 
   // simple single-sig function modifier.
   modifier onlyOwner {
-    if (!isOwner(msg.sender)) {
-      throw;
-    }
+    require(isOwner(msg.sender));
     _;
   }
-  
-  /** 
-   * @dev Modifier for multisig functions. 
+
+  /**
+   * @dev Modifier for multisig functions.
    * @param _operation The operation must have an intrinsic hash in order that later attempts can be
    * realised as the same underlying operation and thus count as confirmations.
    */
@@ -53,8 +51,8 @@ contract Shareable {
     }
   }
 
-  /** 
-   * @dev Constructor is given the number of sigs required to do protected "onlymanyowners" 
+  /**
+   * @dev Constructor is given the number of sigs required to do protected "onlymanyowners"
    * transactions as well as the selection of addresses capable of confirming them.
    * @param _owners A list of owners.
    * @param _required The amount required for a transaction to be approved.
@@ -67,9 +65,7 @@ contract Shareable {
       ownerIndex[_owners[i]] = 2 + i;
     }
     required = _required;
-    if (required > owners.length) {
-      throw;
-    }
+    require(required <= owners.length);
   }
 
   /**
@@ -138,9 +134,7 @@ contract Shareable {
     // determine what index the present sender is:
     uint256 index = ownerIndex[msg.sender];
     // make sure they're an owner
-    if (index == 0) {
-      throw;
-    }
+    require(index != 0);
 
     var pending = pendings[_operation];
     // if we're not yet working on this operation, switch over and reset the confirmation status.

+ 3 - 10
contracts/payment/PullPayment.sol

@@ -32,19 +32,12 @@ contract PullPayment {
     address payee = msg.sender;
     uint256 payment = payments[payee];
 
-    if (payment == 0) {
-      throw;
-    }
-
-    if (this.balance < payment) {
-      throw;
-    }
+    require(payment != 0);
+    require(this.balance >= payment);
 
     totalPayments = totalPayments.sub(payment);
     payments[payee] = 0;
 
-    if (!payee.send(payment)) {
-      throw;
-    }
+    assert(payee.send(payment));
   }
 }

+ 6 - 6
contracts/token/LimitedTransferToken.sol

@@ -4,11 +4,11 @@ import "./ERC20.sol";
 
 /**
  * @title LimitedTransferToken
- * @dev LimitedTransferToken defines the generic interface and the implementation to limit token 
- * transferability for different events. It is intended to be used as a base class for other token 
- * contracts. 
+ * @dev LimitedTransferToken defines the generic interface and the implementation to limit token
+ * transferability for different events. It is intended to be used as a base class for other token
+ * contracts.
  * LimitedTransferToken has been designed to allow for different limiting factors,
- * this can be achieved by recursively calling super.transferableTokens() until the base class is 
+ * this can be achieved by recursively calling super.transferableTokens() until the base class is
  * hit. For example:
  *     function transferableTokens(address holder, uint64 time) constant public returns (uint256) {
  *       return min256(unlockedTokens, super.transferableTokens(holder, time));
@@ -23,7 +23,7 @@ contract LimitedTransferToken is ERC20 {
    * @dev Checks whether it can transfer or otherwise throws.
    */
   modifier canTransfer(address _sender, uint256 _value) {
-   if (_value > transferableTokens(_sender, uint64(now))) throw;
+   require(_value <= transferableTokens(_sender, uint64(now)));
    _;
   }
 
@@ -48,7 +48,7 @@ contract LimitedTransferToken is ERC20 {
 
   /**
    * @dev Default transferable tokens function returns all tokens for a holder (no limit).
-   * @dev Overwriting transferableTokens(address holder, uint64 time) is the way to provide the 
+   * @dev Overwriting transferableTokens(address holder, uint64 time) is the way to provide the
    * specific logic for limiting token transferability for a holder over time.
    */
   function transferableTokens(address holder, uint64 time) constant public returns (uint256) {

+ 1 - 1
contracts/token/MintableToken.sol

@@ -21,7 +21,7 @@ contract MintableToken is StandardToken, Ownable {
 
 
   modifier canMint() {
-    if(mintingFinished) throw;
+    require(!mintingFinished);
     _;
   }
 

+ 2 - 2
contracts/token/StandardToken.sol

@@ -27,7 +27,7 @@ contract StandardToken is ERC20, BasicToken {
     var _allowance = allowed[_from][msg.sender];
 
     // Check is not needed because sub(_allowance, _value) will already throw if this condition is not met
-    // if (_value > _allowance) throw;
+    // require (_value <= _allowance);
 
     balances[_to] = balances[_to].add(_value);
     balances[_from] = balances[_from].sub(_value);
@@ -47,7 +47,7 @@ contract StandardToken is ERC20, BasicToken {
     //  allowance to zero by calling `approve(_spender, 0)` if it is not
     //  already 0 to mitigate the race condition described here:
     //  https://github.com/ethereum/EIPs/issues/20#issuecomment-263524729
-    if ((_value != 0) && (allowed[msg.sender][_spender] != 0)) throw;
+    require((_value == 0) || (allowed[msg.sender][_spender] == 0));
 
     allowed[msg.sender][_spender] = _value;
     Approval(msg.sender, _spender, _value);

+ 4 - 11
contracts/token/VestedToken.sol

@@ -45,11 +45,9 @@ contract VestedToken is StandardToken, LimitedTransferToken {
   ) public {
 
     // Check for date inconsistencies that may cause unexpected behavior
-    if (_cliff < _start || _vesting < _cliff) {
-      throw;
-    }
+    require(_cliff >= _start && _vesting >= _cliff);
 
-    if (tokenGrantsCount(_to) > MAX_GRANTS_PER_ADDRESS) throw;   // To prevent a user being spammed and have his balance locked (out of gas attack when calculating vesting).
+    require(tokenGrantsCount(_to) <= MAX_GRANTS_PER_ADDRESS);   // To prevent a user being spammed and have his balance locked (out of gas attack when calculating vesting).
 
     uint256 count = grants[_to].push(
                 TokenGrant(
@@ -76,13 +74,8 @@ contract VestedToken is StandardToken, LimitedTransferToken {
   function revokeTokenGrant(address _holder, uint256 _grantId) public {
     TokenGrant grant = grants[_holder][_grantId];
 
-    if (!grant.revokable) { // Check if grant was revokable
-      throw;
-    }
-
-    if (grant.granter != msg.sender) { // Only granter can revoke it
-      throw;
-    }
+    require(grant.revokable);
+    require(grant.granter == msg.sender); // Only granter can revoke it
 
     address receiver = grant.burnsOnRevoke ? 0xdead : msg.sender;