Merge pull request #330 from ethereum/gascheck

assets/css/browser-solidity.css

@@ -299,6 +299,9 @@ body {
     margin-right: 1em;
     cursor: pointer;
 }
+#staticanalysismodules label {
+    display: block;
+}
 
 #header .origin,
 #header #executionContext {

src/app/formalVerification.js

@@ -25,7 +25,10 @@ function FormalVerification (outputElement, compilerEvent) {
 
 FormalVerification.prototype.compilationFinished = function (compilationResult) {
   if (compilationResult.formal === undefined) {
-    this.event.trigger('compilationFinished', [false, 'Formal verification not supported by this compiler version.', $('#formalVerificationErrors'), true])
+    this.event.trigger(
+      'compilationFinished',
+      [false, 'Formal verification not supported by this compiler version.', $('#formalVerificationErrors'), {noAnnotations: true}]
+    )
   } else {
     if (compilationResult.formal['why3'] !== undefined) {
       $('#formalVerificationInput', this.outputElement).val(
@@ -37,10 +40,10 @@ FormalVerification.prototype.compilationFinished = function (compilationResult)
     if (compilationResult.formal.errors !== undefined) {
       var errors = compilationResult.formal.errors
       for (var i = 0; i < errors.length; i++) {
-        this.event.trigger('compilationFinished', [false, errors[i], $('#formalVerificationErrors'), true])
+        this.event.trigger('compilationFinished', [false, errors[i], $('#formalVerificationErrors'), {noAnnotations: true}])
       }
     } else {
-      this.event.trigger('compilationFinished', [true, null, null, true])
+      this.event.trigger('compilationFinished', [true, null, null, {noAnnotations: true}])
     }
   }
 }

src/app/renderer.js

@@ -11,9 +11,9 @@ function Renderer (editor, updateFiles, udapp, executionContext, formalVerificat
   this.udapp = udapp
   this.executionContext = executionContext
   var self = this
-  formalVerificationEvent.register('compilationFinished', this, function (success, message, container, noAnnotations) {
+  formalVerificationEvent.register('compilationFinished', this, function (success, message, container, options) {
     if (!success) {
-      self.error(message, container, noAnnotations)
+      self.error(message, container, options)
     }
   })
   compilerEvent.register('compilationFinished', this, function (success, data, source) {
@@ -34,13 +34,19 @@ function Renderer (editor, updateFiles, udapp, executionContext, formalVerificat
   })
 }
 
-Renderer.prototype.error = function (message, container, noAnnotations, type) {
+Renderer.prototype.error = function (message, container, options) {
   var self = this
-  if (!type) {
-    type = utils.errortype(message)
+  var opt = options || {}
+  if (!opt.type) {
+    opt.type = utils.errortype(message)
   }
-  var $pre = $('<pre />').text(message)
-  var $error = $('<div class="sol ' + type + '"><div class="close"><i class="fa fa-close"></i></div></div>').prepend($pre)
+  var $pre
+  if (opt.isHTML) {
+    $pre = $(opt.useSpan ? '<span />' : '<pre />').html(message)
+  } else {
+    $pre = $(opt.useSpan ? '<span />' : '<pre />').text(message)
+  }
+  var $error = $('<div class="sol ' + opt.type + '"><div class="close"><i class="fa fa-close"></i></div></div>').prepend($pre)
   if (container === undefined) {
     container = $('#output')
   }
@@ -50,12 +56,12 @@ Renderer.prototype.error = function (message, container, noAnnotations, type) {
     var errFile = err[1]
     var errLine = parseInt(err[2], 10) - 1
     var errCol = err[4] ? parseInt(err[4], 10) : 0
-    if (!noAnnotations && (errFile === '' || errFile === utils.fileNameFromKey(self.editor.getCacheFile()))) {
+    if (!opt.noAnnotations && (errFile === '' || errFile === utils.fileNameFromKey(self.editor.getCacheFile()))) {
       self.editor.addAnnotation({
         row: errLine,
         column: errCol,
         text: message,
-        type: type
+        type: opt.type
       })
     }
     $error.click(function (ev) {
@@ -63,7 +69,6 @@ Renderer.prototype.error = function (message, container, noAnnotations, type) {
         // Switch to file
         self.editor.setCacheFile(utils.fileKey(errFile))
         self.updateFiles()
-      // @TODO could show some error icon in files with errors
       }
       self.editor.handleErrorClick(errLine, errCol)
     })

src/app/staticanalysis/modules/gasCosts.js

+var name = 'gas costs'
+var desc = 'Warn if the gas requiremets of functions are too high.'
+
+function gasCosts () {
+}
+
+gasCosts.prototype.report = function (compilationResults) {
+  var report = []
+  for (var contractName in compilationResults.contracts) {
+    var contract = compilationResults.contracts[contractName]
+    if (
+      contract.gasEstimates === undefined ||
+      contract.gasEstimates.external === undefined
+    ) {
+      continue
+    }
+    var fallback = contract.gasEstimates.external['']
+    if (fallback !== undefined) {
+      if (fallback === null || fallback >= 2100) {
+        report.push({
+          warning: `Fallback function of contract ${contractName} requires too much gas (${fallback}).<br />
+          If the fallback function requires more than 2300 gas, the contract cannot receive Ether.`
+        })
+      }
+    }
+
+    for (var functionName in contract.gasEstimates.external) {
+      if (functionName === '') {
+        continue
+      }
+      var gas = contract.gasEstimates.external[functionName]
+      var gasString = gas === null ? 'unknown or not constant' : 'high: ' + gas
+      if (gas === null || gas >= 3000000) {
+        report.push({
+          warning: `Gas requirement of function ${contractName}.${functionName} ${gasString}.<br />
+          If the gas requirement of a function is higher than the block gas limit, it cannot be executed.
+          Please avoid loops in your functions or actions that modify large areas of storage
+          (this includes clearing or copying arrays in storage)`
+        })
+      }
+    }
+  }
+  return report
+}
+
+module.exports = {
+  name: name,
+  description: desc,
+  Module: gasCosts
+}

src/app/staticanalysis/modules/list.js

 module.exports = [
-  require('./txOrigin')
+  require('./txOrigin'),
+  require('./gasCosts')
 ]

src/app/staticanalysis/modules/txOrigin.js

@@ -2,7 +2,7 @@ var name = 'tx origin'
 var desc = 'warn if tx.origin is used'
 
 function txOrigin () {
-  this.txOriginNode = []
+  this.txOriginNodes = []
 }
 
 txOrigin.prototype.visit = function (node) {
@@ -12,23 +12,18 @@ txOrigin.prototype.visit = function (node) {
   node.children && node.children.length &&
   node.children[0].attributes.type === 'tx' &&
   node.children[0].attributes.value === 'tx') {
-    this.txOriginNode.push(node)
+    this.txOriginNodes.push(node)
   }
 }
 
-txOrigin.prototype.report = function (node) {
-  var report = []
-  this.txOriginNode.map(function (item, i) {
-    report.push({
-      warning: `use of tx.origin: "tx.origin" is useful only in very exceptional cases.\n
+txOrigin.prototype.report = function () {
+  return this.txOriginNodes.map(function (item, i) {
+    return {
+      warning: `use of tx.origin: "tx.origin" is useful only in very exceptional cases.<br />
                 If you use it for authentication, you usually want to replace it by "msg.sender", because otherwise any contract you call can act on your behalf.`,
       location: item.src
-    })
+    }
   })
-  return {
-    name: name,
-    report: report
-  }
 }
 
 module.exports = {

src/app/staticanalysis/staticAnalysisRunner.js

@@ -5,20 +5,30 @@ var list = require('./modules/list')
 function staticAnalysisRunner () {
 }
 
-staticAnalysisRunner.prototype.run = function (ast, toRun, callback) {
+staticAnalysisRunner.prototype.run = function (compilationResult, toRun, callback) {
+  var self = this
+  var modules = toRun.map(function (i) {
+    var m = self.modules()[i]
+    return { 'name': m.name, 'mod': new m.Module() }
+  })
+
+  // Also provide convenience analysis via the AST walker.
   var walker = new AstWalker()
-  for (var k in ast) {
-    walker.walk(ast[k].AST, {'*': function (node) {
-      toRun.map(function (item, i) {
-        item.visit(node)
+  for (var k in compilationResult.sources) {
+    walker.walk(compilationResult.sources[k].AST, {'*': function (node) {
+      modules.map(function (item, i) {
+        if (item.mod.visit !== undefined) {
+          item.mod.visit(node)
+        }
       })
       return true
     }})
   }
 
-  var reports = []
-  toRun.map(function (item, i) {
-    reports.push(item.report())
+  // Here, modules can just collect the results from the AST walk,
+  // but also perform new analysis.
+  var reports = modules.map(function (item, i) {
+    return { name: item.name, report: item.mod.report(compilationResult) }
   })
   callback(reports)
 }

src/app/staticanalysis/staticAnalysisView.js

@@ -28,8 +28,10 @@ staticAnalysisView.prototype.render = function () {
   var self = this
   var view = yo`<div>
     <strong>Static Analysis</strong>
-    <div>Select analyser to run against current compiled contracts <label><input id="autorunstaticanalysis" type="checkbox" checked="true">Auto run Static Analysis</label></div>    
+    <label for="autorunstaticanalysis"><input id="autorunstaticanalysis" type="checkbox" checked="true">Auto run</label>
+    <div id="staticanalysismodules">
     ${this.modulesView}
+    </div>
     <div>
       <button onclick=${function () { self.run() }} >Run</button>
     </div>
@@ -45,14 +47,10 @@ staticAnalysisView.prototype.selectedModules = function () {
   if (!this.view) {
     return []
   }
-  var selected = this.view.querySelectorAll('[name="staticanalysismodule"]')
+  var selected = this.view.querySelectorAll('[name="staticanalysismodule"]:checked')
   var toRun = []
   for (var i = 0; i < selected.length; i++) {
-    var el = selected[i]
-    if (el.checked) {
-      var analyser = this.runner.modules()[el.attributes['index'].value]
-      toRun.push(new analyser.Module())
-    }
+    toRun.push(selected[i].attributes['index'].value)
   }
   return toRun
 }
@@ -66,18 +64,21 @@ staticAnalysisView.prototype.run = function () {
   warningContainer.empty()
   if (this.lastCompilationResult) {
     var self = this
-    this.runner.run(this.lastCompilationResult.sources, selected, function (results) {
+    this.runner.run(this.lastCompilationResult, selected, function (results) {
       results.map(function (result, i) {
         result.report.map(function (item, i) {
-          var split = item.location.split(':')
-          var file = split[2]
-          var location = {
-            start: parseInt(split[0]),
-            length: parseInt(split[1])
+          var location = ''
+          if (item.location !== undefined) {
+            var split = item.location.split(':')
+            var file = split[2]
+            location = {
+              start: parseInt(split[0]),
+              length: parseInt(split[1])
+            }
+            location = self.offsetToColumnConverter.offsetToLineColumn(location, file, self.editor, self.lastCompilationResult)
+            location = self.lastCompilationResult.sourceList[file] + ':' + (location.start.line + 1) + ':' + (location.start.column + 1) + ':'
           }
-          location = self.offsetToColumnConverter.offsetToLineColumn(location, file, self.editor, self.lastCompilationResult)
-          location = self.lastCompilationResult.sourceList[file] + ':' + (location.start.line + 1) + ':' + (location.start.column + 1) + ':'
-          self.renderer.error(location + ' ' + item.warning, warningContainer, false, 'warning')
+          self.renderer.error(location + ' ' + item.warning, warningContainer, {type: 'warning', useSpan: true, isHTML: true})
         })
       })
       if (warningContainer.html() === '') {

test-browser/helpers/contracts.js

@@ -21,6 +21,7 @@ function testContracts (browser, contractCode, compiledContractNames, callback)
       .clearValue('#input textarea')
       .click('.newFile')
       .setValue('#input textarea', contractCode, function () {})
-      .waitForElementPresent('.contract .create', 2000)
-  checkCompiledContracts(browser, compiledContractNames, callback)
+      .waitForElementPresent('.contract .create', 2000, true, function () {
+        checkCompiledContracts(browser, compiledContractNames, callback)
+      })
 }

test-browser/helpers/dom.js

+'use strict'
+
+module.exports = {
+  listSelectorContains: listSelectorContains
+}
+
+function listSelectorContains (textsToFind, selector, browser, callback) {
+  browser
+    .elements('css selector', selector, function (warnings) {
+      warnings.value.map(function (warning, index) {
+        browser.elementIdText(warning.ELEMENT, function (text) {
+          browser.assert.equal(text.value.indexOf(textsToFind[index]) !== -1, true)
+          if (index === warnings.value.length - 1) {
+            callback()
+          }
+        })
+      })
+    })
+}
+

test-browser/tests/staticanalysis.js

@@ -2,10 +2,17 @@
 var contractHelper = require('../helpers/contracts')
 var init = require('../helpers/init')
 var sauce = require('./sauce')
+var dom = require('../helpers/dom')
 
 var sources = {
   'sources': {
-    'Untitled': `contract test1 { address test = tx.origin; } contract test2 {}`
+    'Untitled': `
+contract test1 { address test = tx.origin; }
+contract test2 {}
+contract TooMuchGas {
+  uint x;
+  function() { x++; }
+}`
   }
 }
 
@@ -25,12 +32,18 @@ module.exports = {
 function runTests (browser) {
   browser
     .waitForElementVisible('.newFile', 10000)
-  contractHelper.testContracts(browser, sources.sources.Untitled, ['test1', 'test2'], function () {
+  contractHelper.testContracts(browser, sources.sources.Untitled, ['TooMuchGas', 'test1', 'test2'], function () {
     browser
-    .click('.staticanalysisView')
-    .click('#staticanalysisView button')
-    .waitForElementPresent('#staticanalysisresult .warning')
-    .assert.containsText('#staticanalysisresult .warning pre', 'Untitled:1:33: use of tx.origin')
-    .end()
+      .click('.staticanalysisView')
+      .click('#staticanalysisView button')
+      .waitForElementPresent('#staticanalysisresult .warning', 2000, true, function () {
+        dom.listSelectorContains(['Untitled:1:34: use of tx.origin',
+          'Fallback function of contract TooMuchGas requires too much gas'],
+          '#staticanalysisresult .warning span',
+          browser, function () {
+            browser.end()
+          }
+        )
+      })
   })
 }