Merge pull request #2533 from ethereum/fixLoadingNodeModules

Fix loading content from node_modules & browser tests

contracts/node_modules/openzeppelin-solidity/contracts/math/SafeMath.sol

@@ -0,0 +1,156 @@
+pragma solidity ^0.5.0;
+
+/**
+ * @dev Wrappers over Solidity's arithmetic operations with added overflow
+ * checks.
+ *
+ * Arithmetic operations in Solidity wrap on overflow. This can easily result
+ * in bugs, because programmers usually assume that an overflow raises an
+ * error, which is the standard behavior in high level programming languages.
+ * `SafeMath` restores this intuition by reverting the transaction when an
+ * operation overflows.
+ *
+ * Using this library instead of the unchecked operations eliminates an entire
+ * class of bugs, so it's recommended to use it always.
+ */
+library SafeMath {
+    /**
+     * @dev Returns the addition of two unsigned integers, reverting on
+     * overflow.
+     *
+     * Counterpart to Solidity's `+` operator.
+     *
+     * Requirements:
+     * - Addition cannot overflow.
+     */
+    function add(uint256 a, uint256 b) internal pure returns (uint256) {
+        uint256 c = a + b;
+        require(c >= a, "SafeMath: addition overflow");
+
+        return c;
+    }
+
+    /**
+     * @dev Returns the subtraction of two unsigned integers, reverting on
+     * overflow (when the result is negative).
+     *
+     * Counterpart to Solidity's `-` operator.
+     *
+     * Requirements:
+     * - Subtraction cannot overflow.
+     */
+    function sub(uint256 a, uint256 b) internal pure returns (uint256) {
+        return sub(a, b, "SafeMath: subtraction overflow");
+    }
+
+    /**
+     * @dev Returns the subtraction of two unsigned integers, reverting with custom message on
+     * overflow (when the result is negative).
+     *
+     * Counterpart to Solidity's `-` operator.
+     *
+     * Requirements:
+     * - Subtraction cannot overflow.
+     *
+     * _Available since v2.4.0._
+     */
+    function sub(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) {
+        require(b <= a, errorMessage);
+        uint256 c = a - b;
+
+        return c;
+    }
+
+    /**
+     * @dev Returns the multiplication of two unsigned integers, reverting on
+     * overflow.
+     *
+     * Counterpart to Solidity's `*` operator.
+     *
+     * Requirements:
+     * - Multiplication cannot overflow.
+     */
+    function mul(uint256 a, uint256 b) internal pure returns (uint256) {
+        // Gas optimization: this is cheaper than requiring 'a' not being zero, but the
+        // benefit is lost if 'b' is also tested.
+        // See: https://github.com/OpenZeppelin/openzeppelin-contracts/pull/522
+        if (a == 0) {
+            return 0;
+        }
+
+        uint256 c = a * b;
+        require(c / a == b, "SafeMath: multiplication overflow");
+
+        return c;
+    }
+
+    /**
+     * @dev Returns the integer division of two unsigned integers. Reverts on
+     * division by zero. The result is rounded towards zero.
+     *
+     * Counterpart to Solidity's `/` operator. Note: this function uses a
+     * `revert` opcode (which leaves remaining gas untouched) while Solidity
+     * uses an invalid opcode to revert (consuming all remaining gas).
+     *
+     * Requirements:
+     * - The divisor cannot be zero.
+     */
+    function div(uint256 a, uint256 b) internal pure returns (uint256) {
+        return div(a, b, "SafeMath: division by zero");
+    }
+
+    /**
+     * @dev Returns the integer division of two unsigned integers. Reverts with custom message on
+     * division by zero. The result is rounded towards zero.
+     *
+     * Counterpart to Solidity's `/` operator. Note: this function uses a
+     * `revert` opcode (which leaves remaining gas untouched) while Solidity
+     * uses an invalid opcode to revert (consuming all remaining gas).
+     *
+     * Requirements:
+     * - The divisor cannot be zero.
+     *
+     * _Available since v2.4.0._
+     */
+    function div(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) {
+        // Solidity only automatically asserts when dividing by 0
+        require(b > 0, errorMessage);
+        uint256 c = a / b;
+        // assert(a == b * c + a % b); // There is no case in which this doesn't hold
+
+        return c;
+    }
+
+    /**
+     * @dev Returns the remainder of dividing two unsigned integers. (unsigned integer modulo),
+     * Reverts when dividing by zero.
+     *
+     * Counterpart to Solidity's `%` operator. This function uses a `revert`
+     * opcode (which leaves remaining gas untouched) while Solidity uses an
+     * invalid opcode to revert (consuming all remaining gas).
+     *
+     * Requirements:
+     * - The divisor cannot be zero.
+     */
+    function mod(uint256 a, uint256 b) internal pure returns (uint256) {
+        return mod(a, b, "SafeMath: modulo by zero");
+    }
+
+    /**
+     * @dev Returns the remainder of dividing two unsigned integers. (unsigned integer modulo),
+     * Reverts with custom message when dividing by zero.
+     *
+     * Counterpart to Solidity's `%` operator. This function uses a `revert`
+     * opcode (which leaves remaining gas untouched) while Solidity uses an
+     * invalid opcode to revert (consuming all remaining gas).
+     *
+     * Requirements:
+     * - The divisor cannot be zero.
+     *
+     * _Available since v2.4.0._
+     */
+    function mod(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) {
+        require(b != 0, errorMessage);
+        return a % b;
+    }
+}

