From a7697e79aebf920a71c0e4962eb471d084bc10c5 Mon Sep 17 00:00:00 2001 From: Harry Altman Date: Mon, 21 Aug 2023 16:01:23 -0400 Subject: [PATCH] Apply no-op logic from mapping keys to modifier arguments --- packages/debugger/lib/data/sagas/index.js | 37 +++++-------------- packages/debugger/lib/data/selectors/index.js | 12 +++--- packages/debugger/lib/helpers/index.js | 21 +++++++++++ 3 files changed, 36 insertions(+), 34 deletions(-) diff --git a/packages/debugger/lib/data/sagas/index.js b/packages/debugger/lib/data/sagas/index.js index 39539af09b9..6fcaf812dfb 100644 --- a/packages/debugger/lib/data/sagas/index.js +++ b/packages/debugger/lib/data/sagas/index.js @@ -7,7 +7,8 @@ import { prefixName, stableKeccak256, makeAssignment, - makePath + makePath, + peelAwayPotentialEVMNoOp } from "lib/helpers"; import { TICK } from "lib/trace/actions"; @@ -776,6 +777,8 @@ function* decodeMappingKeyCore(indexDefinition, keyDefinition) { const modifierDepth = yield select(data.current.modifierDepth); const inModifier = yield select(data.current.inModifier); + let peeledIndexDefinition; + //why the loop? see the end of the block it heads for an explanatory //comment while (true) { @@ -864,41 +867,19 @@ function* decodeMappingKeyCore(indexDefinition, keyDefinition) { } } //there's still one more reason we might have failed to decode it: - //certain (silent) type conversions aren't sourcemapped either. + //certain operations aren't sourcemapped because they're no-ops on + //the EVM side. This includes, for instance, certain type conversions. //(thankfully, any type conversion that actually *does* something seems //to be sourcemapped.) So if we've failed to decode it, we try again - //with the argument of the type conversion, if it is one; we leave + //with the argument of the no-op operation; we leave //indexValue undefined so the loop will continue //(note that this case is last for a reason; if this were earlier, it //would catch *non*-silent type conversions, which we want to just read //off the stack) else if ( - indexDefinition.nodeType === "FunctionCall" && - indexDefinition.kind === "typeConversion" - ) { - debug("type conversion case"); - indexDefinition = indexDefinition.arguments[0]; - } - //...also prior to 0.5.0, unary + was legal, which needs to be accounted - //for for the same reason - else if ( - indexDefinition.nodeType === "UnaryOperation" && - indexDefinition.operator === "+" - ) { - debug("unary + case"); - indexDefinition = indexDefinition.subExpression; - } - //...and starting in 0.8.8, we'd better handle wrap and unwrap as well for - //the same reason! - else if ( - indexDefinition.nodeType === "FunctionCall" && - indexDefinition.kind === "functionCall" && - ["wrap", "unwrap"].includes( - Codec.Ast.Utils.functionClass(indexDefinition.expression) - ) + (peeledIndexDefinition = peelAwayPotentialEVMNoOp(indexDefinition)) ) { - debug("wrap/unwrap case"); - indexDefinition = indexDefinition.arguments[0]; + indexDefinition = peeledIndexDefinition; } //otherwise, we've just totally failed to decode it, so we mark //indexValue as null (as distinct from undefined) to indicate this. In diff --git a/packages/debugger/lib/data/selectors/index.js b/packages/debugger/lib/data/selectors/index.js index 6bb1f315f88..d89ae974c2b 100644 --- a/packages/debugger/lib/data/selectors/index.js +++ b/packages/debugger/lib/data/selectors/index.js @@ -10,7 +10,8 @@ import { stableKeccak256, makePath, topLevelNodeTypes, - isTopLevelNode + isTopLevelNode, + peelAwayPotentialEVMNoOp } from "lib/helpers"; import trace from "lib/trace/selectors"; @@ -1336,18 +1337,17 @@ const data = createSelectorTree({ } //now: are we on the node corresponding to an argument, or, if - //it's a type conversion, its nested argument? + //it's a potential EVM no-op, its nested argument? if (index === undefined) { return false; } let argument = invocation.arguments[index]; - while (argument.kind === "typeConversion") { + do { if (node.id === argument.id) { return true; } - argument = argument.arguments[0]; - } - return node.id === argument.id; + } while ((argument = peelAwayPotentialEVMNoOp(argument))); + return false; } ), diff --git a/packages/debugger/lib/helpers/index.js b/packages/debugger/lib/helpers/index.js index ed8fbdebc57..efa0b805504 100644 --- a/packages/debugger/lib/helpers/index.js +++ b/packages/debugger/lib/helpers/index.js @@ -1,6 +1,27 @@ import * as Codec from "@truffle/codec"; import stringify from "json-stable-stringify"; +export function peelAwayPotentialEVMNoOp(node) { + if (node.nodeType === "FunctionCall" && node.kind === "typeConversion") { + //some type conversions are no-ops; + //not all are, but it's not worth the trouble to detect which ones, + //the places where this is used no it's only checking for *potential* no-ops + return node.arguments[0]; + } else if (node.nodeType === "UnaryOperation" && node.operator === "+") { + //prior to 0.5.0, unary + was legal, which was a no-op + return node.subExpression; + } else if ( + node.nodeType === "FunctionCall" && + node.kind === "functionCall" && + ["wrap", "unwrap"].includes(Codec.Ast.Utils.functionClass(node.expression)) + ) { + //starting in 0.8.8, there are wrap and unwrap which are no-ops + return node.arguments[0]; + } else { + return null; + } +} + /** AST node types that are skipped by stepNext() to filter out some noise */ export function isDeliberatelySkippedNodeType(node) { const skippedTypes = [