Merge branch 'release/2.0' into feature/truffle4-plus-solidity-0.5

.solcover.js

@@ -10,9 +10,10 @@ module.exports = {
     'Oracle/DSThing.sol',
     'Oracle/DSValue.sol',
     'Oracle/Medianizer.sol',
-    'Oracle/PriceFeed.sol'
+    'Oracle/PriceFeed.sol',
+    'ForTestingOnly'
   ],
   testrpcOptions: '--port 8556 --account=0x4f3edf983ac636a65a842ce7c78d9aa706d3b113bce9c46f30d7d21715b23b1d,50000000000000000000000 --account=0x6cbed15c793ce57650b9877cf6fa156fbef513c4e6134f022a85b1ffdd59b2a1,50000000000000000000000 --account=0x6370fd033278c143179d81c5526140625662b8daa446c22ee2d73db3707e620c,50000000000000000000000 --account=0x646f1ce2fdad0e6deeeb5c7e8e5543bdde65e86029e2fd9fc169899c440a7913,50000000000000000000000 --account=0xadd53f9a7e588d003326d1cbf9e4a43c061aadd9bc938c843a79e7b4fd2ad743,50000000000000000000000 --account=0x395df67f0c2d2d9fe1ad08d1bc8b6627011959b79c53d7dd6a3536a33ab8a4fd,50000000000000000000000 --account=0xe485d098507f54e7733a205420dfddbe58db035fa577fc294ebd14db90767a52,50000000000000000000000',
   testCommand: 'truffle test -s',
   copyPackages: ['@gnosis.pm'],
-};
+};

contracts/DutchExchange.sol

@@ -2,6 +2,7 @@ pragma solidity ^0.5.2;
 
 import "./TokenFRT.sol";
 import "@gnosis.pm/owl-token/contracts/TokenOWL.sol";
+import "./base/SafeTransfer.sol";
 import "./base/TokenWhitelist.sol";
 import "./base/DxMath.sol";
 import "./base/EthOracle.sol";
@@ -11,7 +12,8 @@ import "./base/DxUpgrade.sol";
 /// @author Alex Herrmann - <alex@gnosis.pm>
 /// @author Dominik Teiml - <dominik@gnosis.pm>
 
-contract DutchExchange is DxUpgrade, TokenWhitelist, EthOracle {
+contract DutchExchange is DxUpgrade, TokenWhitelist, EthOracle, SafeTransfer {
+
     // The price is a rational number, so we need a concept of a fraction
     struct fraction {
         uint num;
@@ -238,10 +240,7 @@ contract DutchExchange is DxUpgrade, TokenWhitelist, EthOracle {
 
     function deposit(address tokenAddress, uint amount) public returns (uint) {
         // R1
-        require(
-            Token(tokenAddress).transferFrom(msg.sender, address(this), amount),
-            "The deposit transaction must succeed"
-        );
+        require(safeTransfer(tokenAddress, msg.sender, amount, true), "The deposit transaction must succeed");
 
         uint newBal = add(balances[tokenAddress][msg.sender], amount);
 
@@ -263,7 +262,7 @@ contract DutchExchange is DxUpgrade, TokenWhitelist, EthOracle {
         balances[tokenAddress][msg.sender] = newBal;
 
         // R2
-        require(Token(tokenAddress).transfer(msg.sender, amount), "The withdraw transfer must succeed");
+        require(safeTransfer(tokenAddress, msg.sender, amount, false), "The withdraw transfer must succeed");
         emit NewWithdrawal(tokenAddress, amount);
 
         return newBal;

contracts/ForTestingOnly/BadToken.sol

@@ -0,0 +1,23 @@
+/// Implements ERC 20 Token standard: https://github.com/ethereum/EIPs/blob/master/EIPS/eip-20-token-standard.md
+pragma solidity ^0.4.21;
+
+
+/// @title Abstract token contract - Functions to be implemented by token contracts
+contract BadToken {
+
+    /*
+     *  Events
+     */
+    event Transfer(address indexed from, address indexed to, uint value);
+    event Approval(address indexed owner, address indexed spender, uint value);
+
+    /*
+     *  Public functions
+     */
+    function transfer(address to, uint value) public;
+    function transferFrom(address from, address to, uint value) public;
+    function approve(address spender, uint value) public returns (bool);
+    function balanceOf(address owner) public view returns (uint);
+    function allowance(address owner, address spender) public view returns (uint);
+    function totalSupply() public view returns (uint);
+}

contracts/ForTestingOnly/SubStandardToken.sol

@@ -0,0 +1,98 @@
+pragma solidity ^0.4.21;
+import "./BadToken.sol";
+import "@gnosis.pm/util-contracts/contracts/Math.sol";
+import "@gnosis.pm/util-contracts/contracts/Proxy.sol";
+import {StandardTokenData} from "@gnosis.pm/util-contracts/contracts/GnosisStandardToken.sol";
+
+/// @title Standard token contract with overflow protection
+contract SubStandardToken is BadToken, StandardTokenData {
+    using Math for *;
+
+    /*
+     *  Public functions
+     */
+    /// @dev Transfers sender's tokens to a given address. Returns success
+    /// @param to Address of token receiver
+    /// @param value Number of tokens to transfer
+    /// @return Was transfer successful?
+    function transfer(address to, uint value)
+        public
+        // returns (bool)
+    {
+        if (   !balances[msg.sender].safeToSub(value)
+            || !balances[to].safeToAdd(value))
+            return;
+            // return false;
+        balances[msg.sender] -= value;
+        balances[to] += value;
+        emit Transfer(msg.sender, to, value);
+        // return true;
+    }
+
+    /// @dev Allows allowed third party to transfer tokens from one address to another. Returns success
+    /// @param from Address from where tokens are withdrawn
+    /// @param to Address to where tokens are sent
+    /// @param value Number of tokens to transfer
+    /// @return Was transfer successful?
+    function transferFrom(address from, address to, uint value)
+        public
+        // returns (bool)
+    {
+        if (   !balances[from].safeToSub(value)
+            || !allowances[from][msg.sender].safeToSub(value)
+            || !balances[to].safeToAdd(value))
+            return;
+            // return false;
+        balances[from] -= value;
+        allowances[from][msg.sender] -= value;
+        balances[to] += value;
+        emit Transfer(from, to, value);
+        // return true;
+    }
+
+    /// @dev Sets approved amount of tokens for spender. Returns success
+    /// @param spender Address of allowed account
+    /// @param value Number of approved tokens
+    /// @return Was approval successful?
+    function approve(address spender, uint value)
+        public
+        returns (bool)
+    {
+        allowances[msg.sender][spender] = value;
+        emit Approval(msg.sender, spender, value);
+        return true;
+    }
+
+    /// @dev Returns number of allowed tokens for given address
+    /// @param owner Address of token owner
+    /// @param spender Address of token spender
+    /// @return Remaining allowance for spender
+    function allowance(address owner, address spender)
+        public
+        view
+        returns (uint)
+    {
+        return allowances[owner][spender];
+    }
+
+    /// @dev Returns number of tokens owned by given address
+    /// @param owner Address of token owner
+    /// @return Balance of owner
+    function balanceOf(address owner)
+        public
+        view
+        returns (uint)
+    {
+        return balances[owner];
+    }
+
+    /// @dev Returns total supply of tokens
+    /// @return Total supply
+    function totalSupply()
+        public
+        view
+        returns (uint)
+    {
+        return totalTokens;
+    }
+}

contracts/ForTestingOnly/TokenGNO.sol

@@ -0,0 +1,18 @@
+pragma solidity ^0.4.21;
+
+import "./SubStandardToken.sol";
+
+contract BadGNO is SubStandardToken {
+    string public constant symbol = "GNO";
+    string public constant name = "Gnosis";
+    uint8 public constant decimals = 18;
+
+    function BadGNO(
+    	uint amount
+    )
+    	public 
+    {
+        totalTokens = amount;
+    	balances[msg.sender] = amount;
+    }
+}

contracts/base/SafeTransfer.sol

@@ -0,0 +1,34 @@
+
+pragma solidity ^0.4.21;
+
+import "@gnosis.pm/util-contracts/contracts/Token.sol";
+
+interface BadToken {
+    function transfer(address to, uint value) external;
+    function transferFrom(address from, address to, uint value) external;
+}
+
+contract SafeTransfer {
+    function safeTransfer(address token, address to, uint value, bool from) internal returns (bool result) {
+        if (from) {
+            BadToken(token).transferFrom(msg.sender, address(this), value);
+        } else {
+            BadToken(token).transfer(to, value);
+        }
+
+        assembly {
+          switch returndatasize()   
+            case 0 {                      // This is our BadToken
+              result := not(0)          // result is true
+            }
+            case 32 {                     // This is our GoodToken
+              returndatacopy(0, 0, 32) 
+              result := mload(0)        // result == returndata of external call
+            }
+            default {                     // This is not an ERC20 token
+              result := 0 
+            }
+        }
+        return result;
+    }
+}

test/dutchExchange-depositWithdrawBadToken.js

@@ -0,0 +1,70 @@
+const {
+  log: utilsLog,
+  gasLogger
+} = require('./utils')
+
+const { getContracts } = require('./testFunctions')
+
+const BadGNO = artifacts.require('BadGNO')
+
+// Test VARS
+let dx, badGNO
+let contracts
+
+const separateLogs = () => utilsLog('\n    ----------------------------------')
+const log = (...args) => utilsLog('\t', ...args)
+
+contract('DutchExchange - addTokenPair', accounts => {
+  const [master, seller1] = accounts
+
+  const startBal = {
+    startingETH: 90.0.toWei(),
+    startingGNO: 90.0.toWei(),
+    ethUSDPrice: 1008.0.toWei(),
+    sellingAmount: 50.0.toWei()
+  }
+
+  beforeEach(separateLogs)
+  afterEach(gasLogger)
+
+  before(async () => {
+    // get contracts
+    contracts = await getContracts();
+    // destructure contracts into upper state
+    ({
+      DutchExchange: dx
+    } = contracts)
+
+    badGNO = await BadGNO.new(1e22, { from: master })
+    await Promise.all([
+      badGNO.transfer(seller1, startBal.startingGNO, { from: master }),
+      badGNO.approve(dx.address, startBal.startingGNO, { from: seller1 })
+    ])
+  })
+
+  it('deposits bad ERC20 tokens', async () => {
+    log('Depositing Bad GNO')
+    const tx = await dx.deposit(badGNO.address, startBal.startingGNO, { from: seller1 })
+    log('tx: ', JSON.stringify(tx.logs, null, 2))
+    log('Succeeded Depositing Bad GNO')
+    log('startBal.startingGNO: ', startBal.startingGNO)
+
+    const deposited = await dx.balances(badGNO.address, seller1)
+    log('deposited: ', deposited.toString())
+
+    assert(deposited.eq(startBal.startingGNO), 'deposited amount was exactly equal startingGNO')
+  })
+
+  it('withdraws bad ERC20 tokens', async () => {
+    log('Withdrawing Bad GNO')
+    const tx = await dx.withdraw(badGNO.address, startBal.startingGNO, { from: seller1 })
+    log('tx: ', JSON.stringify(tx.logs, null, 2))
+    log('Succeeded Withdrawing Bad GNO')
+    log('startBal.startingGNO: ', startBal.startingGNO)
+
+    const deposited = await dx.balances(badGNO.address, seller1)
+    log('deposited: ', deposited.toString())
+
+    assert(deposited.eq(0), 'deposited amount is exactly 0 after startingGNO was withdrawn')
+  })
+})