contracts/node_modules/openzeppelin-solidity/contracts/sample.sol

@@ -0,0 +1 @@
+import "https://github.com/OpenZeppelin/openzeppelin-contracts/contracts/token/ERC20/ERC20.sol"; contract test11 {}

package.json

@@ -170,9 +170,10 @@
     "nightwatch_local_signingMessage": "nightwatch ./test-browser/tests/signingMessage.js --config nightwatch.js --env chrome ",
     "nightwatch_local_specialFunctions": "nightwatch ./test-browser/tests/specialFunctions.js --config nightwatch.js --env chrome ",
     "nightwatch_local_solidityUnittests": "nightwatch ./test-browser/tests/solidityUnittests.js --config nightwatch.js --env chrome ",
-    "nightwatch_local_remixd": "nightwatch ./test-browser/tests/remix.js --config nightwatch.js --env chrome ",
+    "nightwatch_local_remixd": "nightwatch ./test-browser/tests/remixd.js --config nightwatch.js --env chrome ",
     "nightwatch_local_console": "nightwatch ./test-browser/tests/console.js --config nightwatch.js --env chrome ",
     "nightwatch_local_gist": "nightwatch ./test-browser/tests/gist.js --config nightwatch.js --env chrome ",
+    "nightwatch_local_workspace": "nightwatch ./test-browser/tests/workspace.js --config nightwatch.js --env chrome ",
     "onchange": "onchange build/app.js -- npm-run-all lint",
     "prepublish": "mkdirp build; npm-run-all -ls downloadsolc_root build",
     "remixd": "remixd -s ./contracts --remix-ide http://127.0.0.1:8080",

src/app/files/fileManager.js

@@ -314,6 +314,10 @@ class FileManager extends Plugin {
     })
   }
 
+  getProvider (name) {
+    return this._deps.filesProviders[name]
+  }
+
   fileProviderOf (file) {
     if (file.indexOf('localhost') === 0) {
       return this._deps.filesProviders['localhost']

src/app/tabs/compileTab/compileTab.js

@@ -99,6 +99,15 @@ class CompileTab {
       })
   }
 
