Skip to content

Fix demo workflows: bootstrap hardhat owner-matrix addresses#3842

Open
MontrealAI wants to merge 1 commit into
mainfrom
codex/fix-demo-workflows-bootstrap-hardhat-owner-matrix-addresses
Open

Fix demo workflows: bootstrap hardhat owner-matrix addresses#3842
MontrealAI wants to merge 1 commit into
mainfrom
codex/fix-demo-workflows-bootstrap-hardhat-owner-matrix-addresses

Conversation

@MontrealAI

Copy link
Copy Markdown
Owner

Motivation

  • Demo runs in CI were failing because the Owner parameter matrix step validated zero addresses for JobRegistry.taxPolicy and RewardEngine.address.
  • The owner-matrix generation must remain strict for real networks but provide deterministic, non-zero, on-chain addresses for Hardhat demos.
  • Provide a centralized, deterministic Hardhat-only bootstrap to deploy minimal/mock contracts and produce an address book used by the owner-parameter matrix.

Description

  • Added scripts/v2/lib/hardhatOwnerMatrixBootstrap.ts which deploys minimal demo contracts on Hardhat (TaxPolicy, Thermostat, Mock FeePool/Reputation/EnergyOracle and RewardEngineMB) and writes a demo address book.
  • Updated scripts/v2/ownerParameterMatrix.ts to detect Hardhat context (chainId/network name), verify that candidate demo addresses have on-chain code, and invoke the bootstrap helper when addresses are missing or invalid; bootstrapped addresses are written to config overrides that the matrix loaders consume.
  • Replaced in-file deployment plumbing in scripts/v2/demoHardhatOwnerMatrixConfig.ts with calls to the new centralized bootstrap helper.
  • Adjusted scripts/v2/__tests__/ownerParameterMatrix.test.ts to mock bootstrapHardhatOwnerMatrix and assert the prepare/override flow; tests were updated to reflect the new bootstrapping path.

Testing

  • Unit tests updated: scripts/v2/__tests__/ownerParameterMatrix.test.ts now mocks bootstrapHardhatOwnerMatrix and validates override file generation (tests modified but not executed in this patch).
  • No demo or CI workflows were executed as part of this change (no npm run demo:* or npm run ci:* was run locally).
  • Recommend running npm run demo:asi-global / npm run demo:zenith-hypernova and the CI preflight suite (npm run ci:preflight, npm run ci:verify-toolchain, npm run ci:sync-contexts -- --check, etc.) to verify end-to-end behaviour in a Hardhat environment.

Codex Task

addressBook = null;
}
}
if (!addressBook && network && LOCAL_NETWORKS.has(network) && canBootstrap) {

Check warning

Code scanning / CodeQL

Useless conditional Warning

This use of variable 'network' always evaluates to true.

Copilot Autofix

AI 5 months ago

In general, to fix a useless conditional where part of the condition is already guaranteed by earlier checks, you remove the redundant portion and keep only the part that can still vary. This makes the code clearer and removes noise that can confuse readers or tools.

Here, the function throws if !network || !LOCAL_NETWORKS.has(network) at lines 412–415. Therefore, any code after that point runs only when network is truthy and in LOCAL_NETWORKS. On line 438, the condition:

if (!addressBook && network && LOCAL_NETWORKS.has(network) && canBootstrap) {

uses network and LOCAL_NETWORKS.has(network) again. While re-checking LOCAL_NETWORKS.has(network) is harmless (and arguably defensive), the network truthiness check is strictly redundant because the function would have thrown otherwise. The minimal, behavior‑preserving change is to remove network && from that if condition:

if (!addressBook && LOCAL_NETWORKS.has(network) && canBootstrap) {

No new imports, methods, or definitions are required; this is a straightforward conditional simplification within scripts/v2/ownerParameterMatrix.ts.

Suggested changeset 1
scripts/v2/ownerParameterMatrix.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/scripts/v2/ownerParameterMatrix.ts b/scripts/v2/ownerParameterMatrix.ts
--- a/scripts/v2/ownerParameterMatrix.ts
+++ b/scripts/v2/ownerParameterMatrix.ts
@@ -435,7 +435,7 @@
       addressBook = null;
     }
   }
-  if (!addressBook && network && LOCAL_NETWORKS.has(network) && canBootstrap) {
+  if (!addressBook && LOCAL_NETWORKS.has(network) && canBootstrap) {
     const bootstrapPath = resolveBootstrapAddressBookPath(network, addressBookPath);
     addressBook = await runDemoBootstrap(network, bootstrapPath);
   }
EOF
@@ -435,7 +435,7 @@
addressBook = null;
}
}
if (!addressBook && network && LOCAL_NETWORKS.has(network) && canBootstrap) {
if (!addressBook && LOCAL_NETWORKS.has(network) && canBootstrap) {
const bootstrapPath = resolveBootstrapAddressBookPath(network, addressBookPath);
addressBook = await runDemoBootstrap(network, bootstrapPath);
}
Copilot is powered by AI and may make mistakes. Always verify output.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e7d1dfa915

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 267 to +273
async function runDemoBootstrap(
network: string,
addressBookPath?: string
): Promise<void> {
): Promise<DemoAddressBook> {
const outputPath = resolveBootstrapAddressBookPath(network, addressBookPath);
await new Promise<void>((resolve, reject) => {
const child = spawn(
'npx',
[
'hardhat',
'run',
'--no-compile',
DEMO_BOOTSTRAP_SCRIPT,
'--network',
network,
],
{
cwd: process.cwd(),
env: {
...process.env,
...(outputPath ? { [DEMO_ADDRESS_BOOK_ENV]: outputPath } : {}),
},
stdio: 'inherit',
}
);
child.on('error', (error) => reject(error));
child.on('close', (code) => {
if (code === 0) {
resolve();
} else {
reject(new Error(`Demo bootstrap exited with code ${code ?? 'unknown'}`));
}
});
});
const payload = await bootstrapHardhatOwnerMatrix(outputPath);
return {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Honor selected network when bootstrapping demo contracts

The new runDemoBootstrap calls bootstrapHardhatOwnerMatrix without setting or passing the CLI --network selection, so when owner:parameters is run via npx ts-node ... ownerParameterMatrix.ts --network localhost (as in package.json), Hardhat’s runtime defaults to the in-process hardhat network. That deploys the demo contracts on the wrong chain but still writes overrides/configs for localhost, yielding address books that don’t exist on the intended node and breaking downstream demo/CI steps that expect those addresses to be live on localhost. Consider threading the selected network into the Hardhat runtime (e.g., HARDHAT_NETWORK) or reverting to a hardhat run --network bootstrap.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants