Fix a bug in revert_trace.ts

3 files changed, 32 insertions, 17 deletions

diff --git a/packages/sol-cov/src/revert_trace.ts b/packages/sol-cov/src/revert_trace.ts
index 59edf5db4..04d410f0a 100644
--- a/packages/sol-cov/src/revert_trace.ts
+++ b/packages/sol-cov/src/revert_trace.ts
@@ -8,7 +8,7 @@ import { utils } from './utils';
 
 export function getRevertTrace(structLogs: StructLog[], startAddress: string): EvmCallStack {
     const evmCallStack: EvmCallStack = [];
-    let currentAddress = startAddress;
+    const addressStack = [startAddress];
     if (_.isEmpty(structLogs)) {
         return [];
     }
@@ -16,19 +16,17 @@ export function getRevertTrace(structLogs: StructLog[], startAddress: string): E
     // tslint:disable-next-line:prefer-for-of
     for (let i = 0; i < normalizedStructLogs.length; i++) {
         const structLog = normalizedStructLogs[i];
-        if (structLog.depth !== evmCallStack.length) {
+        if (structLog.depth !== addressStack.length - 1) {
             throw new Error("Malformed trace. Trace depth doesn't match call stack depth");
         }
         // After that check we have a guarantee that call stack is never empty
         // If it would: callStack.length - 1 === structLog.depth === -1
         // That means that we can always safely pop from it
 
-        // TODO(albrow): split out isCallLike and isEndOpcode
         if (utils.isCallLike(structLog.op)) {
-            const evmCallStackEntry = _.last(evmCallStack) as EvmCallStackEntry;
-            const prevAddress = _.isUndefined(evmCallStackEntry) ? currentAddress : evmCallStackEntry.address;
+            const currentAddress = _.last(addressStack) as string;
             const jumpAddressOffset = 1;
-            currentAddress = utils.getAddressFromStackEntry(
+            const newAddress = utils.getAddressFromStackEntry(
                 structLog.stack[structLog.stack.length - jumpAddressOffset - 1],
             );
 
@@ -40,8 +38,9 @@ export function getRevertTrace(structLogs: StructLog[], startAddress: string): E
             // function. We manually check if the call depth had changed to handle that case.
             const nextStructLog = normalizedStructLogs[i + 1];
             if (nextStructLog.depth !== structLog.depth) {
+                addressStack.push(newAddress);
                 evmCallStack.push({
-                    address: prevAddress,
+                    address: currentAddress,
                     structLog,
                 });
             }
@@ -50,6 +49,7 @@ export function getRevertTrace(structLogs: StructLog[], startAddress: string): E
             const nextStructLog = normalizedStructLogs[i + 1];
             if (_.isUndefined(nextStructLog) || nextStructLog.depth !== structLog.depth) {
                 evmCallStack.pop();
+                addressStack.pop();
             }
             if (structLog.op === OpCode.SelfDestruct) {
                 // After contract execution, we look at all sub-calls to external contracts, and for each one, fetch
@@ -63,7 +63,7 @@ export function getRevertTrace(structLogs: StructLog[], startAddress: string): E
             }
         } else if (structLog.op === OpCode.Revert) {
             evmCallStack.push({
-                address: currentAddress,
+                address: _.last(addressStack) as string,
                 structLog,
             });
             return evmCallStack;
@@ -73,6 +73,17 @@ export function getRevertTrace(structLogs: StructLog[], startAddress: string): E
                 "Detected a contract created from within another contract. Sol-cov currently doesn't support that scenario. We'll just skip that trace",
             );
             return [];
+        } else {
+            if (structLog !== _.last(normalizedStructLogs)) {
+                const nextStructLog = normalizedStructLogs[i + 1];
+                if (nextStructLog.depth === structLog.depth) {
+                    continue;
+                } else if (nextStructLog.depth === structLog.depth - 1) {
+                    addressStack.pop();
+                } else {
+                    throw new Error('Malformed trace. Unexpected call depth change');
+                }
+            }
         }
     }
     if (evmCallStack.length !== 0) {
diff --git a/packages/sol-cov/src/revert_trace_subprovider.ts b/packages/sol-cov/src/revert_trace_subprovider.ts
index ea878058c..aef9c7568 100644
--- a/packages/sol-cov/src/revert_trace_subprovider.ts
+++ b/packages/sol-cov/src/revert_trace_subprovider.ts
@@ -186,6 +186,10 @@ export class RevertTraceSubprovider extends Subprovider {
         }
         for (const evmCallStackEntry of evmCallStack) {
             const isContractCreation = evmCallStackEntry.address === constants.NEW_CONTRACT;
+            if (isContractCreation) {
+                this._logger.error('Contract creation not supported');
+                continue;
+            }
             const bytecode = await this._web3Wrapper.getContractCodeAsync(evmCallStackEntry.address);
             const contractData = utils.getContractDataIfExists(this._contractsData, bytecode);
             if (_.isUndefined(contractData)) {
@@ -223,7 +227,7 @@ export class RevertTraceSubprovider extends Subprovider {
             sourceRanges.push(sourceRange);
         }
         if (sourceRanges.length > 0) {
-            this._logger.error('\n\nStack trace:\n');
+            this._logger.error('\n\nStack trace for REVERT:\n');
             _.forEach(sourceRanges, sourceRange => {
                 this._logger.error(
                     `${sourceRange.fileName}:${sourceRange.location.start.line}:${sourceRange.location.start.column}`,
diff --git a/packages/sol-cov/src/trace.ts b/packages/sol-cov/src/trace.ts
index b43fd19a2..fad2e5e08 100644
--- a/packages/sol-cov/src/trace.ts
+++ b/packages/sol-cov/src/trace.ts
@@ -11,7 +11,7 @@ export interface TraceByContractAddress {
 export function getTracesByContractAddress(structLogs: StructLog[], startAddress: string): TraceByContractAddress {
     const traceByContractAddress: TraceByContractAddress = {};
     let currentTraceSegment = [];
-    const callStack = [startAddress];
+    const addressStack = [startAddress];
     if (_.isEmpty(structLogs)) {
         return traceByContractAddress;
     }
@@ -19,7 +19,7 @@ export function getTracesByContractAddress(structLogs: StructLog[], startAddress
     // tslint:disable-next-line:prefer-for-of
     for (let i = 0; i < normalizedStructLogs.length; i++) {
         const structLog = normalizedStructLogs[i];
-        if (structLog.depth !== callStack.length - 1) {
+        if (structLog.depth !== addressStack.length - 1) {
             throw new Error("Malformed trace. Trace depth doesn't match call stack depth");
         }
         // After that check we have a guarantee that call stack is never empty
@@ -28,7 +28,7 @@ export function getTracesByContractAddress(structLogs: StructLog[], startAddress
         currentTraceSegment.push(structLog);
 
         if (utils.isCallLike(structLog.op)) {
-            const currentAddress = _.last(callStack) as string;
+            const currentAddress = _.last(addressStack) as string;
             const jumpAddressOffset = 1;
             const newAddress = utils.getAddressFromStackEntry(
                 structLog.stack[structLog.stack.length - jumpAddressOffset - 1],
@@ -41,14 +41,14 @@ export function getTracesByContractAddress(structLogs: StructLog[], startAddress
             // function. We manually check if the call depth had changed to handle that case.
             const nextStructLog = normalizedStructLogs[i + 1];
             if (nextStructLog.depth !== structLog.depth) {
-                callStack.push(newAddress);
+                addressStack.push(newAddress);
                 traceByContractAddress[currentAddress] = (traceByContractAddress[currentAddress] || []).concat(
                     currentTraceSegment,
                 );
                 currentTraceSegment = [];
             }
         } else if (utils.isEndOpcode(structLog.op)) {
-            const currentAddress = callStack.pop() as string;
+            const currentAddress = addressStack.pop() as string;
             traceByContractAddress[currentAddress] = (traceByContractAddress[currentAddress] || []).concat(
                 currentTraceSegment,
             );
@@ -75,7 +75,7 @@ export function getTracesByContractAddress(structLogs: StructLog[], startAddress
                 if (nextStructLog.depth === structLog.depth) {
                     continue;
                 } else if (nextStructLog.depth === structLog.depth - 1) {
-                    const currentAddress = callStack.pop() as string;
+                    const currentAddress = addressStack.pop() as string;
                     traceByContractAddress[currentAddress] = (traceByContractAddress[currentAddress] || []).concat(
                         currentTraceSegment,
                     );
@@ -86,11 +86,11 @@ export function getTracesByContractAddress(structLogs: StructLog[], startAddress
             }
         }
     }
-    if (callStack.length !== 0) {
+    if (addressStack.length !== 0) {
         logUtils.warn('Malformed trace. Call stack non empty at the end');
     }
     if (currentTraceSegment.length !== 0) {
-        const currentAddress = callStack.pop() as string;
+        const currentAddress = addressStack.pop() as string;
         traceByContractAddress[currentAddress] = (traceByContractAddress[currentAddress] || []).concat(
             currentTraceSegment,
         );