Merge pull request #474 from ethereum/fix#470

apps/remix-ide-e2e/src/commands/checkVariableDebug.ts

@@ -33,7 +33,7 @@ function checkDebug (browser: NightwatchBrowser, id: string, debugValue: Nightwa
     }
     const equal = deepequal(debugValue, value)
     if (!equal) {
-      browser.assert.fail('checkDebug on ' + id, 'info about error\n ' + JSON.stringify(debugValue) + '\n ' + JSON.stringify(value), '')
+      browser.assert.fail(JSON.stringify(value), 'info about error\n ' + JSON.stringify(debugValue) + '\n ' + JSON.stringify(value), '')
     }
     done()
   })

apps/remix-ide-e2e/src/commands/goToVMTraceStep.ts

@@ -2,11 +2,8 @@ import { NightwatchBrowser } from 'nightwatch'
 import EventEmitter from "events"
 class GoToVmTraceStep extends EventEmitter {
   command (this: NightwatchBrowser, step: number, incr?: number): NightwatchBrowser {
-    this.api.perform((done) => {
-      goToVMtraceStep(this.api, step, incr, () => {
-        done()
-        this.emit('complete')
-      })
+    goToVMtraceStep(this.api, step, incr, () => {        
+      this.emit('complete')
     })
     return this
   }

apps/remix-ide-e2e/src/tests/debugger.test.ts

@@ -93,7 +93,43 @@ module.exports = {
         _decimals = 18;
     }`) != -1, 
     'current displayed content is not from the ERC20 source code')
-    })
+    })        
+  },
+
+  'Should display correct source highlighting while debugging a contract which has ABIEncoderV2': function (browser: NightwatchBrowser) {
+    /*
+      localVariable_step266_ABIEncoder and localVariable_step717_ABIEncoder
+      still contains unwanted values (related to decoding calldata types)
+      This is still an issue @todo(https://github.com/ethereum/remix-project/issues/481), so this test will fail when this issue is fixed
+    */
+    browser
+    .clickLaunchIcon('solidity')
+    .setSolidityCompilerVersion('soljson-v0.6.12+commit.27d51765.js')
+    .clickLaunchIcon('udapp')
+    .testContracts('withABIEncoderV2.sol', sources[2]['browser/withABIEncoderV2.sol'], ['test'])
+    .selectContract('test')
+    .createContract('')
+    .clickInstance(2)
+    .clickFunction('test1 - transact (not payable)', {types: 'bytes userData', values: '0x000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000015b38da6a701c568545dcfcb03fcb875f56beddc4'})
+    .debugTransaction(4)
+    .pause(2000)    
+    .goToVMTraceStep(261)
+    .pause(1000)
+    /*
+      for the test below:
+      source highlight should remain line `bytes32 idAsk = abi.decode(userData[:33], (bytes32));`
+      At this vmtrace index, the sourcemap has file = -1 because the execution is in the generated sources (ABIEncoderV2)
+      the atIndex of SourceLocationTracker was buggy and return an incorrect value, this is fixed
+      But the debugger uses now validSourcelocation, which means file is not -1.
+      In that case the source highlight at 261 should be the same as for step 262
+    */
+    .waitForElementPresent('.highlightLine7') 
+    .goToVMTraceStep(266)
+    .pause(1000)
+    .checkVariableDebug('soliditylocals', localVariable_step266_ABIEncoder) // locals should not be initiated at this point, only idAsk should
+    .goToVMTraceStep(717)
+    .pause(5000)
+    .checkVariableDebug('soliditylocals', localVariable_step717_ABIEncoder) // all locals should be initiaed
     .end()
   },
 
@@ -136,7 +172,92 @@ const sources = [
   },
   {
     'browser/externalImport.sol': {content: 'import "https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/ERC20.sol"; contract test7 {}'}
+  },
+  {
+    'browser/withABIEncoderV2.sol': {content: `
+    pragma experimental ABIEncoderV2;
+
+    contract test {
+    // 000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000015b38da6a701c568545dcfcb03fcb875f56beddc4
+    // 0000000000000000000000000000000000000000000000000000000000000002
+    function test1 (bytes calldata userData) external returns (bytes memory, bytes32, bytes32, uint) {
+        bytes32 idAsk = abi.decode(userData[:33], (bytes32));
+        bytes32 idOffer = abi.decode(userData[32:64], (bytes32));
+              
+        bytes memory ro  = abi.encodePacked(msg.sender, msg.sender, idAsk, idOffer);
+        return (ro, idAsk, idOffer, userData.length);
+    }
+    
+    
+    function testgp (bytes calldata userData) external returns (bytes4) {
+        return  abi.decode(userData[:4], (bytes4));
+    }
+}
+    `}
   }
 ]
 
+const localVariable_step266_ABIEncoder = {
+  "<1>": {
+      "length": "0xNaN",
+      "type": "bytes",
+      "value": "0x"
+  },
+  "<2>": {
+      "type": "bytes32",
+      "value": "0x0000000000000000000000000000000000000000000000000000000000000000"
+  },
+  "<3>": {
+      "type": "bytes32",
+      "value": "0x0000000000000000000000000000000000000000000000000000000000000000"
+  },
+  "<4>": {
+      "type": "uint256",
+      "value": "0"
+  },
+  "idAsk": {
+      "type": "bytes32",
+      "value": "0x0000000000000000000000000000000000000000000000000000000000000002"
+  },
+  "userData": {
+      "error": "<decoding failed - no decoder for calldata>",
+      "type": "bytes"
+  }
+}
 
+const localVariable_step717_ABIEncoder = {
+  "<1>": {
+      "length": "0xd0",
+      "type": "bytes",
+      "value": "0x5b38da6a701c568545dcfcb03fcb875f56beddc45b38da6a701c568545dcfcb03fcb875f56beddc400000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000001"
+  },
+  "<2>": {
+      "type": "bytes32",
+      "value": "0x0000000000000000000000000000000000000000000000000000000000000002"
+  },
+  "<3>": {
+      "type": "bytes32",
+      "value": "0x0000000000000000000000000000000000000000000000000000000000000001"
+  },
+  "<4>": {
+      "type": "uint256",
+      "value": "84"
+  },
+  "idAsk": {
+      "type": "bytes32",
+      "value": "0x0000000000000000000000000000000000000000000000000000000000000002"
+  },
+  "idOffer": {
+      "type": "bytes32",
+      "value": "0x0000000000000000000000000000000000000000000000000000000000000001"
+  },
+  "ro": {
+      "length": "0xd0",
+      "type": "bytes",
+      "value": "0x5b38da6a701c568545dcfcb03fcb875f56beddc45b38da6a701c568545dcfcb03fcb875f56beddc400000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000001"
+  },
+  "userData": {
+      "error": "<decoding failed - no decoder for calldata>",
+      "type": "bytes"
+  }
+}
\ No newline at end of file

libs/remix-debug/src/Ethdebugger.js

@@ -60,6 +60,10 @@ Ethdebugger.prototype.sourceLocationFromVMTraceIndex = async function (address,
   return this.callTree.sourceLocationTracker.getSourceLocationFromVMTraceIndex(address, stepIndex, this.solidityProxy.contracts)
 }
 
+Ethdebugger.prototype.getValidSourceLocationFromVMTraceIndex = async function (address, stepIndex) {
+  return this.callTree.sourceLocationTracker.getValidSourceLocationFromVMTraceIndex(address, stepIndex, this.solidityProxy.contracts)
+}
+
 Ethdebugger.prototype.sourceLocationFromInstructionIndex = async function (address, instIndex, callback) {
   return this.callTree.sourceLocationTracker.getSourceLocationFromInstructionIndex(address, instIndex, this.solidityProxy.contracts)
 }

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

@@ -80,7 +80,7 @@ class BreakpointManager {
     while (currentStep > 0 && currentStep < this.debugger.traceManager.trace.length) {
       try {
         previousSourceLocation = sourceLocation
-        sourceLocation = await this.debugger.callTree.extractSourceLocation(currentStep)
+        sourceLocation = await this.debugger.callTree.extractValidSourceLocation(currentStep)
       } catch (e) {
         console.log('cannot jump to breakpoint ' + e)
         return

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

@@ -53,7 +53,7 @@ Debugger.prototype.registerAndHighlightCodeItem = async function (index) {
     const compilationResultForAddress = await this.compilationResult(address)
     if (!compilationResultForAddress) return
 
-    this.debugger.callTree.sourceLocationTracker.getSourceLocationFromVMTraceIndex(address, index, compilationResultForAddress.data.contracts).then((rawLocation) => {
+    this.debugger.callTree.sourceLocationTracker.getValidSourceLocationFromVMTraceIndex(address, index, compilationResultForAddress.data.contracts).then((rawLocation) => {
       if (compilationResultForAddress && compilationResultForAddress.data) {
         var lineColumnPos = this.offsetToLineColumnConverter.offsetToLineColumn(rawLocation, rawLocation.file, compilationResultForAddress.source.sources, compilationResultForAddress.data.sources)
         this.event.trigger('newSourceLocation', [lineColumnPos, rawLocation])
@@ -114,7 +114,7 @@ Debugger.prototype.debugTx = function (tx, loadingCb) {
   this.step_manager = new StepManager(this.debugger, this.debugger.traceManager)
 
   this.debugger.codeManager.event.register('changed', this, (code, address, instIndex) => {
-    this.debugger.callTree.sourceLocationTracker.getSourceLocationFromVMTraceIndex(address, this.step_manager.currentStepIndex, this.debugger.solidityProxy.contracts).then((sourceLocation) => {
+    this.debugger.callTree.sourceLocationTracker.getValidSourceLocationFromVMTraceIndex(address, this.step_manager.currentStepIndex, this.debugger.solidityProxy.contracts).then((sourceLocation) => {
       this.vmDebuggerLogic.event.trigger('sourceLocationChanged', [sourceLocation])
     })
   })

libs/remix-debug/src/solidity-decoder/internalCallTree.js

@@ -134,6 +134,16 @@ class InternalCallTree {
       throw new Error('InternalCallTree - Cannot retrieve sourcelocation for step ' + step + ' ' + error)
     }
   }
+
+  async extractValidSourceLocation (step) {
+    try {
+      const address = this.traceManager.getCurrentCalledAddressAt(step)
+      const location = await this.sourceLocationTracker.getValidSourceLocationFromVMTraceIndex(address, step, this.solidityProxy.contracts)
+      return location
+    } catch (error) {
+      throw new Error('InternalCallTree - Cannot retrieve valid sourcelocation for step ' + step + ' ' + error)
+    }
+  }
 }
 
 async function buildTree (tree, step, scopeId, isExternalCall) {

libs/remix-debug/src/source/sourceLocationTracker.js

@@ -3,6 +3,7 @@ const EventManager = require('../eventManager')
 const helper = require('../trace/traceHelper')
 const SourceMappingDecoder = require('./sourceMappingDecoder')
 const remixLib = require('@remix-project/remix-lib')
+const { map } = require('jquery')
 const util = remixLib.util
 
 /**
@@ -16,12 +17,11 @@ function SourceLocationTracker (_codeManager) {
 }
 
 /**
- * Return the source location associated with the given @arg index
+ * Return the source location associated with the given @arg index (instruction index)
  *
  * @param {String} address - contract address from which the source location is retrieved
  * @param {Int} index - index in the instruction list from where the source location is retrieved
  * @param {Object} contractDetails - AST of compiled contracts
- * @param {Function} cb - callback function
  */
 SourceLocationTracker.prototype.getSourceLocationFromInstructionIndex = async function (address, index, contracts) {
   const sourceMap = await extractSourceMap(this, this.codeManager, address, contracts)
@@ -29,19 +29,35 @@ SourceLocationTracker.prototype.getSourceLocationFromInstructionIndex = async fu
 }
 
 /**
- * Return the source location associated with the given @arg pc
+ * Return the source location associated with the given @arg vmTraceIndex
  *
  * @param {String} address - contract address from which the source location is retrieved
  * @param {Int} vmtraceStepIndex - index of the current code in the vmtrace
  * @param {Object} contractDetails - AST of compiled contracts
- * @param {Function} cb - callback function
  */
-SourceLocationTracker.prototype.getSourceLocationFromVMTraceIndex = async function (address, vmtraceStepIndex, contracts) {
+SourceLocationTracker.prototype.getSourceLocationFromVMTraceIndex = async function (address, vmtraceStepIndex, contracts) {  
   const sourceMap = await extractSourceMap(this, this.codeManager, address, contracts)
   const index = this.codeManager.getInstructionIndex(address, vmtraceStepIndex)
   return this.sourceMappingDecoder.atIndex(index, sourceMap)
 }
 
+/**
+ * Return a valid source location associated with the given @arg vmTraceIndex
+ *
+ * @param {String} address - contract address from which the source location is retrieved
+ * @param {Int} vmtraceStepIndex - index of the current code in the vmtrace
+ * @param {Object} contractDetails - AST of compiled contracts
+ */
+SourceLocationTracker.prototype.getValidSourceLocationFromVMTraceIndex = async function (address, vmtraceStepIndex, contracts) {
+  let map = { file: -1}
+  while (vmtraceStepIndex >= 0 && map.file === -1) {
+    map = await this.getSourceLocationFromVMTraceIndex(address, vmtraceStepIndex, contracts)
+    vmtraceStepIndex = vmtraceStepIndex - 1    
+  }
+  console.log(map, vmtraceStepIndex)
+  return map
+}
+
 SourceLocationTracker.prototype.clearCache = function () {
   this.sourceMapByAddress = {}
 }
@@ -52,7 +68,7 @@ function getSourceMap (address, code, contracts) {
   for (let file in contracts) {
     for (let contract in contracts[file]) {
       const bytecode = contracts[file][contract].evm.bytecode
-      const deployedBytecode = contracts[file][contract].evm.deployedBytecode
+      const deployedBytecode = contracts[file][contract].evm.deployedBytecode     
       if (!deployedBytecode) continue
 
       bytes = isCreation ? bytecode.object : deployedBytecode.object

libs/remix-debug/src/source/sourceMappingDecoder.js

@@ -205,19 +205,13 @@ function atIndex (index, mapping) {
       continue
     }
     current = current.split(':')
-    if (current[2] === '-1') { // if the current step has -1 for the file attribute, we discard it
-       // case: 'file' is not yet assigned, while processing the srcmap (reverse looping) to find 'start', 'length' (etc..), we tumble on -1 for the file.
-       // in that case the step has to be discarded
-      if (ret.file === undefined) ret = {}
-      continue
-    }
     if (ret.start === undefined && current[0] && current[0] !== '-1' && current[0].length) {
       ret.start = parseInt(current[0])
     }
     if (ret.length === undefined && current[1] && current[1] !== '-1' && current[1].length) {
       ret.length = parseInt(current[1])
     }
-    if (ret.file === undefined && current[2] && current[2] !== '-1' && current[2].length) {
+    if (ret.file === undefined && current[2] && current[2].length) {
       ret.file = parseInt(current[2])
     }
     if (ret.jump === undefined && current[3] && current[3].length) {

libs/remix-debug/test/resources/testWeb3.js

@@ -6,6 +6,10 @@ web3Override.debug = {}
 var data = init.readFile(require('path').resolve(__dirname, 'testWeb3.json'))
 data = JSON.parse(data)
 
+var traceWithABIEncoder = init.readFile(require('path').resolve(__dirname, 'traceWithABIEncoder.json'))
+traceWithABIEncoder = 
+
+data.testTraces['0x20ef65b8b186ca942fcccd634f37074dde49b541c27994fc7596740ef44cfd53'] = JSON.parse(traceWithABIEncoder)
 web3Override.eth.getCode = function (address, callback) {
   if (callback) {
     callback(null, data.testCodes[address])

libs/remix-debug/test/sourceLocationTracker.js

+'use strict'
+const tape = require('tape')
+const TraceManager = require('../src/trace/traceManager')
+const CodeManager = require('../src/code/codeManager')
+const web3Test = require('./resources/testWeb3')
+const sourceMapping = require('./resources/sourceMapping')
+const SourceLocationTracker = require('../src/source/sourceLocationTracker')
+const compiler = require('solc')
+const compilerInput = require('./helpers/compilerHelper').compilerInput
+
+tape('SourceLocationTracker', function (t) {
+  t.test('SourceLocationTracker.getSourceLocationFromVMTraceIndex - simple contract', async function (st) {
+
+    const traceManager = new TraceManager({web3: web3Test})
+    let codeManager = new CodeManager(traceManager)     
+
+    let output = compiler.compile(compilerInput(contracts))
+    output = JSON.parse(output)
+    
+    codeManager.codeResolver.cacheExecutingCode('0x0d3a18d64dfe4f927832ab58d6451cecc4e517c5', '0x' + output.contracts['test.sol']['test'].evm.deployedBytecode.object)
+    
+    const tx = web3Test.eth.getTransaction('0x20ef65b8b186ca942fcccd634f37074dde49b541c27994fc7596740ef44cfd52')
+
+    traceManager.resolveTrace(tx).then(async () => {     
+
+      const sourceLocationTracker = new SourceLocationTracker(codeManager)
+      
+      try {
+        const map = await sourceLocationTracker.getSourceLocationFromVMTraceIndex('0x0d3a18d64dfe4f927832ab58d6451cecc4e517c5', 0, output.contracts)
+        st.equal(map.file, 0)
+        st.equal(map.start, 0)
+      } catch (e) {
+        console.log(e)
+      }
+      st.end()
+
+    }).catch((e) => {
+      t.fail(' - traceManager.resolveTrace - failed ')
+      console.error(e)
+    })
+  })
+
+  t.test('SourceLocationTracker.getSourceLocationFromVMTraceIndex - ABIEncoder V2 contract', async function (st) {
+
+    const traceManager = new TraceManager({web3: web3Test})
+    let codeManager = new CodeManager(traceManager)     
+
+    let output = compiler.compile(compilerInput(ABIEncoderV2))
+    output = JSON.parse(output)
+    
+    codeManager.codeResolver.cacheExecutingCode('0x0d3a18d64dfe4f927832ab58d6451cecc4e517c5', '0x' + output.contracts['test.sol']['test'].evm.deployedBytecode.object)
+    
+    const tx = web3Test.eth.getTransaction('0x20ef65b8b186ca942fcccd634f37074dde49b541c27994fc7596740ef44cfd53')
+
+    traceManager.resolveTrace(tx).then(async () => {     
+
+      const sourceLocationTracker = new SourceLocationTracker(codeManager)
+      
+      try {
+        let map = await sourceLocationTracker.getSourceLocationFromVMTraceIndex('0x0d3a18d64dfe4f927832ab58d6451cecc4e517c5', 0, output.contracts)
+        console.log(map)
+        st.equal(map.file, 0)
+        st.equal(map.start, 35)
+
+        map = await sourceLocationTracker.getSourceLocationFromVMTraceIndex('0x0d3a18d64dfe4f927832ab58d6451cecc4e517c5', 45, output.contracts)
+        st.equal(map.file, -1)
+
+        map = await sourceLocationTracker.getValidSourceLocationFromVMTraceIndex('0x0d3a18d64dfe4f927832ab58d6451cecc4e517c5', 45, output.contracts)
+        st.equal(map.file, 0)
+        st.equal(map.start, 303)
+        st.equal(map.length, 448)
+      } catch (e) {
+        console.log(e)
+      }
+      st.end()
+
+    }).catch(() => {
+      t.fail(' - traceManager.resolveTrace - failed ')
+    })
+  })
+})
+
+const contracts = `contract test {
+    function f1() public returns (uint) {
+        uint t = 4;
+        return t;
+    }
+    
+    function f2() public {
+        
+    }
+}
+`
+
+const ABIEncoderV2 = `pragma experimental ABIEncoderV2;
+
+contract test {
+    // 000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000015b38da6a701c568545dcfcb03fcb875f56beddc4
+    // 0000000000000000000000000000000000000000000000000000000000000002
+    function testg (bytes calldata userData) external returns (bytes memory, bytes32, bytes32, uint) {
+        bytes32 idAsk = abi.decode(userData[:33], (bytes32));
+        bytes32 idOffer = abi.decode(userData[32:64], (bytes32));
+        // bytes4 sellerAddress = abi.decode(userData[:4], (bytes4));       
+        bytes memory ro  = abi.encodePacked(msg.sender, msg.sender, idAsk, idOffer);
+        return (ro, idAsk, idOffer, userData.length);
+    }
+    
+    
+    function testgp (bytes calldata userData) external returns (bytes4) {
+        return  abi.decode(userData[:4], (bytes4));
+    }
+}
+`

libs/remix-debug/test/sourceMappingDecoder.js

@@ -85,9 +85,9 @@ tape('SourceMappingDecoder', function (t) {
     // TokenSaleChallenge - function test(uint256)
     const tokenSaleChallengeMap = sourceMappingDecoder.atIndex(170, sourceMapping.tokenSaleChallengeSourceMap)
     console.log(tokenSaleChallengeMap)
-    st.equal(tokenSaleChallengeMap.start, 211)
-    st.equal(tokenSaleChallengeMap.length, 48)
-    st.equal(tokenSaleChallengeMap.file, 0)
+    st.equal(tokenSaleChallengeMap.start, 45)
+    st.equal(tokenSaleChallengeMap.length, 16)
+    st.equal(tokenSaleChallengeMap.file, -1)
     st.equal(tokenSaleChallengeMap.jump, '-')
   })
 

libs/remix-debug/test/tests.js

@@ -4,6 +4,7 @@ require('./traceManager.js')
 require('./codeManager.js')
 require('./disassembler.js')
 require('./sourceMappingDecoder.js')
+require('./sourceLocationTracker.js')
 require('./decoder/decodeInfo.js')
 require('./decoder/storageLocation.js')
 require('./decoder/storageDecoder.js')