Cleanup stack traces tests code

NomicFoundation · Dec 23, 2024 · 5308fcc · 5308fcc
1 parent 86a6178
commit 5308fcc
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 45 deletions.
diff --git a/hardhat-tests/test/internal/hardhat-network/stack-traces/execution.ts b/hardhat-tests/test/internal/hardhat-network/stack-traces/execution.ts
@@ -102,6 +102,11 @@ interface TxData {
   gas?: bigint;
 }
 
+/**
+ * Returns a SolidityStackTrace object if the transaction failed, the contract address if
+ * the transaction successfully deployed a contract, or undefined if the transaction
+ * succeeded and didn't deploy a contract.
+ */
 export async function traceTransaction(
   provider: EdrProviderWrapper,
   txData: TxData
@@ -137,14 +142,15 @@ export async function traceTransaction(
     params: [response.result ?? response.error.data.transactionHash],
   });
 
-  const stackTrace = responseObject.stackTrace();
-
-  const contractAddress = receipt.contractAddress?.slice(2);
+  const stackTrace: SolidityStackTrace | string | null =
+    responseObject.stackTrace();
 
   if (typeof stackTrace === "string") {
-    throw new Error("shouldn't happen"); // FVTODO
+    throw new Error("this shouldn't happen");
   }
 
+  const contractAddress = receipt.contractAddress?.slice(2);
+
   if (stackTrace === null) {
     return contractAddress;
   }

diff --git a/hardhat-tests/test/internal/hardhat-network/stack-traces/test.ts b/hardhat-tests/test/internal/hardhat-network/stack-traces/test.ts
@@ -483,10 +483,10 @@ async function runTest(
   const txIndexToContract: Map<number, DeployedContract> = new Map();
 
   for (const [txIndex, tx] of testDefinition.transactions.entries()) {
-    let stackTraceOrContractAddress: SolidityStackTrace | string | undefined;
+    let stackTrace: SolidityStackTrace | undefined;
 
     if ("file" in tx) {
-      stackTraceOrContractAddress = await runDeploymentTransactionTest(
+      const stackTraceOrContractAddress = await runDeploymentTransactionTest(
         txIndex,
         tx,
         provider,
@@ -500,6 +500,8 @@ async function runTest(
           name: tx.contract,
           address: Buffer.from(stackTraceOrContractAddress, "hex"),
         });
+      } else {
+        stackTrace = stackTraceOrContractAddress;
       }
     } else {
       const contract = txIndexToContract.get(tx.to);
@@ -509,7 +511,7 @@ async function runTest(
         `No contract was deployed in tx ${tx.to} but transaction ${txIndex} is trying to call it`
       );
 
-      stackTraceOrContractAddress = await runCallTransactionTest(
+      stackTrace = await runCallTransactionTest(
         txIndex,
         tx,
         provider,
@@ -518,45 +520,26 @@ async function runTest(
       );
     }
 
-    try {
-      if (tx.stackTrace === undefined) {
-        if (
-          !(
-            stackTraceOrContractAddress === undefined ||
-            typeof stackTraceOrContractAddress === "string"
-          )
-        ) {
-          assert.fail(`Transaction ${txIndex} shouldn't have failed`);
-        }
-      } else {
-        assert.isFalse(
-          stackTraceOrContractAddress === undefined ||
-            typeof stackTraceOrContractAddress === "string",
-          `Transaction ${txIndex} should have failed`
-        );
+    if (tx.stackTrace === undefined) {
+      if (stackTrace !== undefined) {
+        assert.fail(`Transaction ${txIndex} shouldn't have failed`);
       }
-    } catch (error) {
-      // printMessageTrace(decodedTrace); FVTODO
-
-      throw error;
+    } else {
+      assert.isFalse(
+        stackTrace === undefined,
+        `Transaction ${txIndex} should have failed`
+      );
     }
 
-    if (
-      stackTraceOrContractAddress !== undefined &&
-      typeof stackTraceOrContractAddress !== "string"
-    ) {
-      try {
-        compareStackTraces(
-          txIndex,
-          stackTraceOrContractAddress,
-          tx.stackTrace!,
-          compilerOptions.optimizer
-        );
-        if (testDefinition.print !== undefined && testDefinition.print) {
-          console.log(`Transaction ${txIndex} stack trace`);
-        }
-      } catch (err) {
-        throw err;
+    if (stackTrace !== undefined) {
+      compareStackTraces(
+        txIndex,
+        stackTrace,
+        tx.stackTrace!,
+        compilerOptions.optimizer
+      );
+      if (testDefinition.print !== undefined && testDefinition.print) {
+        console.log(`Transaction ${txIndex} stack trace`);
       }
     }
 
@@ -622,7 +605,7 @@ async function runDeploymentTransactionTest(
   provider: EdrProviderWrapper,
   compilerOutput: CompilerOutput,
   txIndexToContract: Map<number, DeployedContract>
-): Promise<SolidityStackTrace | string | undefined> {
+): Promise<SolidityStackTrace | string> {
   const file = compilerOutput.contracts[tx.file];
 
   assert.isDefined(
@@ -657,6 +640,12 @@ async function runDeploymentTransactionTest(
     gas: tx.gas !== undefined ? BigInt(tx.gas) : undefined,
   });
 
+  if (trace === undefined) {
+    throw new Error(
+      "deployment transactions should either deploy a contract or fail"
+    );
+  }
+
   return trace;
 }
 
@@ -666,7 +655,7 @@ async function runCallTransactionTest(
   provider: EdrProviderWrapper,
   compilerOutput: CompilerOutput,
   contract: DeployedContract
-): Promise<SolidityStackTrace | string | undefined> {
+): Promise<SolidityStackTrace | undefined> {
   const compilerContract =
     compilerOutput.contracts[contract.file][contract.name];
 
@@ -691,6 +680,10 @@ async function runCallTransactionTest(
     gas: tx.gas !== undefined ? BigInt(tx.gas) : undefined,
   });
 
+  if (typeof trace === "string") {
+    throw new Error("call transactions should not deploy contracts");
+  }
+
   return trace;
 }