fix: use global params where other module params exist

If there is a parameter defined in Global parameters, and a different parameter defined at the module level, we were failing to find the global parameter. This has been fixed with a more explicit lookup in `resolveModuleParameter`. Two tests have been added to check module param lookup where a different global parameter exists, and global parameter lookup where a different module parameter exists.
NomicFoundation · Nov 20, 2024 · 64aa809 · 64aa809
1 parent 04f2765
commit 64aa809
Show file tree

Hide file tree

Showing 3 changed files with 119 additions and 17 deletions.
diff --git a/packages/core/src/internal/utils/resolve-module-parameter.ts b/packages/core/src/internal/utils/resolve-module-parameter.ts
@@ -22,26 +22,28 @@ export function resolveModuleParameter(
     return _resolveDefaultValue(moduleParamRuntimeValue, context.accounts);
   }
 
-  const moduleParameters =
-    context.deploymentParameters[moduleParamRuntimeValue.moduleId] ??
-    context.deploymentParameters.$global;
+  const potentialParamAtModuleLevel =
+    context.deploymentParameters?.[moduleParamRuntimeValue.moduleId]?.[
+      moduleParamRuntimeValue.name
+    ];
 
-  if (moduleParameters === undefined) {
-    assertIgnitionInvariant(
-      moduleParamRuntimeValue.defaultValue !== undefined,
-      `No default value provided for module parameter ${moduleParamRuntimeValue.moduleId}/${moduleParamRuntimeValue.name}`
-    );
-
-    return _resolveDefaultValue(moduleParamRuntimeValue, context.accounts);
+  if (potentialParamAtModuleLevel !== undefined) {
+    return potentialParamAtModuleLevel;
   }
 
-  const moduleParamValue = moduleParameters[moduleParamRuntimeValue.name];
+  const potentialParamAtGlobalLevel =
+    context.deploymentParameters?.$global?.[moduleParamRuntimeValue.name];
 
-  if (moduleParamValue === undefined) {
-    return _resolveDefaultValue(moduleParamRuntimeValue, context.accounts);
+  if (potentialParamAtGlobalLevel !== undefined) {
+    return potentialParamAtGlobalLevel;
   }
 
-  return moduleParamValue;
+  assertIgnitionInvariant(
+    moduleParamRuntimeValue.defaultValue !== undefined,
+    `No default value provided for module parameter ${moduleParamRuntimeValue.moduleId}/${moduleParamRuntimeValue.name}`
+  );
+
+  return _resolveDefaultValue(moduleParamRuntimeValue, context.accounts);
 }
 
 function _resolveDefaultValue(

diff --git a/packages/core/test/execution/future-processor/helpers/build-initialize-message-for.ts b/packages/core/test/execution/future-processor/helpers/build-initialize-message-for.ts
@@ -555,6 +555,64 @@ describe("buildInitializeMessageFor", () => {
       });
     });
 
+    describe("resolves value when module parameter is a global parameter", () => {
+      beforeEach(async () => {
+        namedContractDeployment.value =
+          new ModuleParameterRuntimeValueImplementation<bigint>(
+            "MyModule",
+            "passedValue",
+            undefined
+          );
+
+        message = (await buildInitializeMessageFor(
+          namedContractDeployment,
+          exampleDeploymentState,
+          basicStrategy,
+          {
+            $global: {
+              passedValue: BigInt(99),
+            },
+          },
+          mockDeploymentLoader,
+          exampleAccounts,
+          getDefaultSender(exampleAccounts)
+        )) as DeploymentExecutionStateInitializeMessage;
+      });
+
+      it("should record the value", async () => {
+        assert.deepStrictEqual(message.value, BigInt(99));
+      });
+    });
+
+    describe("resolves to default value for module parameter when no deployment parameters have been given", () => {
+      const expectedDefaultValue = BigInt(100);
+
+      beforeEach(async () => {
+        namedContractDeployment.value =
+          new ModuleParameterRuntimeValueImplementation<bigint>(
+            "MyModule",
+            "passedValue",
+            expectedDefaultValue
+          );
+
+        const deploymentParameters = undefined as any;
+
+        message = (await buildInitializeMessageFor(
+          namedContractDeployment,
+          exampleDeploymentState,
+          basicStrategy,
+          deploymentParameters,
+          mockDeploymentLoader,
+          exampleAccounts,
+          getDefaultSender(exampleAccounts)
+        )) as DeploymentExecutionStateInitializeMessage;
+      });
+
+      it("should record the default value", async () => {
+        assert.deepStrictEqual(message.value, expectedDefaultValue);
+      });
+    });
+
     describe("resolves from when runtime account used", () => {
       beforeEach(async () => {
         namedContractDeployment.from = new AccountRuntimeValueImplementation(1);

diff --git a/packages/hardhat-plugin/test/module-parameters.ts b/packages/hardhat-plugin/test/module-parameters.ts
@@ -94,6 +94,22 @@ describe("module parameters", () => {
       );
     });
 
+    it("should use a global parameter if no module parameter is available", async function () {
+      const ignitionModule = buildModule("Test", (m) => {
+        const unlockTime = m.getParameter("unlockTime");
+
+        const lock = m.contract("Lock", [unlockTime]);
+
+        return { lock };
+      });
+
+      const result = await this.hre.ignition.deploy(ignitionModule, {
+        parameters: { $global: { unlockTime: 1893499200000 } },
+      });
+
+      assert.equal(await result.lock.read.unlockTime(), 1893499200000);
+    });
+
     it("should use a global parameter instead of the default value", async function () {
       const ignitionModule = buildModule("Test", (m) => {
         const unlockTime = m.getParameter("unlockTime", 100);
@@ -110,17 +126,43 @@ describe("module parameters", () => {
       assert.equal(await result.lock.read.unlockTime(), 1893499200000);
     });
 
-    it("should use a global parameter if available", async function () {
+    it("should use the module parameter even if global parameters exist but not that specific parameter", async function () {
       const ignitionModule = buildModule("Test", (m) => {
-        const unlockTime = m.getParameter("unlockTime");
+        const unlockTime = m.getParameter("moduleLevelParam");
 
         const lock = m.contract("Lock", [unlockTime]);
 
         return { lock };
       });
 
       const result = await this.hre.ignition.deploy(ignitionModule, {
-        parameters: { $global: { unlockTime: 1893499200000 } },
+        parameters: {
+          $global: { globalLevelParam: "should-not-be-read" },
+          Test: {
+            moduleLevelParam: 1893499200000,
+          },
+        },
+      });
+
+      assert.equal(await result.lock.read.unlockTime(), 1893499200000);
+    });
+
+    it("should use the global parameter even if module parameters exist but not that specific parameter", async function () {
+      const ignitionModule = buildModule("Test", (m) => {
+        const unlockTime = m.getParameter("globalLevelParam");
+
+        const lock = m.contract("Lock", [unlockTime]);
+
+        return { lock };
+      });
+
+      const result = await this.hre.ignition.deploy(ignitionModule, {
+        parameters: {
+          $global: { globalLevelParam: 1893499200000 },
+          Test: {
+            moduleLevelParam: "should-not-be-read",
+          },
+        },
       });
 
       assert.equal(await result.lock.read.unlockTime(), 1893499200000);