Merge pull request #60 from ethereum/refactor_remix_debug1b

remove unneded if-elses; refactor; convert callbacks to promises

libs/remix-debug/src/debugger/VmDebugger.js

@@ -117,21 +117,30 @@ class VmDebuggerLogic {
         this.event.trigger('traceCurrentStepUpdate', [error, step])
       })
 
-      this._traceManager.getMemExpand(index, (error, addmem) => {
-        this.event.trigger('traceMemExpandUpdate', [error, addmem])
-      })
+      try {
+        const addmem = this._traceManager.getMemExpand(index)
+        this.event.trigger('traceMemExpandUpdate', [null, addmem])
+      } catch (error) {
+        this.event.trigger('traceMemExpandUpdate', [error])
+      }
 
-      this._traceManager.getStepCost(index, (error, gas) => {
-        this.event.trigger('traceStepCostUpdate', [error, gas])
-      })
+      try {
+        const gas = this._traceManager.getStepCost(index)
+        this.event.trigger('traceStepCostUpdate', [null, gas])
+      } catch (error) {
+        this.event.trigger('traceStepCostUpdate', [error])
+      }
 
       this._traceManager.getCurrentCalledAddressAt(index, (error, address) => {
         this.event.trigger('traceCurrentCalledAddressAtUpdate', [error, address])
       })
 
