Fix demo workflows: bootstrap hardhat owner-matrix addresses in CI#3840
Fix demo workflows: bootstrap hardhat owner-matrix addresses in CI#3840MontrealAI wants to merge 1 commit into
Conversation
| addressBook = await deriveDemoAddressBookFromConfigs(network); | ||
| } | ||
| if (!addressBook && network && LOCAL_NETWORKS.has(network) && shouldBootstrapDemo()) { | ||
| if (!addressBook && network && LOCAL_NETWORKS.has(network) && bootstrapEnabled) { |
Check warning
Code scanning / CodeQL
Useless conditional Warning
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 months ago
In general, when a condition includes a sub-expression that is provably always true at that program point (e.g., due to prior guards that throw or return in all other cases), you should remove the redundant part and keep only the part that can actually vary. This makes the intent clearer and avoids misleading readers into thinking there is a meaningful runtime check there.
Here, the initial guard:
if (!network || !LOCAL_NETWORKS.has(network)) {
throw new Error(
`${DEMO_ADDRESS_BOOK_ENV} is only supported on local hardhat networks`
);
}ensures that any subsequent code runs only when network is defined and LOCAL_NETWORKS.has(network) is true. Therefore, the later condition at line 419:
if (!addressBook && network && LOCAL_NETWORKS.has(network) && bootstrapEnabled) {can be simplified by removing the redundant network && LOCAL_NETWORKS.has(network) check. The behavior is unchanged because those conditions are already guaranteed on all reachable paths. The best minimal fix is to change that if to:
if (!addressBook && bootstrapEnabled) {No new imports or helper methods are required; we only adjust the conditional expression inside the existing function in scripts/v2/ownerParameterMatrix.ts.
| @@ -416,7 +416,7 @@ | ||
| if (!addressBook) { | ||
| addressBook = await deriveDemoAddressBookFromConfigs(network); | ||
| } | ||
| if (!addressBook && network && LOCAL_NETWORKS.has(network) && bootstrapEnabled) { | ||
| if (!addressBook && bootstrapEnabled) { | ||
| const bootstrapPath = resolveBootstrapAddressBookPath(network, addressBookPath); | ||
| await runDemoBootstrap(network, bootstrapPath); | ||
| try { |
Motivation
taxPolicyandrewardEngineaddresses were resolving to the zero address when the local demo address book was absent.Description
prepareDemoOverridesby settingbootstrapEnabled = shouldBootstrapDemo() || process.env.CI === 'true'and use that flag to runrunDemoBootstrap.process.env.CI = 'true') to verify the bootstrap path is invoked and writes demo overrides, and ensure test suite restores the originalCIenvironment after each test.scripts/v2/ownerParameterMatrix.tsandscripts/v2/__tests__/ownerParameterMatrix.test.ts.Testing
scripts/v2/__tests__/ownerParameterMatrix.test.tsnow includes a CI-bootstrap test scenario covering the new behavior. (This test was added/modified in the patch.)CIenv state between tests.Codex Task