Merge pull request #474 from ethereum/fix#470

Fix source highlight and local/state decoding while using ABIEncoderV2
pull/5370/head
yann300 4 years ago committed by GitHub
commit 34d5983542
  1. 2
      apps/remix-ide-e2e/src/commands/checkVariableDebug.ts
  2. 7
      apps/remix-ide-e2e/src/commands/goToVMTraceStep.ts
  3. 121
      apps/remix-ide-e2e/src/tests/debugger.test.ts
  4. 4
      libs/remix-debug/src/Ethdebugger.js
  5. 2
      libs/remix-debug/src/code/breakpointManager.js
  6. 4
      libs/remix-debug/src/debugger/debugger.js
  7. 10
      libs/remix-debug/src/solidity-decoder/internalCallTree.js
  8. 24
      libs/remix-debug/src/source/sourceLocationTracker.js
  9. 8
      libs/remix-debug/src/source/sourceMappingDecoder.js
  10. 4
      libs/remix-debug/test/resources/testWeb3.js
  11. 8
      libs/remix-debug/test/resources/testWeb3.json
  12. 1
      libs/remix-debug/test/resources/traceWithABIEncoder.json
  13. 113
      libs/remix-debug/test/sourceLocationTracker.js
  14. 6
      libs/remix-debug/test/sourceMappingDecoder.js
  15. 1
      libs/remix-debug/test/tests.js

@ -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()
})

@ -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
}

@ -94,6 +94,42 @@ module.exports = {
}`) != -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"
}
}

@ -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)
}

@ -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

@ -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])
})
})

@ -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) {

@ -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,12 +29,11 @@ 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) {
const sourceMap = await extractSourceMap(this, this.codeManager, address, contracts)
@ -42,6 +41,23 @@ SourceLocationTracker.prototype.getSourceLocationFromVMTraceIndex = async functi
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 = {}
}

@ -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) {

@ -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])

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

@ -0,0 +1,113 @@
'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));
}
}
`

@ -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, '-')
})

@ -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')

Loading…
Cancel
Save