-      this._traceManager.getRemainingGas(index, (error, remaining) => {
-        this.event.trigger('traceRemainingGasUpdate', [error, remaining])
-      })
+      try {
+        const remaining = this._traceManager.getRemainingGas(index)
+        this.event.trigger('traceRemainingGasUpdate', [null, remaining])
+      } catch (error) {
+        this.event.trigger('traceRemainingGasUpdate', [error])
+      }
 
       this._traceManager.getReturnValue(index, (error, returnValue) => {
         if (error) {

libs/remix-debug/src/debugger/stepManager.js

@@ -191,17 +191,17 @@ class DebuggerStepManager {
   }
 
   resolveToReducedTrace (value, incr) {
-    if (this.debugger.callTree.reducedTrace.length) {
-      var nextSource = util.findClosestIndex(value, this.debugger.callTree.reducedTrace)
-      nextSource = nextSource + incr
-      if (nextSource <= 0) {
-        nextSource = 0
-      } else if (nextSource > this.debugger.callTree.reducedTrace.length) {
-        nextSource = this.debugger.callTree.reducedTrace.length - 1
-      }
-      return this.debugger.callTree.reducedTrace[nextSource]
+    if (!this.debugger.callTree.reducedTrace.length) {
+      return value
+    }
+    var nextSource = util.findClosestIndex(value, this.debugger.callTree.reducedTrace)
+    nextSource = nextSource + incr
+    if (nextSource <= 0) {
+      nextSource = 0
+    } else if (nextSource > this.debugger.callTree.reducedTrace.length) {
+      nextSource = this.debugger.callTree.reducedTrace.length - 1
     }
-    return value
+    return this.debugger.callTree.reducedTrace[nextSource]
   }
 
 }

libs/remix-lib/src/code/breakpointManager.js

@@ -67,11 +67,10 @@ class BreakpointManager {
         (sourceLocation.start <= previousSourceLocation.start &&
         sourceLocation.start + sourceLocation.length >= previousSourceLocation.start + previousSourceLocation.length)) {
         return false
-      } else {
-        self.jumpToCallback(currentStep)
-        self.event.trigger('breakpointHit', [sourceLocation, currentStep])
-        return true
       }
+      self.jumpToCallback(currentStep)
+      self.event.trigger('breakpointHit', [sourceLocation, currentStep])
+      return true
     }
 
     let sourceLocation
@@ -97,22 +96,21 @@ class BreakpointManager {
         this.previousLine = lineColumn.start.line
         if (this.hasBreakpointAtLine(sourceLocation.file, lineColumn.start.line)) {
           lineHadBreakpoint = true
-          if (direction === 1) {
-            if (hitLine(currentStep, sourceLocation, previousSourceLocation, this)) {
-              return
-            }
+          if (direction === 1 && hitLine(currentStep, sourceLocation, previousSourceLocation, this)) {
+            return
           }
         }
       }
       currentStep += direction
     }
     this.event.trigger('NoBreakpointHit', [])
-    if (defaultToLimit) {
-      if (direction === -1) {
-        this.jumpToCallback(0)
-      } else if (direction === 1) {
-        this.jumpToCallback(this.debugger.traceManager.trace.length - 1)
-      }
+    if (!defaultToLimit) {
+      return
+    }
+    if (direction === -1) {
+      this.jumpToCallback(0)
+    } else if (direction === 1) {
+      this.jumpToCallback(this.debugger.traceManager.trace.length - 1)
     }
   }
 
@@ -125,16 +123,16 @@ class BreakpointManager {
     */
   hasBreakpointAtLine (fileIndex, line) {
     const filename = this.debugger.solidityProxy.fileNameFromIndex(fileIndex)
-    if (filename && this.breakpoints[filename]) {
-      const sources = this.breakpoints[filename]
-      for (let k in sources) {
-        const source = sources[k]
-        if (line === source.row) {
-          return true
-        }
+    if (!(filename && this.breakpoints[filename])) {
+      return false
+    }
+    const sources = this.breakpoints[filename]
+    for (let k in sources) {
+      const source = sources[k]
+      if (line === source.row) {
+        return true
       }
     }
-    return false
   }
 
   /**
@@ -170,15 +168,16 @@ class BreakpointManager {
     * @param {Object} sourceLocation - position of the breakpoint { file: '<file index>', row: '<line number' }
     */
   remove (sourceLocation) {
-    if (this.breakpoints[sourceLocation.fileName]) {
-      var sources = this.breakpoints[sourceLocation.fileName]
-      for (let k in sources) {
-        const source = sources[k]
-        if (sourceLocation.row === source.row) {
-          sources.splice(k, 1)
-          this.event.trigger('breakpointRemoved', [sourceLocation])
-          break
-        }
+    var sources = this.breakpoints[sourceLocation.fileName]
+    if (!sources) {
+      return
+    }
+    for (let k in sources) {
+      const source = sources[k]
+      if (sourceLocation.row === source.row) {
+        sources.splice(k, 1)
+        this.event.trigger('breakpointRemoved', [sourceLocation])
+        break
       }
     }
   }

libs/remix-lib/src/code/codeManager.js

@@ -37,16 +37,14 @@ CodeManager.prototype.resolveStep = function (stepIndex, tx) {
   if (stepIndex < 0) return
   this.event.trigger('resolvingStep')
   if (stepIndex === 0) {
-    retrieveCodeAndTrigger(this, tx.to, stepIndex, tx)
-  } else {
-    this.traceManager.getCurrentCalledAddressAt(stepIndex, (error, address) => {
-      if (error) {
-        console.log(error)
-      } else {
-        retrieveCodeAndTrigger(this, address, stepIndex, tx)
-      }
-    })
+    return retrieveCodeAndTrigger(this, tx.to, stepIndex, tx)
   }
+  this.traceManager.getCurrentCalledAddressAt(stepIndex, (error, address) => {
+    if (error) {
+      return console.log(error)
+    }
+    retrieveCodeAndTrigger(this, address, stepIndex, tx)
+  })
 }
 
 /**
@@ -56,23 +54,21 @@ CodeManager.prototype.resolveStep = function (stepIndex, tx) {
  * @param {Function} cb - callback function, return the bytecode
  */
 CodeManager.prototype.getCode = function (address, cb) {
-  if (traceHelper.isContractCreation(address)) {
-    var codes = this.codeResolver.getExecutingCodeFromCache(address)
-    if (!codes) {
-      this.traceManager.getContractCreationCode(address, (error, hexCode) => {
-        if (!error) {
-          codes = this.codeResolver.cacheExecutingCode(address, hexCode)
-          cb(null, codes)
-        }
-      })
-    } else {
-      cb(null, codes)
-    }
-  } else {
-    this.codeResolver.resolveCode(address, (address, code) => {
+  if (!traceHelper.isContractCreation(address)) {
+    return this.codeResolver.resolveCode(address).then((code) => {
       cb(null, code)
     })
   }
+  var codes = this.codeResolver.getExecutingCodeFromCache(address)
+  if (codes) {
+    return cb(null, codes)
+  }
+  this.traceManager.getContractCreationCode(address, (error, hexCode) => {
+    if (!error) {
+      codes = this.codeResolver.cacheExecutingCode(address, hexCode)
+      cb(null, codes)
+    }
+  })
 }
 
 /**
@@ -88,16 +84,14 @@ CodeManager.prototype.getFunctionFromStep = function (stepIndex, sourceMap, ast)
     if (error) {
       console.log(error)
       return { error: 'Cannot retrieve current address for ' + stepIndex }
-    } else {
-      this.traceManager.getCurrentPC(stepIndex, (error, pc) => {
-        if (error) {
-          console.log(error)
-          return { error: 'Cannot retrieve current PC for ' + stepIndex }
-        } else {
-          return this.getFunctionFromPC(address, pc, sourceMap, ast)
-        }
-      })
     }
+    this.traceManager.getCurrentPC(stepIndex, (error, pc) => {
+      if (error) {
+        console.log(error)
+        return { error: 'Cannot retrieve current PC for ' + stepIndex }
+      }
+      return this.getFunctionFromPC(address, pc, sourceMap, ast)
+    })
   })
 }
 
@@ -112,11 +106,10 @@ CodeManager.prototype.getInstructionIndex = function (address, step, callback) {
   this.traceManager.getCurrentPC(step, (error, pc) => {
     if (error) {
       console.log(error)
-      callback('Cannot retrieve current PC for ' + step, null)
-    } else {
-      const itemIndex = this.codeResolver.getInstructionIndex(address, pc)
-      callback(null, itemIndex)
+      return callback('Cannot retrieve current PC for ' + step, null)
     }
+    const itemIndex = this.codeResolver.getInstructionIndex(address, pc)
+    callback(null, itemIndex)
   })
 }
 
@@ -136,21 +129,19 @@ CodeManager.prototype.getFunctionFromPC = function (address, pc, sourceMap, ast)
 
 function retrieveCodeAndTrigger (codeMananger, address, stepIndex, tx) {
   codeMananger.getCode(address, (error, result) => {
-    if (!error) {
-      retrieveIndexAndTrigger(codeMananger, address, stepIndex, result.instructions)
-    } else {
-      console.log(error)
+    if (error) {
+      return console.log(error)
     }
+    retrieveIndexAndTrigger(codeMananger, address, stepIndex, result.instructions)
   })
 }
 
 function retrieveIndexAndTrigger (codeMananger, address, step, code) {
   codeMananger.getInstructionIndex(address, step, (error, result) => {
-    if (!error) {
-      codeMananger.event.trigger('changed', [code, address, result])
-    } else {
-      console.log(error)
+    if (error) {
+      return console.log(error)
     }
+    codeMananger.event.trigger('changed', [code, address, result])
   })
 }
 

libs/remix-lib/src/code/codeResolver.js

@@ -15,24 +15,20 @@ CodeResolver.prototype.clear = function () {
   this.instructionsIndexByBytesOffset = {}
 }
 
-CodeResolver.prototype.resolveCode = function (address, callBack) {
-  const cache = this.getExecutingCodeFromCache(address)
-  if (cache) {
-    return callBack(address, cache)
-  }
-
-  this.loadCode(address, (code) => {
-    callBack(address, this.cacheExecutingCode(address, code))
-  })
-}
-
-CodeResolver.prototype.loadCode = function (address, callback) {
-  this.web3.eth.getCode(address, (error, result) => {
-    if (error) {
-      console.log(error)
-    } else {
-      callback(result)
+CodeResolver.prototype.resolveCode = async function (address) {
+  return new Promise((resolve, reject) => {
+    const cache = this.getExecutingCodeFromCache(address)
+    if (cache) {
+      return resolve(cache)
     }
+
+    this.web3.eth.getCode(address, (error, code) => {
+      if (error) {
+        // return console.log(error)
+        return reject(error)
+      }
+      return resolve(this.cacheExecutingCode(address, code))
+    })
   })
 }
 
@@ -53,14 +49,14 @@ CodeResolver.prototype.formatCode = function (hexCode) {
 }
 
 CodeResolver.prototype.getExecutingCodeFromCache = function (address) {
-  if (this.instructionsByAddress[address]) {
-    return {
-      instructions: this.instructionsByAddress[address],
-      instructionsIndexByBytesOffset: this.instructionsIndexByBytesOffset[address],
-      bytecode: this.bytecodeByAddress[address]
-    }
+  if (!this.instructionsByAddress[address]) {
+    return null
+  }
+  return {
+    instructions: this.instructionsByAddress[address],
+    instructionsIndexByBytesOffset: this.instructionsIndexByBytesOffset[address],
+    bytecode: this.bytecodeByAddress[address]
   }
-  return null
 }
 
 CodeResolver.prototype.getInstructionIndex = function (address, pc) {

libs/remix-lib/src/code/disassembler.js

@@ -41,9 +41,8 @@ const toString = function (expr) {
     return expr.label + ':'
   } else if (expr.args) {
     return expr.name.toLowerCase() + '(' + expr.args.reverse().map(toString).join(', ') + ')'
-  } else {
-    return expr.name.toLowerCase()
   }
+  return expr.name.toLowerCase()
 }
 
 const disassemble = function (input) {

libs/remix-lib/src/helpers/traceHelper.js

@@ -55,9 +55,8 @@ module.exports = {
     const step = trace[index]
     if (this.isCallInstruction(step)) {
       return index + 1 < trace.length && trace[index + 1].stack.length !== 0
-    } else {
-      return false
     }
+    return false
   },
 
   contractCreationToken: function (index) {

libs/remix-lib/src/trace/traceAnalyser.js

@@ -6,7 +6,7 @@ function TraceAnalyser (_cache) {
   this.trace = null
 }
 
-TraceAnalyser.prototype.analyse = function (trace, tx, callback) {
+TraceAnalyser.prototype.analyse = function (trace, tx) {
   this.trace = trace
   this.traceCache.pushStoreChanges(0, tx.to)
   let context = {
@@ -27,7 +27,7 @@ TraceAnalyser.prototype.analyse = function (trace, tx, callback) {
     context = this.buildStorage(k, step, context)
     this.buildReturnValues(k, step)
   }
-  callback(null, true)
+  return true
 }
 
 TraceAnalyser.prototype.buildReturnValues = function (index, step) {

libs/remix-lib/src/trace/traceCache.js

@@ -39,31 +39,29 @@ TraceCache.prototype.pushMemoryChanges = function (value) {
 // in the vm/geth/eth. TODO add the error property (with about the error in all clients)
 TraceCache.prototype.pushCall = function (step, index, address, callStack, reverted) {
   let validReturnStep = step.op === 'RETURN' || step.op === 'STOP'
-  if (validReturnStep || reverted) {
-    if (this.currentCall) {
-      this.currentCall.call.return = index - 1
-      if (!validReturnStep) {
-        this.currentCall.call.reverted = reverted
-      }
-      var parent = this.currentCall.parent
-      this.currentCall = parent ? { call: parent.call, parent: parent.parent } : null
+  if ((validReturnStep || reverted) && (this.currentCall)) {
+    this.currentCall.call.return = index - 1
+    if (!validReturnStep) {
+      this.currentCall.call.reverted = reverted
     }
+    var parent = this.currentCall.parent
+    this.currentCall = parent ? { call: parent.call, parent: parent.parent } : null
+    return
+  }
+  let call = {
+    op: step.op,
+    address: address,
+    callStack: callStack,
+    calls: {},
+    start: index
+  }
+  this.addresses.push(address)
+  if (this.currentCall) {
+    this.currentCall.call.calls[index] = call
   } else {
-    let call = {
-      op: step.op,
-      address: address,
-      callStack: callStack,
-      calls: {},
-      start: index
-    }
-    this.addresses.push(address)
-    if (this.currentCall) {
-      this.currentCall.call.calls[index] = call
-    } else {
-      this.callsTree = { call: call }
-    }
-    this.currentCall = { call: call, parent: this.currentCall }
+    this.callsTree = { call: call }
   }
+  this.currentCall = { call: call, parent: this.currentCall }
 }
 
 TraceCache.prototype.pushReturnValue = function (step, value) {

libs/remix-lib/src/trace/traceManager.js

@@ -1,6 +1,5 @@
 'use strict'
 const TraceAnalyser = require('./traceAnalyser')
-const TraceRetriever = require('./traceRetriever')
 const TraceCache = require('./traceCache')
 const TraceStepManager = require('./traceStepManager')
 
@@ -13,43 +12,49 @@ function TraceManager (options) {
   this.trace = null
   this.traceCache = new TraceCache()
   this.traceAnalyser = new TraceAnalyser(this.traceCache)
-  this.traceRetriever = new TraceRetriever({web3: this.web3})
   this.traceStepManager = new TraceStepManager(this.traceAnalyser)
   this.tx
 }
 
 // init section
-TraceManager.prototype.resolveTrace = function (tx, callback) {
+TraceManager.prototype.resolveTrace = async function (tx, callback) {
   this.tx = tx
   this.init()
   if (!this.web3) callback('web3 not loaded', false)
   this.isLoading = true
-  var self = this
-  this.traceRetriever.getTrace(tx.hash, (error, result) => {
-    if (error) {
-      console.log(error)
-      self.isLoading = false
-      callback(error, false)
-    } else {
-      if (result.structLogs.length > 0) {
-        self.trace = result.structLogs
-        self.traceAnalyser.analyse(result.structLogs, tx, function (error, result) {
-          if (error) {
-            self.isLoading = false
-            console.log(error)
-            callback(error, false)
-          } else {
-            self.isLoading = false
-            callback(null, true)
-          }
-        })
-      } else {
-        var mes = tx.hash + ' is not a contract invocation or contract creation.'
-        console.log(mes)
-        self.isLoading = false
-        callback(mes, false)
-      }
+  try {
+    const result = await this.getTrace(tx.hash)
+
+    if (result.structLogs.length > 0) {
+      this.trace = result.structLogs
+
+      this.traceAnalyser.analyse(result.structLogs, tx)
+      this.isLoading = false
+      return callback(null, true)
     }
+    var mes = tx.hash + ' is not a contract invocation or contract creation.'
+    console.log(mes)
+    this.isLoading = false
+    callback(mes, false)
+  } catch (error) {
+    console.log(error)
+    this.isLoading = false
+    callback(error, false)
+  }
+}
+
+TraceManager.prototype.getTrace = function (txHash) {
+  return new Promise((resolve, reject) => {
+    const options = {
+      disableStorage: true,
+      disableMemory: false,
+      disableStack: false,
+      fullStorage: false
+    }
+    this.web3.debug.traceTransaction(txHash, options, function (error, result) {
+      if (error) return reject(error)
+      resolve(result)
+    })
   })
 }
 
@@ -206,28 +211,24 @@ TraceManager.prototype.getCurrentStep = function (stepIndex, callback) {
   callback(null, this.traceCache.steps[stepIndex])
 }
 
-TraceManager.prototype.getMemExpand = function (stepIndex, callback) {
-  const check = this.checkRequestedStep(stepIndex)
-  if (check) {
-    return callback(check, null)
-  }
-  callback(null, this.trace[stepIndex].memexpand ? this.trace[stepIndex].memexpand : '')
+TraceManager.prototype.getMemExpand = function (stepIndex) {
+  return (this.getStepProperty(stepIndex, 'memexpand') || '')
 }
 
-TraceManager.prototype.getStepCost = function (stepIndex, callback) {
-  const check = this.checkRequestedStep(stepIndex)
-  if (check) {
-    return callback(check, null)
-  }
-  callback(null, this.trace[stepIndex].gasCost)
+TraceManager.prototype.getStepCost = function (stepIndex) {
+  return this.getStepProperty(stepIndex, 'gasCost')
 }
 
-TraceManager.prototype.getRemainingGas = function (stepIndex, callback) {
+TraceManager.prototype.getRemainingGas = function (stepIndex) {
+  return this.getStepProperty(stepIndex, 'gas')
+}
+
+TraceManager.prototype.getStepProperty = function (stepIndex, property) {
   const check = this.checkRequestedStep(stepIndex)
   if (check) {
-    return callback(check, null)
+    throw new Error(check)
   }
-  callback(null, this.trace[stepIndex].gas)
+  return this.trace[stepIndex][property]
 }
 
 TraceManager.prototype.isCreationStep = function (stepIndex) {

libs/remix-lib/src/trace/traceStepManager.js

@@ -21,18 +21,16 @@ TraceStepManager.prototype.findStepOverBack = function (currentStep) {
   if (this.isReturnInstruction(currentStep)) {
     const call = util.findCall(currentStep, this.traceAnalyser.traceCache.callsTree.call)
     return call.start > 0 ? call.start - 1 : 0
-  } else {
-    return currentStep > 0 ? currentStep - 1 : 0
   }
+  return currentStep > 0 ? currentStep - 1 : 0
 }
 
 TraceStepManager.prototype.findStepOverForward = function (currentStep) {
   if (this.isCallInstruction(currentStep)) {
     const call = util.findCall(currentStep + 1, this.traceAnalyser.traceCache.callsTree.call)
     return call.return + 1 < this.traceAnalyser.trace.length ? call.return + 1 : this.traceAnalyser.trace.length - 1
-  } else {
-    return this.traceAnalyser.trace.length >= currentStep + 1 ? currentStep + 1 : currentStep
   }
+  return this.traceAnalyser.trace.length >= currentStep + 1 ? currentStep + 1 : currentStep
 }
 
 TraceStepManager.prototype.findNextCall = function (currentStep) {
@@ -42,12 +40,10 @@ TraceStepManager.prototype.findNextCall = function (currentStep) {
     var callStart = util.findLowerBound(currentStep, subCalls) + 1
     if (subCalls.length > callStart) {
       return subCalls[callStart] - 1
-    } else {
-      return currentStep
     }
-  } else {
     return currentStep
   }
+  return currentStep
 }
 
 TraceStepManager.prototype.findStepOut = function (currentStep) {

libs/remix-lib/test/traceManager.js

@@ -239,39 +239,36 @@ tape('TraceManager', function (t) {
   })
 
   t.test('TraceManager.getMemExpand', function (st) {
-    traceManager.getMemExpand(2, function (error, result) {
+    try {
+      const result = traceManager.getMemExpand(2)
       console.log(result)
-      if (error) {
-        st.fail(error)
-      } else {
-        st.ok(result === '3')
-        st.end()
-      }
-    })
+      st.ok(result === '3')
+      st.end()
+    } catch (error) {
+      st.fail(error)
+    }
   })
 
   t.test('TraceManager.getStepCost', function (st) {
-    traceManager.getStepCost(34, function (error, result) {
+    try {
+      const result = traceManager.getStepCost(23)
       console.log(result)
-      if (error) {
-        st.fail(error)
-      } else {
-        st.ok(result === '3')
-        st.end()
-      }
-    })
+      st.ok(result === '3')
+      st.end()
+    } catch (error) {
+      st.fail(error)
+    }
   })
 
   t.test('TraceManager.getRemainingGas', function (st) {
-    traceManager.getRemainingGas(55, function (error, result) {
+    try {
+      const result = traceManager.getRemainingGas(55)
       console.log(result)
-      if (error) {
-        st.fail(error)
-      } else {
-        st.ok(result === '79306')
-        st.end()
-      }
-    })
+      st.ok(result === '79306')
+      st.end()
+    } catch (error) {
+      st.fail(error)
+    }
   })
 
   t.test('TraceManager.findStepOverBack', function (st) {