Repository: NomicFoundation/hardhat-ignition (ts)

Issue:
Reconciliation of `undefined` `from`s is too restrictive
We are currently failing to reconcile a future with its execution state if its `from` is `undefined`, and the execution state's `from` is different from the `defaultSender` yet included in `accounts`. This case should be allowed.

Pull request description:
fix: allow undefined froms to pass reconciliation (PR 2)
If a `from` is undefined on the first run, allow it to reconcile on the second run using the account that was selected during initialization of the future on the first run. This is only valid if the first run from account (the default sender on the first run) is still in the list of accounts.

Fixes #413.

Patch:
diff --git a/packages/core/src/errors.ts b/packages/core/src/errors.ts
index e4dd8f27..18d65d37 100644
--- a/packages/core/src/errors.ts
+++ b/packages/core/src/errors.ts
@@ -7,8 +7,8 @@ import { ERRORS, ErrorDescriptor, getErrorCode } from "./errors-list";
  * @alpha
  */
 export class CustomError extends Error {
-  constructor(message: string, cause?: Error) {
-    super(message, { cause });
+  constructor(message: string) {
+    super(message);
     this.name = this.constructor.name;
   }
 }
@@ -22,8 +22,7 @@ export class CustomError extends Error {
 export class IgnitionError extends CustomError {
   constructor(
     errorDescriptor: ErrorDescriptor,
-    messageArguments: Record<string, string | number> = {},
-    cause?: Error
+    messageArguments: Record<string, string | number> = {}
   ) {
     const prefix = `${getErrorCode(errorDescriptor)}: `;
     const formattedMessage = applyErrorMessageTemplate(
@@ -31,7 +30,9 @@ export class IgnitionError extends CustomError {
       messageArguments
     );
 
-    super(prefix + formattedMessage, cause);
+    super(prefix + formattedMessage);
+
+    this.name = this.constructor.name;
   }
 }
 
@@ -55,9 +56,14 @@ export class IgnitionPluginError extends CustomError {
 
   public readonly pluginName: string;
 
-  constructor(pluginName: string, message: string, cause?: Error) {
-    super(message, cause);
+  constructor(pluginName: string, message: string) {
+    super(message);
     this.pluginName = pluginName;
+
+    // This is required to allow calls to `resetStackFrom`,
+    // otherwise the function is not available on the
+    // error instance
+    Object.setPrototypeOf(this, new.target.prototype);
   }
 }
 
@@ -79,6 +85,11 @@ export class NomicIgnitionPluginError extends IgnitionPluginError {
   }
 
   private readonly _isNomicIgnitionPluginError = true;
+
+  constructor(pluginName: string, message: string) {
+    super(pluginName, message);
+    Object.setPrototypeOf(this, NomicIgnitionPluginError.prototype);
+  }
 }
 
 /**
diff --git a/packages/core/src/internal/reconciliation/helpers/reconcile-from.ts b/packages/core/src/internal/reconciliation/helpers/reconcile-from.ts
index 287be156..f9fe4e33 100644
--- a/packages/core/src/internal/reconciliation/helpers/reconcile-from.ts
+++ b/packages/core/src/internal/reconciliation/helpers/reconcile-from.ts
@@ -1,11 +1,11 @@
 import {
+  ContractCallFuture,
   ContractDeploymentFuture,
   LibraryDeploymentFuture,
-  ContractCallFuture,
   NamedArtifactContractDeploymentFuture,
   NamedArtifactLibraryDeploymentFuture,
-  StaticCallFuture,
   SendDataFuture,
+  StaticCallFuture,
 } from "../../../types/module";
 import { resolveFutureFrom } from "../../execution/future-processor/helpers/future-resolvers";
 import {
@@ -37,6 +37,10 @@ export function reconcileFrom(
     | StaticCallExecutionState,
   context: ReconciliationContext
 ): ReconciliationFutureResultFailure | undefined {
+  if (future.from === undefined && context.accounts.includes(exState.from)) {
+    return undefined;
+  }
+
   const resolvedFrom = resolveFutureFrom(
     future.from,
     context.accounts,
diff --git a/packages/core/src/internal/utils/adjacency-list.ts b/packages/core/src/internal/utils/adjacency-list.ts
index 5041d734..b8fa6b1a 100644
--- a/packages/core/src/internal/utils/adjacency-list.ts
+++ b/packages/core/src/internal/utils/adjacency-list.ts
@@ -1,5 +1,3 @@
-import { assertIgnitionInvariant } from "./assertions";
-
 export class AdjacencyList {
   /**
    * A mapping from futures to each futures dependencies.
@@ -73,10 +71,9 @@ export class AdjacencyList {
     for (const dependent of dependents) {
       const toSet = this._list.get(dependent);
 
-      assertIgnitionInvariant(
-        toSet !== undefined,
-        "Dependency sets should be defined"
-      );
+      if (toSet === undefined) {
+        throw new Error("Dependency sets should be defined");
+      }
 
       const setWithoutFuture = new Set<string>(
         [...toSet].filter((n) => n !== futureId)


Test patch:
diff --git a/packages/core/test/reconciliation/futures/reconcileNamedContractCall.ts b/packages/core/test/reconciliation/futures/reconcileNamedContractCall.ts
index 37aa17c4..88cc138e 100644
--- a/packages/core/test/reconciliation/futures/reconcileNamedContractCall.ts
+++ b/packages/core/test/reconciliation/futures/reconcileNamedContractCall.ts
@@ -94,6 +94,40 @@ describe("Reconciliation - named contract call", () => {
     );
   });
 
+  it("should reconcile when the from is undefined but the exState's from is in the accounts list", async () => {
+    const moduleDefinition = buildModule("Module", (m) => {
+      const contract1 = m.contract("Contract1");
+
+      m.call(contract1, "function1", [1, "a", contract1], { from: undefined });
+
+      return { contract1 };
+    });
+
+    await assertSuccessReconciliation(
+      moduleDefinition,
+      createDeploymentState(
+        {
+          ...exampleDeploymentState,
+          id: "Module#Contract1",
+          status: ExecutionStatus.SUCCESS,
+          result: {
+            type: ExecutionResultType.SUCCESS,
+            address: differentAddress,
+          },
+        },
+        {
+          ...exampleContractCallState,
+          id: "Module#Contract1.function1",
+          futureType: FutureType.CONTRACT_CALL,
+          status: ExecutionStatus.SUCCESS,
+          functionName: "function1",
+          args: [1, "a", differentAddress],
+          from: exampleAccounts[2],
+        }
+      )
+    );
+  });
+
   it("should find changes to contract unreconciliable", async () => {
     const moduleDefinition = buildModule("Module", (m) => {
       const contract1 = m.contract("Contract1");
diff --git a/packages/core/test/reconciliation/futures/reconcileNamedContractDeployment.ts b/packages/core/test/reconciliation/futures/reconcileNamedContractDeployment.ts
index 7271abf4..3ba19a6b 100644
--- a/packages/core/test/reconciliation/futures/reconcileNamedContractDeployment.ts
+++ b/packages/core/test/reconciliation/futures/reconcileNamedContractDeployment.ts
@@ -96,6 +96,34 @@ describe("Reconciliation - named contract", () => {
     );
   });
 
+  /**
+   * This test here is in a first run, the from is undefined and the defaultSender is used.
+   * On the second run the from is undefined but a different defaultSender is now in play.
+   * We say this should reconcile but the account from the first run should be used, as long
+   * as it is in the accounts list
+   */
+  it("should reconcile where the future is undefined but the exState's from is in the accounts list", async () => {
+    const moduleDefinition = buildModule("Module", (m) => {
+      const contract1 = m.contract("Contract", [], {
+        id: "Example",
+        from: undefined,
+      });
+
+      return { contract1 };
+    });
+
+    await assertSuccessReconciliation(
+      moduleDefinition,
+      createDeploymentState({
+        ...exampleDeploymentState,
+        id: "Module#Example",
+        status: ExecutionStatus.STARTED,
+        contractName: "Contract",
+        from: exampleAccounts[3],
+      })
+    );
+  });
+
   it("should find changes to contract name unreconciliable", async () => {
     const moduleDefinition = buildModule("Module", (m) => {
       const contract1 = m.contract("ContractChanged", [], {
diff --git a/packages/core/test/reconciliation/futures/reconcileNamedStaticCall.ts b/packages/core/test/reconciliation/futures/reconcileNamedStaticCall.ts
index 691ec4c3..e8892a54 100644
--- a/packages/core/test/reconciliation/futures/reconcileNamedStaticCall.ts
+++ b/packages/core/test/reconciliation/futures/reconcileNamedStaticCall.ts
@@ -95,6 +95,43 @@ describe("Reconciliation - named static call", () => {
     );
   });
 
+  it("should reconcile when the from is undefined but the exState's from is in the accounts list", async () => {
+    const moduleDefinition = buildModule("Module", (m) => {
+      const contract1 = m.contract("Contract1");
+
+      m.staticCall(contract1, "function1", [1, "a"], 0, {
+        from: undefined,
+      });
+
+      return { contract1 };
+    });
+
+    await assertSuccessReconciliation(
+      moduleDefinition,
+      createDeploymentState(
+        {
+          ...exampleDeploymentState,
+          id: "Module#Contract1",
+          status: ExecutionStatus.SUCCESS,
+          result: {
+            type: ExecutionResultType.SUCCESS,
+            address: exampleAddress,
+          },
+        },
+        {
+          ...exampleStaticCallState,
+          id: "Module#Contract1.function1",
+          futureType: FutureType.STATIC_CALL,
+          status: ExecutionStatus.SUCCESS,
+          contractAddress: exampleAddress,
+          functionName: "function1",
+          args: [1, "a"],
+          from: exampleAccounts[4],
+        }
+      )
+    );
+  });
+
   it("should find changes to contract unreconciliable", async () => {
     const moduleDefinition = buildModule("Module", (m) => {
       const contract1 = m.contract("Contract1");
diff --git a/packages/core/test/reconciliation/futures/reconcileSendData.ts b/packages/core/test/reconciliation/futures/reconcileSendData.ts
index 06dca8e5..6303f365 100644
--- a/packages/core/test/reconciliation/futures/reconcileSendData.ts
+++ b/packages/core/test/reconciliation/futures/reconcileSendData.ts
@@ -56,6 +56,31 @@ describe("Reconciliation - send data", () => {
     );
   });
 
+  /**
+   * This test here is in a first run, the from is undefined and the defaultSender is used.
+   * On the second run the from is undefined but a different defaultSender is now in play.
+   * We say this should reconcile but the account from the first run should be used, as long
+   * as it is in the accounts list.
+   */
+  it("should reconcile where the future is undefined but the exState's from is in the accounts list", async () => {
+    const moduleDefinition = buildModule("Module", (m) => {
+      m.send("test_send", exampleAddress, 0n, "example_data", {
+        from: undefined,
+      });
+
+      return {};
+    });
+
+    await assertSuccessReconciliation(
+      moduleDefinition,
+      createDeploymentState({
+        ...exampleSendState,
+        id: "Module#test_send",
+        status: ExecutionStatus.STARTED,
+      })
+    );
+  });
+
   it("should reconcile between undefined and 0x for data", async () => {
     const moduleDefinition = buildModule("Module", (m) => {
       m.send("test_send", exampleAddress, 0n, undefined);