From bfb647704e53814c6fd039c8fbe1c0c9d03ed1e7 Mon Sep 17 00:00:00 2001
From: Harry Altman <harry.j.altman@gmail.com>
Date: Tue, 14 Apr 2020 18:17:52 -0400
Subject: [PATCH 01/18] Combine willCreate into willCall in solidity selectors

---
 packages/debugger/lib/solidity/sagas/index.js     |  6 ++----
 packages/debugger/lib/solidity/selectors/index.js | 11 +++++------
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/packages/debugger/lib/solidity/sagas/index.js b/packages/debugger/lib/solidity/sagas/index.js
index 725820ad291..b54beb8b3b1 100644
--- a/packages/debugger/lib/solidity/sagas/index.js
+++ b/packages/debugger/lib/solidity/sagas/index.js
@@ -41,10 +41,8 @@ function* functionDepthSaga() {
     } else {
       yield put(actions.jump(jumpDirection));
     }
-  } else if (
-    (yield select(solidity.current.willCall)) ||
-    (yield select(solidity.current.willCreate))
-  ) {
+  } else if (yield select(solidity.current.willCall)) {
+    //note: includes creations
     debug("checking if guard needed");
     let guard = yield select(solidity.current.callRequiresPhantomFrame);
     yield put(actions.externalCall(guard));
diff --git a/packages/debugger/lib/solidity/selectors/index.js b/packages/debugger/lib/solidity/selectors/index.js
index ad59c54e0fd..58017f47a06 100644
--- a/packages/debugger/lib/solidity/selectors/index.js
+++ b/packages/debugger/lib/solidity/selectors/index.js
@@ -298,13 +298,12 @@ let solidity = createSelectorTree({
 
     /**
      * solidity.current.willCall
+     * note: includes creations
      */
-    willCall: createLeaf([evm.current.step.isCall], x => x),
-
-    /**
-     * solidity.current.willCreate
-     */
-    willCreate: createLeaf([evm.current.step.isCreate], x => x),
+    willCall: createLeaf(
+      [evm.current.step.isCall, evm.current.step.isCreate],
+      (isCall, isCreate) => isCall || isCreate
+    ),
 
     /**
      * solidity.current.willCallOrCreateButInstantlyReturn

From 29f1aa98a6366e5fd92995e3ee9ce5821331b116 Mon Sep 17 00:00:00 2001
From: Harry Altman <harry.j.altman@gmail.com>
Date: Tue, 14 Apr 2020 18:59:20 -0400
Subject: [PATCH 02/18] Add rudimentary stacktrace implementation

---
 .../debugger/lib/stacktrace/actions/index.js  |  71 +++++++
 packages/debugger/lib/stacktrace/reducers.js  | 132 +++++++++++++
 .../debugger/lib/stacktrace/sagas/index.js    |  70 +++++++
 .../lib/stacktrace/selectors/index.js         | 187 ++++++++++++++++++
 packages/debugger/package.json                |   1 +
 yarn.lock                                     |   6 +
 6 files changed, 467 insertions(+)
 create mode 100644 packages/debugger/lib/stacktrace/actions/index.js
 create mode 100644 packages/debugger/lib/stacktrace/reducers.js
 create mode 100644 packages/debugger/lib/stacktrace/sagas/index.js
 create mode 100644 packages/debugger/lib/stacktrace/selectors/index.js

diff --git a/packages/debugger/lib/stacktrace/actions/index.js b/packages/debugger/lib/stacktrace/actions/index.js
new file mode 100644
index 00000000000..1f808f0f44a
--- /dev/null
+++ b/packages/debugger/lib/stacktrace/actions/index.js
@@ -0,0 +1,71 @@
+export const JUMP_IN = "STACKTRACE_JUMP_IN";
+export function jumpIn(from, functionNode) {
+  return {
+    type: JUMP_IN,
+    from,
+    functionNode
+  };
+}
+
+export const JUMP_OUT = "STACKTRACE_JUMP_OUT";
+export function jumpOut() {
+  return {
+    type: JUMP_OUT
+  };
+}
+
+export const EXTERNAL_CALL = "STACKTRACE_EXTERNAL_CALL";
+export function externalCall(from, skippedInReports) {
+  return {
+    type: EXTERNAL_CALL,
+    from,
+    skippedInReports
+  };
+}
+
+export const EXTERNAL_RETURN = "STACKTRACE_EXTERNAL_RETURN";
+export function externalReturn(from, status) {
+  return {
+    type: EXTERNAL_RETURN,
+    from,
+    status
+  };
+}
+
+export const EXECUTE_RETURN = "EXECUTE_RETURN";
+export function executeReturn(counter) {
+  return {
+    type: EXECUTE_RETURN,
+    counter
+  };
+}
+
+export const MARK_RETURN_POSITION = "MARK_RETURN_POSITION";
+export function markReturnPosition(location) {
+  return {
+    type: MARK_RETURN_POSITION,
+    location
+  };
+}
+
+//note: no reducer explicitly listens for this, but justReturned depends on it
+export const CLEAR_RETURN_MARKER = "CLEAR_RETURN_MARKER";
+export function clearReturnMarker() {
+  return {
+    type: CLEAR_RETURN_MARKER
+  };
+}
+
+export const RESET = "STACKTRACE_RESET";
+export function reset() {
+  return {
+    type: RESET
+  };
+}
+
+export const UNLOAD_TRANSACTION = "STACKTRACE_UNLOAD_TRANSACTION";
+export function unloadTransaction() {
+  return {
+    type: UNLOAD_TRANSACTION
+  };
+}
diff --git a/packages/debugger/lib/stacktrace/reducers.js b/packages/debugger/lib/stacktrace/reducers.js
new file mode 100644
index 00000000000..c07cbbfc331
--- /dev/null
+++ b/packages/debugger/lib/stacktrace/reducers.js
@@ -0,0 +1,132 @@
+import { combineReducers } from "redux";
+
+import * as actions from "./actions";
+
+function callstack(state = [], action) {
+  let top;
+  let newFrame;
+  switch (action.type) {
+    case actions.JUMP_IN:
+      let { from, functionNode } = action;
+      newFrame = {
+        type: "internal",
+        calledFromPosition: from,
+        name:
+          functionNode && functionNode.nodeType === "FunctionDefinition"
+            ? functionNode.name
+            : undefined,
+        //note we don't currently account for getters because currently
+        //we can't; fallback, receive, constructors, & modifiers also remain
+        //unaccounted for at present
+        //(none of these things have associated jump-in markings!)
+        skippedInReports: false
+      };
+      return [...state, newFrame];
+    case actions.JUMP_OUT:
+      top = state[state.length - 1];
+      if (top && top.type === "internal") {
+        return state.slice(0, -1);
+      } else {
+        return state;
+      }
+    case actions.EXTERNAL_CALL:
+      newFrame = {
+        type: "external",
+        calledFromPosition: action.from,
+        skippedInReports: action.skippedInReports,
+        name: undefined
+      };
+      return [...state, newFrame];
+    case actions.EXECUTE_RETURN:
+      let newState = state.slice(); //clone the state
+      //I'm going to write this the C way, hope you don't mind :P
+      let counter = action.counter;
+      while (counter > 0 && newState.length > 0) {
+        top = newState[newState.length - 1];
+        if (top.type === "external") {
+          counter--;
+        }
+        newState.pop();
+      }
+      return newState;
+    case actions.RESET:
+      return [state[0]];
+    case actions.UNLOAD_TRANSACTION:
+      return [];
+    default:
+      //note that we don't do anything on EXTERNAL_RETURN!
+      //the callstack only changes on EXECUTE_RETURN!
+      return state;
+  }
+}
+
+function returnCounter(state = 0, action) {
+  switch (action.type) {
+    case actions.EXTERNAL_RETURN:
+      return state + 1;
+    case actions.EXECUTE_RETURN:
+    case actions.RESET:
+    case actions.UNLOAD_TRANSACTION:
+      return 0;
+    default:
+      return state;
+  }
+}
+
+function markedPosition(state = null, action) {
+  switch (action.type) {
+    case actions.MARK_RETURN_POSITION:
+      return actions.location;
+    case actions.EXECUTE_RETURN:
+    case actions.RESET:
+    case actions.UNLOAD_TRANSACTION:
+      return null;
+    default:
+      return state;
+  }
+}
+
+function innerReturnPosition(state = null, action) {
+  switch (action.type) {
+    case actions.EXTERNAL_RETURN:
+      return action.from;
+    case actions.EXECUTE_RETURN:
+    case actions.RESET:
+    case actions.UNLOAD_TRANSACTION:
+      return null;
+    default:
+      return state;
+  }
+}
+
+function innerReturnStatus(state = null, action) {
+  switch (action.type) {
+    case actions.EXTERNAL_RETURN:
+      return action.status;
+    case actions.EXECUTE_RETURN:
+    case actions.RESET:
+    case actions.UNLOAD_TRANSACTION:
+      return null;
+    default:
+      return state;
+  }
+}
+
+function justReturned(_state = false, action) {
+  return action.type === actions.EXTERNAL_RETURN;
+}
+
+const proc = combineReducers({
+  callstack,
+  returnCounter,
+  markedPosition,
+  innerReturnPosition,
+  innerReturnStatus,
+  justReturned
+});
+
+const reducer = combineReducers({
+  proc
+});
+
+export default reducer;
diff --git a/packages/debugger/lib/stacktrace/sagas/index.js b/packages/debugger/lib/stacktrace/sagas/index.js
new file mode 100644
index 00000000000..123ab533186
--- /dev/null
+++ b/packages/debugger/lib/stacktrace/sagas/index.js
@@ -0,0 +1,70 @@
+import debugModule from "debug";
+const debug = debugModule("debugger:stacktrace:sagas");
+
+import { put, takeEvery, select } from "redux-saga/effects";
+import { prefixName } from "lib/helpers";
+
+import * as actions from "../actions";
+import { TICK } from "lib/trace/actions";
+import * as trace from "lib/trace/sagas";
+
+import stacktrace from "../selectors";
+
+function* tickSaga() {
+  debug("got TICK");
+
+  yield* stacktraceSaga();
+  yield* trace.signalTickSagaCompletion();
+}
+
+function* stacktraceSaga() {
+  //different possible outcomes:
+  const { source, sourceRange } = yield select(stacktrace.current.location);
+  const currentLocation = { source, sourceRange }; //leave out the node
+  if (yield select(stacktrace.current.willJumpIn)) {
+    const nextLocation = yield select(stacktrace.next.location);
+    yield put(actions.jumpIn(currentLocation, nextLocation.node)); //in this case we only want the node
+  } else if (yield select(stacktrace.current.willJumpOut)) {
+    yield put(actions.jumpOut());
+  } else if (yield select(stacktrace.current.willCall)) {
+    //an external frame marked "skip in reports" will be, for reporting
+    //purposes, combined with the frame above, unless that also is a
+    //marker frame (combined in the appropriate sense)
+    //note: includes creations
+    const skipInReports = yield select(
+      stacktrace.current.nextFrameIsSkippedInReports
+    );
+    yield put(actions.externalCall(currentLocation, skipInReports));
+  } else if (yield select(stacktrace.current.willReturn)) {
+    const status = yield select(stacktrace.current.returnStatus);
+    yield put(actions.externalReturn(currentLocation, status));
+  } else if (yield select(stacktrace.current.positionChanged)) {
+    const returnCounter = yield select(stacktrace.current.returnCounter);
+    yield put(actions.executeReturn(returnCounter));
+  } else if (yield select(stacktrace.current.justReturned)) {
+    yield put(actions.markReturnPosition(currentLocation));
+  } else {
+    yield put(actions.clearReturnMarker());
+  }
+}
+
+export function* reset() {
+  yield put(actions.reset());
+}
+
+export function* unload() {
+  yield put(actions.unloadTransaction());
+}
+
+export function* begin() {
+  const skipInReports = yield select(
+    stacktrace.current.nextFrameIsSkippedInReports
+  );
+  yield put(actions.externalCall(null, skipInReports));
+}
+
+export function* saga() {
+  yield takeEvery(TICK, tickSaga);
+}
+
+export default prefixName("stacktrace", saga);
diff --git a/packages/debugger/lib/stacktrace/selectors/index.js b/packages/debugger/lib/stacktrace/selectors/index.js
new file mode 100644
index 00000000000..b3e79f13a96
--- /dev/null
+++ b/packages/debugger/lib/stacktrace/selectors/index.js
@@ -0,0 +1,187 @@
+import debugModule from "debug";
+const debug = debugModule("debugger:stacktrace:selectors");
+
+import { createSelectorTree, createLeaf } from "reselect-tree";
+
+import solidity from "lib/solidity/selectors";
+
+import zipWith from "lodash.zipwith";
+
+const identity = x => x;
+
+function createMultistepSelectors(stepSelector) {
+  return {
+    /**
+     * .location
+     */
+    location: {
+      /**
+       * .source
+       */
+      source: createLeaf([stepSelector.source], identity),
+      /**
+       * .sourceRange
+       */
+      sourceRange: createLeaf([stepSelector.sourceRange], identity),
+      /**
+       * .node
+       */
+      node: createLeaf([stepSelector.node], identity)
+    }
+  };
+}
+
+let stacktrace = createSelectorTree({
+  /**
+   * stacktrace.state
+   */
+  state: state => state.stacktrace,
+
+  /**
+   * stacktrace.current
+   */
+  current: {
+    /**
+     * stacktrace.current.callstack
+     */
+    callstack: createLeaf(["/state"], state => state.proc.callstack),
+    /**
+     * stacktrace.current.returnCounter
+     */
+    returnCounter: createLeaf(["/state"], state => state.proc.returnCounter),
+    /**
+     * stacktrace.current.markedPosition
+     */
+    markedPosition: createLeaf(["/state"], state => state.proc.markedPosition),
+    /**
+     * stacktrace.current.innerReturnPosition
+     */
+    innerReturnPosition: createLeaf(
+      ["/state"],
+      state => state.proc.innerReturnPosition
+    ),
+    /**
+     * stacktrace.current.innerReturnStatus
+     */
+    innerReturnStatus: createLeaf(
+      ["/state"],
+      state => state.proc.innerReturnStatus
+    ),
+    /**
+     * stacktrace.current.justReturned
+     */
+    justReturned: createLeaf(["/state"], state => state.proc.justReturned),
+    ...createMultistepSelectors(solidity.current),
+    /**
+     * stacktrace.current.willJumpIn
+     */
+    willJumpIn: createLeaf(
+      [solidity.current.willJump, solidity.current.jumpDirection],
+      (willJump, jumpDirection) => willJump && jumpDirection === "i"
+    ),
+    /**
+     * stacktrace.current.willJumpOut
+     */
+    willJumpOut: createLeaf(
+      [solidity.current.willJump, solidity.current.jumpDirection],
+      (willJump, jumpDirection) => willJump && jumpDirection === "o"
+    ),
+    /**
+     * stacktrace.current.willCall
+     * note: includes creations!
+     */
+    willCall: createLeaf([solidity.current.willCall], identity),
+    /**
+     * stacktrace.current.nextFrameIsSkippedInReports
+     */
+    nextFrameIsSkippedInReports: createLeaf(
+      [solidity.current.nextFrameIsPhantom],
+      identity
+    ),
+    /**
+     * stacktrace.current.willReturn
+     */
+    willReturn: createLeaf([solidity.current.willReturn], identity),
+    /**
+     * stacktrace.current.returnStatus
+     */
+    returnStatus: createLeaf(
+      ["./willReturn", solidity.current.willFail],
+      (returns, failing) => (returns ? !failing : null)
+    ),
+    /**
+     * stacktrace.current.positionChanged
+     */
+    positionChanged: createLeaf(
+      ["./location", "./markedPosition"],
+      (currentLocation, markedLocation) =>
+        Boolean(markedLocation) && //if there's no marked position, we don't need this check
+        Boolean(currentLocation) && //if current location is unmapped, we consider ourselves to have not moved
+        (currentLocation.source.compilationId !==
+          markedLocation.source.compilationId ||
+          currentLocation.source.id !== markedLocation.source.id ||
+          currentLocation.sourceRange.start !==
+            markedLocation.sourceRange.start ||
+          currentLocation.sourceRange.length !==
+            markedLocation.sourceRange.length)
+    ),
+    /**
+     * stacktrace.current.report
+     * Contains the report object for outside consumption.
+     * Still needs to be processed into a string, mind you.
+     */
+    report: createLeaf(
+      ["./callstack", "./innerReturnPosition", "./innerReturnStatus"],
+      (rawStack, finalLocation, status) => {
+        //step 1: process skipped frames
+        let callstack = [];
+        //we're doing a C-style loop here!
+        //because we want to skip some items <grin>
+        for (let i = 0; i < rawStack.length; i++) {
+          const frame = rawStack[i];
+          if (
+            frame.skippedInReports &&
+            i < rawStack.length - 1 &&
+            rawStack[i + 1].type === "internal"
+          ) {
+            const combinedFrame = {
+              //only including the relevant info here
+              calledFromPosition: frame.calledFromPosition,
+              name: rawStack[i + 1].name
+            };
+            callstack.push(combinedFrame);
+            i++; //!! SKIP THE NEXT FRAME!
+          } else {
+            //ordinary case: just push the frame
+            callstack.push(frame);
+          }
+        }
+        debug("callstack: %O", callstack);
+        //step 2: shift everything over by 1 and recombine :)
+        let locations = callstack.map(frame => frame.calledFromPosition);
+        //remove initial null, add final location on end
+        locations.shift();
+        locations.push(finalLocation);
+        debug("locations: %O", locations);
+        const names = callstack.map(frame => frame.name);
+        debug("names: %O", names);
+        let report = zipWith(locations, names, (location, name) => ({
+          location,
+          name
+        }));
+        //finally: set the status in the top frame
+        report[report.length - 1].status = status;
+        return report;
+      }
+    )
+  },
+
+  /**
+   * stacktrace.next
+   */
+  next: {
+    ...createMultistepSelectors(solidity.next)
+  }
+});
+
+export default stacktrace;
diff --git a/packages/debugger/package.json b/packages/debugger/package.json
index 18f79514ee1..702d8b0513f 100644
--- a/packages/debugger/package.json
+++ b/packages/debugger/package.json
@@ -30,6 +30,7 @@
     "json-stable-stringify": "^1.0.1",
     "lodash.flatten": "^4.4.0",
     "lodash.sum": "^4.0.2",
+    "lodash.zipwith": "^4.2.0",
     "redux": "^3.7.2",
     "redux-cli-logger": "^2.0.1",
     "redux-saga": "1.0.0",
diff --git a/yarn.lock b/yarn.lock
index bab26004d65..7d8d7519cf9 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -9457,6 +9457,11 @@ lodash.uniq@^4.5.0:
   resolved "https://registry.yarnpkg.com/lodash.uniq/-/lodash.uniq-4.5.0.tgz#d0225373aeb652adc1bc82e4945339a842754773"
   integrity sha1-0CJTc662Uq3BvILklFM5qEJ1R3M=
 
+lodash.zipwith@^4.2.0:
+  version "4.2.0"
+  resolved "https://registry.yarnpkg.com/lodash.zipwith/-/lodash.zipwith-4.2.0.tgz#afacf03fd2f384af29e263c3c6bda3b80e3f51fd"
+  integrity sha1-r6zwP9LzhK8p4mPDxr2juA4/Uf0=
+
 lodash@4.17.14:
   version "4.17.14"
   resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.14.tgz#9ce487ae66c96254fe20b599f21b6816028078ba"
@@ -15639,6 +15644,7 @@ websocket@1.0.29, "websocket@github:web3-js/WebSocket-Node#polyfill/globalThis":
   dependencies:
     debug "^2.2.0"
     es5-ext "^0.10.50"
+    gulp "^4.0.2"
     nan "^2.14.0"
     typedarray-to-buffer "^3.1.5"
     yaeti "^0.0.6"

From 2a6cae9962bac595f66f51d6e89c3732ae7b80cc Mon Sep 17 00:00:00 2001
From: Harry Altman <harry.j.altman@gmail.com>
Date: Tue, 14 Apr 2020 19:28:47 -0400
Subject: [PATCH 03/18] Hook up stacktracing to debugger library via
 moduleOptions

---
 .../debugger/lib/controller/sagas/index.js    |  3 ++
 packages/debugger/lib/debugger.js             | 26 ++++++++--
 packages/debugger/lib/session/index.js        |  6 ++-
 packages/debugger/lib/session/reducers.js     |  2 +
 packages/debugger/lib/session/sagas/index.js  | 47 ++++++++++++-------
 packages/debugger/lib/store/common.js         |  3 +-
 packages/debugger/lib/trace/sagas/index.js    | 25 +++++-----
 7 files changed, 74 insertions(+), 38 deletions(-)

diff --git a/packages/debugger/lib/controller/sagas/index.js b/packages/debugger/lib/controller/sagas/index.js
index d47bbed260a..989e5f75fcf 100644
--- a/packages/debugger/lib/controller/sagas/index.js
+++ b/packages/debugger/lib/controller/sagas/index.js
@@ -9,6 +9,7 @@ import * as trace from "lib/trace/sagas";
 import * as data from "lib/data/sagas";
 import * as evm from "lib/evm/sagas";
 import * as solidity from "lib/solidity/sagas";
+import * as stacktrace from "lib/stacktrace/sagas";
 
 import * as actions from "../actions";
 
@@ -275,10 +276,12 @@ function* continueUntilBreakpoint(action) {
 
 /**
  * reset -- reset the state of the debugger
+ * (we'll just reset all submodules regardless of which are in use)
  */
 export function* reset() {
   yield* data.reset();
   yield* evm.reset();
   yield* solidity.reset();
   yield* trace.reset();
+  yield* stacktrace.reset();
 }
diff --git a/packages/debugger/lib/debugger.js b/packages/debugger/lib/debugger.js
index b33b6562d67..7e3e07b056e 100644
--- a/packages/debugger/lib/debugger.js
+++ b/packages/debugger/lib/debugger.js
@@ -11,6 +11,7 @@ import traceSelector from "./trace/selectors";
 import evmSelector from "./evm/selectors";
 import soliditySelector from "./solidity/selectors";
 import sessionSelector from "./session/selectors";
+import stacktraceSelector from "./stacktrace/selectors";
 import controllerSelector from "./controller/selectors";
 
 import { Compilations } from "@truffle/codec";
@@ -45,11 +46,19 @@ export default class Debugger {
    * @return {Debugger} instance
    */
   static async forTx(txHash, options = {}) {
-    let { contracts, files, provider, compilations } = options;
+    let {
+      contracts,
+      files,
+      provider,
+      compilations,
+      stacktrace,
+      noData
+    } = options;
+    let moduleOptions = { stacktrace, noData };
     if (!compilations) {
       compilations = Compilations.Utils.shimArtifacts(contracts, files);
     }
-    let session = new Session(compilations, provider, txHash);
+    let session = new Session(compilations, provider, moduleOptions, txHash);
 
     try {
       await session.ready();
@@ -69,11 +78,19 @@ export default class Debugger {
    * @return {Debugger} instance
    */
   static async forProject(options = {}) {
-    let { contracts, files, provider, compilations } = options;
+    let {
+      contracts,
+      files,
+      provider,
+      compilations,
+      stacktrace,
+      noData
+    } = options;
+    let moduleOptions = { stacktrace, noData };
     if (!compilations) {
       compilations = Compilations.Utils.shimArtifacts(contracts, files);
     }
-    let session = new Session(compilations, provider);
+    let session = new Session(compilations, provider, moduleOptions);
 
     await session.ready();
 
@@ -110,6 +127,7 @@ export default class Debugger {
       trace: traceSelector,
       evm: evmSelector,
       solidity: soliditySelector,
+      stacktrace: stacktraceSelector,
       session: sessionSelector,
       controller: controllerSelector
     });
diff --git a/packages/debugger/lib/session/index.js b/packages/debugger/lib/session/index.js
index ac0d9151216..10ac34f9138 100644
--- a/packages/debugger/lib/session/index.js
+++ b/packages/debugger/lib/session/index.js
@@ -29,11 +29,13 @@ export default class Session {
    * txHash parameter is now optional!
    * @private
    */
-  constructor(compilations, provider, txHash) {
+  constructor(compilations, provider, moduleOptions, txHash) {
     /**
      * @private
      */
-    let { store, sagaMiddleware } = configureStore(reducer, rootSaga);
+    let { store, sagaMiddleware } = configureStore(reducer, rootSaga, [
+      moduleOptions
+    ]);
     this._store = store;
     this._sagaMiddleware = sagaMiddleware;
 
diff --git a/packages/debugger/lib/session/reducers.js b/packages/debugger/lib/session/reducers.js
index bf95850806f..6f5d01c3a9f 100644
--- a/packages/debugger/lib/session/reducers.js
+++ b/packages/debugger/lib/session/reducers.js
@@ -8,6 +8,7 @@ import evm from "lib/evm/reducers";
 import solidity from "lib/solidity/reducers";
 import trace from "lib/trace/reducers";
 import controller from "lib/controller/reducers";
+import stacktrace from "lib/stacktrace/reducers";
 
 import * as actions from "./actions";
 
@@ -85,6 +86,7 @@ const reduceState = combineReducers({
   data,
   evm,
   solidity,
+  stacktrace,
   trace,
   controller
 });
diff --git a/packages/debugger/lib/session/sagas/index.js b/packages/debugger/lib/session/sagas/index.js
index 038761b7cfc..164288d9e15 100644
--- a/packages/debugger/lib/session/sagas/index.js
+++ b/packages/debugger/lib/session/sagas/index.js
@@ -8,6 +8,7 @@ import { prefixName } from "lib/helpers";
 import * as ast from "lib/ast/sagas";
 import * as controller from "lib/controller/sagas";
 import * as solidity from "lib/solidity/sagas";
+import * as stacktrace from "lib/stacktrace/sagas";
 import * as evm from "lib/evm/sagas";
 import * as trace from "lib/trace/sagas";
 import * as data from "lib/data/sagas";
@@ -34,9 +35,9 @@ function* listenerSaga() {
   }
 }
 
-export function* saga() {
+export function* saga(moduleOptions) {
   debug("starting listeners");
-  yield* forkListeners();
+  yield* forkListeners(moduleOptions);
 
   // receiving & saving contracts into state
   debug("waiting for contract information");
@@ -56,13 +57,15 @@ export function* saga() {
   let { txHash, provider } = yield take(actions.START);
   debug("starting");
 
-  debug("visiting ASTs");
-  // visit asts
-  yield* ast.visitAll();
+  if (!moduleOptions.noData) {
+    debug("visiting ASTs");
+    // visit asts
+    yield* ast.visitAll();
 
-  //save allocation table
-  debug("saving allocation table");
-  yield* data.recordAllocations();
+    //save allocation table
+    debug("saving allocation table");
+    yield* data.recordAllocations();
+  }
 
   //initialize web3 adapter
   yield* web3.init(provider);
@@ -90,15 +93,22 @@ export function* processTransaction(txHash) {
 
 export default prefixName("session", saga);
 
-function* forkListeners() {
+function* forkListeners(moduleOptions) {
   yield fork(listenerSaga); //session listener; this one is separate, sorry
   //(I didn't want to mess w/ the existing structure of defaults)
-  return yield all(
-    [controller, data, evm, solidity, trace, web3].map(
-      app => fork(app.saga)
-      //ast no longer has a listener
-    )
-  );
+  let mainApps = [evm, solidity];
+  let otherApps = [trace, controller, web3];
+  if (!moduleOptions.noData) {
+    mainApps.push(data);
+  }
+  if (moduleOptions.stacktrace) {
+    mainApps.push(stacktrace);
+  }
+  const submoduleCount = mainApps.length;
+  //I'm being lazy, so I'll just pass the submodule count to all of
+  //them even though only trace cares :P
+  const apps = mainApps.concat(otherApps);
+  return yield all(apps.map(app => fork(app.saga, submoduleCount)));
 }
 
 function* fetchTx(txHash) {
@@ -135,8 +145,9 @@ function* fetchTx(txHash) {
   );
 
   debug("sending initial call");
-  yield* evm.begin(result);
-  yield* solidity.begin(); //note: these must occur in this order
+  yield* evm.begin(result); //note: this must occur *before* the other two
+  yield* solidity.begin();
+  yield* stacktrace.begin();
 }
 
 function* recordContexts(...contexts) {
@@ -161,12 +172,14 @@ function* error(err) {
   yield put(actions.error(err));
 }
 
+//we'll just unload all modules regardless of which ones are currently present...
 export function* unload() {
   debug("unloading");
   yield* data.reset();
   yield* solidity.unload();
   yield* evm.unload();
   yield* trace.unload();
+  yield* stacktrace.unload();
   yield put(actions.unloadTransaction());
 }
 
diff --git a/packages/debugger/lib/store/common.js b/packages/debugger/lib/store/common.js
index fb2603a9233..d890f35e569 100644
--- a/packages/debugger/lib/store/common.js
+++ b/packages/debugger/lib/store/common.js
@@ -7,6 +7,7 @@ import createSagaMiddleware from "redux-saga";
 export default function configureStore(
   reducer,
   saga,
+  sagaArgs,
   initialState,
   composeEnhancers
 ) {
@@ -23,7 +24,7 @@ export default function configureStore(
     composeEnhancers(applyMiddleware(sagaMiddleware))
   );
 
-  sagaMiddleware.run(saga);
+  sagaMiddleware.run(saga, ...sagaArgs);
 
   return { store, sagaMiddleware };
 }
diff --git a/packages/debugger/lib/trace/sagas/index.js b/packages/debugger/lib/trace/sagas/index.js
index 4e62b9ca902..40df5af9c58 100644
--- a/packages/debugger/lib/trace/sagas/index.js
+++ b/packages/debugger/lib/trace/sagas/index.js
@@ -18,9 +18,7 @@ export function* advance() {
   debug("TOCK taken");
 }
 
-const SUBMODULE_COUNT = 3; //data, evm, solidity
-
-function* next() {
+function* next(submoduleCount) {
   let remaining = yield select(trace.stepsRemaining);
   debug("remaining: %o", remaining);
   let steps = yield select(trace.steps);
@@ -30,7 +28,7 @@ function* next() {
   if (remaining > 0) {
     debug("putting TICK");
     // updates state for current step
-    waitingForSubmodules = SUBMODULE_COUNT;
+    waitingForSubmodules = submoduleCount;
     yield put(actions.tick());
     debug("put TICK");
 
@@ -66,14 +64,13 @@ export function* processTrace(steps) {
   let addresses = [
     ...new Set(
       steps
-        .map(
-          ({ op, stack }) =>
-            isCallMnemonic(op)
-              ? //if it's a call, just fetch the address off the stack
-                Codec.Evm.Utils.toAddress(stack[stack.length - 2])
-              : //if it's not a call, just return undefined (we've gone back to
-                //skipping creates)
-                undefined
+        .map(({ op, stack }) =>
+          isCallMnemonic(op)
+            ? //if it's a call, just fetch the address off the stack
+              Codec.Evm.Utils.toAddress(stack[stack.length - 2])
+            : //if it's not a call, just return undefined (we've gone back to
+              //skipping creates)
+              undefined
         )
         //filter out zero addresses from failed creates (as well as undefineds)
         .filter(
@@ -94,8 +91,8 @@ export function* unload() {
   yield put(actions.unloadTransaction());
 }
 
-export function* saga() {
-  yield takeEvery(actions.NEXT, next);
+export function* saga(submoduleCount) {
+  yield takeEvery(actions.NEXT, next, submoduleCount);
 }
 
 export default prefixName("trace", saga);

From 9dcbe97831a4b567fe63accdfd1b58fc64709105 Mon Sep 17 00:00:00 2001
From: Harry Altman <harry.j.altman@gmail.com>
Date: Tue, 14 Apr 2020 20:26:41 -0400
Subject: [PATCH 04/18] Remove extra info from sources in stacktrace

---
 packages/debugger/lib/stacktrace/sagas/index.js | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/packages/debugger/lib/stacktrace/sagas/index.js b/packages/debugger/lib/stacktrace/sagas/index.js
index 123ab533186..6c1090bc06d 100644
--- a/packages/debugger/lib/stacktrace/sagas/index.js
+++ b/packages/debugger/lib/stacktrace/sagas/index.js
@@ -19,7 +19,11 @@ function* tickSaga() {
 
 function* stacktraceSaga() {
   //different possible outcomes:
-  const { source, sourceRange } = yield select(stacktrace.current.location);
+  const {
+    source: { id, compilationId, sourcePath },
+    sourceRange
+  } = yield select(stacktrace.current.location);
+  const source = { id, compilationId, sourcePath }; //leave out everything else
   const currentLocation = { source, sourceRange }; //leave out the node
   if (yield select(stacktrace.current.willJumpIn)) {
     const nextLocation = yield select(stacktrace.next.location);

From afa06eb9cd981690394df79e222afd01ec5875b6 Mon Sep 17 00:00:00 2001
From: Harry Altman <harry.j.altman@gmail.com>
Date: Tue, 14 Apr 2020 21:12:01 -0400
Subject: [PATCH 05/18] Fix "has location changed?" tracking system

---
 .../debugger/lib/stacktrace/actions/index.js  |  8 -----
 packages/debugger/lib/stacktrace/reducers.js  | 18 ++++++++--
 .../debugger/lib/stacktrace/sagas/index.js    | 26 ++++++++++----
 .../lib/stacktrace/selectors/index.js         | 35 +++++++++++++------
 4 files changed, 58 insertions(+), 29 deletions(-)

diff --git a/packages/debugger/lib/stacktrace/actions/index.js b/packages/debugger/lib/stacktrace/actions/index.js
index 1f808f0f44a..6268620c135 100644
--- a/packages/debugger/lib/stacktrace/actions/index.js
+++ b/packages/debugger/lib/stacktrace/actions/index.js
@@ -48,14 +48,6 @@ export function markReturnPosition(location) {
   };
 }
 
-//note: no reducer explicitly listens for this, but justReturned depends on it
-export const CLEAR_RETURN_MARKER = "CLEAR_RETURN_MARKER";
-export function clearReturnMarker() {
-  return {
-    type: CLEAR_RETURN_MARKER
-  };
-}
-
 export const RESET = "STACKTRACE_RESET";
 export function reset() {
   return {
diff --git a/packages/debugger/lib/stacktrace/reducers.js b/packages/debugger/lib/stacktrace/reducers.js
index c07cbbfc331..f3da757f57c 100644
--- a/packages/debugger/lib/stacktrace/reducers.js
+++ b/packages/debugger/lib/stacktrace/reducers.js
@@ -1,3 +1,6 @@
+import debugModule from "debug";
+const debug = debugModule("debugger:stacktrace:reducers");
+
 import { combineReducers } from "redux";
 
 import * as actions from "./actions";
@@ -76,7 +79,7 @@ function returnCounter(state = 0, action) {
 function markedPosition(state = null, action) {
   switch (action.type) {
     case actions.MARK_RETURN_POSITION:
-      return actions.location;
+      return action.location;
     case actions.EXECUTE_RETURN:
     case actions.RESET:
     case actions.UNLOAD_TRANSACTION:
@@ -112,8 +115,17 @@ function innerReturnStatus(state = null, action) {
   }
 }
 
-function justReturned(_state = false, action) {
-  return action.type === actions.EXTERNAL_RETURN;
+function justReturned(state = false, action) {
+  switch (action.type) {
+    case actions.EXTERNAL_RETURN:
+      debug("setting returned flag");
+      return true;
+    case actions.MARK_RETURN_POSITION:
+      debug("clearing returned flag");
+      return false;
+    default:
+      return state;
+  }
 }
 
 const proc = combineReducers({
diff --git a/packages/debugger/lib/stacktrace/sagas/index.js b/packages/debugger/lib/stacktrace/sagas/index.js
index 6c1090bc06d..1d4fb432c16 100644
--- a/packages/debugger/lib/stacktrace/sagas/index.js
+++ b/packages/debugger/lib/stacktrace/sagas/index.js
@@ -11,8 +11,6 @@ import * as trace from "lib/trace/sagas";
 import stacktrace from "../selectors";
 
 function* tickSaga() {
-  debug("got TICK");
-
   yield* stacktraceSaga();
   yield* trace.signalTickSagaCompletion();
 }
@@ -25,6 +23,7 @@ function* stacktraceSaga() {
   } = yield select(stacktrace.current.location);
   const source = { id, compilationId, sourcePath }; //leave out everything else
   const currentLocation = { source, sourceRange }; //leave out the node
+  let returnedRightNow = false;
   if (yield select(stacktrace.current.willJumpIn)) {
     const nextLocation = yield select(stacktrace.next.location);
     yield put(actions.jumpIn(currentLocation, nextLocation.node)); //in this case we only want the node
@@ -41,14 +40,27 @@ function* stacktraceSaga() {
     yield put(actions.externalCall(currentLocation, skipInReports));
   } else if (yield select(stacktrace.current.willReturn)) {
     const status = yield select(stacktrace.current.returnStatus);
+    debug("returning!");
     yield put(actions.externalReturn(currentLocation, status));
-  } else if (yield select(stacktrace.current.positionChanged)) {
+    returnedRightNow = true;
+  }
+  //the following checks are separate and happen even if one of the above
+  //branches was taken (so, we may have up to 3 actions, sorry)
+  //(note that shouldn't actually happen, realistically you'll only have 1,
+  //but notionally it could be up to 3)
+  if (!returnedRightNow && (yield select(stacktrace.current.justReturned))) {
+    debug("location: %o", currentLocation);
+    if (currentLocation.source.id !== undefined) {
+      //if we're in unmapped code, don't mark yet
+      yield put(actions.markReturnPosition(currentLocation));
+    }
+  }
+  if (yield select(stacktrace.current.positionWillChange)) {
+    debug("executing!");
+    debug("location: %o", yield select(stacktrace.next.location));
+    debug("marked: %o", yield select(stacktrace.current.markedPosition));
     const returnCounter = yield select(stacktrace.current.returnCounter);
     yield put(actions.executeReturn(returnCounter));
-  } else if (yield select(stacktrace.current.justReturned)) {
-    yield put(actions.markReturnPosition(currentLocation));
-  } else {
-    yield put(actions.clearReturnMarker());
   }
 }
 
diff --git a/packages/debugger/lib/stacktrace/selectors/index.js b/packages/debugger/lib/stacktrace/selectors/index.js
index b3e79f13a96..0a79afe82f8 100644
--- a/packages/debugger/lib/stacktrace/selectors/index.js
+++ b/packages/debugger/lib/stacktrace/selectors/index.js
@@ -45,14 +45,17 @@ let stacktrace = createSelectorTree({
      * stacktrace.current.callstack
      */
     callstack: createLeaf(["/state"], state => state.proc.callstack),
+
     /**
      * stacktrace.current.returnCounter
      */
     returnCounter: createLeaf(["/state"], state => state.proc.returnCounter),
+
     /**
      * stacktrace.current.markedPosition
      */
     markedPosition: createLeaf(["/state"], state => state.proc.markedPosition),
+
     /**
      * stacktrace.current.innerReturnPosition
      */
@@ -60,6 +63,7 @@ let stacktrace = createSelectorTree({
       ["/state"],
       state => state.proc.innerReturnPosition
     ),
+
     /**
      * stacktrace.current.innerReturnStatus
      */
@@ -67,11 +71,14 @@ let stacktrace = createSelectorTree({
       ["/state"],
       state => state.proc.innerReturnStatus
     ),
+
     /**
      * stacktrace.current.justReturned
      */
     justReturned: createLeaf(["/state"], state => state.proc.justReturned),
+
     ...createMultistepSelectors(solidity.current),
+
     /**
      * stacktrace.current.willJumpIn
      */
@@ -79,6 +86,7 @@ let stacktrace = createSelectorTree({
       [solidity.current.willJump, solidity.current.jumpDirection],
       (willJump, jumpDirection) => willJump && jumpDirection === "i"
     ),
+
     /**
      * stacktrace.current.willJumpOut
      */
@@ -86,11 +94,13 @@ let stacktrace = createSelectorTree({
       [solidity.current.willJump, solidity.current.jumpDirection],
       (willJump, jumpDirection) => willJump && jumpDirection === "o"
     ),
+
     /**
      * stacktrace.current.willCall
      * note: includes creations!
      */
     willCall: createLeaf([solidity.current.willCall], identity),
+
     /**
      * stacktrace.current.nextFrameIsSkippedInReports
      */
@@ -98,10 +108,12 @@ let stacktrace = createSelectorTree({
       [solidity.current.nextFrameIsPhantom],
       identity
     ),
+
     /**
      * stacktrace.current.willReturn
      */
     willReturn: createLeaf([solidity.current.willReturn], identity),
+
     /**
      * stacktrace.current.returnStatus
      */
@@ -109,22 +121,23 @@ let stacktrace = createSelectorTree({
       ["./willReturn", solidity.current.willFail],
       (returns, failing) => (returns ? !failing : null)
     ),
+
     /**
-     * stacktrace.current.positionChanged
+     * stacktrace.current.positionWillChange
      */
-    positionChanged: createLeaf(
-      ["./location", "./markedPosition"],
-      (currentLocation, markedLocation) =>
+    positionWillChange: createLeaf(
+      ["/next/location", "./markedPosition"],
+      (nextLocation, markedLocation) =>
         Boolean(markedLocation) && //if there's no marked position, we don't need this check
-        Boolean(currentLocation) && //if current location is unmapped, we consider ourselves to have not moved
-        (currentLocation.source.compilationId !==
+        Boolean(nextLocation.source) &&
+        nextLocation.source.id !== undefined && //if next location is unmapped, we consider ourselves to have not moved
+        (nextLocation.source.compilationId !==
           markedLocation.source.compilationId ||
-          currentLocation.source.id !== markedLocation.source.id ||
-          currentLocation.sourceRange.start !==
-            markedLocation.sourceRange.start ||
-          currentLocation.sourceRange.length !==
-            markedLocation.sourceRange.length)
+          nextLocation.source.id !== markedLocation.source.id ||
+          nextLocation.sourceRange.start !== markedLocation.sourceRange.start ||
+          nextLocation.sourceRange.length !== markedLocation.sourceRange.length)
     ),
+
     /**
      * stacktrace.current.report
      * Contains the report object for outside consumption.

From 25ed2c24cc160d77227e124225db638872190ed0 Mon Sep 17 00:00:00 2001
From: Harry Altman <harry.j.altman@gmail.com>
Date: Wed, 15 Apr 2020 00:44:26 -0400
Subject: [PATCH 06/18] Redo stacktrace system

---
 .../debugger/lib/stacktrace/actions/index.js  | 29 +++++++-----
 packages/debugger/lib/stacktrace/reducers.js  | 47 +++++++++----------
 .../debugger/lib/stacktrace/sagas/index.js    | 40 +++++++++-------
 .../lib/stacktrace/selectors/index.js         | 47 +++++++++++--------
 4 files changed, 89 insertions(+), 74 deletions(-)

diff --git a/packages/debugger/lib/stacktrace/actions/index.js b/packages/debugger/lib/stacktrace/actions/index.js
index 6268620c135..1223aa7fd45 100644
--- a/packages/debugger/lib/stacktrace/actions/index.js
+++ b/packages/debugger/lib/stacktrace/actions/index.js
@@ -1,49 +1,52 @@
 export const JUMP_IN = "STACKTRACE_JUMP_IN";
-export function jumpIn(from, functionNode) {
+export function jumpIn(location, functionNode) {
   return {
     type: JUMP_IN,
-    from,
+    location,
     functionNode
   };
 }
 
 export const JUMP_OUT = "STACKTRACE_JUMP_OUT";
-export function jumpOut() {
+export function jumpOut(location) {
   return {
-    type: JUMP_OUT
+    type: JUMP_OUT,
+    location
   };
 }
 
 export const EXTERNAL_CALL = "STACKTRACE_EXTERNAL_CALL";
-export function externalCall(from, skippedInReports) {
+export function externalCall(location, skippedInReports) {
   return {
     type: EXTERNAL_CALL,
-    from,
+    location,
     skippedInReports
   };
 }
 
 export const EXTERNAL_RETURN = "STACKTRACE_EXTERNAL_RETURN";
-export function externalReturn(from, status) {
+export function externalReturn(from, status, location) {
   return {
     type: EXTERNAL_RETURN,
     from,
-    status
+    status,
+    location
   };
 }
 
 export const EXECUTE_RETURN = "EXECUTE_RETURN";
-export function executeReturn(counter) {
+export function executeReturn(counter, location) {
   return {
     type: EXECUTE_RETURN,
-    counter
+    counter,
+    location
   };
 }
 
-export const MARK_RETURN_POSITION = "MARK_RETURN_POSITION";
-export function markReturnPosition(location) {
+export const UPDATE_POSITION = "UPDATE_POSITION";
+export function updatePosition(location) {
   return {
-    type: MARK_RETURN_POSITION,
+    type: UPDATE_POSITION,
     location
   };
 }
diff --git a/packages/debugger/lib/stacktrace/reducers.js b/packages/debugger/lib/stacktrace/reducers.js
index f3da757f57c..55640fec23f 100644
--- a/packages/debugger/lib/stacktrace/reducers.js
+++ b/packages/debugger/lib/stacktrace/reducers.js
@@ -10,10 +10,10 @@ function callstack(state = [], action) {
   let newFrame;
   switch (action.type) {
     case actions.JUMP_IN:
-      let { from, functionNode } = action;
+      let { location, functionNode } = action;
       newFrame = {
         type: "internal",
-        calledFromPosition: from,
+        calledFromLocation: location,
         name:
           functionNode && functionNode.nodeType === "FunctionDefinition"
             ? functionNode.name
@@ -35,7 +35,7 @@ function callstack(state = [], action) {
     case actions.EXTERNAL_CALL:
       newFrame = {
         type: "external",
-        calledFromPosition: action.from,
+        calledFromLocation: action.location,
         skippedInReports: action.skippedInReports,
         name: undefined
       };
@@ -76,11 +76,20 @@ function returnCounter(state = 0, action) {
   }
 }
 
-function markedPosition(state = null, action) {
+function lastPosition(state = null, action) {
   switch (action.type) {
-    case actions.MARK_RETURN_POSITION:
-      return action.location;
+    case actions.JUMP_IN:
+    case actions.JUMP_OUT:
+    case actions.ETERNAL_CALL:
+    case actions.EXTERNAL_RETURN:
+    case actions.UPDATE_POSITION:
     case actions.EXECUTE_RETURN:
+      const { location } = action;
+      if (location.source.id === undefined) {
+        //don't update for unmapped!
+        return state;
+      }
+      return location;
     case actions.RESET:
     case actions.UNLOAD_TRANSACTION:
       return null;
@@ -92,7 +101,9 @@ function markedPosition(state = null, action) {
 function innerReturnPosition(state = null, action) {
   switch (action.type) {
     case actions.EXTERNAL_RETURN:
-      return action.from;
+      //we want the innermost return, so don't update
+      //this if it's not presently null
+      return state || action.from;
     case actions.EXECUTE_RETURN:
     case actions.RESET:
     case actions.UNLOAD_TRANSACTION:
@@ -105,7 +116,9 @@ function innerReturnPosition(state = null, action) {
 function innerReturnStatus(state = null, action) {
   switch (action.type) {
     case actions.EXTERNAL_RETURN:
-      return action.status;
+      //we want the innermost return, so don't update
+      //this if it's not presently null
+      return state === null ? action.status : state;
     case actions.EXECUTE_RETURN:
     case actions.RESET:
     case actions.UNLOAD_TRANSACTION:
@@ -115,26 +128,12 @@ function innerReturnStatus(state = null, action) {
   }
 }
 
-function justReturned(state = false, action) {
-  switch (action.type) {
-    case actions.EXTERNAL_RETURN:
-      debug("setting returned flag");
-      return true;
-    case actions.MARK_RETURN_POSITION:
-      debug("clearing returned flag");
-      return false;
-    default:
-      return state;
-  }
-}
-
 const proc = combineReducers({
   callstack,
   returnCounter,
-  markedPosition,
+  lastPosition,
   innerReturnPosition,
-  innerReturnStatus,
-  justReturned
+  innerReturnStatus
 });
 
 const reducer = combineReducers({
diff --git a/packages/debugger/lib/stacktrace/sagas/index.js b/packages/debugger/lib/stacktrace/sagas/index.js
index 1d4fb432c16..b125e9013a8 100644
--- a/packages/debugger/lib/stacktrace/sagas/index.js
+++ b/packages/debugger/lib/stacktrace/sagas/index.js
@@ -23,12 +23,16 @@ function* stacktraceSaga() {
   } = yield select(stacktrace.current.location);
   const source = { id, compilationId, sourcePath }; //leave out everything else
   const currentLocation = { source, sourceRange }; //leave out the node
+  const lastLocation = yield select(stacktrace.current.lastPosition); //get this upfront due to sequencing issues
   let returnedRightNow = false;
+  let positionUpdated = false;
   if (yield select(stacktrace.current.willJumpIn)) {
     const nextLocation = yield select(stacktrace.next.location);
     yield put(actions.jumpIn(currentLocation, nextLocation.node)); //in this case we only want the node
+    positionUpdated = true;
   } else if (yield select(stacktrace.current.willJumpOut)) {
-    yield put(actions.jumpOut());
+    yield put(actions.jumpOut(currentLocation));
+    positionUpdated = true;
   } else if (yield select(stacktrace.current.willCall)) {
     //an external frame marked "skip in reports" will be, for reporting
     //purposes, combined with the frame above, unless that also is a
@@ -38,29 +42,31 @@ function* stacktraceSaga() {
       stacktrace.current.nextFrameIsSkippedInReports
     );
     yield put(actions.externalCall(currentLocation, skipInReports));
-  } else if (yield select(stacktrace.current.willReturn)) {
+    positionUpdated = true;
+  } else if (yield select(stacktrace.current.willReturnOrFail)) {
     const status = yield select(stacktrace.current.returnStatus);
     debug("returning!");
-    yield put(actions.externalReturn(currentLocation, status));
+    yield put(actions.externalReturn(lastLocation, status, currentLocation));
     returnedRightNow = true;
+    positionUpdated = true;
   }
-  //the following checks are separate and happen even if one of the above
-  //branches was taken (so, we may have up to 3 actions, sorry)
-  //(note that shouldn't actually happen, realistically you'll only have 1,
-  //but notionally it could be up to 3)
-  if (!returnedRightNow && (yield select(stacktrace.current.justReturned))) {
-    debug("location: %o", currentLocation);
-    if (currentLocation.source.id !== undefined) {
-      //if we're in unmapped code, don't mark yet
-      yield put(actions.markReturnPosition(currentLocation));
-    }
-  }
-  if (yield select(stacktrace.current.positionWillChange)) {
+  //we'll handle this next case separately of the above,
+  //so notionally 2 actions could fire, but it's pretty unlikely
+  if (
+    !returnedRightNow && //otherwise this will trigger in an inconsistent state
+    (yield select(stacktrace.current.returnCounter)) > 0 &&
+    (yield select(stacktrace.current.positionWillChange))
+  ) {
     debug("executing!");
     debug("location: %o", yield select(stacktrace.next.location));
-    debug("marked: %o", yield select(stacktrace.current.markedPosition));
+    debug("marked: %o", yield select(stacktrace.current.lastPosition));
     const returnCounter = yield select(stacktrace.current.returnCounter);
-    yield put(actions.executeReturn(returnCounter));
+    yield put(actions.executeReturn(returnCounter, currentLocation));
+    positionUpdated = true;
+  }
+  if (!positionUpdated) {
+    //finally, if no other action updated the position, do so here
+    yield put(actions.updatePosition(currentLocation));
   }
 }
 
diff --git a/packages/debugger/lib/stacktrace/selectors/index.js b/packages/debugger/lib/stacktrace/selectors/index.js
index 0a79afe82f8..914400a3f66 100644
--- a/packages/debugger/lib/stacktrace/selectors/index.js
+++ b/packages/debugger/lib/stacktrace/selectors/index.js
@@ -52,9 +52,9 @@ let stacktrace = createSelectorTree({
     returnCounter: createLeaf(["/state"], state => state.proc.returnCounter),
 
     /**
-     * stacktrace.current.markedPosition
+     * stacktrace.current.lastPosition
      */
-    markedPosition: createLeaf(["/state"], state => state.proc.markedPosition),
+    lastPosition: createLeaf(["/state"], state => state.proc.lastPosition),
 
     /**
      * stacktrace.current.innerReturnPosition
@@ -72,11 +72,6 @@ let stacktrace = createSelectorTree({
       state => state.proc.innerReturnStatus
     ),
 
-    /**
-     * stacktrace.current.justReturned
-     */
-    justReturned: createLeaf(["/state"], state => state.proc.justReturned),
-
     ...createMultistepSelectors(solidity.current),
 
     /**
@@ -105,7 +100,7 @@ let stacktrace = createSelectorTree({
      * stacktrace.current.nextFrameIsSkippedInReports
      */
     nextFrameIsSkippedInReports: createLeaf(
-      [solidity.current.nextFrameIsPhantom],
+      [solidity.current.callRequiresPhantomFrame],
       identity
     ),
 
@@ -114,28 +109,40 @@ let stacktrace = createSelectorTree({
      */
     willReturn: createLeaf([solidity.current.willReturn], identity),
 
+    /**
+     * stacktrace.current.willFail
+     */
+    willFail: createLeaf([solidity.current.willFail], identity),
+
+    /**
+     * stacktrace.current.willReturnOrFail
+     */
+    willReturnOrFail: createLeaf(
+      ["./willReturn", "./willFail"],
+      (willReturn, willFail) => willReturn || willFail
+    ),
+
     /**
      * stacktrace.current.returnStatus
      */
-    returnStatus: createLeaf(
-      ["./willReturn", solidity.current.willFail],
-      (returns, failing) => (returns ? !failing : null)
+    returnStatus: createLeaf(["./willReturn", "./willFail"], (returns, fails) =>
+      returns ? true : fails ? false : null
     ),
 
     /**
      * stacktrace.current.positionWillChange
      */
     positionWillChange: createLeaf(
-      ["/next/location", "./markedPosition"],
-      (nextLocation, markedLocation) =>
-        Boolean(markedLocation) && //if there's no marked position, we don't need this check
+      ["/next/location", "./lastPosition"],
+      (nextLocation, lastLocation) =>
+        Boolean(lastLocation) && //if there's no last position, we don't need this check
         Boolean(nextLocation.source) &&
         nextLocation.source.id !== undefined && //if next location is unmapped, we consider ourselves to have not moved
         (nextLocation.source.compilationId !==
-          markedLocation.source.compilationId ||
-          nextLocation.source.id !== markedLocation.source.id ||
-          nextLocation.sourceRange.start !== markedLocation.sourceRange.start ||
-          nextLocation.sourceRange.length !== markedLocation.sourceRange.length)
+          lastLocation.source.compilationId ||
+          nextLocation.source.id !== lastLocation.source.id ||
+          nextLocation.sourceRange.start !== lastLocation.sourceRange.start ||
+          nextLocation.sourceRange.length !== lastLocation.sourceRange.length)
     ),
 
     /**
@@ -159,7 +166,7 @@ let stacktrace = createSelectorTree({
           ) {
             const combinedFrame = {
               //only including the relevant info here
-              calledFromPosition: frame.calledFromPosition,
+              calledFromLocation: frame.calledFromLocation,
               name: rawStack[i + 1].name
             };
             callstack.push(combinedFrame);
@@ -171,7 +178,7 @@ let stacktrace = createSelectorTree({
         }
         debug("callstack: %O", callstack);
         //step 2: shift everything over by 1 and recombine :)
-        let locations = callstack.map(frame => frame.calledFromPosition);
+        let locations = callstack.map(frame => frame.calledFromLocation);
         //remove initial null, add final location on end
         locations.shift();
         locations.push(finalLocation);

From 08ee7536e4ffc124bd0261b985b1b42d2ab4a5ef Mon Sep 17 00:00:00 2001
From: Harry Altman <harry.j.altman@gmail.com>
Date: Wed, 15 Apr 2020 01:12:59 -0400
Subject: [PATCH 07/18] Allow getting stacktraces mid-execution

---
 .../lib/stacktrace/selectors/index.js         | 21 +++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/packages/debugger/lib/stacktrace/selectors/index.js b/packages/debugger/lib/stacktrace/selectors/index.js
index 914400a3f66..fd253d232fa 100644
--- a/packages/debugger/lib/stacktrace/selectors/index.js
+++ b/packages/debugger/lib/stacktrace/selectors/index.js
@@ -151,8 +151,13 @@ let stacktrace = createSelectorTree({
      * Still needs to be processed into a string, mind you.
      */
     report: createLeaf(
-      ["./callstack", "./innerReturnPosition", "./innerReturnStatus"],
-      (rawStack, finalLocation, status) => {
+      [
+        "./callstack",
+        "./innerReturnPosition",
+        "./innerReturnStatus",
+        "./lastPosition"
+      ],
+      (rawStack, finalLocation, status, backupLocation) => {
         //step 1: process skipped frames
         let callstack = [];
         //we're doing a C-style loop here!
@@ -181,7 +186,13 @@ let stacktrace = createSelectorTree({
         let locations = callstack.map(frame => frame.calledFromLocation);
         //remove initial null, add final location on end
         locations.shift();
-        locations.push(finalLocation);
+        if (finalLocation) {
+          locations.push(finalLocation);
+        } else {
+          //used for if someone wants a stacktrace during execution
+          //rather than at the end.
+          locations.push(backupLocation);
+        }
         debug("locations: %O", locations);
         const names = callstack.map(frame => frame.name);
         debug("names: %O", names);
@@ -190,7 +201,9 @@ let stacktrace = createSelectorTree({
           name
         }));
         //finally: set the status in the top frame
-        report[report.length - 1].status = status;
+        if (status !== null) {
+          report[report.length - 1].status = status;
+        }
         return report;
       }
     )

From 0f6156ae2f7b8989664bcacfe3cd49b90828187f Mon Sep 17 00:00:00 2001
From: Harry Altman <harry.j.altman@gmail.com>
Date: Wed, 15 Apr 2020 01:33:47 -0400
Subject: [PATCH 08/18] Replace stackframe & noData with lightMode

---
 packages/debugger/lib/debugger.js            | 24 ++++----------------
 packages/debugger/lib/session/sagas/index.js | 11 ++++-----
 2 files changed, 8 insertions(+), 27 deletions(-)

diff --git a/packages/debugger/lib/debugger.js b/packages/debugger/lib/debugger.js
index 7e3e07b056e..0256a39b2f6 100644
--- a/packages/debugger/lib/debugger.js
+++ b/packages/debugger/lib/debugger.js
@@ -46,19 +46,11 @@ export default class Debugger {
    * @return {Debugger} instance
    */
   static async forTx(txHash, options = {}) {
-    let {
-      contracts,
-      files,
-      provider,
-      compilations,
-      stacktrace,
-      noData
-    } = options;
-    let moduleOptions = { stacktrace, noData };
+    let { contracts, files, provider, compilations, lightMode } = options;
     if (!compilations) {
       compilations = Compilations.Utils.shimArtifacts(contracts, files);
     }
-    let session = new Session(compilations, provider, moduleOptions, txHash);
+    let session = new Session(compilations, provider, { lightMode }, txHash);
 
     try {
       await session.ready();
@@ -78,19 +70,11 @@ export default class Debugger {
    * @return {Debugger} instance
    */
   static async forProject(options = {}) {
-    let {
-      contracts,
-      files,
-      provider,
-      compilations,
-      stacktrace,
-      noData
-    } = options;
-    let moduleOptions = { stacktrace, noData };
+    let { contracts, files, provider, compilations, lightMode } = options;
     if (!compilations) {
       compilations = Compilations.Utils.shimArtifacts(contracts, files);
     }
-    let session = new Session(compilations, provider, moduleOptions);
+    let session = new Session(compilations, provider, { lightMode });
 
     await session.ready();
 
diff --git a/packages/debugger/lib/session/sagas/index.js b/packages/debugger/lib/session/sagas/index.js
index 164288d9e15..bfcda8fe271 100644
--- a/packages/debugger/lib/session/sagas/index.js
+++ b/packages/debugger/lib/session/sagas/index.js
@@ -57,7 +57,7 @@ export function* saga(moduleOptions) {
   let { txHash, provider } = yield take(actions.START);
   debug("starting");
 
-  if (!moduleOptions.noData) {
+  if (!moduleOptions.lightMode) {
     debug("visiting ASTs");
     // visit asts
     yield* ast.visitAll();
@@ -96,14 +96,11 @@ export default prefixName("session", saga);
 function* forkListeners(moduleOptions) {
   yield fork(listenerSaga); //session listener; this one is separate, sorry
   //(I didn't want to mess w/ the existing structure of defaults)
-  let mainApps = [evm, solidity];
-  let otherApps = [trace, controller, web3];
-  if (!moduleOptions.noData) {
+  let mainApps = [evm, solidity, stacktrace];
+  if (!moduleOptions.lightMode) {
     mainApps.push(data);
   }
-  if (moduleOptions.stacktrace) {
-    mainApps.push(stacktrace);
-  }
+  let otherApps = [trace, controller, web3];
   const submoduleCount = mainApps.length;
   //I'm being lazy, so I'll just pass the submodule count to all of
   //them even though only trace cares :P

From 4a8a4fac2584292d3ee3ca5d36fa298f100ab8b5 Mon Sep 17 00:00:00 2001
From: Harry Altman <harry.j.altman@gmail.com>
Date: Wed, 15 Apr 2020 18:35:27 -0400
Subject: [PATCH 09/18] Hook up stacktracing in debugger CLI

---
 packages/core/lib/debug/interpreter.js |  5 +++
 packages/core/lib/debug/printer.js     | 24 +++++++++++-
 packages/debug-utils/index.js          | 53 ++++++++++++++++++++++----
 3 files changed, 73 insertions(+), 9 deletions(-)

diff --git a/packages/core/lib/debug/interpreter.js b/packages/core/lib/debug/interpreter.js
index 44d7e8dd493..cb6d75ec884 100644
--- a/packages/core/lib/debug/interpreter.js
+++ b/packages/core/lib/debug/interpreter.js
@@ -432,6 +432,8 @@ class DebugInterpreter {
       //check if transaction failed
       if (!this.session.view(evm.transaction.status)) {
         this.printer.printRevertMessage();
+        this.printer.print("");
+        this.printer.printStacktrace(true); //final stacktrace
       } else {
         //case if transaction succeeded
         this.printer.print("Transaction completed successfully.");
@@ -522,6 +524,9 @@ class DebugInterpreter {
           this.enabledExpressions
         );
         break;
+      case "s":
+        this.printer.printStacktrace(false); //intermediate stacktrace
+        break;
       case "o":
       case "i":
       case "u":
diff --git a/packages/core/lib/debug/printer.js b/packages/core/lib/debug/printer.js
index c2de2cde015..6e27c99d71b 100644
--- a/packages/core/lib/debug/printer.js
+++ b/packages/core/lib/debug/printer.js
@@ -8,7 +8,15 @@ const DebugUtils = require("@truffle/debug-utils");
 const Codec = require("@truffle/codec");
 
 const selectors = require("@truffle/debugger").selectors;
-const { session, solidity, trace, controller, data, evm } = selectors;
+const {
+  session,
+  solidity,
+  trace,
+  controller,
+  data,
+  evm,
+  stacktrace
+} = selectors;
 
 class DebugPrinter {
   constructor(config, session) {
@@ -231,7 +239,9 @@ class DebugPrinter {
   }
 
   printRevertMessage() {
-    this.config.logger.log("Transaction halted with a RUNTIME ERROR.");
+    this.config.logger.log(
+      DebugUtils.truffleColors.red("Transaction halted with a RUNTIME ERROR.")
+    );
     this.config.logger.log("");
     let rawRevertMessage = this.session.view(evm.current.step.returnValue);
     let revertDecodings = Codec.decodeRevert(
@@ -287,6 +297,16 @@ class DebugPrinter {
     );
   }
 
+  printStacktrace(final) {
+    this.config.logger.log("Stacktrace:");
+    this.config.logger.log(
+      DebugUtils.formatStacktrace(
+        this.session.view(stacktrace.current.report),
+        final
+      )
+    );
+  }
+
   async printWatchExpressionsResults(expressions) {
     debug("expressions %o", expressions);
     for (let expression of expressions) {
diff --git a/packages/debug-utils/index.js b/packages/debug-utils/index.js
index a5fd45b82d8..8ff1227f2da 100644
--- a/packages/debug-utils/index.js
+++ b/packages/debug-utils/index.js
@@ -31,7 +31,8 @@ const commandReference = {
   "q": "quit",
   "r": "reset",
   "t": "load new transaction",
-  "T": "unload transaction"
+  "T": "unload transaction",
+  "s": "print stacktrace"
 };
 
 const truffleColors = {
@@ -51,6 +52,8 @@ const truffleColors = {
 const DEFAULT_TAB_WIDTH = 8;
 
 var DebugUtils = {
+  truffleColors, //make these externally available
+
   gatherArtifacts: async function(config) {
     // Gather all available contract artifacts
     let files = await dir.promiseFiles(config.contracts_build_directory);
@@ -118,8 +121,8 @@ var DebugUtils = {
     };
 
     //now: walk each AST
-    return compilation.sources.every(
-      source => (source ? allIDsUnseenSoFar(source.ast) : true)
+    return compilation.sources.every(source =>
+      source ? allIDsUnseenSoFar(source.ast) : true
     );
   },
 
@@ -188,10 +191,11 @@ var DebugUtils = {
     ];
 
     var commandSections = [
+      //TODO
       ["o", "i", "u", "n"],
       [";"],
       ["p"],
-      ["l", "h"],
+      ["l", "s", "h"],
       ["q", "r", "t", "T"],
       ["b", "B", "c"],
       ["+", "-"],
@@ -552,6 +556,42 @@ var DebugUtils = {
       .join(OS.EOL);
   },
 
+  formatStacktrace: function(stacktrace, final = true, indent = 2) {
+    //we want to print inner to outer, so first, let's
+    //reverse
+    stacktrace = stacktrace.slice().reverse(); //reverse is in-place so clone first
+    let lines = stacktrace.map(({ name, location }) => {
+      let functionName = name ? `function ${name}` : "unknown function";
+      if (location) {
+        let {
+          source: { sourcePath },
+          sourceRange: {
+            lines: {
+              start: { line, column }
+            }
+          }
+        } = location;
+        return `at ${functionName} (${sourcePath}:${line + 1}:${column + 1})`; //add 1 to account for 0-indexing
+      } else {
+        return `at ${functionName} (unknown location)`;
+      }
+    });
+    if (final) {
+      let status = stacktrace[0].status;
+      if (status != undefined) {
+        lines.unshift(
+          status
+            ? "Error: Improper return (may be an unexpected self-destruct)"
+            : "Error: Revert or exceptional halt"
+        );
+      }
+    }
+    let indented = lines.map((line, index) =>
+      index === 0 ? line : " ".repeat(indent) + line
+    );
+    return indented.join(OS.EOL);
+  },
+
   colorize: function(code) {
     //I'd put these outside the function
     //but then it gives me errors, because
@@ -698,9 +738,8 @@ var DebugUtils = {
   cleanThis: function(variables, replacement) {
     return Object.assign(
       {},
-      ...Object.entries(variables).map(
-        ([variable, value]) =>
-          variable === "this" ? { [replacement]: value } : { [variable]: value }
+      ...Object.entries(variables).map(([variable, value]) =>
+        variable === "this" ? { [replacement]: value } : { [variable]: value }
       )
     );
   }

From 6c9098e452712fae6ef8ff1500782b39bfb8c88f Mon Sep 17 00:00:00 2001
From: Harry Altman <harry.j.altman@gmail.com>
Date: Wed, 15 Apr 2020 19:04:21 -0400
Subject: [PATCH 10/18] Handle instant returns in stacktrace

---
 packages/debugger/lib/solidity/sagas/index.js |  7 +-----
 .../debugger/lib/solidity/selectors/index.js  | 24 +++++++------------
 2 files changed, 10 insertions(+), 21 deletions(-)

diff --git a/packages/debugger/lib/solidity/sagas/index.js b/packages/debugger/lib/solidity/sagas/index.js
index b54beb8b3b1..0c7e4a35b4e 100644
--- a/packages/debugger/lib/solidity/sagas/index.js
+++ b/packages/debugger/lib/solidity/sagas/index.js
@@ -27,11 +27,6 @@ function* functionDepthSaga() {
     //we do this case first so we can be sure we're not failing in any of the
     //other cases below!
     yield put(actions.externalReturn());
-  } else if (
-    yield select(solidity.current.willCallOrCreateButInstantlyReturn)
-  ) {
-    //do nothing
-    //again, we put this second so we can be sure the other cases are not this
   } else if (yield select(solidity.current.willJump)) {
     let jumpDirection = yield select(solidity.current.jumpDirection);
     debug("checking guard");
@@ -42,7 +37,7 @@ function* functionDepthSaga() {
       yield put(actions.jump(jumpDirection));
     }
   } else if (yield select(solidity.current.willCall)) {
-    //note: includes creations
+    //note: includes creations; does not include insta-returns
     debug("checking if guard needed");
     let guard = yield select(solidity.current.callRequiresPhantomFrame);
     yield put(actions.externalCall(guard));
diff --git a/packages/debugger/lib/solidity/selectors/index.js b/packages/debugger/lib/solidity/selectors/index.js
index 58017f47a06..7a282228fb0 100644
--- a/packages/debugger/lib/solidity/selectors/index.js
+++ b/packages/debugger/lib/solidity/selectors/index.js
@@ -161,10 +161,8 @@ let solidity = createSelectorTree({
     /**
      * solidity.current.humanReadableSourceMap
      */
-    humanReadableSourceMap: createLeaf(
-      ["./sourceMap"],
-      sourceMap =>
-        sourceMap ? SolidityUtils.getHumanReadableSourceMap(sourceMap) : null
+    humanReadableSourceMap: createLeaf(["./sourceMap"], sourceMap =>
+      sourceMap ? SolidityUtils.getHumanReadableSourceMap(sourceMap) : null
     ),
 
     /**
@@ -298,19 +296,15 @@ let solidity = createSelectorTree({
 
     /**
      * solidity.current.willCall
-     * note: includes creations
+     * note: includes creations, does *not* include instareturns
      */
     willCall: createLeaf(
-      [evm.current.step.isCall, evm.current.step.isCreate],
-      (isCall, isCreate) => isCall || isCreate
-    ),
-
-    /**
-     * solidity.current.willCallOrCreateButInstantlyReturn
-     */
-    willCallOrCreateButInstantlyReturn: createLeaf(
-      [evm.current.step.isInstantCallOrCreate],
-      x => x
+      [
+        evm.current.step.isCall,
+        evm.current.step.isCreate,
+        evm.current.step.isInstantCallOrCreate
+      ],
+      (isCall, isCreate, isInstant) => (isCall || isCreate) && !isInstant
     ),
 
     /**

From 419bccfb3aa6783129dc34d03ad94e0ab8f80532 Mon Sep 17 00:00:00 2001
From: Harry Altman <harry.j.altman@gmail.com>
Date: Wed, 15 Apr 2020 19:33:52 -0400
Subject: [PATCH 11/18] Fix willPositionChange logic

---
 .../lib/stacktrace/selectors/index.js         | 27 ++++++++++++-------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/packages/debugger/lib/stacktrace/selectors/index.js b/packages/debugger/lib/stacktrace/selectors/index.js
index fd253d232fa..2085e8be888 100644
--- a/packages/debugger/lib/stacktrace/selectors/index.js
+++ b/packages/debugger/lib/stacktrace/selectors/index.js
@@ -133,16 +133,23 @@ let stacktrace = createSelectorTree({
      * stacktrace.current.positionWillChange
      */
     positionWillChange: createLeaf(
-      ["/next/location", "./lastPosition"],
-      (nextLocation, lastLocation) =>
-        Boolean(lastLocation) && //if there's no last position, we don't need this check
-        Boolean(nextLocation.source) &&
-        nextLocation.source.id !== undefined && //if next location is unmapped, we consider ourselves to have not moved
-        (nextLocation.source.compilationId !==
-          lastLocation.source.compilationId ||
-          nextLocation.source.id !== lastLocation.source.id ||
-          nextLocation.sourceRange.start !== lastLocation.sourceRange.start ||
-          nextLocation.sourceRange.length !== lastLocation.sourceRange.length)
+      ["/next/location", "/current/location", "./lastPosition"],
+      (nextLocation, currentLocation, lastLocation) => {
+        let oldLocation =
+          currentLocation.source.id !== undefined
+            ? currentLocation
+            : lastLocation;
+        return (
+          Boolean(oldLocation) && //if there's no current or last position, we don't need this check
+          Boolean(nextLocation.source) &&
+          nextLocation.source.id !== undefined && //if next location is unmapped, we consider ourselves to have not moved
+          (nextLocation.source.compilationId !==
+            oldLocation.source.compilationId ||
+            nextLocation.source.id !== oldLocation.source.id ||
+            nextLocation.sourceRange.start !== oldLocation.sourceRange.start ||
+            nextLocation.sourceRange.length !== oldLocation.sourceRange.length)
+        );
+      }
     ),
 
     /**

From 62542ff1f78197035e66a674a03dd26d4f03317c Mon Sep 17 00:00:00 2001
From: Harry Altman <harry.j.altman@gmail.com>
Date: Wed, 15 Apr 2020 20:23:10 -0400
Subject: [PATCH 12/18] Add tests of stacktracing

---
 packages/debugger/test/stacktrace.js | 270 +++++++++++++++++++++++++++
 1 file changed, 270 insertions(+)
 create mode 100644 packages/debugger/test/stacktrace.js

diff --git a/packages/debugger/test/stacktrace.js b/packages/debugger/test/stacktrace.js
new file mode 100644
index 00000000000..f0bc7f38ee5
--- /dev/null
+++ b/packages/debugger/test/stacktrace.js
@@ -0,0 +1,270 @@
+import debugModule from "debug";
+const debug = debugModule("test:stacktrace"); // eslint-disable-line no-unused-vars
+
+import { assert } from "chai";
+
+import Ganache from "ganache-core";
+
+import { prepareContracts, lineOf } from "./helpers";
+import Debugger from "lib/debugger";
+
+import solidity from "lib/solidity/selectors";
+import stacktrace from "lib/stacktrace/selectors";
+
+const __STACKTRACE = `
+pragma solidity ^0.6.6;
+
+contract StacktraceTest {
+
+  Boom boom = new Boom();
+  event Num(uint);
+  function(bool) internal run0;
+
+  function run(uint fnId) public {
+    function(bool) internal[4] memory run0s = [
+      runRequire, runPay, runInternal, runBoom
+    ];
+    if(fnId < run0s.length) {
+      run0 = run0s[fnId];
+      this.run2(true);
+      this.run3(false);
+    }
+    emit Num(1);
+  }
+
+  constructor() public payable {
+  }
+
+  function run3(bool succeed) public {
+    run2(succeed);
+  }
+
+  function run2(bool succeed) public {
+    this.run1(succeed);
+  }
+
+  function run1(bool succeed) public {
+    run0(succeed); //CALL
+  }
+
+  function runRequire(bool succeed) public {
+    require(succeed); //REQUIRE
+    emit Num(1);
+  }
+
+  function runPay(bool succeed) public {
+    if(!succeed) {
+      payable(address(this)).transfer(1); //PAY
+    }
+  }
+
+  function runBoom(bool succeed) public {
+    if(!succeed) {
+      emit Num(boom.boom()); //UHOH
+    }
+  }
+
+  function runInternal(bool succeed) public {
+    function() internal garbage;
+    if(!succeed) {
+      garbage(); //GARBAGE
+    }
+  }
+}
+
+contract Boom {
+  function boom() public returns (uint) {
+    selfdestruct(address(this)); //BOOM
+  }
+
+  receive() external payable{
+  }
+}
+`;
+
+let sources = {
+  "StacktraceTest.sol": __STACKTRACE
+};
+
+const __MIGRATION = `
+let StacktraceTest = artifacts.require("StacktraceTest");
+
+module.exports = function(deployer) {
+  deployer.deploy(StacktraceTest, { value: 1 });
+};
+`;
+
+let migrations = {
+  "2_deploy_contracts.js": __MIGRATION
+};
+
+describe("Stack tracing", function() {
+  var provider;
+
+  var abstractions;
+  var compilations;
+
+  before("Create Provider", async function() {
+    provider = Ganache.provider({ seed: "debugger", gasLimit: 7000000 });
+  });
+
+  before("Prepare contracts and artifacts", async function() {
+    this.timeout(30000);
+
+    let prepared = await prepareContracts(provider, sources, migrations);
+    abstractions = prepared.abstractions;
+    compilations = prepared.compilations;
+  });
+
+  it("Generates correct stack trace on a revert", async function() {
+    let instance = await abstractions.StacktraceTest.deployed();
+    //HACK: because this transaction fails, we have to extract the hash from
+    //the resulting exception (there is supposed to be a non-hacky way but it
+    //does not presently work)
+    let txHash;
+    try {
+      await instance.run(0); //this will throw because of the revert
+    } catch (error) {
+      txHash = error.hashes[0]; //it's the only hash involved
+    }
+
+    let bugger = await Debugger.forTx(txHash, { provider, compilations });
+
+    let session = bugger.connect();
+
+    let source = session.view(solidity.current.source);
+    let failLine = lineOf("REQUIRE", source.source);
+    let callLine = lineOf("CALL", source.source);
+
+    await session.continueUntilBreakpoint(); //run till end
+
+    let report = session.view(stacktrace.current.report);
+    let names = report.map(({ name }) => name);
+    assert.deepEqual(names, ["run", "run3", "run2", "run1", "runRequire"]);
+    let status = report[report.length - 1].status;
+    assert.isFalse(status);
+    let location = report[report.length - 1].location;
+    let prevLocation = report[report.length - 2].location;
+    assert.strictEqual(location.sourceRange.lines.start.line, failLine);
+    assert.strictEqual(prevLocation.sourceRange.lines.start.line, callLine);
+  });
+
+  it("Generates correct stack trace on paying an unpayable contract", async function() {
+    let instance = await abstractions.StacktraceTest.deployed();
+    //HACK: because this transaction fails, we have to extract the hash from
+    //the resulting exception (there is supposed to be a non-hacky way but it
+    //does not presently work)
+    let txHash;
+    try {
+      await instance.run(1); //this will throw because of the revert
+    } catch (error) {
+      txHash = error.hashes[0]; //it's the only hash involved
+    }
+
+    let bugger = await Debugger.forTx(txHash, { provider, compilations });
+
+    let session = bugger.connect();
+
+    let source = session.view(solidity.current.source);
+    let failLine = lineOf("PAY", source.source);
+    let callLine = lineOf("CALL", source.source);
+
+    await session.continueUntilBreakpoint(); //run till end
+
+    let report = session.view(stacktrace.current.report);
+    let names = report.map(({ name }) => name);
+    assert.deepEqual(names, [
+      "run",
+      "run3",
+      "run2",
+      "run1",
+      "runPay",
+      undefined
+    ]);
+    let status = report[report.length - 1].status;
+    assert.isFalse(status);
+    let location = report[report.length - 2].location; //note, -2 because of undefined on top
+    let prevLocation = report[report.length - 3].location; //similar
+    assert.strictEqual(location.sourceRange.lines.start.line, failLine);
+    assert.strictEqual(prevLocation.sourceRange.lines.start.line, callLine);
+  });
+
+  it("Generates correct stack trace on calling an invalid internal function", async function() {
+    let instance = await abstractions.StacktraceTest.deployed();
+    //HACK: because this transaction fails, we have to extract the hash from
+    //the resulting exception (there is supposed to be a non-hacky way but it
+    //does not presently work)
+    let txHash;
+    try {
+      await instance.run(2); //this will throw because of the revert
+    } catch (error) {
+      txHash = error.hashes[0]; //it's the only hash involved
+    }
+
+    let bugger = await Debugger.forTx(txHash, { provider, compilations });
+
+    let session = bugger.connect();
+
+    let source = session.view(solidity.current.source);
+    let failLine = lineOf("GARBAGE", source.source);
+    let callLine = lineOf("CALL", source.source);
+
+    await session.continueUntilBreakpoint(); //run till end
+
+    let report = session.view(stacktrace.current.report);
+    let names = report.map(({ name }) => name);
+    assert.deepEqual(names, [
+      "run",
+      "run3",
+      "run2",
+      "run1",
+      "runInternal",
+      undefined
+    ]);
+    let status = report[report.length - 1].status;
+    assert.isFalse(status);
+    let location = report[report.length - 2].location; //note, -2 because of undefined on top
+    let prevLocation = report[report.length - 3].location; //similar
+    assert.strictEqual(location.sourceRange.lines.start.line, failLine);
+    assert.strictEqual(prevLocation.sourceRange.lines.start.line, callLine);
+  });
+
+  it("Generates correct stack trace on unexpected self-destruct", async function() {
+    let instance = await abstractions.StacktraceTest.deployed();
+    //HACK: because this transaction fails, we have to extract the hash from
+    //the resulting exception (there is supposed to be a non-hacky way but it
+    //does not presently work)
+    let txHash;
+    try {
+      await instance.run(3); //this will throw because of the revert
+    } catch (error) {
+      txHash = error.hashes[0]; //it's the only hash involved
+    }
+
+    let bugger = await Debugger.forTx(txHash, { provider, compilations });
+
+    let session = bugger.connect();
+
+    let source = session.view(solidity.current.source);
+    let failLine = lineOf("BOOM", source.source);
+    let callLine = lineOf("UHOH", source.source);
+    let prevCallLine = lineOf("CALL", source.source);
+
+    await session.continueUntilBreakpoint(); //run till end
+
+    let report = session.view(stacktrace.current.report);
+    let names = report.map(({ name }) => name);
+    assert.deepEqual(names, ["run", "run3", "run2", "run1", "runBoom", "boom"]);
+    let status = report[report.length - 1].status;
+    assert.isTrue(status);
+    let location = report[report.length - 1].location;
+    let prevLocation = report[report.length - 2].location;
+    let prev2Location = report[report.length - 3].location;
+    assert.strictEqual(location.sourceRange.lines.start.line, failLine);
+    assert.strictEqual(prevLocation.sourceRange.lines.start.line, callLine);
+    assert.strictEqual(
+      prev2Location.sourceRange.lines.start.line,
+      prevCallLine
+    );
+  });
+});

From 3256206427bcc1b19a6d8bc5c6970b6dc93ccf7e Mon Sep 17 00:00:00 2001
From: Harry Altman <harry.j.altman@gmail.com>
Date: Thu, 16 Apr 2020 00:15:06 -0400
Subject: [PATCH 13/18] Resequence stacktrace saga to handle edge cases

---
 .../debugger/lib/stacktrace/sagas/index.js    | 49 ++++++++++---------
 1 file changed, 26 insertions(+), 23 deletions(-)

diff --git a/packages/debugger/lib/stacktrace/sagas/index.js b/packages/debugger/lib/stacktrace/sagas/index.js
index b125e9013a8..758c9094d7f 100644
--- a/packages/debugger/lib/stacktrace/sagas/index.js
+++ b/packages/debugger/lib/stacktrace/sagas/index.js
@@ -23,9 +23,32 @@ function* stacktraceSaga() {
   } = yield select(stacktrace.current.location);
   const source = { id, compilationId, sourcePath }; //leave out everything else
   const currentLocation = { source, sourceRange }; //leave out the node
-  const lastLocation = yield select(stacktrace.current.lastPosition); //get this upfront due to sequencing issues
-  let returnedRightNow = false;
+  const lastLocation = yield select(stacktrace.current.lastPosition);
   let positionUpdated = false;
+  //first: are we returning?
+  if (yield select(stacktrace.current.willReturnOrFail)) {
+    const status = yield select(stacktrace.current.returnStatus);
+    debug("returning!");
+    yield put(actions.externalReturn(lastLocation, status, currentLocation));
+    positionUpdated = true;
+  } else if (
+    //next: are we *executing* a return?
+    //note this needs to be an else if or else this could execute
+    //in an inconsistent state
+    (yield select(stacktrace.current.returnCounter)) > 0 &&
+    (yield select(stacktrace.current.positionWillChange))
+  ) {
+    debug("executing!");
+    debug("location: %o", yield select(stacktrace.next.location));
+    debug("marked: %o", lastLocation);
+    const returnCounter = yield select(stacktrace.current.returnCounter);
+    yield put(actions.executeReturn(returnCounter, currentLocation));
+    positionUpdated = true;
+  }
+  //we now process the other possibilities.
+  //technically, an EXECUTE_RETURN could happen as well as those below,
+  //resulting in 2 actions instead of just one, but it's pretty unlikely.
+  //(an EXTERNAL_RETURN, OTOH, is obviously exclusive of the possibilities below)
   if (yield select(stacktrace.current.willJumpIn)) {
     const nextLocation = yield select(stacktrace.next.location);
     yield put(actions.jumpIn(currentLocation, nextLocation.node)); //in this case we only want the node
@@ -43,29 +66,9 @@ function* stacktraceSaga() {
     );
     yield put(actions.externalCall(currentLocation, skipInReports));
     positionUpdated = true;
-  } else if (yield select(stacktrace.current.willReturnOrFail)) {
-    const status = yield select(stacktrace.current.returnStatus);
-    debug("returning!");
-    yield put(actions.externalReturn(lastLocation, status, currentLocation));
-    returnedRightNow = true;
-    positionUpdated = true;
-  }
-  //we'll handle this next case separately of the above,
-  //so notionally 2 actions could fire, but it's pretty unlikely
-  if (
-    !returnedRightNow && //otherwise this will trigger in an inconsistent state
-    (yield select(stacktrace.current.returnCounter)) > 0 &&
-    (yield select(stacktrace.current.positionWillChange))
-  ) {
-    debug("executing!");
-    debug("location: %o", yield select(stacktrace.next.location));
-    debug("marked: %o", yield select(stacktrace.current.lastPosition));
-    const returnCounter = yield select(stacktrace.current.returnCounter);
-    yield put(actions.executeReturn(returnCounter, currentLocation));
-    positionUpdated = true;
   }
+  //finally, if no other action updated the position, do so here
   if (!positionUpdated) {
-    //finally, if no other action updated the position, do so here
     yield put(actions.updatePosition(currentLocation));
   }
 }

From e75f7dcf226a9b6163b7e06dcd90e42e44824ee7 Mon Sep 17 00:00:00 2001
From: Harry Altman <harry.j.altman@gmail.com>
Date: Thu, 16 Apr 2020 00:17:03 -0400
Subject: [PATCH 14/18] Increase timeout on stacktrace tests

---
 packages/debugger/test/stacktrace.js | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/packages/debugger/test/stacktrace.js b/packages/debugger/test/stacktrace.js
index f0bc7f38ee5..d58b3c0f344 100644
--- a/packages/debugger/test/stacktrace.js
+++ b/packages/debugger/test/stacktrace.js
@@ -117,6 +117,7 @@ describe("Stack tracing", function() {
   });
 
   it("Generates correct stack trace on a revert", async function() {
+    this.timeout(12000);
     let instance = await abstractions.StacktraceTest.deployed();
     //HACK: because this transaction fails, we have to extract the hash from
     //the resulting exception (there is supposed to be a non-hacky way but it
@@ -150,6 +151,7 @@ describe("Stack tracing", function() {
   });
 
   it("Generates correct stack trace on paying an unpayable contract", async function() {
+    this.timeout(12000);
     let instance = await abstractions.StacktraceTest.deployed();
     //HACK: because this transaction fails, we have to extract the hash from
     //the resulting exception (there is supposed to be a non-hacky way but it
@@ -190,6 +192,7 @@ describe("Stack tracing", function() {
   });
 
   it("Generates correct stack trace on calling an invalid internal function", async function() {
+    this.timeout(12000);
     let instance = await abstractions.StacktraceTest.deployed();
     //HACK: because this transaction fails, we have to extract the hash from
     //the resulting exception (there is supposed to be a non-hacky way but it
@@ -230,6 +233,7 @@ describe("Stack tracing", function() {
   });
 
   it("Generates correct stack trace on unexpected self-destruct", async function() {
+    this.timeout(12000);
     let instance = await abstractions.StacktraceTest.deployed();
     //HACK: because this transaction fails, we have to extract the hash from
     //the resulting exception (there is supposed to be a non-hacky way but it

From 2d93ca55baa577b84f0f94cae9eb1461be408aeb Mon Sep 17 00:00:00 2001
From: Harry Altman <harry.j.altman@gmail.com>
Date: Thu, 16 Apr 2020 18:21:04 -0400
Subject: [PATCH 15/18] Add prefixes to all actions (and delete an unused one)

---
 .../debugger/lib/controller/actions/index.js  | 24 +++++++++----------
 packages/debugger/lib/data/actions/index.js   | 12 +++++-----
 packages/debugger/lib/evm/actions/index.js    | 18 +++++++-------
 .../debugger/lib/session/actions/index.js     | 19 +++++----------
 .../debugger/lib/solidity/actions/index.js    |  8 +++----
 .../debugger/lib/stacktrace/actions/index.js  |  4 ++--
 packages/debugger/lib/trace/actions/index.js  | 12 +++++-----
 7 files changed, 45 insertions(+), 52 deletions(-)

diff --git a/packages/debugger/lib/controller/actions/index.js b/packages/debugger/lib/controller/actions/index.js
index c0c48db621c..47015a80860 100644
--- a/packages/debugger/lib/controller/actions/index.js
+++ b/packages/debugger/lib/controller/actions/index.js
@@ -1,29 +1,29 @@
-export const ADVANCE = "ADVANCE";
+export const ADVANCE = "CONTROLLER_ADVANCE";
 export function advance(count) {
   return { type: ADVANCE, count };
 }
 
-export const STEP_NEXT = "STEP_NEXT";
+export const STEP_NEXT = "CONTROLLER_STEP_NEXT";
 export function stepNext() {
   return { type: STEP_NEXT };
 }
 
-export const STEP_OVER = "STEP_OVER";
+export const STEP_OVER = "CONTROLLER_STEP_OVER";
 export function stepOver() {
   return { type: STEP_OVER };
 }
 
-export const STEP_INTO = "STEP_INTO";
+export const STEP_INTO = "CONTROLLER_STEP_INTO";
 export function stepInto() {
   return { type: STEP_INTO };
 }
 
-export const STEP_OUT = "STEP_OUT";
+export const STEP_OUT = "CONTROLLER_STEP_OUT";
 export function stepOut() {
   return { type: STEP_OUT };
 }
 
-export const RESET = "RESET";
+export const RESET = "CONTROLLER_RESET";
 export function reset() {
   return { type: RESET };
 }
@@ -33,7 +33,7 @@ export function interrupt() {
   return { type: INTERRUPT };
 }
 
-export const CONTINUE = "CONTINUE";
+export const CONTINUE = "CONTROLLER_CONTINUE";
 export function continueUntilBreakpoint(breakpoints) {
   //"continue" is not a legal name
   return {
@@ -42,7 +42,7 @@ export function continueUntilBreakpoint(breakpoints) {
   };
 }
 
-export const ADD_BREAKPOINT = "ADD_BREAKPOINT";
+export const ADD_BREAKPOINT = "CONTROLLER_ADD_BREAKPOINT";
 export function addBreakpoint(breakpoint) {
   return {
     type: ADD_BREAKPOINT,
@@ -50,7 +50,7 @@ export function addBreakpoint(breakpoint) {
   };
 }
 
-export const REMOVE_BREAKPOINT = "REMOVE_BREAKPOINT";
+export const REMOVE_BREAKPOINT = "CONTROLLER_REMOVE_BREAKPOINT";
 export function removeBreakpoint(breakpoint) {
   return {
     type: REMOVE_BREAKPOINT,
@@ -58,21 +58,21 @@ export function removeBreakpoint(breakpoint) {
   };
 }
 
-export const REMOVE_ALL_BREAKPOINTS = "REMOVE_ALL_BREAKPOINTS";
+export const REMOVE_ALL_BREAKPOINTS = "CONTROLLER_REMOVE_ALL_BREAKPOINTS";
 export function removeAllBreakpoints() {
   return {
     type: REMOVE_ALL_BREAKPOINTS
   };
 }
 
-export const START_STEPPING = "START_STEPPING";
+export const START_STEPPING = "CONTROLLER_START_STEPPING";
 export function startStepping() {
   return {
     type: START_STEPPING
   };
 }
 
-export const DONE_STEPPING = "DONE_STEPPING";
+export const DONE_STEPPING = "CONTROLLER_DONE_STEPPING";
 export function doneStepping() {
   return {
     type: DONE_STEPPING
diff --git a/packages/debugger/lib/data/actions/index.js b/packages/debugger/lib/data/actions/index.js
index 6877cf07a30..77538cba503 100644
--- a/packages/debugger/lib/data/actions/index.js
+++ b/packages/debugger/lib/data/actions/index.js
@@ -1,4 +1,4 @@
-export const SCOPE = "SCOPE";
+export const SCOPE = "DATA_SCOPE";
 export function scope(id, pointer, parentId, sourceId, compilationId) {
   return {
     type: SCOPE,
@@ -10,7 +10,7 @@ export function scope(id, pointer, parentId, sourceId, compilationId) {
   };
 }
 
-export const DECLARE = "DECLARE_VARIABLE";
+export const DECLARE = "DATA_DECLARE_VARIABLE";
 export function declare(node, compilationId) {
   return {
     type: DECLARE,
@@ -19,7 +19,7 @@ export function declare(node, compilationId) {
   };
 }
 
-export const ASSIGN = "ASSIGN";
+export const ASSIGN = "DATA_ASSIGN";
 export function assign(assignments) {
   return {
     type: ASSIGN,
@@ -27,7 +27,7 @@ export function assign(assignments) {
   };
 }
 
-export const MAP_PATH_AND_ASSIGN = "MAP_PATH_AND_ASSIGN";
+export const MAP_PATH_AND_ASSIGN = "DATA_MAP_PATH_AND_ASSIGN";
 export function mapPathAndAssign(
   address,
   slot,
@@ -50,7 +50,7 @@ export function reset() {
   return { type: RESET };
 }
 
-export const DEFINE_TYPE = "DEFINE_TYPE";
+export const DEFINE_TYPE = "DATA_DEFINE_TYPE";
 export function defineType(node, compilationId) {
   return {
     type: DEFINE_TYPE,
@@ -59,7 +59,7 @@ export function defineType(node, compilationId) {
   };
 }
 
-export const ALLOCATE = "ALLOCATE";
+export const ALLOCATE = "DATA_ALLOCATE";
 export function allocate(storage, memory, abi, state) {
   return {
     type: ALLOCATE,
diff --git a/packages/debugger/lib/evm/actions/index.js b/packages/debugger/lib/evm/actions/index.js
index 9f4807ba136..e04a25ae0c2 100644
--- a/packages/debugger/lib/evm/actions/index.js
+++ b/packages/debugger/lib/evm/actions/index.js
@@ -43,7 +43,7 @@ export function addInstance(address, context, binary) {
   };
 }
 
-export const SAVE_GLOBALS = "SAVE_GLOBALS";
+export const SAVE_GLOBALS = "EVM_SAVE_GLOBALS";
 export function saveGlobals(origin, gasprice, block) {
   return {
     type: SAVE_GLOBALS,
@@ -53,7 +53,7 @@ export function saveGlobals(origin, gasprice, block) {
   };
 }
 
-export const SAVE_STATUS = "SAVE_STATUS";
+export const SAVE_STATUS = "EVM_SAVE_STATUS";
 export function saveStatus(status) {
   return {
     type: SAVE_STATUS,
@@ -61,7 +61,7 @@ export function saveStatus(status) {
   };
 }
 
-export const CALL = "CALL";
+export const CALL = "EVM_CALL";
 export function call(address, data, storageAddress, sender, value) {
   return {
     type: CALL,
@@ -73,7 +73,7 @@ export function call(address, data, storageAddress, sender, value) {
   };
 }
 
-export const CREATE = "CREATE";
+export const CREATE = "EVM_CREATE";
 export function create(binary, storageAddress, sender, value) {
   return {
     type: CREATE,
@@ -84,14 +84,14 @@ export function create(binary, storageAddress, sender, value) {
   };
 }
 
-export const RETURN_CALL = "RETURN_CALL";
+export const RETURN_CALL = "EVM_RETURN_CALL";
 export function returnCall() {
   return {
     type: RETURN_CALL
   };
 }
 
-export const RETURN_CREATE = "RETURN_CREATE";
+export const RETURN_CREATE = "EVM_RETURN_CREATE";
 export function returnCreate(address, code, context) {
   return {
     type: RETURN_CREATE,
@@ -101,14 +101,14 @@ export function returnCreate(address, code, context) {
   };
 }
 
-export const FAIL = "FAIL";
+export const FAIL = "EVM_FAIL";
 export function fail() {
   return {
     type: FAIL
   };
 }
 
-export const STORE = "STORE";
+export const STORE = "EVM_STORE";
 export function store(address, slot, value) {
   return {
     type: STORE,
@@ -118,7 +118,7 @@ export function store(address, slot, value) {
   };
 }
 
-export const LOAD = "LOAD";
+export const LOAD = "EVM_LOAD";
 export function load(address, slot, value) {
   return {
     type: LOAD,
diff --git a/packages/debugger/lib/session/actions/index.js b/packages/debugger/lib/session/actions/index.js
index 1e3444e186e..bf70339c338 100644
--- a/packages/debugger/lib/session/actions/index.js
+++ b/packages/debugger/lib/session/actions/index.js
@@ -7,7 +7,7 @@ export function start(provider, txHash) {
   };
 }
 
-export const LOAD_TRANSACTION = "LOAD_TRANSACTION";
+export const LOAD_TRANSACTION = "SESSION_LOAD_TRANSACTION";
 export function loadTransaction(txHash) {
   return {
     type: LOAD_TRANSACTION,
@@ -20,7 +20,7 @@ export function interrupt() {
   return { type: INTERRUPT };
 }
 
-export const UNLOAD_TRANSACTION = "UNLOAD_TRANSACTION";
+export const UNLOAD_TRANSACTION = "SESSION_UNLOAD_TRANSACTION";
 export function unloadTransaction() {
   return {
     type: UNLOAD_TRANSACTION
@@ -49,14 +49,7 @@ export function error(error) {
   };
 }
 
-export const CLEAR_ERROR = "CLEAR_ERROR";
-export function clearError() {
-  return {
-    type: CLEAR_ERROR
-  };
-}
-
-export const RECORD_CONTRACTS = "RECORD_CONTRACTS";
+export const RECORD_CONTRACTS = "SESSION_RECORD_CONTRACTS";
 export function recordContracts(contexts, sources) {
   return {
     type: RECORD_CONTRACTS,
@@ -65,7 +58,7 @@ export function recordContracts(contexts, sources) {
   };
 }
 
-export const SAVE_TRANSACTION = "SAVE_TRANSACTION";
+export const SAVE_TRANSACTION = "SESSION_SAVE_TRANSACTION";
 export function saveTransaction(transaction) {
   return {
     type: SAVE_TRANSACTION,
@@ -73,7 +66,7 @@ export function saveTransaction(transaction) {
   };
 }
 
-export const SAVE_RECEIPT = "SAVE_RECEIPT";
+export const SAVE_RECEIPT = "SESSION_SAVE_RECEIPT";
 export function saveReceipt(receipt) {
   return {
     type: SAVE_RECEIPT,
@@ -81,7 +74,7 @@ export function saveReceipt(receipt) {
   };
 }
 
-export const SAVE_BLOCK = "SAVE_BLOCK";
+export const SAVE_BLOCK = "SESSION_SAVE_BLOCK";
 export function saveBlock(block) {
   return {
     type: SAVE_BLOCK,
diff --git a/packages/debugger/lib/solidity/actions/index.js b/packages/debugger/lib/solidity/actions/index.js
index d4db1ebb378..3519a0324ec 100644
--- a/packages/debugger/lib/solidity/actions/index.js
+++ b/packages/debugger/lib/solidity/actions/index.js
@@ -6,7 +6,7 @@ export function addSources(compilations) {
   };
 }
 
-export const JUMP = "JUMP";
+export const JUMP = "SOLIDITY_JUMP";
 export function jump(jumpDirection) {
   return {
     type: JUMP,
@@ -14,17 +14,17 @@ export function jump(jumpDirection) {
   };
 }
 
-export const EXTERNAL_CALL = "EXTERNAL_CALL";
+export const EXTERNAL_CALL = "SOLIDITY_EXTERNAL_CALL";
 export function externalCall(guard) {
   return { type: EXTERNAL_CALL, guard };
 }
 
-export const EXTERNAL_RETURN = "EXTERNAL_RETURN";
+export const EXTERNAL_RETURN = "SOLIDITY_EXTERNAL_RETURN";
 export function externalReturn() {
   return { type: EXTERNAL_RETURN };
 }
 
-export const CLEAR_PHANTOM_GUARD = "CLEAR_PHANTOM_GUARD";
+export const CLEAR_PHANTOM_GUARD = "SOLIDITY_CLEAR_PHANTOM_GUARD";
 export function clearPhantomGuard() {
   return { type: CLEAR_PHANTOM_GUARD };
 }
diff --git a/packages/debugger/lib/stacktrace/actions/index.js b/packages/debugger/lib/stacktrace/actions/index.js
index 1223aa7fd45..72a5fa5e49a 100644
--- a/packages/debugger/lib/stacktrace/actions/index.js
+++ b/packages/debugger/lib/stacktrace/actions/index.js
@@ -34,7 +34,7 @@ export function externalReturn(from, status, location) {
   };
 }
 
-export const EXECUTE_RETURN = "EXECUTE_RETURN";
+export const EXECUTE_RETURN = "STACKTRACE_EXECUTE_RETURN";
 export function executeReturn(counter, location) {
   return {
     type: EXECUTE_RETURN,
@@ -43,7 +43,7 @@ export function executeReturn(counter, location) {
   };
 }
 
-export const UPDATE_POSITION = "UPDATE_POSITION";
+export const UPDATE_POSITION = "STACKTRACE_UPDATE_POSITION";
 export function updatePosition(location) {
   return {
     type: UPDATE_POSITION,
diff --git a/packages/debugger/lib/trace/actions/index.js b/packages/debugger/lib/trace/actions/index.js
index 2970e69076d..1aced36a41d 100644
--- a/packages/debugger/lib/trace/actions/index.js
+++ b/packages/debugger/lib/trace/actions/index.js
@@ -1,4 +1,4 @@
-export const SAVE_STEPS = "SAVE_STEPS";
+export const SAVE_STEPS = "TRACE_SAVE_STEPS";
 export function saveSteps(steps) {
   return {
     type: SAVE_STEPS,
@@ -6,22 +6,22 @@ export function saveSteps(steps) {
   };
 }
 
-export const NEXT = "NEXT";
+export const NEXT = "TRACE_NEXT";
 export function next() {
   return { type: NEXT };
 }
 
-export const TICK = "TICK";
+export const TICK = "TRACE_TICK";
 export function tick() {
   return { type: TICK };
 }
 
-export const TOCK = "TOCK";
+export const TOCK = "TRACE_TOCK";
 export function tock() {
   return { type: TOCK };
 }
 
-export const END_OF_TRACE = "EOT";
+export const END_OF_TRACE = "TRACE_EOT";
 export function endTrace() {
   return { type: END_OF_TRACE };
 }
@@ -36,7 +36,7 @@ export function unloadTransaction() {
   return { type: UNLOAD_TRANSACTION };
 }
 
-export const BACKTICK = "BACKTICK";
+export const BACKTICK = "TRACE_BACKTICK";
 export function backtick() {
   return { type: BACKTICK };
 }

From 59511228919e240b76324100d1dc20a6e7d97c48 Mon Sep 17 00:00:00 2001
From: Harry Altman <harry.j.altman@gmail.com>
Date: Thu, 16 Apr 2020 19:00:15 -0400
Subject: [PATCH 16/18] Rename report -> finalReport, add report as
 intermediate

---
 packages/core/lib/debug/interpreter.js        |   7 +-
 packages/core/lib/debug/printer.js            |  10 +-
 packages/debug-utils/index.js                 |  18 +--
 packages/debugger/lib/helpers/index.js        |  19 +++
 packages/debugger/lib/stacktrace/reducers.js  |  20 +--
 .../debugger/lib/stacktrace/sagas/index.js    |  22 +--
 .../lib/stacktrace/selectors/index.js         | 138 +++++++++++-------
 packages/debugger/test/stacktrace.js          |  61 +++++++-
 8 files changed, 195 insertions(+), 100 deletions(-)

diff --git a/packages/core/lib/debug/interpreter.js b/packages/core/lib/debug/interpreter.js
index cb6d75ec884..e768066ebf3 100644
--- a/packages/core/lib/debug/interpreter.js
+++ b/packages/core/lib/debug/interpreter.js
@@ -525,7 +525,12 @@ class DebugInterpreter {
         );
         break;
       case "s":
-        this.printer.printStacktrace(false); //intermediate stacktrace
+        if (this.session.view(selectors.session.status.loaded)) {
+          this.printer.printStacktrace(
+            this.session.view(trace.finished) //print final report if finished,
+            //intermediate if not
+          );
+        }
         break;
       case "o":
       case "i":
diff --git a/packages/core/lib/debug/printer.js b/packages/core/lib/debug/printer.js
index 6e27c99d71b..aa4a69a8a04 100644
--- a/packages/core/lib/debug/printer.js
+++ b/packages/core/lib/debug/printer.js
@@ -299,12 +299,10 @@ class DebugPrinter {
 
   printStacktrace(final) {
     this.config.logger.log("Stacktrace:");
-    this.config.logger.log(
-      DebugUtils.formatStacktrace(
-        this.session.view(stacktrace.current.report),
-        final
-      )
-    );
+    let report = final
+      ? this.session.view(stacktrace.current.finalReport)
+      : this.session.view(stacktrace.current.report);
+    this.config.logger.log(DebugUtils.formatStacktrace(report));
   }
 
   async printWatchExpressionsResults(expressions) {
diff --git a/packages/debug-utils/index.js b/packages/debug-utils/index.js
index 8ff1227f2da..593d6d8ebf0 100644
--- a/packages/debug-utils/index.js
+++ b/packages/debug-utils/index.js
@@ -556,7 +556,7 @@ var DebugUtils = {
       .join(OS.EOL);
   },
 
-  formatStacktrace: function(stacktrace, final = true, indent = 2) {
+  formatStacktrace: function(stacktrace, indent = 2) {
     //we want to print inner to outer, so first, let's
     //reverse
     stacktrace = stacktrace.slice().reverse(); //reverse is in-place so clone first
@@ -576,15 +576,13 @@ var DebugUtils = {
         return `at ${functionName} (unknown location)`;
       }
     });
-    if (final) {
-      let status = stacktrace[0].status;
-      if (status != undefined) {
-        lines.unshift(
-          status
-            ? "Error: Improper return (may be an unexpected self-destruct)"
-            : "Error: Revert or exceptional halt"
-        );
-      }
+    let status = stacktrace[0].status;
+    if (status != undefined) {
+      lines.unshift(
+        status
+          ? "Error: Improper return (may be an unexpected self-destruct)"
+          : "Error: Revert or exceptional halt"
+      );
     }
     let indented = lines.map((line, index) =>
       index === 0 ? line : " ".repeat(indent) + line
diff --git a/packages/debugger/lib/helpers/index.js b/packages/debugger/lib/helpers/index.js
index cde995d6a25..2e82bdb632d 100644
--- a/packages/debugger/lib/helpers/index.js
+++ b/packages/debugger/lib/helpers/index.js
@@ -31,6 +31,25 @@ export function prefixName(prefix, fn) {
   return fn;
 }
 
+/**
+ * returns a new array which is a copy of array but with
+ * elements popped from the top until numToRemove elements
+ * satisfying the predicate have been removed (or until the
+ * array is empty)
+ */
+export function popNWhere(array, numToRemove, predicate) {
+  let newArray = array.slice();
+  //I'm going to write this the C way, hope you don't mind :P
+  while (numToRemove > 0 && newArray.length > 0) {
+    let top = newArray[newArray.length - 1];
+    if (predicate(top)) {
+      numToRemove--;
+    }
+    newArray.pop();
+  }
+  return newArray;
+}
+
 /**
  * @return 0x-prefix string of keccak256 hash
  */
diff --git a/packages/debugger/lib/stacktrace/reducers.js b/packages/debugger/lib/stacktrace/reducers.js
index 55640fec23f..509c42bf354 100644
--- a/packages/debugger/lib/stacktrace/reducers.js
+++ b/packages/debugger/lib/stacktrace/reducers.js
@@ -2,11 +2,11 @@ import debugModule from "debug";
 const debug = debugModule("debugger:stacktrace:reducers");
 
 import { combineReducers } from "redux";
+import { popNWhere } from "lib/helpers";
 
 import * as actions from "./actions";
 
 function callstack(state = [], action) {
-  let top;
   let newFrame;
   switch (action.type) {
     case actions.JUMP_IN:
@@ -26,7 +26,7 @@ function callstack(state = [], action) {
       };
       return [...state, newFrame];
     case actions.JUMP_OUT:
-      top = state[state.length - 1];
+      let top = state[state.length - 1];
       if (top && top.type === "internal") {
         return state.slice(0, -1);
       } else {
@@ -41,17 +41,11 @@ function callstack(state = [], action) {
       };
       return [...state, newFrame];
     case actions.EXECUTE_RETURN:
-      let newState = state.slice(); //clone the state
-      //I'm going to write this the C way, hope you don't mind :P
-      let counter = action.counter;
-      while (counter > 0 && newState.length > 0) {
-        top = newState[newState.length - 1];
-        if (top.type === "external") {
-          counter--;
-        }
-        newState.pop();
-      }
-      return newState;
+      return popNWhere(
+        state,
+        action.counter,
+        frame => frame.type === "external"
+      );
     case actions.RESET:
       return [state[0]];
     case actions.UNLOAD_TRANSACTION:
diff --git a/packages/debugger/lib/stacktrace/sagas/index.js b/packages/debugger/lib/stacktrace/sagas/index.js
index 758c9094d7f..0ed6adae559 100644
--- a/packages/debugger/lib/stacktrace/sagas/index.js
+++ b/packages/debugger/lib/stacktrace/sagas/index.js
@@ -16,15 +16,11 @@ function* tickSaga() {
 }
 
 function* stacktraceSaga() {
-  //different possible outcomes:
-  const {
-    source: { id, compilationId, sourcePath },
-    sourceRange
-  } = yield select(stacktrace.current.location);
-  const source = { id, compilationId, sourcePath }; //leave out everything else
-  const currentLocation = { source, sourceRange }; //leave out the node
+  const currentLocation = yield select(stacktrace.current.strippedLocation);
   const lastLocation = yield select(stacktrace.current.lastPosition);
+  const returnCounter = yield select(stacktrace.current.returnCounter);
   let positionUpdated = false;
+  //different possible outcomes:
   //first: are we returning?
   if (yield select(stacktrace.current.willReturnOrFail)) {
     const status = yield select(stacktrace.current.returnStatus);
@@ -35,13 +31,12 @@ function* stacktraceSaga() {
     //next: are we *executing* a return?
     //note this needs to be an else if or else this could execute
     //in an inconsistent state
-    (yield select(stacktrace.current.returnCounter)) > 0 &&
+    returnCounter > 0 &&
     (yield select(stacktrace.current.positionWillChange))
   ) {
     debug("executing!");
     debug("location: %o", yield select(stacktrace.next.location));
     debug("marked: %o", lastLocation);
-    const returnCounter = yield select(stacktrace.current.returnCounter);
     yield put(actions.executeReturn(returnCounter, currentLocation));
     positionUpdated = true;
   }
@@ -58,9 +53,14 @@ function* stacktraceSaga() {
     positionUpdated = true;
   } else if (yield select(stacktrace.current.willCall)) {
     //an external frame marked "skip in reports" will be, for reporting
-    //purposes, combined with the frame above, unless that also is a
-    //marker frame (combined in the appropriate sense)
+    //purposes, combined with the frame above, unless that also is an
+    //external frame (combined in the appropriate sense)
     //note: includes creations
+    //note: does *not* include calls that insta-return.  logically speaking,
+    //such calls should be a call + a return in one, right? and we could do that,
+    //making a call while also incrementing the return counter.  but the stacktraces
+    //this would generate would, I think, be more confusing than helpful, so I'm
+    //deliberately not doing that.
     const skipInReports = yield select(
       stacktrace.current.nextFrameIsSkippedInReports
     );
diff --git a/packages/debugger/lib/stacktrace/selectors/index.js b/packages/debugger/lib/stacktrace/selectors/index.js
index 2085e8be888..084f7afac25 100644
--- a/packages/debugger/lib/stacktrace/selectors/index.js
+++ b/packages/debugger/lib/stacktrace/selectors/index.js
@@ -6,9 +6,54 @@ import { createSelectorTree, createLeaf } from "reselect-tree";
 import solidity from "lib/solidity/selectors";
 
 import zipWith from "lodash.zipwith";
+import { popNWhere } from "lib/helpers";
 
 const identity = x => x;
 
+function generateReport(rawStack, location, status) {
+  //step 1: process skipped frames
+  let callstack = [];
+  //we're doing a C-style loop here!
+  //because we want to skip some items <grin>
+  for (let i = 0; i < rawStack.length; i++) {
+    const frame = rawStack[i];
+    if (
+      frame.skippedInReports &&
+      i < rawStack.length - 1 &&
+      rawStack[i + 1].type === "internal"
+    ) {
+      const combinedFrame = {
+        //only including the relevant info here
+        calledFromLocation: frame.calledFromLocation,
+        name: rawStack[i + 1].name
+      };
+      callstack.push(combinedFrame);
+      i++; //!! SKIP THE NEXT FRAME!
+    } else {
+      //ordinary case: just push the frame
+      callstack.push(frame);
+    }
+  }
+  debug("callstack: %O", callstack);
+  //step 2: shift everything over by 1 and recombine :)
+  let locations = callstack.map(frame => frame.calledFromLocation);
+  //remove initial null, add final location on end
+  locations.shift();
+  locations.push(location);
+  debug("locations: %O", locations);
+  const names = callstack.map(frame => frame.name);
+  debug("names: %O", names);
+  let report = zipWith(locations, names, (location, name) => ({
+    location,
+    name
+  }));
+  //finally: set the status in the top frame
+  if (status !== null) {
+    report[report.length - 1].status = status;
+  }
+  return report;
+}
+
 function createMultistepSelectors(stepSelector) {
   return {
     /**
@@ -27,7 +72,18 @@ function createMultistepSelectors(stepSelector) {
        * .node
        */
       node: createLeaf([stepSelector.node], identity)
-    }
+    },
+
+    /**
+     * .strippedLocation
+     */
+    strippedLocation: createLeaf(
+      ["./location/source", "./location/sourceRange"],
+      ({ id, compilationId, sourcePath }, sourceRange) => ({
+        source: { id, compilationId, sourcePath },
+        sourceRange
+      })
+    )
   };
 }
 
@@ -153,66 +209,40 @@ let stacktrace = createSelectorTree({
     ),
 
     /**
-     * stacktrace.current.report
+     * stacktrace.current.finalReport
      * Contains the report object for outside consumption.
      * Still needs to be processed into a string, mind you.
      */
+    finalReport: createLeaf(
+      ["./callstack", "./innerReturnPosition", "./innerReturnStatus"],
+      (callstack, finalLocation, status) =>
+        generateReport(callstack, finalLocation, status)
+    ),
+
+    /**
+     * stacktrace.current.report
+     * Similar to stacktrace.current.report, but meant for use as at
+     * an intermediate point instead of at the end (it reflects how things
+     * actually currently are rather than taking into account exited
+     * stackframes that caused the revert)
+     */
     report: createLeaf(
       [
         "./callstack",
-        "./innerReturnPosition",
-        "./innerReturnStatus",
-        "./lastPosition"
+        "./returnCounter",
+        "./lastPosition",
+        "/current/strippedLocation"
       ],
-      (rawStack, finalLocation, status, backupLocation) => {
-        //step 1: process skipped frames
-        let callstack = [];
-        //we're doing a C-style loop here!
-        //because we want to skip some items <grin>
-        for (let i = 0; i < rawStack.length; i++) {
-          const frame = rawStack[i];
-          if (
-            frame.skippedInReports &&
-            i < rawStack.length - 1 &&
-            rawStack[i + 1].type === "internal"
-          ) {
-            const combinedFrame = {
-              //only including the relevant info here
-              calledFromLocation: frame.calledFromLocation,
-              name: rawStack[i + 1].name
-            };
-            callstack.push(combinedFrame);
-            i++; //!! SKIP THE NEXT FRAME!
-          } else {
-            //ordinary case: just push the frame
-            callstack.push(frame);
-          }
-        }
-        debug("callstack: %O", callstack);
-        //step 2: shift everything over by 1 and recombine :)
-        let locations = callstack.map(frame => frame.calledFromLocation);
-        //remove initial null, add final location on end
-        locations.shift();
-        if (finalLocation) {
-          locations.push(finalLocation);
-        } else {
-          //used for if someone wants a stacktrace during execution
-          //rather than at the end.
-          locations.push(backupLocation);
-        }
-        debug("locations: %O", locations);
-        const names = callstack.map(frame => frame.name);
-        debug("names: %O", names);
-        let report = zipWith(locations, names, (location, name) => ({
-          location,
-          name
-        }));
-        //finally: set the status in the top frame
-        if (status !== null) {
-          report[report.length - 1].status = status;
-        }
-        return report;
-      }
+      (callstack, returnCounter, lastPosition, currentLocation) =>
+        generateReport(
+          popNWhere(
+            callstack,
+            returnCounter,
+            frame => frame.type === "external"
+          ),
+          currentLocation || lastPosition,
+          null
+        )
     )
   },
 
diff --git a/packages/debugger/test/stacktrace.js b/packages/debugger/test/stacktrace.js
index d58b3c0f344..b927d277b6d 100644
--- a/packages/debugger/test/stacktrace.js
+++ b/packages/debugger/test/stacktrace.js
@@ -48,8 +48,8 @@ contract StacktraceTest {
   }
 
   function runRequire(bool succeed) public {
+    emit Num(1); //EMIT
     require(succeed); //REQUIRE
-    emit Num(1);
   }
 
   function runPay(bool succeed) public {
@@ -139,7 +139,7 @@ describe("Stack tracing", function() {
 
     await session.continueUntilBreakpoint(); //run till end
 
-    let report = session.view(stacktrace.current.report);
+    let report = session.view(stacktrace.current.finalReport);
     let names = report.map(({ name }) => name);
     assert.deepEqual(names, ["run", "run3", "run2", "run1", "runRequire"]);
     let status = report[report.length - 1].status;
@@ -150,6 +150,57 @@ describe("Stack tracing", function() {
     assert.strictEqual(prevLocation.sourceRange.lines.start.line, callLine);
   });
 
+  it("Generates correct stack trace at an intermediate state", async function() {
+    this.timeout(12000);
+    let instance = await abstractions.StacktraceTest.deployed();
+    //HACK: because this transaction fails, we have to extract the hash from
+    //the resulting exception (there is supposed to be a non-hacky way but it
+    //does not presently work)
+    let txHash;
+    try {
+      await instance.run(0); //this will throw because of the revert
+    } catch (error) {
+      txHash = error.hashes[0]; //it's the only hash involved
+    }
+
+    let bugger = await Debugger.forTx(txHash, { provider, compilations });
+
+    let session = bugger.connect();
+
+    let source = session.view(solidity.current.source);
+    let breakLine = lineOf("EMIT", source.source);
+    let callLine = lineOf("CALL", source.source);
+    let breakpoint = {
+      sourceId: source.id,
+      compilationId: source.compilationId,
+      line: breakLine
+    };
+    await session.addBreakpoint(breakpoint);
+    await session.continueUntilBreakpoint(); //run till EMIT
+
+    let report = session.view(stacktrace.current.report);
+    let names = report.map(({ name }) => name);
+    assert.deepEqual(names, ["run", "run2", "run1", "runRequire"]);
+    let status = report[report.length - 1].status;
+    assert.isUndefined(status);
+    let location = report[report.length - 1].location;
+    let prevLocation = report[report.length - 2].location;
+    assert.strictEqual(location.sourceRange.lines.start.line, breakLine);
+    assert.strictEqual(prevLocation.sourceRange.lines.start.line, callLine);
+
+    await session.continueUntilBreakpoint(); //run till EMIT again
+
+    report = session.view(stacktrace.current.report);
+    names = report.map(({ name }) => name);
+    assert.deepEqual(names, ["run", "run3", "run2", "run1", "runRequire"]);
+    status = report[report.length - 1].status;
+    assert.isUndefined(status);
+    location = report[report.length - 1].location;
+    prevLocation = report[report.length - 2].location;
+    assert.strictEqual(location.sourceRange.lines.start.line, breakLine);
+    assert.strictEqual(prevLocation.sourceRange.lines.start.line, callLine);
+  });
+
   it("Generates correct stack trace on paying an unpayable contract", async function() {
     this.timeout(12000);
     let instance = await abstractions.StacktraceTest.deployed();
@@ -173,7 +224,7 @@ describe("Stack tracing", function() {
 
     await session.continueUntilBreakpoint(); //run till end
 
-    let report = session.view(stacktrace.current.report);
+    let report = session.view(stacktrace.current.finalReport);
     let names = report.map(({ name }) => name);
     assert.deepEqual(names, [
       "run",
@@ -214,7 +265,7 @@ describe("Stack tracing", function() {
 
     await session.continueUntilBreakpoint(); //run till end
 
-    let report = session.view(stacktrace.current.report);
+    let report = session.view(stacktrace.current.finalReport);
     let names = report.map(({ name }) => name);
     assert.deepEqual(names, [
       "run",
@@ -256,7 +307,7 @@ describe("Stack tracing", function() {
 
     await session.continueUntilBreakpoint(); //run till end
 
-    let report = session.view(stacktrace.current.report);
+    let report = session.view(stacktrace.current.finalReport);
     let names = report.map(({ name }) => name);
     assert.deepEqual(names, ["run", "run3", "run2", "run1", "runBoom", "boom"]);
     let status = report[report.length - 1].status;

From 8d120a5746d9e7fb8c1cfadef8e89ee8ef7e24e2 Mon Sep 17 00:00:00 2001
From: Harry Altman <harry.j.altman@gmail.com>
Date: Thu, 16 Apr 2020 19:39:00 -0400
Subject: [PATCH 17/18] Make s command non-repeatable, oops

---
 packages/core/lib/debug/interpreter.js | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/packages/core/lib/debug/interpreter.js b/packages/core/lib/debug/interpreter.js
index e768066ebf3..bf1cc82b480 100644
--- a/packages/core/lib/debug/interpreter.js
+++ b/packages/core/lib/debug/interpreter.js
@@ -588,7 +588,8 @@ class DebugInterpreter {
       cmd !== "r" &&
       cmd !== "-" &&
       cmd !== "t" &&
-      cmd !== "T"
+      cmd !== "T" &&
+      cmd !== "s"
     ) {
       this.lastCommand = cmd;
     }

From 19c7ea2d41f82a8e4c00a639bd9e153ac94f6166 Mon Sep 17 00:00:00 2001
From: Harry Altman <harry.j.altman@gmail.com>
Date: Thu, 16 Apr 2020 20:08:35 -0400
Subject: [PATCH 18/18] Add contract names to stacktraces

---
 packages/debug-utils/index.js                 | 15 +++--
 .../debugger/lib/stacktrace/actions/index.js  |  8 ++-
 packages/debugger/lib/stacktrace/reducers.js  | 15 ++++-
 .../debugger/lib/stacktrace/sagas/index.js    | 13 ++--
 .../lib/stacktrace/selectors/index.js         | 54 ++++++++++++++---
 packages/debugger/test/stacktrace.js          | 59 +++++++++++++++----
 6 files changed, 130 insertions(+), 34 deletions(-)

diff --git a/packages/debug-utils/index.js b/packages/debug-utils/index.js
index 593d6d8ebf0..a9ff4e5b42f 100644
--- a/packages/debug-utils/index.js
+++ b/packages/debug-utils/index.js
@@ -560,8 +560,15 @@ var DebugUtils = {
     //we want to print inner to outer, so first, let's
     //reverse
     stacktrace = stacktrace.slice().reverse(); //reverse is in-place so clone first
-    let lines = stacktrace.map(({ name, location }) => {
-      let functionName = name ? `function ${name}` : "unknown function";
+    let lines = stacktrace.map(({ functionName, contractName, location }) => {
+      let name;
+      if (contractName && functionName) {
+        name = `${contractName}.${functionName}`;
+      } else if (contractName) {
+        name = contractName;
+      } else {
+        name = "unknown function";
+      }
       if (location) {
         let {
           source: { sourcePath },
@@ -571,9 +578,9 @@ var DebugUtils = {
             }
           }
         } = location;
-        return `at ${functionName} (${sourcePath}:${line + 1}:${column + 1})`; //add 1 to account for 0-indexing
+        return `at ${name} (${sourcePath}:${line + 1}:${column + 1})`; //add 1 to account for 0-indexing
       } else {
-        return `at ${functionName} (unknown location)`;
+        return `at ${name} (unknown location)`;
       }
     });
     let status = stacktrace[0].status;
diff --git a/packages/debugger/lib/stacktrace/actions/index.js b/packages/debugger/lib/stacktrace/actions/index.js
index 72a5fa5e49a..067195a9057 100644
--- a/packages/debugger/lib/stacktrace/actions/index.js
+++ b/packages/debugger/lib/stacktrace/actions/index.js
@@ -1,9 +1,10 @@
 export const JUMP_IN = "STACKTRACE_JUMP_IN";
-export function jumpIn(location, functionNode) {
+export function jumpIn(location, functionNode, contractNode) {
   return {
     type: JUMP_IN,
     location,
-    functionNode
+    functionNode,
+    contractNode
   };
 }
 
@@ -16,10 +17,11 @@ export function jumpOut(location) {
 }
 
 export const EXTERNAL_CALL = "STACKTRACE_EXTERNAL_CALL";
-export function externalCall(location, skippedInReports) {
+export function externalCall(location, contractNode, skippedInReports) {
   return {
     type: EXTERNAL_CALL,
     location,
+    contractNode,
     skippedInReports
   };
 }
diff --git a/packages/debugger/lib/stacktrace/reducers.js b/packages/debugger/lib/stacktrace/reducers.js
index 509c42bf354..8844cba9b5e 100644
--- a/packages/debugger/lib/stacktrace/reducers.js
+++ b/packages/debugger/lib/stacktrace/reducers.js
@@ -10,14 +10,18 @@ function callstack(state = [], action) {
   let newFrame;
   switch (action.type) {
     case actions.JUMP_IN:
-      let { location, functionNode } = action;
+      let { location, functionNode, contractNode } = action;
       newFrame = {
         type: "internal",
         calledFromLocation: location,
-        name:
+        functionName:
           functionNode && functionNode.nodeType === "FunctionDefinition"
             ? functionNode.name
             : undefined,
+        contractName:
+          contractNode && contractNode.nodeType === "ContractDefinition"
+            ? contractNode.name
+            : undefined,
         //note we don't currently account for getters because currently
         //we can't; fallback, receive, constructors, & modifiers also remain
         //unaccounted for at present
@@ -37,7 +41,12 @@ function callstack(state = [], action) {
         type: "external",
         calledFromLocation: action.location,
         skippedInReports: action.skippedInReports,
-        name: undefined
+        functionName: undefined,
+        contractName:
+          action.contractNode &&
+          action.contractNode.nodeType === "ContractDefinition"
+            ? action.contractNode.name
+            : undefined
       };
       return [...state, newFrame];
     case actions.EXECUTE_RETURN:
diff --git a/packages/debugger/lib/stacktrace/sagas/index.js b/packages/debugger/lib/stacktrace/sagas/index.js
index 0ed6adae559..5c186546af3 100644
--- a/packages/debugger/lib/stacktrace/sagas/index.js
+++ b/packages/debugger/lib/stacktrace/sagas/index.js
@@ -46,7 +46,8 @@ function* stacktraceSaga() {
   //(an EXTERNAL_RETURN, OTOH, is obviously exclusive of the possibilities below)
   if (yield select(stacktrace.current.willJumpIn)) {
     const nextLocation = yield select(stacktrace.next.location);
-    yield put(actions.jumpIn(currentLocation, nextLocation.node)); //in this case we only want the node
+    const nextParent = yield select(stacktrace.next.contractNode);
+    yield put(actions.jumpIn(currentLocation, nextLocation.node, nextParent));
     positionUpdated = true;
   } else if (yield select(stacktrace.current.willJumpOut)) {
     yield put(actions.jumpOut(currentLocation));
@@ -64,7 +65,10 @@ function* stacktraceSaga() {
     const skipInReports = yield select(
       stacktrace.current.nextFrameIsSkippedInReports
     );
-    yield put(actions.externalCall(currentLocation, skipInReports));
+    const nextLocation = yield select(stacktrace.next.location);
+    yield put(
+      actions.externalCall(currentLocation, nextLocation.node, skipInReports)
+    );
     positionUpdated = true;
   }
   //finally, if no other action updated the position, do so here
@@ -83,9 +87,10 @@ export function* unload() {
 
 export function* begin() {
   const skipInReports = yield select(
-    stacktrace.current.nextFrameIsSkippedInReports
+    stacktrace.transaction.bottomFrameIsSkippedInReports
   );
-  yield put(actions.externalCall(null, skipInReports));
+  const currentLocation = yield select(stacktrace.current.location);
+  yield put(actions.externalCall(null, currentLocation.node, skipInReports));
 }
 
 export function* saga() {
diff --git a/packages/debugger/lib/stacktrace/selectors/index.js b/packages/debugger/lib/stacktrace/selectors/index.js
index 084f7afac25..11bee048334 100644
--- a/packages/debugger/lib/stacktrace/selectors/index.js
+++ b/packages/debugger/lib/stacktrace/selectors/index.js
@@ -5,6 +5,7 @@ import { createSelectorTree, createLeaf } from "reselect-tree";
 
 import solidity from "lib/solidity/selectors";
 
+import jsonpointer from "json-pointer";
 import zipWith from "lodash.zipwith";
 import { popNWhere } from "lib/helpers";
 
@@ -23,9 +24,8 @@ function generateReport(rawStack, location, status) {
       rawStack[i + 1].type === "internal"
     ) {
       const combinedFrame = {
-        //only including the relevant info here
-        calledFromLocation: frame.calledFromLocation,
-        name: rawStack[i + 1].name
+        ...rawStack[i + 1],
+        calledFromLocation: frame.calledFromLocation
       };
       callstack.push(combinedFrame);
       i++; //!! SKIP THE NEXT FRAME!
@@ -41,11 +41,14 @@ function generateReport(rawStack, location, status) {
   locations.shift();
   locations.push(location);
   debug("locations: %O", locations);
-  const names = callstack.map(frame => frame.name);
+  const names = callstack.map(({ functionName, contractName }) => ({
+    functionName,
+    contractName
+  }));
   debug("names: %O", names);
-  let report = zipWith(locations, names, (location, name) => ({
-    location,
-    name
+  let report = zipWith(locations, names, (location, nameInfo) => ({
+    ...nameInfo,
+    location
   }));
   //finally: set the status in the top frame
   if (status !== null) {
@@ -71,7 +74,11 @@ function createMultistepSelectors(stepSelector) {
       /**
        * .node
        */
-      node: createLeaf([stepSelector.node], identity)
+      node: createLeaf([stepSelector.node], identity),
+      /**
+       * .pointer
+       */
+      pointer: createLeaf([stepSelector.pointer], identity)
     },
 
     /**
@@ -83,6 +90,24 @@ function createMultistepSelectors(stepSelector) {
         source: { id, compilationId, sourcePath },
         sourceRange
       })
+    ),
+
+    /**
+     * .contractNode
+     * WARNING: ad-hoc selector only meant to be used
+     * when you're on a function node!
+     * should probably be replaced by something better;
+     * the data submodule handles these things a better way
+     */
+    contractNode: createLeaf(
+      ["./location/source", "./location/pointer"],
+      ({ ast }, pointer) =>
+        pointer
+          ? jsonpointer.get(
+              ast,
+              pointer.replace(/\/nodes\/\d+$/, "") //cut off end
+            )
+          : ast
     )
   };
 }
@@ -93,6 +118,19 @@ let stacktrace = createSelectorTree({
    */
   state: state => state.stacktrace,
 
+  /**
+   * stacktrace.transaction
+   */
+  transaction: {
+    /**
+     * stacktrace.transaction.bottomFrameIsSkippedInReports
+     */
+    bottomFrameIsSkippedInReports: createLeaf(
+      [solidity.transaction.bottomStackframeRequiresPhantomFrame],
+      identity
+    )
+  },
+
   /**
    * stacktrace.current
    */
diff --git a/packages/debugger/test/stacktrace.js b/packages/debugger/test/stacktrace.js
index b927d277b6d..62ee443d020 100644
--- a/packages/debugger/test/stacktrace.js
+++ b/packages/debugger/test/stacktrace.js
@@ -140,8 +140,16 @@ describe("Stack tracing", function() {
     await session.continueUntilBreakpoint(); //run till end
 
     let report = session.view(stacktrace.current.finalReport);
-    let names = report.map(({ name }) => name);
-    assert.deepEqual(names, ["run", "run3", "run2", "run1", "runRequire"]);
+    let functionNames = report.map(({ functionName }) => functionName);
+    assert.deepEqual(functionNames, [
+      "run",
+      "run3",
+      "run2",
+      "run1",
+      "runRequire"
+    ]);
+    let contractNames = report.map(({ contractName }) => contractName);
+    assert(contractNames.every(name => name === "StacktraceTest"));
     let status = report[report.length - 1].status;
     assert.isFalse(status);
     let location = report[report.length - 1].location;
@@ -179,8 +187,10 @@ describe("Stack tracing", function() {
     await session.continueUntilBreakpoint(); //run till EMIT
 
     let report = session.view(stacktrace.current.report);
-    let names = report.map(({ name }) => name);
-    assert.deepEqual(names, ["run", "run2", "run1", "runRequire"]);
+    let functionNames = report.map(({ functionName }) => functionName);
+    assert.deepEqual(functionNames, ["run", "run2", "run1", "runRequire"]);
+    let contractNames = report.map(({ contractName }) => contractName);
+    assert(contractNames.every(name => name === "StacktraceTest"));
     let status = report[report.length - 1].status;
     assert.isUndefined(status);
     let location = report[report.length - 1].location;
@@ -191,8 +201,16 @@ describe("Stack tracing", function() {
     await session.continueUntilBreakpoint(); //run till EMIT again
 
     report = session.view(stacktrace.current.report);
-    names = report.map(({ name }) => name);
-    assert.deepEqual(names, ["run", "run3", "run2", "run1", "runRequire"]);
+    functionNames = report.map(({ functionName }) => functionName);
+    assert.deepEqual(functionNames, [
+      "run",
+      "run3",
+      "run2",
+      "run1",
+      "runRequire"
+    ]);
+    contractNames = report.map(({ contractName }) => contractName);
+    assert(contractNames.every(name => name === "StacktraceTest"));
     status = report[report.length - 1].status;
     assert.isUndefined(status);
     location = report[report.length - 1].location;
@@ -225,8 +243,8 @@ describe("Stack tracing", function() {
     await session.continueUntilBreakpoint(); //run till end
 
     let report = session.view(stacktrace.current.finalReport);
-    let names = report.map(({ name }) => name);
-    assert.deepEqual(names, [
+    let functionNames = report.map(({ functionName }) => functionName);
+    assert.deepEqual(functionNames, [
       "run",
       "run3",
       "run2",
@@ -234,6 +252,8 @@ describe("Stack tracing", function() {
       "runPay",
       undefined
     ]);
+    let contractNames = report.map(({ contractName }) => contractName);
+    assert(contractNames.every(name => name === "StacktraceTest"));
     let status = report[report.length - 1].status;
     assert.isFalse(status);
     let location = report[report.length - 2].location; //note, -2 because of undefined on top
@@ -266,8 +286,8 @@ describe("Stack tracing", function() {
     await session.continueUntilBreakpoint(); //run till end
 
     let report = session.view(stacktrace.current.finalReport);
-    let names = report.map(({ name }) => name);
-    assert.deepEqual(names, [
+    let functionNames = report.map(({ functionName }) => functionName);
+    assert.deepEqual(functionNames, [
       "run",
       "run3",
       "run2",
@@ -275,6 +295,10 @@ describe("Stack tracing", function() {
       "runInternal",
       undefined
     ]);
+    let contractNames = report.map(({ contractName }) => contractName);
+    assert.isUndefined(contractNames[contractNames.length - 1]);
+    contractNames.pop();
+    assert(contractNames.every(name => name === "StacktraceTest"));
     let status = report[report.length - 1].status;
     assert.isFalse(status);
     let location = report[report.length - 2].location; //note, -2 because of undefined on top
@@ -308,8 +332,19 @@ describe("Stack tracing", function() {
     await session.continueUntilBreakpoint(); //run till end
 
     let report = session.view(stacktrace.current.finalReport);
-    let names = report.map(({ name }) => name);
-    assert.deepEqual(names, ["run", "run3", "run2", "run1", "runBoom", "boom"]);
+    let functionNames = report.map(({ functionName }) => functionName);
+    assert.deepEqual(functionNames, [
+      "run",
+      "run3",
+      "run2",
+      "run1",
+      "runBoom",
+      "boom"
+    ]);
+    let contractNames = report.map(({ contractName }) => contractName);
+    assert.strictEqual(contractNames[contractNames.length - 1], "Boom");
+    contractNames.pop();
+    assert(contractNames.every(name => name === "StacktraceTest"));
     let status = report[report.length - 1].status;
     assert.isTrue(status);
     let location = report[report.length - 1].location;