+  /**
+    * import the content of @arg url.
+    * first look in the browser localstorage (browser explorer) or locahost explorer. if the url start with `browser/*` or  `localhost/*`
+    * then check if the @arg url is located in the localhost, in the node_modules or installed_contracts folder
+    * then check if the @arg url match any external url
+    *
+    * @param {String} url  - URL of the content. can be basically anything like file located in the browser explorer, in the localhost explorer, raw HTTP, github address etc...
+    * @param {Function} cb  - callback
+    */
   importFileCb (url, filecb) {
     if (url.indexOf('remix_tests.sol') !== -1) return filecb(null, remixTests.assertLibCode)
 
@@ -107,26 +116,39 @@ class CompileTab {
       if (provider.type === 'localhost' && !provider.isConnected()) {
         return filecb(`file provider ${provider.type} not available while trying to resolve ${url}`)
       }
-      return provider.exists(url, (error, exist) => {
+      provider.exists(url, (error, exist) => {
         if (error) return filecb(error)
-        if (exist) {
-          return provider.get(url, filecb)
+        if (!exist && provider.type === 'localhost') return filecb(`not found ${url}`)
+
+        /*
+          if the path is absolute and the file does not exist, we can stop here
+          Doesn't make sense to try to resolve "localhost/node_modules/localhost/node_modules/<path>" and we'll end in an infinite loop.
+        */
+        if (!exist && url.startsWith('browser/')) return filecb(`not found ${url}`)
+        if (!exist && url.startsWith('localhost/')) return filecb(`not found ${url}`)
+
+        if (exist) return provider.get(url, filecb)
+
+        // try to resolve localhost modules (aka truffle imports) - e.g from the node_modules folder
+        const localhostProvider = this.fileManager.getProvider('localhost')
+        if (localhostProvider.isConnected()) {
+          var splitted = /([^/]+)\/(.*)$/g.exec(url)
+          return async.tryEach([
+            (cb) => { this.importFileCb('localhost/installed_contracts/' + url, cb) },
+            (cb) => { if (!splitted) { cb('URL not parseable: ' + url) } else { this.importFileCb('localhost/installed_contracts/' + splitted[1] + '/contracts/' + splitted[2], cb) } },
+            (cb) => { this.importFileCb('localhost/node_modules/' + url, cb) },
+            (cb) => { if (!splitted) { cb('URL not parseable: ' + url) } else { this.importFileCb('localhost/node_modules/' + splitted[1] + '/contracts/' + splitted[2], cb) } }],
+            (error, result) => {
+              if (error) return this.importExternal(url, filecb)
+              filecb(null, result)
+            }
+          )
+        } else {
+          // try to resolve external content
+          this.importExternal(url, filecb)
         }
-        this.importExternal(url, filecb)
       })
     }
-    if (this.compilerImport.isRelativeImport(url)) {
-      // try to resolve localhost modules (aka truffle imports)
-      var splitted = /([^/]+)\/(.*)$/g.exec(url)
-      return async.tryEach([
-        (cb) => { this.importFileCb('localhost/installed_contracts/' + url, cb) },
-        (cb) => { if (!splitted) { cb('URL not parseable: ' + url) } else { this.importFileCb('localhost/installed_contracts/' + splitted[1] + '/contracts/' + splitted[2], cb) } },
-        (cb) => { this.importFileCb('localhost/node_modules/' + url, cb) },
-        (cb) => { if (!splitted) { cb('URL not parseable: ' + url) } else { this.importFileCb('localhost/node_modules/' + splitted[1] + '/contracts/' + splitted[2], cb) } }],
-        (error, result) => { filecb(error, result) }
-      )
-    }
-    this.importExternal(url, filecb)
   }
 
 }

test-browser/commands/clickLaunchIcon.js

@@ -2,7 +2,7 @@ const EventEmitter = require('events')
 
 class ClickLaunchIcon extends EventEmitter {
   command (icon) {
-    this.api.click('#icon-panel div[plugin="' + icon + '"]').perform(() => {
+    this.api.waitForElementVisible('#icon-panel div[plugin="' + icon + '"]').click('#icon-panel div[plugin="' + icon + '"]').perform(() => {
       this.emit('complete')
     })
     return this

test-browser/commands/setSolidityCompilerVersion.js

@@ -0,0 +1,15 @@
+const EventEmitter = require('events')
+
+class SetSolidityCompilerVersion extends EventEmitter {
+  command (version) {
+    this.api
+      .click(`#compileTabView #versionSelector [value="${version}"]`)
+      .pause(5000)
+      .perform(() => {
+        this.emit('complete')
+      })
+    return this
+  }
+}
+
+module.exports = SetSolidityCompilerVersion

test-browser/commands/verifyContracts.js

@@ -1,9 +1,9 @@
 const EventEmitter = require('events')
 
 class VerifyContracts extends EventEmitter {
-  command (compiledContractNames) {
+  command (compiledContractNames, opts = { wait: 1000 }) {
     this.api.perform((done) => {
-      verifyContracts(this.api, compiledContractNames, () => {
+      verifyContracts(this.api, compiledContractNames, opts, () => {
         done()
         this.emit('complete')
       })
@@ -12,9 +12,10 @@ class VerifyContracts extends EventEmitter {
   }
 }
 
-function getCompiledContracts (browser, callback) {
+function getCompiledContracts (browser, opts, callback) {
   browser
   .clickLaunchIcon('solidity')
+  .pause(opts.wait)
   .waitForElementPresent('#compileTabView select#compiledContracts option')
   .execute(function () {
     var contracts = document.querySelectorAll('#compileTabView select#compiledContracts option')
@@ -32,8 +33,8 @@ function getCompiledContracts (browser, callback) {
   })
 }
 
-function verifyContracts (browser, compiledContractNames, callback) {
-  getCompiledContracts(browser, (result) => {
+function verifyContracts (browser, compiledContractNames, opts, callback) {
+  getCompiledContracts(browser, opts, (result) => {
     if (result.value) {
       for (var contract in compiledContractNames) {
         console.log(' - ' + compiledContractNames[contract], result.value)

test-browser/helpers/init.js

@@ -24,5 +24,8 @@ function initModules (browser, callback) {
     .scrollAndClick('#pluginManager article[id="remixPluginManagerListItem_solidityStaticAnalysis"] button')
     .scrollAndClick('#pluginManager article[id="remixPluginManagerListItem_debugger"] button')
     .scrollAndClick('#icon-panel div[plugin="fileExplorers"]')
+    .clickLaunchIcon('settings')
+    .setValue('#gistaccesstoken', process.env.gist_token)
+    .click('#savegisttoken')
     .perform(() => { callback() })
 }

test-browser/tests/gist.js

@@ -17,9 +17,6 @@ module.exports = {
     console.log('token', process.env.gist_token)
     browser
     .waitForElementVisible('#icon-panel', 10000)
-    .clickLaunchIcon('settings')
-    .setValue('#gistaccesstoken', process.env.gist_token)
-    .click('#savegisttoken')
     .clickLaunchIcon('fileExplorers')
     .click('#publishToGist')
     .modalFooterOKClick()

test-browser/tests/remixd.js

@@ -28,6 +28,12 @@ var sources = [
   {
     'localhost/src/gmbh/company.sol': {content: assetsTestContract},
     'localhost/src/gmbh/contract.sol': {content: gmbhTestContract}
+  },
+  {
+    'browser/test_import_node_modules.sol': {content: 'import "openzeppelin-solidity/contracts/math/SafeMath.sol";'}
+  },
+  {
+    'browser/test_import_node_modules_with_github_import.sol': {content: 'import "openzeppelin-solidity/contracts/sample.sol";'}
   }
 ]
 
@@ -41,6 +47,29 @@ module.exports = {
   'Remixd': function (browser) {
     runTests(browser)
   },
+  'Import from node_modules ': function (browser) {
+    /*
+      when a relative import is used (i.e import "openzeppelin-solidity/contracts/math/SafeMath.sol")
+      remix (as well as truffle) try to resolve it against the node_modules and installed_contracts folder.
+    */
+    browser
+      .waitForElementVisible('#icon-panel', 2000)
+      .clickLaunchIcon('fileExplorers')
+      .addFile('test_import_node_modules.sol', sources[3]['browser/test_import_node_modules.sol'])
+      .clickLaunchIcon('solidity')
+      .testContracts('test_import_node_modules.sol', sources[3]['browser/test_import_node_modules.sol'], ['SafeMath'])
+  },
+  'Import from node_modules and reference a github import': function (browser) {
+    browser
+      .waitForElementVisible('#icon-panel', 2000)
+      .clickLaunchIcon('fileExplorers')
+      .addFile('test_import_node_modules_with_github_import.sol', sources[4]['browser/test_import_node_modules_with_github_import.sol'])
+      .clickLaunchIcon('solidity')
+      .testContracts('test_import_node_modules_with_github_import.sol', sources[4]['browser/test_import_node_modules_with_github_import.sol'], ['ERC20', 'test11'])
+      .clickLaunchIcon('pluginManager')
+      .scrollAndClick('#pluginManager article[id="remixPluginManagerListItem_remixd"] button')
+      .end()
+  },
   tearDown: sauce
 }
 
@@ -98,9 +127,6 @@ function runTests (browser, testData) {
     .waitForElementNotPresent('[data-path="localhost/folder1/contract_' + browserName + '.sol"]') // check if renamed (old) file is not present
     .waitForElementNotPresent('[data-path="localhost/folder1/contract_' + browserName + '_toremove.sol"]') // check if removed (old) file is not present
     .click('[data-path="localhost/folder1/renamed_contract_' + browserName + '.sol"]')
-    .clickLaunchIcon('pluginManager')
-    .scrollAndClick('#pluginManager article[id="remixPluginManagerListItem_remixd"] button')
-    .end()
 }
 
 function testImportFromRemixd (browser, callback) {

test-browser/tests/solidityImport.js

@@ -22,129 +22,40 @@ module.exports = {
   'Test Failed Import': function (browser) {
     browser.addFile('Untitled3.sol', sources[2]['browser/Untitled3.sol'])
           .clickLaunchIcon('solidity')
-          .assert.containsText('#compileTabView .error pre', 'Unable to import "browser/Untitled11.sol": File not found')
-          .end()
+          .assert.containsText('#compileTabView .error pre', 'not found browser/Untitled11.sol')
   },
-  tearDown: sauce
-}
-
-var abstractENS = `
-contract AbstractENS {
-    function owner(bytes32 node) public view returns(address);
-    function resolver(bytes32 node) public view returns(address);
-    function ttl(bytes32 node) public view returns(uint64);
-    function setOwner(bytes32 node, address owner) public;
-    function setSubnodeOwner(bytes32 node, bytes32 label, address owner) public;
-    function setResolver(bytes32 node, address resolver) public;
-    function setTTL(bytes32 node, uint64 ttl) public;
-
-    // Logged when the owner of a node assigns a new owner to a subnode.
-    event NewOwner(bytes32 indexed node, bytes32 indexed label, address owner);
-
-    // Logged when the owner of a node transfers ownership to a new account.
-    event Transfer(bytes32 indexed node, address owner);
-
-    // Logged when the resolver for a node changes.
-    event NewResolver(bytes32 indexed node, address resolver);
-
-    // Logged when the TTL of a node changes
-    event NewTTL(bytes32 indexed node, uint64 ttl);
-}`
-
-var ENS = `pragma solidity ^0.4.0;
-
-import './AbstractENS.sol';
-
-/**
- * The ENS registry contract.
- */
-contract ENS is AbstractENS {
-  struct Record {
-      address owner;
-      address resolver;
-      uint64 ttl;
-  }
-
-  mapping(bytes32=>Record) records;
-
-  // Permits modifications only by the owner of the specified node.
-  modifier only_owner(bytes32 node) {
-      if (records[node].owner != msg.sender) revert();
-      _;
-  }
-
-  /**
-   * Constructs a new ENS registrar.
-   */
-  constructor() public {
-      records[0].owner = msg.sender;
-  }
-
-  /**
-   * Returns the address that owns the specified node.
-   */
-  function owner(bytes32 node) public view returns (address) {
-      return records[node].owner;
-  }
 
-  /**
-   * Returns the address of the resolver for the specified node.
-   */
-  function resolver(bytes32 node) public view returns (address) {
-      return records[node].resolver;
-  }
-
-  /**
-   * Returns the TTL of a node, and any records associated with it.
-   */
-  function ttl(bytes32 node) public view returns (uint64) {
-      return records[node].ttl;
-  }
-
-  /**
-   * Transfers ownership of a node to a new address. May only be called by the current
-   * owner of the node.
-   * @param node The node to transfer ownership of.
-   * @param owner The address of the new owner.
-   */
-  function setOwner(bytes32 node, address owner) public only_owner(node) {
-      emit Transfer(node, owner);
-      records[node].owner = owner;
-  }
+  'Test Github Import - from master branch': function (browser) {
+    browser
+          .setSolidityCompilerVersion('soljson-v0.5.0+commit.1d4f565a.js')
+          .addFile('Untitled4.sol', sources[3]['browser/Untitled4.sol'])
+          .clickLaunchIcon('fileExplorers')
+          .verifyContracts(['test7', 'ERC20', 'SafeMath'], {wait: 10000})
+  },
 
-  /**
-   * Transfers ownership of a subnode sha3(node, label) to a new address. May only be
-   * called by the owner of the parent node.
-   * @param node The parent node.
-   * @param label The hash of the label specifying the subnode.
-   * @param owner The address of the new owner.
-   */
-  function setSubnodeOwner(bytes32 node, bytes32 label, address owner) public only_owner(node) {
-      bytes32 subnode = keccak256(abi.encodePacked(node, label));
-      emit NewOwner(node, label, owner);
-      records[subnode].owner = owner;
-  }
+  'Test Github Import - from other branch': function (browser) {
+    browser
+          .addFile('Untitled5.sol', sources[4]['browser/Untitled5.sol'])
+          .clickLaunchIcon('fileExplorers')
+          .verifyContracts(['test8', 'ERC20', 'SafeMath'], {wait: 10000})
+  },
 
-  /**
-   * Sets the resolver address for the specified node.
-   * @param node The node to update.
-   * @param resolver The address of the resolver.
-   */
-  function setResolver(bytes32 node, address resolver) public only_owner(node) {
-      emit NewResolver(node, resolver);
-      records[node].resolver = resolver;
-  }
+  'Test Github Import - no branch specified': function (browser) {
+    browser
+          .addFile('Untitled6.sol', sources[5]['browser/Untitled6.sol'])
+          .clickLaunchIcon('fileExplorers')
+          .verifyContracts(['test10', 'ERC20', 'SafeMath'], {wait: 10000})
+  },
 
-  /**
-   * Sets the TTL for the specified node.
-   * @param node The node to update.
-   * @param ttl The TTL in seconds.
-   */
-  function setTTL(bytes32 node, uint64 ttl) public only_owner(node) {
-      emit NewTTL(node, ttl);
-      records[node].ttl = ttl;
-  }
-}`
+  'Test Github Import - raw URL': function (browser) {
+    browser
+          .addFile('Untitled7.sol', sources[6]['browser/Untitled7.sol'])
+          .clickLaunchIcon('fileExplorers')
+          .verifyContracts(['test11', 'ERC20', 'SafeMath'], {wait: 10000})
+          .end()
+  },
+  tearDown: sauce
+}
 
 var sources = [
   {
@@ -158,12 +69,15 @@ var sources = [
     'browser/Untitled3.sol': {content: 'import "./Untitled11.sol"; contract test6 {}'}
   },
   {
-    'browser/Untitled4.sol': {content: 'import "github.com/ethereum/ens/contracts/ENS.sol"; contract test7 {}'},
-    'github.com/ethereum/ens/contracts/ENS.sol': {content: ENS}
+    'browser/Untitled4.sol': {content: 'import "https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/ERC20.sol"; contract test7 {}'}
+  },
+  {
+    'browser/Untitled5.sol': {content: 'import "https://github.com/OpenZeppelin/openzeppelin-contracts/blob/release-v2.3.0/contracts/token/ERC20/ERC20.sol"; contract test8 {}'}
+  },
+  {
+    'browser/Untitled6.sol': {content: 'import "https://github.com/OpenZeppelin/openzeppelin-contracts/contracts/token/ERC20/ERC20.sol"; contract test10 {}'}
   },
   {
-    'browser/Untitled4.sol': {content: 'import "github.com/ethereum/ens/contracts/ENS.sol"; contract test7 {}'},
-    'github.com/ethereum/ens/contracts/ENS.sol': {content: ENS},
-    'github.com/ethereum/ens/contracts/AbstractENS.sol': {content: abstractENS}
+    'browser/Untitled7.sol': {content: 'import "https://raw.githubusercontent.com/OpenZeppelin/openzeppelin-contracts/master/contracts/token/ERC20/ERC20.sol"; contract test11 {}'}
   }
 ]