From ab0ca0c7d2723bdacaa6cac8b5214b604c8da376 Mon Sep 17 00:00:00 2001
From: Eric Lau <ericglau@outlook.com>
Date: Fri, 25 Oct 2024 16:54:52 -0400
Subject: [PATCH 01/24] Check for initializer errors

---
 .../contracts/test/ValidationsInitializer.sol | 223 ++++++++++++++++++
 .../core/src/validate-initializers.test.ts    | 106 +++++++++
 packages/core/src/validate/overrides.ts       |  16 ++
 packages/core/src/validate/report.ts          |  20 ++
 packages/core/src/validate/run.ts             | 135 ++++++++++-
 5 files changed, 499 insertions(+), 1 deletion(-)
 create mode 100644 packages/core/contracts/test/ValidationsInitializer.sol
 create mode 100644 packages/core/src/validate-initializers.test.ts

diff --git a/packages/core/contracts/test/ValidationsInitializer.sol b/packages/core/contracts/test/ValidationsInitializer.sol
new file mode 100644
index 000000000..59afbeef6
--- /dev/null
+++ b/packages/core/contracts/test/ValidationsInitializer.sol
@@ -0,0 +1,223 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.0;
+
+import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
+
+// These contracts are for testing only. They are not safe for use in production, and do not represent best practices.
+
+// ==== Parent contracts ====
+
+contract Parent_NoInitializer {
+  function parentFn() internal {}
+}
+
+contract Parent_InitializerModifier is Initializable {
+  function parentInit() initializer public {}
+}
+
+contract Parent_ReinitializerModifier is Initializable {
+  function parentReinit() reinitializer(2) public {}
+}
+
+contract Parent__OnlyInitializingModifier is Initializable {
+  function __Parent_init() onlyInitializing() internal {}
+}
+
+contract Parent_InitializeName {
+  function initialize() public virtual {}
+}
+
+contract Parent_InitializerName {
+  function initializer() public {}
+}
+
+contract Parent_ReinitializeName {
+  function reinitialize(uint64 version) public {}
+}
+
+contract Parent_ReinitializerName {
+  function reinitializer(uint64 version) public {}
+}
+
+// ==== Child contracts ====
+
+contract Child_Of_NoInitializer_Ok is Parent_NoInitializer {
+  function childFn() public {}
+}
+
+contract Child_Of_InitializerModifier_Ok is Parent_InitializerModifier {
+  function initialize() public {
+    parentInit();
+  }
+}
+
+contract Child_Of_InitializerModifier_UsesSuper_Ok is Parent_InitializerModifier {
+  function initialize() public {
+    super.parentInit();
+  }
+}
+
+contract Child_Of_InitializerModifier_Bad is Parent_InitializerModifier {
+  function initialize() public {}
+}
+
+contract Child_Of_ReinitializerModifier_Ok is Parent_ReinitializerModifier {
+  function initialize() public {
+    parentReinit();
+  }
+}
+
+contract Child_Of_ReinitializerModifier_Bad is Parent_ReinitializerModifier {
+  function initialize() public {}
+}
+
+contract Child_Of_OnlyInitializingModifier_Ok is Parent__OnlyInitializingModifier {
+  function initialize() public {
+    __Parent_init();
+  }
+}
+
+contract Child_Of_OnlyInitializingModifier_Bad is Parent__OnlyInitializingModifier {
+  function initialize() public {}
+}
+
+// This is considered to have a missing initializer because the `regularFn` function is not inferred as an intializer
+contract MissingInitializer_Bad is Parent_InitializerModifier {
+  function regularFn() public {
+    parentInit();
+  }
+}
+
+/// @custom:oz-upgrades-unsafe-allow missing-initializer
+contract MissingInitializer_UnsafeAllow_Contract is Parent_InitializerModifier {
+  function regularFn() public {
+    parentInit();
+  }
+}
+
+contract A is Initializable {
+  function __A_init() onlyInitializing internal {}
+}
+
+contract B is Initializable {
+  function __B_init() onlyInitializing internal {}
+}
+
+contract C is Initializable {
+  function __C_init() onlyInitializing internal {}
+}
+
+contract InitializationOrder_Ok is A, B, C, Parent_NoInitializer {
+  function initialize() public {
+    __A_init();
+    __B_init();
+    __C_init();
+  }
+}
+
+contract InitializationOrder_Ok_2 is A, B, C, Parent_NoInitializer {
+  function initialize() public {
+    __A_init();
+    __B_init();
+    parentFn(); // this is not an initializer so we don't check its linearization order
+    __C_init();
+  }
+}
+
+contract InitializationOrder_WrongOrder_Bad is A, B, C, Parent_NoInitializer {
+  function initialize() public {
+    __A_init();
+    __C_init();
+    parentFn();
+    __B_init();
+  }
+}
+
+/// @custom:oz-upgrades-unsafe-allow incorrect-initializer-order
+contract InitializationOrder_WrongOrder_UnsafeAllow_Contract is A, B, C, Parent_NoInitializer {
+  function initialize() public {
+    __A_init();
+    __C_init();
+    parentFn();
+    __B_init();
+  }
+}
+
+contract InitializationOrder_WrongOrder_UnsafeAllow_Function is A, B, C, Parent_NoInitializer {
+  /// @custom:oz-upgrades-unsafe-allow incorrect-initializer-order
+  function initialize() public {
+    __A_init();
+    __C_init();
+    parentFn();
+    __B_init();
+  }
+}
+
+contract InitializationOrder_MissingCall_Bad is A, B, C, Parent_NoInitializer {
+  function initialize() public {
+    __A_init();
+    __B_init();
+    parentFn();
+  }
+}
+
+/// @custom:oz-upgrades-unsafe-allow missing-initializer-call
+contract InitializationOrder_MissingCall_UnsafeAllow_Contract is A, B, C, Parent_NoInitializer {
+  function initialize() public {
+    __A_init();
+    __B_init();
+    parentFn();
+  }
+}
+
+contract InitializationOrder_MissingCall_UnsafeAllow_Function is A, B, C, Parent_NoInitializer {
+  /// @custom:oz-upgrades-unsafe-allow missing-initializer-call
+  function initialize() public {
+    __A_init();
+    __B_init();
+    parentFn();
+  }
+}
+
+contract InitializationOrder_Duplicate_Bad is A, B, C, Parent_NoInitializer {
+  function initialize() public {
+    __A_init();
+    __B_init();
+    parentFn();
+    __B_init();
+    __C_init();
+  }
+}
+
+/// @custom:oz-upgrades-unsafe-allow duplicate-initializer-call
+contract InitializationOrder_Duplicate_UnsafeAllow_Contract is A, B, C, Parent_NoInitializer {
+  function initialize() public {
+    __A_init();
+    __B_init();
+    parentFn();
+    __B_init();
+    __C_init();
+  }
+}
+
+contract InitializationOrder_Duplicate_UnsafeAllow_Function is A, B, C, Parent_NoInitializer {
+  /// @custom:oz-upgrades-unsafe-allow duplicate-initializer-call
+  function initialize() public {
+    __A_init();
+    __B_init();
+    parentFn();
+    __B_init();
+    __C_init();
+  }
+}
+
+contract InitializationOrder_Duplicate_UnsafeAllow_Call is A, B, C, Parent_NoInitializer {
+  function initialize() public {
+    __A_init();
+    __B_init();
+    parentFn();
+    /// @custom:oz-upgrades-unsafe-allow duplicate-initializer-call
+    __B_init();
+    __C_init();
+  }
+}
\ No newline at end of file
diff --git a/packages/core/src/validate-initializers.test.ts b/packages/core/src/validate-initializers.test.ts
new file mode 100644
index 000000000..df85bda12
--- /dev/null
+++ b/packages/core/src/validate-initializers.test.ts
@@ -0,0 +1,106 @@
+import _test, { TestFn } from 'ava';
+import { artifacts } from 'hardhat';
+
+import {
+  validate,
+  getContractVersion,
+  assertUpgradeSafe,
+  ValidationOptions,
+  RunValidation,
+  ValidationErrors,
+} from './validate';
+import { solcInputOutputDecoder } from './src-decoder';
+
+interface Context {
+  validation: RunValidation;
+}
+
+const test = _test as TestFn<Context>;
+
+test.before(async t => {
+  const contracts = ['contracts/test/ValidationsInitializer.sol:Parent_NoInitializer'];
+
+  t.context.validation = {} as RunValidation;
+  for (const contract of contracts) {
+    const buildInfo = await artifacts.getBuildInfo(contract);
+    if (buildInfo === undefined) {
+      throw new Error(`Build info not found for contract ${contract}`);
+    }
+    const solcOutput = buildInfo.output;
+    const solcInput = buildInfo.input;
+    const decodeSrc = solcInputOutputDecoder(solcInput, solcOutput);
+    Object.assign(t.context.validation, validate(solcOutput, decodeSrc));
+  }
+});
+
+function testValid(name: string, kind: ValidationOptions['kind'], valid: boolean, numExpectedErrors?: number) {
+  testOverride(name, kind, {}, valid, numExpectedErrors);
+}
+
+function testOverride(
+  name: string,
+  kind: ValidationOptions['kind'],
+  opts: ValidationOptions,
+  valid: boolean,
+  numExpectedErrors?: number,
+) {
+  if (numExpectedErrors !== undefined && numExpectedErrors > 0 && valid) {
+    throw new Error('Cannot expect errors for a valid contract');
+  }
+
+  const optKeys = Object.keys(opts);
+  const describeOpts = optKeys.length > 0 ? '(' + optKeys.join(', ') + ')' : '';
+  const testName = [valid ? 'accepts' : 'rejects', kind, name, describeOpts].join(' ');
+  test(testName, t => {
+    const version = getContractVersion(t.context.validation, name);
+    const assertUpgSafe = () => assertUpgradeSafe([t.context.validation], version, { kind, ...opts });
+    if (valid) {
+      t.notThrows(assertUpgSafe);
+    } else {
+      const error = t.throws(assertUpgSafe) as ValidationErrors;
+      if (numExpectedErrors !== undefined) {
+        t.is(error.errors.length, numExpectedErrors);
+      }
+    }
+  });
+}
+
+testValid('Child_Of_NoInitializer_Ok', 'transparent', true);
+
+testValid('Child_Of_InitializerModifier_Ok', 'transparent', true);
+testValid('Child_Of_InitializerModifier_Bad', 'transparent', false, 1);
+testValid('Child_Of_InitializerModifier_UsesSuper_Ok', 'transparent', true);
+
+testValid('Child_Of_ReinitializerModifier_Ok', 'transparent', true);
+testValid('Child_Of_ReinitializerModifier_Bad', 'transparent', false, 1);
+
+testValid('Child_Of_OnlyInitializingModifier_Ok', 'transparent', true);
+testValid('Child_Of_OnlyInitializingModifier_Bad', 'transparent', false, 1);
+
+testValid('MissingInitializer_Bad', 'transparent', false, 1);
+testValid('MissingInitializer_UnsafeAllow_Contract', 'transparent', true);
+testOverride('MissingInitializer_Bad', 'transparent', { unsafeAllow: ['missing-initializer'] }, true);
+
+testValid('InitializationOrder_Ok', 'transparent', true);
+testValid('InitializationOrder_Ok_2', 'transparent', true);
+
+testValid('InitializationOrder_WrongOrder_Bad', 'transparent', false, 1);
+testValid('InitializationOrder_WrongOrder_UnsafeAllow_Contract', 'transparent', true);
+testValid('InitializationOrder_WrongOrder_UnsafeAllow_Function', 'transparent', true);
+testOverride(
+  'InitializationOrder_WrongOrder_Bad',
+  'transparent',
+  { unsafeAllow: ['incorrect-initializer-order'] },
+  true,
+);
+
+testValid('InitializationOrder_MissingCall_Bad', 'transparent', false, 1);
+testValid('InitializationOrder_MissingCall_UnsafeAllow_Contract', 'transparent', true);
+testValid('InitializationOrder_MissingCall_UnsafeAllow_Function', 'transparent', true);
+testOverride('InitializationOrder_MissingCall_Bad', 'transparent', { unsafeAllow: ['missing-initializer-call'] }, true);
+
+testValid('InitializationOrder_Duplicate_Bad', 'transparent', false, 1);
+testValid('InitializationOrder_Duplicate_UnsafeAllow_Contract', 'transparent', true);
+testValid('InitializationOrder_Duplicate_UnsafeAllow_Function', 'transparent', true);
+testValid('InitializationOrder_Duplicate_UnsafeAllow_Call', 'transparent', true);
+testOverride('InitializationOrder_Duplicate_Bad', 'transparent', { unsafeAllow: ['duplicate-initializer-call'] }, true);
diff --git a/packages/core/src/validate/overrides.ts b/packages/core/src/validate/overrides.ts
index 3c0d1a508..964a48a03 100644
--- a/packages/core/src/validate/overrides.ts
+++ b/packages/core/src/validate/overrides.ts
@@ -81,6 +81,22 @@ export const ValidationErrorUnsafeMessages: Record<ValidationError['kind'], stri
     `Internal functions are code pointers which will no longer be valid after an upgrade.`,
     `Make sure you reassign internal functions in storage variables during upgrades.`,
   ],
+  'missing-initializer': [
+    `You are using the \`unsafeAllow.missing-initializer\` flag.`,
+    `Make sure you have manually checked that the contract has an initializer and that it correct calls any parent initializers.`,
+  ],
+  'missing-initializer-call': [
+    `You are using the \`unsafeAllow.missing-initializer-call\` flag.`,
+    `Make sure you have manually checked that the contract has an initializer and that it correct calls any parent initializers.`,
+  ],
+  'duplicate-initializer-call': [
+    `You are using the \`unsafeAllow.duplicate-initializer-call\` flag.`,
+    `Make sure you have manually checked that the contract has an initializer and that it correct calls any parent initializers.`,
+  ],
+  'incorrect-initializer-order': [
+    `You are using the \`unsafeAllow.incorrect-initializer-order\` flag.`,
+    `Make sure you have manually checked that the contract has an initializer and that it correct calls any parent initializers.`,
+  ],
 };
 
 export function withValidationDefaults(opts: ValidationOptions): Required<ValidationOptions> {
diff --git a/packages/core/src/validate/report.ts b/packages/core/src/validate/report.ts
index 44b42cf9a..f40f604c7 100644
--- a/packages/core/src/validate/report.ts
+++ b/packages/core/src/validate/report.ts
@@ -72,6 +72,26 @@ const errorInfo: ErrorDescriptions<ValidationError> = {
       `     flag and ensure you always reassign internal functions in storage during upgrades`,
     link: 'https://zpl.in/upgrades/error-009',
   },
+  'missing-initializer': {
+    msg: () => `Contract is missing an initializer`,
+    hint: () => `Define an initializer function`, // TODO include instruction to call parent initializers, or use a separate error message
+    link: 'https://zpl.in/upgrades/error-010', // TODO define link
+  },
+  'missing-initializer-call': {
+    msg: () => `Contract is missing a call to a parent initializer`,
+    hint: () => `Call the parent initializer in your initializer function`,
+    link: 'https://zpl.in/upgrades/error-011', // TODO define link
+  },
+  'duplicate-initializer-call': {
+    msg: () => `Contract has multiple calls to a parent initializer`,
+    hint: () => `Only call each parent initializer once`,
+    link: 'https://zpl.in/upgrades/error-012', // TODO define link
+  },
+  'incorrect-initializer-order': {
+    msg: () => `Contract has an incorrect order of parent initializer calls`,
+    hint: () => `Call parent initializers in the order they are inherited`,
+    link: 'https://zpl.in/upgrades/error-013', // TODO define link
+  },
 };
 
 function describeError(e: ValidationError, color = true): string {
diff --git a/packages/core/src/validate/run.ts b/packages/core/src/validate/run.ts
index bc6ff266e..6099bb06c 100644
--- a/packages/core/src/validate/run.ts
+++ b/packages/core/src/validate/run.ts
@@ -50,6 +50,10 @@ export const errorKinds = [
   'selfdestruct',
   'missing-public-upgradeto',
   'internal-function-storage',
+  'missing-initializer',
+  'missing-initializer-call',
+  'duplicate-initializer-call',
+  'incorrect-initializer-order',
 ] as const;
 
 export type ValidationError =
@@ -71,7 +75,11 @@ interface ValidationErrorWithName extends ValidationErrorBase {
     | 'external-library-linking'
     | 'struct-definition'
     | 'enum-definition'
-    | 'internal-function-storage';
+    | 'internal-function-storage'
+    | 'missing-initializer'
+    | 'missing-initializer-call'
+    | 'duplicate-initializer-call'
+    | 'incorrect-initializer-order';
 }
 
 interface ValidationErrorConstructor extends ValidationErrorBase {
@@ -209,6 +217,7 @@ export function validate(
           // https://github.com/OpenZeppelin/openzeppelin-upgrades/issues/52
           ...getLinkingErrors(contractDef, bytecode),
           ...getInternalFunctionStorageErrors(contractDef, deref, decodeSrc),
+          ...getInitializerErrors(contractDef, deref, decodeSrc),
         ];
 
         validation[key].layout = extractStorageLayout(
@@ -629,6 +638,130 @@ function* getInternalFunctionStorageErrors(
   }
 }
 
+/**
+ * Reports an error if a parent contract has an initializer and any of the following are true:
+ * - 1. Missing initializer: This contract does not appear to have an initializer.
+ * - 2. Missing initializer call: This contract's initializer is missing a call to a parent initializer.
+ * - 3. Duplicate initializer call: This contract has duplicate calls to the same parent initializer function.
+ * - 4. Incorrect initializer order: This contract does not call parent initializers in the correct order.
+ */
+function* getInitializerErrors(
+  contractDef: ContractDefinition,
+  deref: ASTDereferencer,
+  decodeSrc: SrcDecoder,
+): Generator<ValidationErrorWithName> {
+  if (contractDef.baseContracts.length > 0) {
+    const baseContractDefs = contractDef.baseContracts.map(base =>
+      deref('ContractDefinition', base.baseName.referencedDeclaration),
+    );
+    const baseContractsInitializersMap = new Map(
+      baseContractDefs.map(base => [base.name, getPossibleInitializers(base)]),
+    );
+    const baseContractsWithInitializers = baseContractDefs
+      .filter(base => hasInitializers(base.name, baseContractsInitializersMap))
+      .map(base => base.name);
+
+    if (baseContractsWithInitializers.length > 0) {
+      // Check for missing initializers
+      const contractInitializers = getPossibleInitializers(contractDef);
+      if (contractInitializers.length === 0 && !skipCheck('missing-initializer', contractDef)) {
+        yield {
+          kind: 'missing-initializer',
+          name: contractDef.name,
+          src: decodeSrc(contractDef),
+        };
+      }
+
+      for (const contractInitializer of contractInitializers) {
+        const uninitializedBaseContracts = [...baseContractsWithInitializers];
+        const calledInitializerIds: number[] = [];
+
+        const expressionStatements =
+          contractInitializer.body?.statements?.filter(stmt => stmt.nodeType === 'ExpressionStatement') ?? [];
+        for (const stmt of expressionStatements) {
+          const fnCall = stmt.expression;
+          if (
+            fnCall.nodeType === 'FunctionCall' &&
+            (fnCall.expression.nodeType === 'Identifier' || fnCall.expression.nodeType === 'MemberAccess')
+          ) {
+            const referencedFn = fnCall.expression.referencedDeclaration;
+
+            // If this is a call to a parent initializer, then:
+            // - Check for duplicates
+            // - Check if the parent initializer is called in the correct order
+            for (const [baseName, initializers] of baseContractsInitializersMap) {
+              const foundParentInitializer = initializers.find(init => init.id === referencedFn);
+              if (referencedFn && foundParentInitializer) {
+                const duplicate = calledInitializerIds.includes(referencedFn);
+                if (
+                  duplicate &&
+                  !skipCheck('duplicate-initializer-call', contractDef) &&
+                  !skipCheck('duplicate-initializer-call', contractInitializer) &&
+                  !skipCheck('duplicate-initializer-call', stmt)
+                ) {
+                  yield {
+                    kind: 'duplicate-initializer-call',
+                    name: contractDef.name,
+                    src: decodeSrc(fnCall),
+                  };
+                }
+                calledInitializerIds.push(referencedFn);
+
+                const index = uninitializedBaseContracts.indexOf(baseName);
+                if (
+                  !duplicate &&
+                  index !== 0 &&
+                  !skipCheck('incorrect-initializer-order', contractDef) &&
+                  !skipCheck('incorrect-initializer-order', contractInitializer)
+                ) {
+                  yield {
+                    kind: 'incorrect-initializer-order',
+                    name: contractDef.name,
+                    src: decodeSrc(fnCall),
+                  };
+                }
+                if (index !== -1) {
+                  uninitializedBaseContracts.splice(index, 1);
+                }
+              }
+            }
+          }
+        }
+
+        // If there are any base contracts that were not initialized, report an error
+        if (
+          uninitializedBaseContracts.length > 0 &&
+          !skipCheck('missing-initializer-call', contractDef) &&
+          !skipCheck('missing-initializer-call', contractInitializer)
+        ) {
+          console.log('contractDef', contractDef.name);
+          console.log('uninitializedBaseContracts', uninitializedBaseContracts);
+          yield {
+            kind: 'missing-initializer-call',
+            name: contractDef.name,
+            src: decodeSrc(contractInitializer),
+          };
+        }
+      }
+    }
+  }
+}
+
+function hasInitializers(baseName: string, baseContractsInitializersMap: Map<string, FunctionDefinition[]>) {
+  const initializers = baseContractsInitializersMap.get(baseName);
+  return initializers !== undefined && initializers.length > 0;
+}
+
+function getPossibleInitializers(contractDef: ContractDefinition) {
+  const fns = [...findAll('FunctionDefinition', contractDef)];
+  return fns.filter(
+    fnDef =>
+      fnDef.modifiers.some(modifier =>
+        ['initializer', 'reinitializer', 'onlyInitializing'].includes(modifier.modifierName.name),
+      ) || ['initialize', 'initializer', 'reinitialize', 'reinitializer'].includes(fnDef.name),
+  );
+}
+
 /**
  * Gets variables declared directly in a contract or in a struct definition.
  *

From cfadde999a8dcc165e62b445b4fcc2abbd89c9ff Mon Sep 17 00:00:00 2001
From: Eric Lau <ericglau@outlook.com>
Date: Fri, 29 Nov 2024 15:34:05 -0500
Subject: [PATCH 02/24] Omit abstract functions from initializers, fix tests to
 satisfy new rules

---
 .../core/contracts/test/cli/excludes/AbstractUUPS.sol     | 4 ++++
 .../core/contracts/test/cli/excludes/UsesAbstractUUPS.sol | 4 ++++
 packages/core/src/validate/run.ts                         | 8 ++++----
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/packages/core/contracts/test/cli/excludes/AbstractUUPS.sol b/packages/core/contracts/test/cli/excludes/AbstractUUPS.sol
index 85005b80b..8e4a3e554 100644
--- a/packages/core/contracts/test/cli/excludes/AbstractUUPS.sol
+++ b/packages/core/contracts/test/cli/excludes/AbstractUUPS.sol
@@ -11,4 +11,8 @@ abstract contract AbstractUUPS is UUPSUpgradeable, Abstract1, Abstract2 {
     constructor(uint256 _x, uint256 _y, uint256 _z) Abstract1(_x) Abstract2(_y) {
         z = _z;
     }
+
+    function initialize() initializer public {
+        __UUPSUpgradeable_init();
+    }
 }
\ No newline at end of file
diff --git a/packages/core/contracts/test/cli/excludes/UsesAbstractUUPS.sol b/packages/core/contracts/test/cli/excludes/UsesAbstractUUPS.sol
index 4a6f7f76b..426a330d6 100644
--- a/packages/core/contracts/test/cli/excludes/UsesAbstractUUPS.sol
+++ b/packages/core/contracts/test/cli/excludes/UsesAbstractUUPS.sol
@@ -12,4 +12,8 @@ contract UsesAbstractUUPS is AbstractUUPS {
     function _authorizeUpgrade(address newImplementation) internal pure override {
         revert("Upgrade disabled");
     }
+
+    function initializeChild() initializer public {
+        super.initialize();
+    }
 }
\ No newline at end of file
diff --git a/packages/core/src/validate/run.ts b/packages/core/src/validate/run.ts
index 6099bb06c..662b2e85e 100644
--- a/packages/core/src/validate/run.ts
+++ b/packages/core/src/validate/run.ts
@@ -734,8 +734,6 @@ function* getInitializerErrors(
           !skipCheck('missing-initializer-call', contractDef) &&
           !skipCheck('missing-initializer-call', contractInitializer)
         ) {
-          console.log('contractDef', contractDef.name);
-          console.log('uninitializedBaseContracts', uninitializedBaseContracts);
           yield {
             kind: 'missing-initializer-call',
             name: contractDef.name,
@@ -756,9 +754,11 @@ function getPossibleInitializers(contractDef: ContractDefinition) {
   const fns = [...findAll('FunctionDefinition', contractDef)];
   return fns.filter(
     fnDef =>
-      fnDef.modifiers.some(modifier =>
+      (fnDef.modifiers.some(modifier =>
         ['initializer', 'reinitializer', 'onlyInitializing'].includes(modifier.modifierName.name),
-      ) || ['initialize', 'initializer', 'reinitialize', 'reinitializer'].includes(fnDef.name),
+      ) || ['initialize', 'initializer', 'reinitialize', 'reinitializer'].includes(fnDef.name))
+        &&
+      !(fnDef.virtual && !fnDef.body) // Skip virtual functions without a body, since that indicates an abstract function and is not itself an initializer
   );
 }
 

From 133fdd2162d05fbb47985e380280955a8e0e0eaa Mon Sep 17 00:00:00 2001
From: Eric Lau <ericglau@outlook.com>
Date: Fri, 29 Nov 2024 15:34:44 -0500
Subject: [PATCH 03/24] Update snapshots

---
 packages/core/src/cli/cli.test.ts.md   |   4 ++--
 packages/core/src/cli/cli.test.ts.snap | Bin 3326 -> 3357 bytes
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/core/src/cli/cli.test.ts.md b/packages/core/src/cli/cli.test.ts.md
index bcc365e62..0aa0e90ef 100644
--- a/packages/core/src/cli/cli.test.ts.md
+++ b/packages/core/src/cli/cli.test.ts.md
@@ -21,7 +21,7 @@ Generated by [AVA](https://avajs.dev).
       --requireReference  Can only be used when the --contract option is also provided. Not compatible with --unsafeSkipStorageCheck. If specified, requires either the --reference option to be provided or the contract to have a @custom:oz-upgrades-from annotation.␊
       --referenceBuildInfoDirs "<BUILD_INFO_DIR>[,<BUILD_INFO_DIR>...]"  Optional paths of additional build info directories from previous versions of the project to use for storage layout comparisons. When using this option, refer to one of these directories using prefix '<dirName>:' before the contract name or fully qualified name in the --reference option or @custom:oz-upgrades-from annotation, where <dirName> is the directory short name. Each directory short name must be unique, including compared to the main build info directory. If passing in multiple directories, separate them with commas or call the option multiple times, once for each directory.␊
       --exclude "<GLOB_PATTERN>" [--exclude "<GLOB_PATTERN>"...]  Exclude validations for contracts in source file paths that match any of the given glob patterns. For example, --exclude "contracts/mocks/**/*.sol". Does not apply to reference contracts. If passing in multiple patterns, call the option multiple times, once for each pattern.␊
-      --unsafeAllow "<VALIDATION_ERROR>[,<VALIDATION_ERROR>...]"  Selectively disable one or more validation errors. Comma-separated list with one or more of the following: state-variable-assignment, state-variable-immutable, external-library-linking, struct-definition, enum-definition, constructor, delegatecall, selfdestruct, missing-public-upgradeto, internal-function-storage␊
+      --unsafeAllow "<VALIDATION_ERROR>[,<VALIDATION_ERROR>...]"  Selectively disable one or more validation errors. Comma-separated list with one or more of the following: state-variable-assignment, state-variable-immutable, external-library-linking, struct-definition, enum-definition, constructor, delegatecall, selfdestruct, missing-public-upgradeto, internal-function-storage, missing-initializer, missing-initializer-call, duplicate-initializer-call, incorrect-initializer-order␊
       --unsafeAllowRenames  Configure storage layout check to allow variable renaming.␊
       --unsafeSkipStorageCheck  Skips checking for storage layout compatibility errors. This is a dangerous option meant to be used as a last resort.␊
     `
@@ -43,7 +43,7 @@ Generated by [AVA](https://avajs.dev).
       --requireReference  Can only be used when the --contract option is also provided. Not compatible with --unsafeSkipStorageCheck. If specified, requires either the --reference option to be provided or the contract to have a @custom:oz-upgrades-from annotation.␊
       --referenceBuildInfoDirs "<BUILD_INFO_DIR>[,<BUILD_INFO_DIR>...]"  Optional paths of additional build info directories from previous versions of the project to use for storage layout comparisons. When using this option, refer to one of these directories using prefix '<dirName>:' before the contract name or fully qualified name in the --reference option or @custom:oz-upgrades-from annotation, where <dirName> is the directory short name. Each directory short name must be unique, including compared to the main build info directory. If passing in multiple directories, separate them with commas or call the option multiple times, once for each directory.␊
       --exclude "<GLOB_PATTERN>" [--exclude "<GLOB_PATTERN>"...]  Exclude validations for contracts in source file paths that match any of the given glob patterns. For example, --exclude "contracts/mocks/**/*.sol". Does not apply to reference contracts. If passing in multiple patterns, call the option multiple times, once for each pattern.␊
-      --unsafeAllow "<VALIDATION_ERROR>[,<VALIDATION_ERROR>...]"  Selectively disable one or more validation errors. Comma-separated list with one or more of the following: state-variable-assignment, state-variable-immutable, external-library-linking, struct-definition, enum-definition, constructor, delegatecall, selfdestruct, missing-public-upgradeto, internal-function-storage␊
+      --unsafeAllow "<VALIDATION_ERROR>[,<VALIDATION_ERROR>...]"  Selectively disable one or more validation errors. Comma-separated list with one or more of the following: state-variable-assignment, state-variable-immutable, external-library-linking, struct-definition, enum-definition, constructor, delegatecall, selfdestruct, missing-public-upgradeto, internal-function-storage, missing-initializer, missing-initializer-call, duplicate-initializer-call, incorrect-initializer-order␊
       --unsafeAllowRenames  Configure storage layout check to allow variable renaming.␊
       --unsafeSkipStorageCheck  Skips checking for storage layout compatibility errors. This is a dangerous option meant to be used as a last resort.␊
     `
diff --git a/packages/core/src/cli/cli.test.ts.snap b/packages/core/src/cli/cli.test.ts.snap
index c8b362076b24594ec30e31d57154ba5ee33919e7..f83864c0b2161c82a21c08471f1289f361aa9e4f 100644
GIT binary patch
literal 3357
zcmV+&4dU`aRzV<BZeeh9Xm4~Nb~6eC02#_RWAJrbkJ#FR&?$`l)I&-Q*@()tl8gD=
zm%n}NjUS5$00000000B+Tup2wM-`sgC<&S<A|YIegm^Ln?=JJRGwaRn4#rylEpb@v
z#bcYSRx7)vyLzVBo~queYOlv95Er;24mpHFxF94DS0oS@E{G$S+;ia0g$oz-s(+??
zrpMD`Z)_(SpX~0gs`tM4y`TE?M^4)n)>HMt+;6F7+NE}vx*k)Uc&aP3`q@XeO*DD*
z+TUI(yjsAY;>(3Mul(t?T}3*y3EVq^d%~mqn0g*{8L#<XM-rQ=iX|k4L*g=<XbO)P
zS9UiZtnF`X-rw3^+t^-u0*@EB9`0;xZMK%4luBz<Q%frdo0GOn!4h0cf=?YOdO)D<
zGuH;j9RW6z)Y3xsfp9w>M#1wg)m<t<cWEDpq)~^Uh(oKT(z5LMJ<7FemH>uj0@(62
z6P&o<5#0qXu%GN<pLdznH6jwm6};El+60HWC<4=6XbG3uO!vVOJ&(Cm0^^>qjkLno
zp0BI0Mjhh2TH!eJL~7=svNbbY1&5mlM@V>=NV`ikcvAetU?e2W3tx|XcwhM3mi_p{
zDr`8=7ru<1fUl?m0+z3|=!J3&bSmS=(DRiB<xz_{ePCK;tKX#(;SBIHU{)%Ty<l;5
zYjbCNd39$AV5duglO9E>9p82P@XRMJa~QS5=Z*n6ZmdOYPNM)9kvNRnWgxCQh9r!i
zU@hjl&>ry7t{RCYb*QA=qOiEVet&&?eRFkvKOy`o;UKu%$ELobHqt8?C84b_7ywg1
zTqO|eLuOOEnp^dO6iE<#MU7RJ7Ls%*xTG(9ZTLhaQ-Z53c#MUy(|duDo8q_<yJX--
zoC{3?ksV+f1-GfgIJF}P2Vx-an1VKCyfX^kGoMMi9ZP>kxHd(oon~lQ<3^Q=&(U*R
zPnp*WfLQHP>#0#ap>`;X0?H!yPnkCyR0|X>8WQa?Oua5Sq(JhZH^iV7q}>L0#2S+d
z=7vMd<MQy6YPI@gZYbg?qy({T7Cy`1GNWNQ^dvoG!dGxer9vl+=oC47P7eJTZR9I7
zu<kMyLRAh}iJb@@F;&r&7(pXXQimPE{G!d|Ch5_o<~+2igV8?)?unkj_&AdZna3Dq
zq?)7<3xWK?9tR;nb%hM1sKPq2x|y$n=!f*>?3qu?z_{i5HtHFGM(rpo^a$gb!NzdY
zBg&w|IP`p1GtW(7R|Z8rA~7@SF6{+mw?wZ;OnA32%Hbd(2yqvh^{}x(fY7isrAR<Y
zdW0fVm|Oh*gRPbQhs!%V>)V@4bMSclJ0v5(diXYUbVDi}B^6_p@FkAUTpDuLM2Q~J
zRu>5GM>OoPL&~A!iZ*_xsYJ5fN12aE&vR)xDQY}It%v1E?fUiF^{Nu?Tou*?RVK<2
z&vW~T{%E{ZV=<40E>DSR*e*ach(XJ)E1tvL;@<LujkRU0T=v(ux3`RMk30*#yG31U
zX?956KG;l|jAi^uLN8Gtp*&PdAypMtkp-2A9yYj4>3|=}h9Ss~K%p4#G(l;iY2}be
zhNDyviw?(9r#$?M^?JU>-^xIbP)p)gT-KIE_A4&qPZ{rE8|hoRVlo2@T#RzRmwI3c
z9(*Qb8Eop(4$%~GMtZxBO@p^(=rLoNis!doW<_4C1^QPgfa7xud#!}AYS73GM_hJH
zW#+?5Frw{yF0)Y4(RYknLZXwT-U(?_Ic(r<ig7>ztP1Y1jxXs*^uz>&JS2t?5v(Aw
zCF7kWXNL0@06tMcAIQ~l(OI)LbFqGmXt9H_8Iva1gm<XK$R9D860XDCX3{Rf&s?H3
zNUDU?)lyj9cenr|J8D+o&kFomfj=woX9fPOz@HWPvjTrs;Li&DS%LpKDDa=f^>`)h
zc(l}i{ZXpafA95{wuP^o0H1vLYZ$m(O;e?7mdk2;QMsms+iVesmP+})eV3?>fjy>Z
zwHpv<N0>GrOxI?0z}j77q*4jY&!kRn-fSW(f^>FpVwVTdHAYByNH_@oteJ8P2!{k6
zfpMiNu@m2Qwf0oARy+3GYGP_tGp4KDxKYom{tv@gH=Vw>ZZ=_8nOKu_Ze*c}$4GxV
zjl7NU+q~o7x!Ca+7UJ=f;q3I|-^^>T4c?`aX*ILkXUlG3Kv4Ik=<V)4Y{Av2ub^h1
zCQMz+Ywp{PCTw6ug~^^oldCXM0)Zgu!hs+4e3jU?q)J^oz;a^ne3{#;#C4<B<#u0F
z{3uGz3B*Ez0xG$in2l=@R?Kmk(-v5{MAL_;D^p3DrHM$pMCa)RrC9w+V%lHxq&Vu=
ze9c@{8xjIRnZmet^5&hSad#x)3a0~HpTfq8q<1YhwSp3b+hIBJz!VGXCW%igLfFM#
zJvYbh-u@I^D&NQpO?VI$URe3qre-?W-|rCb0IvFs>&C5j9xvQ|0+l65{CO=Frvfck
zB-$fJb>wOz{$vSSRD<ahh?K-kG!#3gu#lS|E7TG_s;~l@)*|UZpPLlnh%3YnA)0d7
zgv2ow63?2jMqP@Nkb|Rx1XMqu)HBnMxb~$u*JBR&91GCA;<q$&-9Y7OCEpZ0;qtk8
zT(95#RB}TOc0@USxvk813Gs8ATj=g`yyl@!hurkeUthR!QQlv_dHWK$|NZ5S2kUEa
zwZ1TB>De~B7q}4lj#-7k-|m1zm`m+zrBX}VRLUlVv#_JL6VotE(uuxC=40=FoSKgP
zqmY=6<)BR3Px4#*#Tz^TaTJWocreOl7L{7_8=bEf|Cs{lFU9Eqn&s6CcdSuPeD5Sq
zB$kK|sgw-OGl?T_r#SN3MRFvWYcGNx<1oLU4|Ar#_dQ%M%?tjOms8;XcS`WjTH~B*
zPF1hhXOpV4O{&K6>|42bmf^#<QZ)MZIn!t~w#QFST_y(~p3q$ojHjx+$HBgo&%Mn?
zgTkG#^w!qQ9xvD)i@EGk&fG4|Shb&7<4o3AO;}^(Ttfx8NSY*Bl;UC#T97!SA?y8L
zByCbWKL1sO*6!-+daD&}{~WEKQO#geYgEyHr^xs5lyj6zyNWT6Eej{Zh``Cj-(ygF
zRNP9zc5h0bja;I!ze4Q&O)*#*IFBWP%QG(@wNGO@`($(aS1IHCypVJIGd9ZB(-&rw
zVK@F-LO(8&YZ9Ys)Kf9&o10?75=OjN+)E51o{o0>(^SWDEIb1Uj9!C3W4+*NJyWQ}
zvPwN(R=E)`t3+sgV|*qZBl>$O(HGAkdMRthI-Ih@16t;6SFg=FKJMM8&P9!i_@k7F
ztC=G14UXxwNRQ&VCyaODG&6WGD(Sl^=Lo$(y%3>4NV&`}vt1^S)DxT~ql+Nrr8>!Q
zoMyJ}!kigQ7rfB!2M4maoz0?0#jnKbeyR9UqfjW^DHIBOg+k%2;wx_!U)gHoF}z|i
z{`HMF-Y67bFBGp7ieE0gRlM@m;!B0%%WoE6Q6_$ce-<*{3SWnRUiolxAc~J7nkW=h
z2L1Tp!Vt8BL&TNCdNWtA^`R~^fZjbF&}R0LS@F(nFtrW7ds?!$iJgJ;UsKNg$Na@g
z5{eaKp9>a2^)Pspr~&(@pM*@Q<U36WS5V@~ia9pM&3Ux=ptJ6KG*J_+5Ia5*hGmbW
zrrt5&EZo5k7VaiiVp{mpGACiNTS<wc<13@RPr)x|bot!UWigvB@zL2dV-!+ma3(v0
z6#q$);*(R7LK1Fur{qQ>A2%|Bb)GrV3F6&pLU5$ihJ#~c(1YXPlY<;P#~B@;Q#OhA
zJR1|YJO{OTef*Z^n81^dNoJ1Lqj)!M=-Bp*V>6Cr?i%JuZYh3c7MEYYh~n~h(-oJ4
zN$Ew^nM-GYJ-c@JSIUL|emWOU0P}??BQo^8efcViYsni3!R;Rt?8Eyu!Ge~#OEk)h
zSO1XW#cy7n$ct!%+ODF5gIBu`Tfw&e(8Cjg@W~zBP2LL_{^Go@0!-petip)+xYs?9
z|Fb3g-6rfsPK3D+FUdsLn;@9F1UG*NBHAE=+lDN)zn|QQcaV}c#T!)RC}4$qqw`s}
z5rY-}N^l`)+-<A*aD*)kjL(^(!38R2QyD>w_jux%Af&m!Ngz@ueltD9x2A`<5rvRZ
zYBVk_F9M-&B}vhrk>bUXqCTD!^-D{N`Y0)mW~3NPiotQ$NxL#P@>J#DnV9jFz>C+}
zIlpM7a{kWbTJVdM^Ix#3u1_=-UgOHPRATez{3c7&>}RIVcudeP{MQEIg?#j87F=cj
z461!~RPB%plh#r$zi#)kOqw{gxI&2~xQXugpKO8)TPg_I=6~;G1e`yo0?zNJ2snF<
z6=Da;CipeSp8ue6*l=(L;|xcq#@{DN3)>h7+qgJk^DhGpqF7^m6w_xVMcKTIo8p<<
nIHDS8hW?ET?n7q~sgxqp|0W-by3DpYTY~vNYSq{3NLT;>yk3mZ

delta 3318
zcmV<S3<>j{8vYr7K~_N^Q*L2!b7*gLAa*kf0{{R;u9-sUkhENrytqw<e5@OKj-w1u
zy&f?MkD{%9R~8?O2mk;800003?OaQ28^;+Y9V96j1WlS=8ldP86|HQS50jQ-n^gtt
z<wU6~xuT@TFbrwPoh7H<omtP!%A!s!dTFmk4>_cV^imXmXzvBuTQ5bAz2u%l@4fWW
zOYzS>xJxd@rDRJ=L!QWe%>4iNfB$1%{M2qcqVrUJJoh`QnRckvqmIWEC!XpFt$zNg
zWf4sty|MR7;q?N3imw*lzViMXyNYya6S#K-_k>6JG4(v^FkbV$t|S&!m5z`U4vE7o
zqA5IHT-n`!c(As=v3Y-Me{ExX=?Oeu+<LgPv9;M+dQvK_QB6BqLHIgpI}|#CYf12`
zErkmN+CFnEVB8j9F-bdG$UYEm#oZ`)-lMulCFmaQ1Ccap5EQX#wNzS`UEif#t7Zvc
zSSEliPcy-Z10K;m&;r}Z?)G_)b$Uib!nlGDTU(odU^53rV7dn_;V_HoK6HfZF^5WE
z-1D`OR`}ZUbrsgAO?*cy>}Q@x&1_V*X1c3jck^Hi2_Fz?^@s*fik}*cgoJtF>yZcV
z3!huEA3s=y4IBExm(dmQ6;(i><0~!PP>z94WqcT1UujSt?J&C!Osj15dsHHv0bT~o
zN+q&?7c8!BZSHI@ukI`X?DQyb;!>2__8q4W&wS!An^7ygZX1x}m$it^X%yfh5{FT%
z48(E9kc9CQti>D$+5<jXRU@&aHkFijC@gNT-(TNe-&|eaPYAzCI0)|a@l{_@3+WZ~
zlF(Kd41g&hjuMFVA+xAe&8_-CiX;fWqQ<I!N()K46dckQzBYU!k}1Je7Cgqn*yz2$
z$W3uviCr>qBhG~;fyg#6je=X$W}I3Pgaa{<cT7Q>GTt2p@0rge-HxR{BV3yz)J`)r
ztZ}1C#pmd`t*6Xu1wgF!Xy>U>J)w3eivr3b_fMHO>{JUBE$R~OGEBW5Iix`Hpf|)I
zpcSOu2Dil;lM3dBL(Ajx@Re${`ebe>;wYp9u`Cu|lN$pWDg<rhD>Sg~F%?2p4p@ne
z2p%z2(Ucg$OP-`QJA(N|i^)ym(xv7+w5g4gUIQ0@76&0f^@I$hsKPqw^fGS+(GTg%
z*)yM(f$@&xTc~FM8nvRV;1b3&gN@;)N0dQ_ad3S{GtWt3R|Z8rA~7@S9(4n<cZBN_
z6W%)*<*<_wgt!UKTzpv|KxkN+QY4@xJwlNw%q_nEU~6Un;quPT`u67196TO>56K9y
z9^MUq9o>)$dr8GuC47m!GlzzpHBrJPy3+%~`w<Ph?2vNkI--q_X)2Ly_fh5};(87(
zCq<2WsJU2<)UIEzU9T$P%vE7cP-UVV@jR!G=#R!rH5T)z>++PChVKQ41~F*aal~_&
zTijcIu(7s`mCOG6_V$+X?U8GtcekiRJDMGTQl}3VQzl~>f0E!P>LZkgN-3nO!YZ<$
z64Ao~hbbNKBl%(ovMo?3#=A{WnrK=%B$8n-6~v;;vD7IK-(s%oYy4IQdW2dMr{b`-
zB(h&|7=Oxm7vGV7M^{W{V1bKK?z^cA9l?Xgge-$a9oi+DBF;!}$F^v2w+t>bmZ^Au
ze%oQ4$cwc={|W`Lecr)VD`Bh}27a5Oe=2}g!EM&{B^`-sm>7^B#26}47bL#PcsFUw
z;d}ysS5(jja%f!C)U3@Mtm`6k?O@ErWCj-DT`DmmM+~Ec>oDh-bV%?qhbRq_Dj{{X
z6jt3`E`Z3cnibx&!h2SD&kFBZ;XNyVyk~{?tni){-m}7cR(QX}!uwfVCs)FTM@#TG
zAEiq058rHQOZd78@cAddfq^U3G*!CRaae6HDx;Kenk{0}QYqiF9}u;HHYN@`rgDn6
zb_2ui2=mUz)4j7gVAHNKQmF*yGpUiAH=D?cAk7?{*yI89j1dyd4LiZlnkj>SfN)>~
z^?`AvDX|jI^|ba>vsOFyoN8joQ!}Qk+_+KCtNxF|7&M)}w{A9JSD6@)bUI|AiIYfw
zI*q)I@Y%fm-?`ZS7Z&3FlVRrc{ol-MuMOU#l4&)w+ZW4jVL(v#rEqt5AGY9X)K*YS
zPZOrD<u&*1MiVx$(!nH5qCro8m<oYFkPP9#k6OM;EK5?Qt{q^SA6#GN)+%wF=yti?
z*AyR$Qf30Nkf4A{?j}abT7;EsTz<3#RuoZLA2nqvJTqkwX_sg`J<$`ZUr7wdYn~KG
z{hF_tqiRD!AShE9_fFotlQiy*BwS&c&h;s5oJe}ta#JfPQMeVB6Aw&(v9NBE__QK~
zP3+ZkbL{Tzm*7(QdR}P4gQ)Pr%EvM_)4~3Jmv{$o)n{BcZoT(-;qDWtEJ5PuwOE`A
zv>cImA2F&UR~zvoOVFYkOrt=gBxV4h*fE8L+yq&n9pO@i70|R6NeBAeqzHRlAyx>{
zl-(vIj;WBi)`T_cP#he892^}ap!y-Do*5RzwJ*gn7_-6WSb*jgzonVu1S(G}`KI6r
zm(R`Pdj0N8$qhN!5#{vdwldEp#LsPRp?k~mT!cCua??A1ec{GMd4K)p?MvYP_m?*w
ztgpe<`ofqgVau#4-~{1&W_AF--2t00hg#Q4rIxm+luZanVMp(OCWc{{q!Vq8jK@Cu
zEHxbadm%9#%R!m6pX9gri#K=x;wTuE@nDqAC@M7<H#%M|{v!p@Uy9QKG|Q_O?pULo
z_`yk>NKD-xQYjglXA(!=O>yLni{wZ$*IooY#$kRxALdMf?|V3pnHT&kucpBN&y?Vw
zwZ=Ksn5tf{&jwY0XB$+F<Jos|@hroKZ>4DT-E*eVXl#$KoVrX79z3DBAQ(?od5?pA
zDW7|r1ptLBROuzBnLS>xJr;A>qnx>nnK5fWv&NaMv6`^P$bN$gaFH}gvM9yLAhaN{
zM?=;}zf9Vsczph|2(8`K)%8{@TK+j&Kckw#qSmOQ|4Na6@3Se#D3^8?V;ox+PKFVI
zlZoGBP<vF|O2Kw-N}r8fqOre1to=<fm>D>aDS^u~E+4f|V><g}bNbgQ<NTtKbNMqi
z%GT2tW|Ltz{zgJSE|PN+qiWPsG3eWyV!{+gyjJWcIuTDt8~$0UVL29_0R%>`!OvJP
zxLVH?Dlx5pQje!qZp70n5gOkbpGn7v{$Wb=#WRRr${Mi_r|fX!l-YCTwprK5wfoe5
zrBM-ooDy+0Q^dW&_L&xODegzYcoz;cgA1dQevopG&<oTH5&Gkl%ls<aW%5Wp!C5kz
z2vT0EkqpOaX6r7@UcPj}3+;ZeQHopHEP7P@TCDDWSBkGR3WdU*LZPr%C=}i)zV>$U
zwXHU8k1H1AuW!BeR-yQ2p?IZG{A%Hy;+3x#Unvw{eY^OYGVv??S;)96d>#J0^2y{t
z6mJnUQ7EVk`tinvA!rAidn<?aX0BH2LrrD?y?Z*K&FCYu;+@%GY8!n2v}A7+D+B4j
zrkwkKPx*_LBor&eIu|U0>S1sxQ3Li*KM0vp$@iKN&Y;AD6|-#&xBVnGAnU$I6E)Ea
zvEm(JSoTP2>Ky~l!X0d2;cj9kriC{<W``;^D=ATQd}Xxv68v&Tm;YP3EN0Us-qM<8
zj6%u`_DW}v;@>G!e11w&NWwe4DY?<e$Bm4CV4Y`7bb@$ynh<O$wcy~`81&#cc;q0*
z%5g@+=afyNHP6PxCC@=^ULU{YIVSMrW0D!8xfIWg4Q;2Macsu1%vHl2$t}gN%;NH!
z7g1dPe!AjvFetsKI&<j^uxHl}|4g~?-%jVk31GeyWkiO)w=Z8saV>cQAvl#|f_->@
z4knn;GG}~7dGY!mQ@r@?>l1kq^-$YYbg=Pi_hBno)*rffLJ&T=p}Wa*0mDz6*HM5;
zoQYK!5g*sO2l9WhWWU>ly~v3$_u(O#=-3eiLzm!0?m$EfL~sg_rPlY83-LBm(xP~>
zsT>8YaBXxx>y%(H!(Ryw1dTfdH6QkWu!Vu~Ia4$^*u*R<BZ%=FPaG43H0LS_MC!zM
zrib{}^bj|q5Hd=Q#--&&AoQ&yDf%-~yf{+S$CILdX-QEZCB@N<6k|y-*zP)MRpv&X
zs{DHsGrkgdAUZqe7dxq(zcaZO{4(YI7i_BQ6HSH3xUwykSo}G^!O}GAnW;U09uu?+
z|8gKaD39LEf+OZ%K(()qsvUA+(p<{r*X>@GNfWykS19QSZlXK>!%T2sO9dg@{3lLE
z!1;43;QV2VfV0<FAy$xVg0DH&{0A?GFAnx#oMG$K`12%bVH*Qs8y6>R{%N2=6l;u+
zV*0G4D4TontGMSjj;IEjp}#jB!FA{iB9&4^`tRgTQJ2{^XG<{u1KLLodU#g=0Ffh0
AO8@`>


From 991d1da6332b5d040cc89bd5840bee958aa569e7 Mon Sep 17 00:00:00 2001
From: Eric Lau <ericglau@outlook.com>
Date: Mon, 2 Dec 2024 17:57:23 -0500
Subject: [PATCH 04/24] Refactor errors, add linearization WIP

---
 packages/core/src/validate/run.ts | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/packages/core/src/validate/run.ts b/packages/core/src/validate/run.ts
index 662b2e85e..649d3500b 100644
--- a/packages/core/src/validate/run.ts
+++ b/packages/core/src/validate/run.ts
@@ -60,7 +60,8 @@ export type ValidationError =
   | ValidationErrorConstructor
   | ValidationErrorOpcode
   | ValidationErrorWithName
-  | ValidationErrorUpgradeability;
+  | ValidationErrorUpgradeability
+  | ValidationErrorInitializers;
 
 interface ValidationErrorBase {
   src: string;
@@ -75,13 +76,22 @@ interface ValidationErrorWithName extends ValidationErrorBase {
     | 'external-library-linking'
     | 'struct-definition'
     | 'enum-definition'
-    | 'internal-function-storage'
-    | 'missing-initializer'
-    | 'missing-initializer-call'
-    | 'duplicate-initializer-call'
-    | 'incorrect-initializer-order';
+    | 'internal-function-storage';
 }
 
+interface ValidationErrorInitializerGeneric extends ValidationErrorBase {
+  name: string;
+  kind: 'missing-initializer' | 'missing-initializer-call' | 'duplicate-initializer-call';
+}
+
+interface ValidationErrorInitializerOrder extends ValidationErrorBase {
+  name: string;
+  kind: 'incorrect-initializer-order';
+  expectedLinearization: string[];
+}
+
+type ValidationErrorInitializers = ValidationErrorInitializerGeneric | ValidationErrorInitializerOrder;
+
 interface ValidationErrorConstructor extends ValidationErrorBase {
   kind: 'constructor';
   contract: string;
@@ -649,7 +659,7 @@ function* getInitializerErrors(
   contractDef: ContractDefinition,
   deref: ASTDereferencer,
   decodeSrc: SrcDecoder,
-): Generator<ValidationErrorWithName> {
+): Generator<ValidationErrorInitializers> {
   if (contractDef.baseContracts.length > 0) {
     const baseContractDefs = contractDef.baseContracts.map(base =>
       deref('ContractDefinition', base.baseName.referencedDeclaration),
@@ -718,6 +728,7 @@ function* getInitializerErrors(
                     kind: 'incorrect-initializer-order',
                     name: contractDef.name,
                     src: decodeSrc(fnCall),
+                    expectedLinearization: baseContractsWithInitializers,
                   };
                 }
                 if (index !== -1) {

From c43ba46e8301cbcaf8ca07a57e4ff746e634b152 Mon Sep 17 00:00:00 2001
From: Eric Lau <ericglau@outlook.com>
Date: Fri, 6 Dec 2024 18:00:18 -0500
Subject: [PATCH 05/24] Improve error messages

---
 packages/core/src/validate/report.ts | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/packages/core/src/validate/report.ts b/packages/core/src/validate/report.ts
index f40f604c7..d139a5da8 100644
--- a/packages/core/src/validate/report.ts
+++ b/packages/core/src/validate/report.ts
@@ -74,29 +74,29 @@ const errorInfo: ErrorDescriptions<ValidationError> = {
   },
   'missing-initializer': {
     msg: () => `Contract is missing an initializer`,
-    hint: () => `Define an initializer function`, // TODO include instruction to call parent initializers, or use a separate error message
-    link: 'https://zpl.in/upgrades/error-010', // TODO define link
+    hint: () => `Define an initializer function and use it to call the initializers of parent contracts`,
+    link: 'https://zpl.in/upgrades/error-001',
   },
   'missing-initializer-call': {
     msg: () => `Contract is missing a call to a parent initializer`,
     hint: () => `Call the parent initializer in your initializer function`,
-    link: 'https://zpl.in/upgrades/error-011', // TODO define link
+    link: 'https://zpl.in/upgrades/error-001',
   },
   'duplicate-initializer-call': {
     msg: () => `Contract has multiple calls to a parent initializer`,
     hint: () => `Only call each parent initializer once`,
-    link: 'https://zpl.in/upgrades/error-012', // TODO define link
+    link: 'https://zpl.in/upgrades/error-001',
   },
   'incorrect-initializer-order': {
-    msg: () => `Contract has an incorrect order of parent initializer calls`,
+    msg: e => `Contract has an incorrect order of parent initializer calls. Expected initializers to be called for parent contracts in the following order: ${e.expectedLinearization.join(', ')}`,
     hint: () => `Call parent initializers in the order they are inherited`,
-    link: 'https://zpl.in/upgrades/error-013', // TODO define link
+    link: 'https://zpl.in/upgrades/error-001',
   },
 };
 
 function describeError(e: ValidationError, color = true): string {
   const chalk = new _chalk.Instance({ level: color && _chalk.supportsColor ? _chalk.supportsColor.level : 0 });
-  const info = errorInfo[e.kind];
+  const info: any = errorInfo[e.kind]; // union type is too complex for TypeScript to represent, so we use `any`
   const log = [chalk.bold(e.src) + ': ' + info.msg(e as any)];
   const hint = info.hint?.(e as any);
   if (hint) {

From 88eaa6db7027a742d8053c055436f6de6e18d738 Mon Sep 17 00:00:00 2001
From: Eric Lau <ericglau@outlook.com>
Date: Fri, 6 Dec 2024 18:02:30 -0500
Subject: [PATCH 06/24] Use onlyInitializing in test

---
 packages/core/contracts/test/cli/excludes/AbstractUUPS.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/core/contracts/test/cli/excludes/AbstractUUPS.sol b/packages/core/contracts/test/cli/excludes/AbstractUUPS.sol
index 8e4a3e554..4c8ce4555 100644
--- a/packages/core/contracts/test/cli/excludes/AbstractUUPS.sol
+++ b/packages/core/contracts/test/cli/excludes/AbstractUUPS.sol
@@ -12,7 +12,7 @@ abstract contract AbstractUUPS is UUPSUpgradeable, Abstract1, Abstract2 {
         z = _z;
     }
 
-    function initialize() initializer public {
+    function initialize() onlyInitializing public {
         __UUPSUpgradeable_init();
     }
 }
\ No newline at end of file

From ff52d4cd9a01c1e713e73bcb3db5cde5b83c9807 Mon Sep 17 00:00:00 2001
From: Eric Lau <ericglau@outlook.com>
Date: Fri, 6 Dec 2024 18:06:01 -0500
Subject: [PATCH 07/24] Improve types

---
 packages/core/src/validate/run.ts | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/packages/core/src/validate/run.ts b/packages/core/src/validate/run.ts
index 649d3500b..531d6f4ee 100644
--- a/packages/core/src/validate/run.ts
+++ b/packages/core/src/validate/run.ts
@@ -61,7 +61,7 @@ export type ValidationError =
   | ValidationErrorOpcode
   | ValidationErrorWithName
   | ValidationErrorUpgradeability
-  | ValidationErrorInitializers;
+  | ValidationErrorInitializer;
 
 interface ValidationErrorBase {
   src: string;
@@ -79,18 +79,16 @@ interface ValidationErrorWithName extends ValidationErrorBase {
     | 'internal-function-storage';
 }
 
-interface ValidationErrorInitializerGeneric extends ValidationErrorBase {
-  name: string;
+interface ValidationErrorInitializerCall extends ValidationErrorBase {
   kind: 'missing-initializer' | 'missing-initializer-call' | 'duplicate-initializer-call';
 }
 
 interface ValidationErrorInitializerOrder extends ValidationErrorBase {
-  name: string;
   kind: 'incorrect-initializer-order';
   expectedLinearization: string[];
 }
 
-type ValidationErrorInitializers = ValidationErrorInitializerGeneric | ValidationErrorInitializerOrder;
+type ValidationErrorInitializer = ValidationErrorInitializerCall | ValidationErrorInitializerOrder;
 
 interface ValidationErrorConstructor extends ValidationErrorBase {
   kind: 'constructor';
@@ -659,7 +657,7 @@ function* getInitializerErrors(
   contractDef: ContractDefinition,
   deref: ASTDereferencer,
   decodeSrc: SrcDecoder,
-): Generator<ValidationErrorInitializers> {
+): Generator<ValidationErrorInitializer> {
   if (contractDef.baseContracts.length > 0) {
     const baseContractDefs = contractDef.baseContracts.map(base =>
       deref('ContractDefinition', base.baseName.referencedDeclaration),
@@ -677,7 +675,6 @@ function* getInitializerErrors(
       if (contractInitializers.length === 0 && !skipCheck('missing-initializer', contractDef)) {
         yield {
           kind: 'missing-initializer',
-          name: contractDef.name,
           src: decodeSrc(contractDef),
         };
       }
@@ -711,7 +708,6 @@ function* getInitializerErrors(
                 ) {
                   yield {
                     kind: 'duplicate-initializer-call',
-                    name: contractDef.name,
                     src: decodeSrc(fnCall),
                   };
                 }
@@ -726,7 +722,6 @@ function* getInitializerErrors(
                 ) {
                   yield {
                     kind: 'incorrect-initializer-order',
-                    name: contractDef.name,
                     src: decodeSrc(fnCall),
                     expectedLinearization: baseContractsWithInitializers,
                   };
@@ -747,7 +742,6 @@ function* getInitializerErrors(
         ) {
           yield {
             kind: 'missing-initializer-call',
-            name: contractDef.name,
             src: decodeSrc(contractInitializer),
           };
         }

From b65c2b8c7ba97cfdf509d2a9e219d5ef621813d2 Mon Sep 17 00:00:00 2001
From: Eric Lau <ericglau@outlook.com>
Date: Tue, 10 Dec 2024 14:02:36 -0500
Subject: [PATCH 08/24] Check error messages in tests

---
 .../core/src/validate-initializers.test.ts    | 56 ++++++++++++++-----
 packages/core/src/validate/report.ts          |  3 +-
 packages/core/src/validate/run.ts             |  6 +-
 3 files changed, 48 insertions(+), 17 deletions(-)

diff --git a/packages/core/src/validate-initializers.test.ts b/packages/core/src/validate-initializers.test.ts
index df85bda12..e27b3cdd9 100644
--- a/packages/core/src/validate-initializers.test.ts
+++ b/packages/core/src/validate-initializers.test.ts
@@ -33,8 +33,8 @@ test.before(async t => {
   }
 });
 
-function testValid(name: string, kind: ValidationOptions['kind'], valid: boolean, numExpectedErrors?: number) {
-  testOverride(name, kind, {}, valid, numExpectedErrors);
+function testValid(name: string, kind: ValidationOptions['kind'], valid: boolean, expectedErrorContains?: string) {
+  testOverride(name, kind, {}, valid, expectedErrorContains);
 }
 
 function testOverride(
@@ -42,9 +42,9 @@ function testOverride(
   kind: ValidationOptions['kind'],
   opts: ValidationOptions,
   valid: boolean,
-  numExpectedErrors?: number,
+  expectedErrorContains?: string,
 ) {
-  if (numExpectedErrors !== undefined && numExpectedErrors > 0 && valid) {
+  if (expectedErrorContains !== undefined && valid) {
     throw new Error('Cannot expect errors for a valid contract');
   }
 
@@ -58,8 +58,8 @@ function testOverride(
       t.notThrows(assertUpgSafe);
     } else {
       const error = t.throws(assertUpgSafe) as ValidationErrors;
-      if (numExpectedErrors !== undefined) {
-        t.is(error.errors.length, numExpectedErrors);
+      if (expectedErrorContains !== undefined) {
+        t.true(error.message.includes(expectedErrorContains), error.message);
       }
     }
   });
@@ -68,23 +68,43 @@ function testOverride(
 testValid('Child_Of_NoInitializer_Ok', 'transparent', true);
 
 testValid('Child_Of_InitializerModifier_Ok', 'transparent', true);
-testValid('Child_Of_InitializerModifier_Bad', 'transparent', false, 1);
+testValid(
+  'Child_Of_InitializerModifier_Bad',
+  'transparent',
+  false,
+  'Contract is missing a call to a parent initializer',
+);
 testValid('Child_Of_InitializerModifier_UsesSuper_Ok', 'transparent', true);
 
 testValid('Child_Of_ReinitializerModifier_Ok', 'transparent', true);
-testValid('Child_Of_ReinitializerModifier_Bad', 'transparent', false, 1);
+testValid(
+  'Child_Of_ReinitializerModifier_Bad',
+  'transparent',
+  false,
+  'Contract is missing a call to a parent initializer',
+);
 
 testValid('Child_Of_OnlyInitializingModifier_Ok', 'transparent', true);
-testValid('Child_Of_OnlyInitializingModifier_Bad', 'transparent', false, 1);
+testValid(
+  'Child_Of_OnlyInitializingModifier_Bad',
+  'transparent',
+  false,
+  'Contract is missing a call to a parent initializer',
+);
 
-testValid('MissingInitializer_Bad', 'transparent', false, 1);
+testValid('MissingInitializer_Bad', 'transparent', false, 'Contract is missing an initializer');
 testValid('MissingInitializer_UnsafeAllow_Contract', 'transparent', true);
 testOverride('MissingInitializer_Bad', 'transparent', { unsafeAllow: ['missing-initializer'] }, true);
 
 testValid('InitializationOrder_Ok', 'transparent', true);
 testValid('InitializationOrder_Ok_2', 'transparent', true);
 
-testValid('InitializationOrder_WrongOrder_Bad', 'transparent', false, 1);
+testValid(
+  'InitializationOrder_WrongOrder_Bad',
+  'transparent',
+  false,
+  'Contract has an incorrect order of parent initializer calls. Expected initializers to be called for parent contracts in the following order: A, B, C',
+);
 testValid('InitializationOrder_WrongOrder_UnsafeAllow_Contract', 'transparent', true);
 testValid('InitializationOrder_WrongOrder_UnsafeAllow_Function', 'transparent', true);
 testOverride(
@@ -94,12 +114,22 @@ testOverride(
   true,
 );
 
-testValid('InitializationOrder_MissingCall_Bad', 'transparent', false, 1);
+testValid(
+  'InitializationOrder_MissingCall_Bad',
+  'transparent',
+  false,
+  'Contract is missing a call to a parent initializer',
+);
 testValid('InitializationOrder_MissingCall_UnsafeAllow_Contract', 'transparent', true);
 testValid('InitializationOrder_MissingCall_UnsafeAllow_Function', 'transparent', true);
 testOverride('InitializationOrder_MissingCall_Bad', 'transparent', { unsafeAllow: ['missing-initializer-call'] }, true);
 
-testValid('InitializationOrder_Duplicate_Bad', 'transparent', false, 1);
+testValid(
+  'InitializationOrder_Duplicate_Bad',
+  'transparent',
+  false,
+  'Contract has multiple calls to a parent initializer',
+);
 testValid('InitializationOrder_Duplicate_UnsafeAllow_Contract', 'transparent', true);
 testValid('InitializationOrder_Duplicate_UnsafeAllow_Function', 'transparent', true);
 testValid('InitializationOrder_Duplicate_UnsafeAllow_Call', 'transparent', true);
diff --git a/packages/core/src/validate/report.ts b/packages/core/src/validate/report.ts
index d139a5da8..a606ec2d8 100644
--- a/packages/core/src/validate/report.ts
+++ b/packages/core/src/validate/report.ts
@@ -88,7 +88,8 @@ const errorInfo: ErrorDescriptions<ValidationError> = {
     link: 'https://zpl.in/upgrades/error-001',
   },
   'incorrect-initializer-order': {
-    msg: e => `Contract has an incorrect order of parent initializer calls. Expected initializers to be called for parent contracts in the following order: ${e.expectedLinearization.join(', ')}`,
+    msg: e =>
+      `Contract has an incorrect order of parent initializer calls. Expected initializers to be called for parent contracts in the following order: ${e.expectedLinearization.join(', ')}`,
     hint: () => `Call parent initializers in the order they are inherited`,
     link: 'https://zpl.in/upgrades/error-001',
   },
diff --git a/packages/core/src/validate/run.ts b/packages/core/src/validate/run.ts
index 531d6f4ee..6acf305ba 100644
--- a/packages/core/src/validate/run.ts
+++ b/packages/core/src/validate/run.ts
@@ -761,9 +761,9 @@ function getPossibleInitializers(contractDef: ContractDefinition) {
     fnDef =>
       (fnDef.modifiers.some(modifier =>
         ['initializer', 'reinitializer', 'onlyInitializing'].includes(modifier.modifierName.name),
-      ) || ['initialize', 'initializer', 'reinitialize', 'reinitializer'].includes(fnDef.name))
-        &&
-      !(fnDef.virtual && !fnDef.body) // Skip virtual functions without a body, since that indicates an abstract function and is not itself an initializer
+      ) ||
+        ['initialize', 'initializer', 'reinitialize', 'reinitializer'].includes(fnDef.name)) &&
+      !(fnDef.virtual && !fnDef.body), // Skip virtual functions without a body, since that indicates an abstract function and is not itself an initializer
   );
 }
 

From c08ecfd2ba3663009919a58409ad9eaf13cbb8c6 Mon Sep 17 00:00:00 2001
From: Eric Lau <ericglau@outlook.com>
Date: Tue, 10 Dec 2024 14:27:43 -0500
Subject: [PATCH 09/24] Improve error message details

---
 packages/core/src/validate-initializers.test.ts |  2 +-
 packages/core/src/validate/overrides.ts         |  8 ++++----
 packages/core/src/validate/report.ts            |  2 +-
 packages/core/src/validate/run.ts               | 14 +++++++++++---
 4 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/packages/core/src/validate-initializers.test.ts b/packages/core/src/validate-initializers.test.ts
index e27b3cdd9..e68bbff2e 100644
--- a/packages/core/src/validate-initializers.test.ts
+++ b/packages/core/src/validate-initializers.test.ts
@@ -128,7 +128,7 @@ testValid(
   'InitializationOrder_Duplicate_Bad',
   'transparent',
   false,
-  'Contract has multiple calls to a parent initializer',
+  'Contract has duplicate calls to initializer `__B_init` from parent contract `B`',
 );
 testValid('InitializationOrder_Duplicate_UnsafeAllow_Contract', 'transparent', true);
 testValid('InitializationOrder_Duplicate_UnsafeAllow_Function', 'transparent', true);
diff --git a/packages/core/src/validate/overrides.ts b/packages/core/src/validate/overrides.ts
index 964a48a03..a72f63d44 100644
--- a/packages/core/src/validate/overrides.ts
+++ b/packages/core/src/validate/overrides.ts
@@ -83,19 +83,19 @@ export const ValidationErrorUnsafeMessages: Record<ValidationError['kind'], stri
   ],
   'missing-initializer': [
     `You are using the \`unsafeAllow.missing-initializer\` flag.`,
-    `Make sure you have manually checked that the contract has an initializer and that it correct calls any parent initializers.`,
+    `Make sure you have manually checked that the contract has an initializer and that it correctly calls any parent initializers.`,
   ],
   'missing-initializer-call': [
     `You are using the \`unsafeAllow.missing-initializer-call\` flag.`,
-    `Make sure you have manually checked that the contract has an initializer and that it correct calls any parent initializers.`,
+    `Make sure you have manually checked that the contract initializer calls all parent initializers.`,
   ],
   'duplicate-initializer-call': [
     `You are using the \`unsafeAllow.duplicate-initializer-call\` flag.`,
-    `Make sure you have manually checked that the contract has an initializer and that it correct calls any parent initializers.`,
+    `Make sure you have manually checked that the contract initializer calls each parent initializer only once.`,
   ],
   'incorrect-initializer-order': [
     `You are using the \`unsafeAllow.incorrect-initializer-order\` flag.`,
-    `Make sure you have manually checked that the contract has an initializer and that it correct calls any parent initializers.`,
+    `Make sure you have manually checked that the contract initializer calls parent initializers in linearized inheritance order.`,
   ],
 };
 
diff --git a/packages/core/src/validate/report.ts b/packages/core/src/validate/report.ts
index a606ec2d8..1615060fd 100644
--- a/packages/core/src/validate/report.ts
+++ b/packages/core/src/validate/report.ts
@@ -83,7 +83,7 @@ const errorInfo: ErrorDescriptions<ValidationError> = {
     link: 'https://zpl.in/upgrades/error-001',
   },
   'duplicate-initializer-call': {
-    msg: () => `Contract has multiple calls to a parent initializer`,
+    msg: e => `Contract has duplicate calls to initializer \`${e.parentInitializer}\` from parent contract \`${e.parentContract}\``,
     hint: () => `Only call each parent initializer once`,
     link: 'https://zpl.in/upgrades/error-001',
   },
diff --git a/packages/core/src/validate/run.ts b/packages/core/src/validate/run.ts
index 6acf305ba..de8ea714c 100644
--- a/packages/core/src/validate/run.ts
+++ b/packages/core/src/validate/run.ts
@@ -79,8 +79,14 @@ interface ValidationErrorWithName extends ValidationErrorBase {
     | 'internal-function-storage';
 }
 
-interface ValidationErrorInitializerCall extends ValidationErrorBase {
-  kind: 'missing-initializer' | 'missing-initializer-call' | 'duplicate-initializer-call';
+interface ValidationErrorInitializerMissing extends ValidationErrorBase {
+  kind: 'missing-initializer' | 'missing-initializer-call';
+}
+
+interface ValidationErrorInitializerDuplicate extends ValidationErrorBase {
+  kind: 'duplicate-initializer-call';
+  parentInitializer: string;
+  parentContract: string;
 }
 
 interface ValidationErrorInitializerOrder extends ValidationErrorBase {
@@ -88,7 +94,7 @@ interface ValidationErrorInitializerOrder extends ValidationErrorBase {
   expectedLinearization: string[];
 }
 
-type ValidationErrorInitializer = ValidationErrorInitializerCall | ValidationErrorInitializerOrder;
+type ValidationErrorInitializer = ValidationErrorInitializerMissing | ValidationErrorInitializerDuplicate | ValidationErrorInitializerOrder;
 
 interface ValidationErrorConstructor extends ValidationErrorBase {
   kind: 'constructor';
@@ -709,6 +715,8 @@ function* getInitializerErrors(
                   yield {
                     kind: 'duplicate-initializer-call',
                     src: decodeSrc(fnCall),
+                    parentInitializer: foundParentInitializer.name,
+                    parentContract: baseName,
                   };
                 }
                 calledInitializerIds.push(referencedFn);

From e38c1a15c60799442fb82c12352ba1d2acccf4bd Mon Sep 17 00:00:00 2001
From: Eric Lau <ericglau@outlook.com>
Date: Tue, 10 Dec 2024 15:26:27 -0500
Subject: [PATCH 10/24] Add more details to errors

---
 .../core/src/validate-initializers.test.ts    | 10 +++++-----
 packages/core/src/validate/overrides.ts       |  2 +-
 packages/core/src/validate/report.ts          |  8 +++++---
 packages/core/src/validate/run.ts             | 20 ++++++++++++++-----
 4 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/packages/core/src/validate-initializers.test.ts b/packages/core/src/validate-initializers.test.ts
index e68bbff2e..a5f565962 100644
--- a/packages/core/src/validate-initializers.test.ts
+++ b/packages/core/src/validate-initializers.test.ts
@@ -72,7 +72,7 @@ testValid(
   'Child_Of_InitializerModifier_Bad',
   'transparent',
   false,
-  'Contract is missing a call to a parent initializer',
+  'Contract is missing initializer calls for one or more parent contracts: `Parent_InitializerModifier`',
 );
 testValid('Child_Of_InitializerModifier_UsesSuper_Ok', 'transparent', true);
 
@@ -81,7 +81,7 @@ testValid(
   'Child_Of_ReinitializerModifier_Bad',
   'transparent',
   false,
-  'Contract is missing a call to a parent initializer',
+  'Contract is missing initializer calls for one or more parent contracts: `Parent_ReinitializerModifier`',
 );
 
 testValid('Child_Of_OnlyInitializingModifier_Ok', 'transparent', true);
@@ -89,7 +89,7 @@ testValid(
   'Child_Of_OnlyInitializingModifier_Bad',
   'transparent',
   false,
-  'Contract is missing a call to a parent initializer',
+  'Contract is missing initializer calls for one or more parent contracts: `Parent__OnlyInitializingModifier`',
 );
 
 testValid('MissingInitializer_Bad', 'transparent', false, 'Contract is missing an initializer');
@@ -118,7 +118,7 @@ testValid(
   'InitializationOrder_MissingCall_Bad',
   'transparent',
   false,
-  'Contract is missing a call to a parent initializer',
+  'Contract is missing initializer calls for one or more parent contracts: `C`',
 );
 testValid('InitializationOrder_MissingCall_UnsafeAllow_Contract', 'transparent', true);
 testValid('InitializationOrder_MissingCall_UnsafeAllow_Function', 'transparent', true);
@@ -128,7 +128,7 @@ testValid(
   'InitializationOrder_Duplicate_Bad',
   'transparent',
   false,
-  'Contract has duplicate calls to initializer `__B_init` from parent contract `B`',
+  'Contract has duplicate calls to parent initializer `__B_init` for contract `B`',
 );
 testValid('InitializationOrder_Duplicate_UnsafeAllow_Contract', 'transparent', true);
 testValid('InitializationOrder_Duplicate_UnsafeAllow_Function', 'transparent', true);
diff --git a/packages/core/src/validate/overrides.ts b/packages/core/src/validate/overrides.ts
index a72f63d44..3f2cf6c05 100644
--- a/packages/core/src/validate/overrides.ts
+++ b/packages/core/src/validate/overrides.ts
@@ -83,7 +83,7 @@ export const ValidationErrorUnsafeMessages: Record<ValidationError['kind'], stri
   ],
   'missing-initializer': [
     `You are using the \`unsafeAllow.missing-initializer\` flag.`,
-    `Make sure you have manually checked that the contract has an initializer and that it correctly calls any parent initializers.`,
+    `Make sure you have manually checked that the contract has an initializer and that it correctly calls all parent initializers.`,
   ],
   'missing-initializer-call': [
     `You are using the \`unsafeAllow.missing-initializer-call\` flag.`,
diff --git a/packages/core/src/validate/report.ts b/packages/core/src/validate/report.ts
index 1615060fd..ed598c1a7 100644
--- a/packages/core/src/validate/report.ts
+++ b/packages/core/src/validate/report.ts
@@ -78,12 +78,14 @@ const errorInfo: ErrorDescriptions<ValidationError> = {
     link: 'https://zpl.in/upgrades/error-001',
   },
   'missing-initializer-call': {
-    msg: () => `Contract is missing a call to a parent initializer`,
-    hint: () => `Call the parent initializer in your initializer function`,
+    msg: e =>
+      `Contract is missing initializer calls for one or more parent contracts: \`${e.parentContracts.join(', ')}\``,
+    hint: () => `Call the parent initializers in your initializer function`,
     link: 'https://zpl.in/upgrades/error-001',
   },
   'duplicate-initializer-call': {
-    msg: e => `Contract has duplicate calls to initializer \`${e.parentInitializer}\` from parent contract \`${e.parentContract}\``,
+    msg: e =>
+      `Contract has duplicate calls to parent initializer \`${e.parentInitializer}\` for contract \`${e.parentContract}\``,
     hint: () => `Only call each parent initializer once`,
     link: 'https://zpl.in/upgrades/error-001',
   },
diff --git a/packages/core/src/validate/run.ts b/packages/core/src/validate/run.ts
index de8ea714c..49a13fd74 100644
--- a/packages/core/src/validate/run.ts
+++ b/packages/core/src/validate/run.ts
@@ -79,22 +79,31 @@ interface ValidationErrorWithName extends ValidationErrorBase {
     | 'internal-function-storage';
 }
 
-interface ValidationErrorInitializerMissing extends ValidationErrorBase {
-  kind: 'missing-initializer' | 'missing-initializer-call';
+interface ValidationErrorMissingInitializer extends ValidationErrorBase {
+  kind: 'missing-initializer';
 }
 
-interface ValidationErrorInitializerDuplicate extends ValidationErrorBase {
+interface ValidationErrorMissingInitializerCall extends ValidationErrorBase {
+  kind: 'missing-initializer-call';
+  parentContracts: string[];
+}
+
+interface ValidationErrorDuplicateInitializerCall extends ValidationErrorBase {
   kind: 'duplicate-initializer-call';
   parentInitializer: string;
   parentContract: string;
 }
 
-interface ValidationErrorInitializerOrder extends ValidationErrorBase {
+interface ValidationErrorIncorrectInitializerOrder extends ValidationErrorBase {
   kind: 'incorrect-initializer-order';
   expectedLinearization: string[];
 }
 
-type ValidationErrorInitializer = ValidationErrorInitializerMissing | ValidationErrorInitializerDuplicate | ValidationErrorInitializerOrder;
+type ValidationErrorInitializer =
+  | ValidationErrorMissingInitializer
+  | ValidationErrorMissingInitializerCall
+  | ValidationErrorDuplicateInitializerCall
+  | ValidationErrorIncorrectInitializerOrder;
 
 interface ValidationErrorConstructor extends ValidationErrorBase {
   kind: 'constructor';
@@ -751,6 +760,7 @@ function* getInitializerErrors(
           yield {
             kind: 'missing-initializer-call',
             src: decodeSrc(contractInitializer),
+            parentContracts: uninitializedBaseContracts,
           };
         }
       }

From 2c1bfdd598397ab408771e70e836a91a442d5d7e Mon Sep 17 00:00:00 2001
From: Eric Lau <ericglau@outlook.com>
Date: Tue, 10 Dec 2024 15:46:59 -0500
Subject: [PATCH 11/24] Update doc options for hardhat

---
 docs/modules/ROOT/pages/api-hardhat-upgrades.adoc | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/docs/modules/ROOT/pages/api-hardhat-upgrades.adoc b/docs/modules/ROOT/pages/api-hardhat-upgrades.adoc
index 11831b4e0..b483d520d 100644
--- a/docs/modules/ROOT/pages/api-hardhat-upgrades.adoc
+++ b/docs/modules/ROOT/pages/api-hardhat-upgrades.adoc
@@ -17,6 +17,10 @@ The following options are common to some functions.
 ** `"delegatecall"`, `"selfdestruct"`: Allow the use of these operations. Incorrect use of this option can put funds at risk of permanent loss. See xref:faq.adoc#delegatecall-selfdestruct[Can I safely use `delegatecall` and `selfdestruct`?]
 ** `"missing-public-upgradeto"`: Allow UUPS implementations that do not contain a public `upgradeTo` or `upgradeToAndCall` function. Enabling this option is likely to cause a revert due to the built-in UUPS safety mechanism.
 ** `"internal-function-storage"`: Allow internal functions in storage variables. Internal functions are code pointers which will no longer be valid after an upgrade, so they must be reassigned during upgrades. See xref:faq.adoc#internal-function-storage[How can I use internal functions in storage variables?]
+** `"missing-initializer"`: Allows implementations where an initializer function is not detected.
+** `"missing-initializer-call"`: Allows implementations where a parent initializer is not called from the child initializer.
+** `"duplicate-initializer-call"`: Allows implementations where a parent initializer is called more than once from the child initializer.
+** `"incorrect-initializer-order"`: Allows implementations where parent initializers are not called in the order that they are inherited.
 * `unsafeAllowRenames`: (`boolean`) Configure storage layout check to allow variable renaming.
 * `unsafeSkipStorageCheck`: (`boolean`) upgrades the proxy or beacon without first checking for storage layout compatibility errors. This is a dangerous option meant to be used as a last resort.
 * `constructorArgs`: (`unknown[]`) Provide arguments for the constructor of the implementation contract. Note that these are different from initializer arguments, and will be used in the deployment of the implementation contract itself. Can be used to initialize immutable variables.

From 523e54207dfb5e1a7303f903131c805139333eca Mon Sep 17 00:00:00 2001
From: Eric Lau <ericglau@outlook.com>
Date: Tue, 10 Dec 2024 15:49:04 -0500
Subject: [PATCH 12/24] Update cli docs

---
 docs/modules/ROOT/pages/api-core.adoc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/modules/ROOT/pages/api-core.adoc b/docs/modules/ROOT/pages/api-core.adoc
index df1e6dc02..b28f035cc 100644
--- a/docs/modules/ROOT/pages/api-core.adoc
+++ b/docs/modules/ROOT/pages/api-core.adoc
@@ -121,7 +121,7 @@ If any errors are found, the command will exit with a non-zero exit code and pri
 * `--requireReference` - Can only be used when the `--contract` option is also provided. Not compatible with `--unsafeSkipStorageCheck`. If specified, requires either the `--reference` option to be provided or the contract to have a `@custom:oz-upgrades-from` annotation.
 * `--referenceBuildInfoDirs "<BUILD_INFO_DIR>[,<BUILD_INFO_DIR>...]"` - Optional paths of additional build info directories from previous versions of the project to use for storage layout comparisons. When using this option, refer to one of these directories using prefix `<dirName>:` before the contract name or fully qualified name in the `--reference` option or `@custom:oz-upgrades-from` annotation, where `<dirName>` is the directory short name. Each directory short name must be unique, including compared to the main build info directory. If passing in multiple directories, separate them with commas or call the option multiple times, once for each directory.
 * `--exclude "<GLOB_PATTERN>" [--exclude "<GLOB_PATTERN>"...]` - Exclude validations for contracts in source file paths that match any of the given glob patterns. For example, `--exclude "contracts/mocks/\**/*.sol"`. Does not apply to reference contracts. If passing in multiple patterns, call the option multiple times, once for each pattern.
-* `--unsafeAllow "<VALIDATION_ERROR>[,<VALIDATION_ERROR>...]"` - Selectively disable one or more validation errors. Comma-separated list with one or more of the following: `state-variable-assignment, state-variable-immutable, external-library-linking, struct-definition, enum-definition, constructor, delegatecall, selfdestruct, missing-public-upgradeto, internal-function-storage`
+* `--unsafeAllow "<VALIDATION_ERROR>[,<VALIDATION_ERROR>...]"` - Selectively disable one or more validation errors. Comma-separated list with one or more of the following: `state-variable-assignment, state-variable-immutable, external-library-linking, struct-definition, enum-definition, constructor, delegatecall, selfdestruct, missing-public-upgradeto, internal-function-storage, missing-initializer, missing-initializer-call, duplicate-initializer-call, incorrect-initializer-order`
 * `--unsafeAllowRenames` - Configure storage layout check to allow variable renaming.
 * `--unsafeSkipStorageCheck` - Skips checking for storage layout compatibility errors. This is a dangerous option meant to be used as a last resort.
 

From 92e4c2cc0f79e5ea04ba9185d1224328b8e3c250 Mon Sep 17 00:00:00 2001
From: Eric Lau <ericglau@outlook.com>
Date: Tue, 10 Dec 2024 15:56:38 -0500
Subject: [PATCH 13/24] Update changelog

---
 packages/core/CHANGELOG.md | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/packages/core/CHANGELOG.md b/packages/core/CHANGELOG.md
index e69df217c..5d9d60461 100644
--- a/packages/core/CHANGELOG.md
+++ b/packages/core/CHANGELOG.md
@@ -1,5 +1,9 @@
 # Changelog
 
+## Unreleased
+
+- Detect issues in parent initializer calls. ([#1095](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/1095))
+
 ## 1.40.0 (2024-10-10)
 
 - Fix Hardhat compile error when overriding interface functions with public constant variables. ([#1091](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/1091))

From e17b5c4bc3347d33e2c2693a2dadb3f71566d862 Mon Sep 17 00:00:00 2001
From: Eric Lau <ericglau@outlook.com>
Date: Tue, 10 Dec 2024 16:35:30 -0500
Subject: [PATCH 14/24] Ignore public initializers from parent contracts

---
 packages/core/contracts/test/ValidationsInitializer.sol    | 4 ++--
 packages/core/contracts/test/cli/excludes/AbstractUUPS.sol | 2 +-
 .../core/contracts/test/cli/excludes/UsesAbstractUUPS.sol  | 4 ----
 packages/core/src/validate/run.ts                          | 7 ++++---
 4 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/packages/core/contracts/test/ValidationsInitializer.sol b/packages/core/contracts/test/ValidationsInitializer.sol
index 59afbeef6..97a18b714 100644
--- a/packages/core/contracts/test/ValidationsInitializer.sol
+++ b/packages/core/contracts/test/ValidationsInitializer.sol
@@ -12,11 +12,11 @@ contract Parent_NoInitializer {
 }
 
 contract Parent_InitializerModifier is Initializable {
-  function parentInit() initializer public {}
+  function parentInit() initializer internal {}
 }
 
 contract Parent_ReinitializerModifier is Initializable {
-  function parentReinit() reinitializer(2) public {}
+  function parentReinit() reinitializer(2) internal {}
 }
 
 contract Parent__OnlyInitializingModifier is Initializable {
diff --git a/packages/core/contracts/test/cli/excludes/AbstractUUPS.sol b/packages/core/contracts/test/cli/excludes/AbstractUUPS.sol
index 4c8ce4555..8e4a3e554 100644
--- a/packages/core/contracts/test/cli/excludes/AbstractUUPS.sol
+++ b/packages/core/contracts/test/cli/excludes/AbstractUUPS.sol
@@ -12,7 +12,7 @@ abstract contract AbstractUUPS is UUPSUpgradeable, Abstract1, Abstract2 {
         z = _z;
     }
 
-    function initialize() onlyInitializing public {
+    function initialize() initializer public {
         __UUPSUpgradeable_init();
     }
 }
\ No newline at end of file
diff --git a/packages/core/contracts/test/cli/excludes/UsesAbstractUUPS.sol b/packages/core/contracts/test/cli/excludes/UsesAbstractUUPS.sol
index 426a330d6..4a6f7f76b 100644
--- a/packages/core/contracts/test/cli/excludes/UsesAbstractUUPS.sol
+++ b/packages/core/contracts/test/cli/excludes/UsesAbstractUUPS.sol
@@ -12,8 +12,4 @@ contract UsesAbstractUUPS is AbstractUUPS {
     function _authorizeUpgrade(address newImplementation) internal pure override {
         revert("Upgrade disabled");
     }
-
-    function initializeChild() initializer public {
-        super.initialize();
-    }
 }
\ No newline at end of file
diff --git a/packages/core/src/validate/run.ts b/packages/core/src/validate/run.ts
index 49a13fd74..d4b66275f 100644
--- a/packages/core/src/validate/run.ts
+++ b/packages/core/src/validate/run.ts
@@ -678,7 +678,7 @@ function* getInitializerErrors(
       deref('ContractDefinition', base.baseName.referencedDeclaration),
     );
     const baseContractsInitializersMap = new Map(
-      baseContractDefs.map(base => [base.name, getPossibleInitializers(base)]),
+      baseContractDefs.map(base => [base.name, getPossibleInitializers(base, { ignorePublicInitializers: true })]),
     );
     const baseContractsWithInitializers = baseContractDefs
       .filter(base => hasInitializers(base.name, baseContractsInitializersMap))
@@ -773,7 +773,7 @@ function hasInitializers(baseName: string, baseContractsInitializersMap: Map<str
   return initializers !== undefined && initializers.length > 0;
 }
 
-function getPossibleInitializers(contractDef: ContractDefinition) {
+function getPossibleInitializers(contractDef: ContractDefinition, { ignorePublicInitializers = false } = {}) {
   const fns = [...findAll('FunctionDefinition', contractDef)];
   return fns.filter(
     fnDef =>
@@ -781,7 +781,8 @@ function getPossibleInitializers(contractDef: ContractDefinition) {
         ['initializer', 'reinitializer', 'onlyInitializing'].includes(modifier.modifierName.name),
       ) ||
         ['initialize', 'initializer', 'reinitialize', 'reinitializer'].includes(fnDef.name)) &&
-      !(fnDef.virtual && !fnDef.body), // Skip virtual functions without a body, since that indicates an abstract function and is not itself an initializer
+      !(fnDef.virtual && !fnDef.body) && // Skip virtual functions without a body, since that indicates an abstract function and is not itself an initializer
+      (!ignorePublicInitializers || (fnDef.visibility !== 'public' && fnDef.visibility !== 'external')), // For parent contracts, ignore public initializers since they do not strictly need to be called from the child
   );
 }
 

From d8bcedd9cbbe0c2337d181985f1ad07c98130d97 Mon Sep 17 00:00:00 2001
From: Eric Lau <ericglau@outlook.com>
Date: Tue, 10 Dec 2024 16:50:33 -0500
Subject: [PATCH 15/24] Only consider internal parent initializers, and non
 private child initializers

---
 packages/core/src/validate/run.ts | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/packages/core/src/validate/run.ts b/packages/core/src/validate/run.ts
index d4b66275f..9189cf317 100644
--- a/packages/core/src/validate/run.ts
+++ b/packages/core/src/validate/run.ts
@@ -678,7 +678,7 @@ function* getInitializerErrors(
       deref('ContractDefinition', base.baseName.referencedDeclaration),
     );
     const baseContractsInitializersMap = new Map(
-      baseContractDefs.map(base => [base.name, getPossibleInitializers(base, { ignorePublicInitializers: true })]),
+      baseContractDefs.map(base => [base.name, getPossibleInitializers(base, true)]),
     );
     const baseContractsWithInitializers = baseContractDefs
       .filter(base => hasInitializers(base.name, baseContractsInitializersMap))
@@ -686,7 +686,7 @@ function* getInitializerErrors(
 
     if (baseContractsWithInitializers.length > 0) {
       // Check for missing initializers
-      const contractInitializers = getPossibleInitializers(contractDef);
+      const contractInitializers = getPossibleInitializers(contractDef, false);
       if (contractInitializers.length === 0 && !skipCheck('missing-initializer', contractDef)) {
         yield {
           kind: 'missing-initializer',
@@ -773,7 +773,7 @@ function hasInitializers(baseName: string, baseContractsInitializersMap: Map<str
   return initializers !== undefined && initializers.length > 0;
 }
 
-function getPossibleInitializers(contractDef: ContractDefinition, { ignorePublicInitializers = false } = {}) {
+function getPossibleInitializers(contractDef: ContractDefinition, isParentContract: boolean) {
   const fns = [...findAll('FunctionDefinition', contractDef)];
   return fns.filter(
     fnDef =>
@@ -781,8 +781,11 @@ function getPossibleInitializers(contractDef: ContractDefinition, { ignorePublic
         ['initializer', 'reinitializer', 'onlyInitializing'].includes(modifier.modifierName.name),
       ) ||
         ['initialize', 'initializer', 'reinitialize', 'reinitializer'].includes(fnDef.name)) &&
-      !(fnDef.virtual && !fnDef.body) && // Skip virtual functions without a body, since that indicates an abstract function and is not itself an initializer
-      (!ignorePublicInitializers || (fnDef.visibility !== 'public' && fnDef.visibility !== 'external')), // For parent contracts, ignore public initializers since they do not strictly need to be called from the child
+      // Skip virtual functions without a body, since that indicates an abstract function and is not itself an initializer
+      !(fnDef.virtual && !fnDef.body) &&
+      // For parent contracts, only treat internal functions as initializers (since they MUST be called by the child)
+      // For child contracts, treat all non-private functions as initializers (since they can be called by another contract or externally)
+      (isParentContract ? fnDef.visibility === 'internal' : fnDef.visibility !== 'private'),
   );
 }
 

From 210c71b54f037d691d93134a7858c84d92f8ac10 Mon Sep 17 00:00:00 2001
From: Eric Lau <ericglau@outlook.com>
Date: Tue, 10 Dec 2024 16:59:57 -0500
Subject: [PATCH 16/24] Test initializer visibility

---
 .../contracts/test/ValidationsInitializer.sol | 28 ++++++++++++++++---
 .../core/src/validate-initializers.test.ts    |  4 +++
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/packages/core/contracts/test/ValidationsInitializer.sol b/packages/core/contracts/test/ValidationsInitializer.sol
index 97a18b714..b4a7c41f0 100644
--- a/packages/core/contracts/test/ValidationsInitializer.sol
+++ b/packages/core/contracts/test/ValidationsInitializer.sol
@@ -24,19 +24,19 @@ contract Parent__OnlyInitializingModifier is Initializable {
 }
 
 contract Parent_InitializeName {
-  function initialize() public virtual {}
+  function initialize() internal virtual {}
 }
 
 contract Parent_InitializerName {
-  function initializer() public {}
+  function initializer() internal {}
 }
 
 contract Parent_ReinitializeName {
-  function reinitialize(uint64 version) public {}
+  function reinitialize(uint64 version) internal {}
 }
 
 contract Parent_ReinitializerName {
-  function reinitializer(uint64 version) public {}
+  function reinitializer(uint64 version) internal {}
 }
 
 // ==== Child contracts ====
@@ -220,4 +220,24 @@ contract InitializationOrder_Duplicate_UnsafeAllow_Call is A, B, C, Parent_NoIni
     __B_init();
     __C_init();
   }
+}
+
+// ==== Initializer visibility ====
+
+contract Parent_PrivateInitializer {
+  function initialize() private {} // not considered an initializer because it's private
+}
+
+contract Parent_PublicInitializer {
+  function initialize() public {} // does not strictly need to be called by child
+}
+
+contract Child_Of_ParentPrivateInitializer_Ok is Parent_PrivateInitializer { // no initializer required since parent initializer is private
+}
+
+contract Child_Of_ParentPublicInitializer_Ok is Parent_PublicInitializer { // no initializer required since parent initializer is public
+}
+
+contract Child_Has_PrivateInitializer_Bad is Parent__OnlyInitializingModifier { // parent has internal initializer, but child has private
+  function initialize() private {}
 }
\ No newline at end of file
diff --git a/packages/core/src/validate-initializers.test.ts b/packages/core/src/validate-initializers.test.ts
index a5f565962..4399e10ee 100644
--- a/packages/core/src/validate-initializers.test.ts
+++ b/packages/core/src/validate-initializers.test.ts
@@ -134,3 +134,7 @@ testValid('InitializationOrder_Duplicate_UnsafeAllow_Contract', 'transparent', t
 testValid('InitializationOrder_Duplicate_UnsafeAllow_Function', 'transparent', true);
 testValid('InitializationOrder_Duplicate_UnsafeAllow_Call', 'transparent', true);
 testOverride('InitializationOrder_Duplicate_Bad', 'transparent', { unsafeAllow: ['duplicate-initializer-call'] }, true);
+
+testValid('Child_Of_ParentPrivateInitializer_Ok', 'transparent', true);
+testValid('Child_Of_ParentPublicInitializer_Ok', 'transparent', true);
+testValid('Child_Has_PrivateInitializer_Bad', 'transparent', false, 'Contract is missing an initializer');

From 4a4ea90f737b63f976ebbb7b37a4a22454e15f49 Mon Sep 17 00:00:00 2001
From: Eric Lau <ericglau@outlook.com>
Date: Tue, 17 Dec 2024 17:24:39 -0500
Subject: [PATCH 17/24] Address review comments - simplify tests

---
 .../core/src/validate-initializers.test.ts    | 94 ++++++++-----------
 1 file changed, 41 insertions(+), 53 deletions(-)

diff --git a/packages/core/src/validate-initializers.test.ts b/packages/core/src/validate-initializers.test.ts
index 4399e10ee..d46e1c49c 100644
--- a/packages/core/src/validate-initializers.test.ts
+++ b/packages/core/src/validate-initializers.test.ts
@@ -33,108 +33,96 @@ test.before(async t => {
   }
 });
 
-function testValid(name: string, kind: ValidationOptions['kind'], valid: boolean, expectedErrorContains?: string) {
-  testOverride(name, kind, {}, valid, expectedErrorContains);
+function testAccepts(name: string, kind: ValidationOptions['kind']) {
+  testOverride(name, kind, {}, undefined);
+}
+
+function testRejects(name: string, kind: ValidationOptions['kind'], expectedErrorContains: string) {
+  testOverride(name, kind, {}, expectedErrorContains);
 }
 
 function testOverride(
   name: string,
   kind: ValidationOptions['kind'],
   opts: ValidationOptions,
-  valid: boolean,
-  expectedErrorContains?: string,
+  expectErrorContains?: string,
 ) {
-  if (expectedErrorContains !== undefined && valid) {
-    throw new Error('Cannot expect errors for a valid contract');
-  }
+  const expectValid = expectErrorContains === undefined;
 
   const optKeys = Object.keys(opts);
   const describeOpts = optKeys.length > 0 ? '(' + optKeys.join(', ') + ')' : '';
-  const testName = [valid ? 'accepts' : 'rejects', kind, name, describeOpts].join(' ');
+  const testName = [expectValid ? 'accepts' : 'rejects', kind, name, describeOpts].join(' ');
   test(testName, t => {
     const version = getContractVersion(t.context.validation, name);
     const assertUpgSafe = () => assertUpgradeSafe([t.context.validation], version, { kind, ...opts });
-    if (valid) {
+    if (expectValid) {
       t.notThrows(assertUpgSafe);
     } else {
       const error = t.throws(assertUpgSafe) as ValidationErrors;
-      if (expectedErrorContains !== undefined) {
-        t.true(error.message.includes(expectedErrorContains), error.message);
-      }
+      t.true(error.message.includes(expectErrorContains), error.message);
     }
   });
 }
 
-testValid('Child_Of_NoInitializer_Ok', 'transparent', true);
+testAccepts('Child_Of_NoInitializer_Ok', 'transparent');
 
-testValid('Child_Of_InitializerModifier_Ok', 'transparent', true);
-testValid(
+testAccepts('Child_Of_InitializerModifier_Ok', 'transparent');
+testRejects(
   'Child_Of_InitializerModifier_Bad',
   'transparent',
-  false,
   'Contract is missing initializer calls for one or more parent contracts: `Parent_InitializerModifier`',
 );
-testValid('Child_Of_InitializerModifier_UsesSuper_Ok', 'transparent', true);
+testAccepts('Child_Of_InitializerModifier_UsesSuper_Ok', 'transparent');
 
-testValid('Child_Of_ReinitializerModifier_Ok', 'transparent', true);
-testValid(
+testAccepts('Child_Of_ReinitializerModifier_Ok', 'transparent');
+testRejects(
   'Child_Of_ReinitializerModifier_Bad',
   'transparent',
-  false,
   'Contract is missing initializer calls for one or more parent contracts: `Parent_ReinitializerModifier`',
 );
 
-testValid('Child_Of_OnlyInitializingModifier_Ok', 'transparent', true);
-testValid(
+testAccepts('Child_Of_OnlyInitializingModifier_Ok', 'transparent');
+testRejects(
   'Child_Of_OnlyInitializingModifier_Bad',
   'transparent',
-  false,
   'Contract is missing initializer calls for one or more parent contracts: `Parent__OnlyInitializingModifier`',
 );
 
-testValid('MissingInitializer_Bad', 'transparent', false, 'Contract is missing an initializer');
-testValid('MissingInitializer_UnsafeAllow_Contract', 'transparent', true);
-testOverride('MissingInitializer_Bad', 'transparent', { unsafeAllow: ['missing-initializer'] }, true);
+testRejects('MissingInitializer_Bad', 'transparent', 'Contract is missing an initializer');
+testAccepts('MissingInitializer_UnsafeAllow_Contract', 'transparent');
+testOverride('MissingInitializer_Bad', 'transparent', { unsafeAllow: ['missing-initializer'] });
 
-testValid('InitializationOrder_Ok', 'transparent', true);
-testValid('InitializationOrder_Ok_2', 'transparent', true);
+testAccepts('InitializationOrder_Ok', 'transparent');
+testAccepts('InitializationOrder_Ok_2', 'transparent');
 
-testValid(
+testRejects(
   'InitializationOrder_WrongOrder_Bad',
   'transparent',
-  false,
   'Contract has an incorrect order of parent initializer calls. Expected initializers to be called for parent contracts in the following order: A, B, C',
 );
-testValid('InitializationOrder_WrongOrder_UnsafeAllow_Contract', 'transparent', true);
-testValid('InitializationOrder_WrongOrder_UnsafeAllow_Function', 'transparent', true);
-testOverride(
-  'InitializationOrder_WrongOrder_Bad',
-  'transparent',
-  { unsafeAllow: ['incorrect-initializer-order'] },
-  true,
-);
+testAccepts('InitializationOrder_WrongOrder_UnsafeAllow_Contract', 'transparent');
+testAccepts('InitializationOrder_WrongOrder_UnsafeAllow_Function', 'transparent');
+testOverride('InitializationOrder_WrongOrder_Bad', 'transparent', { unsafeAllow: ['incorrect-initializer-order'] });
 
-testValid(
+testRejects(
   'InitializationOrder_MissingCall_Bad',
   'transparent',
-  false,
   'Contract is missing initializer calls for one or more parent contracts: `C`',
 );
-testValid('InitializationOrder_MissingCall_UnsafeAllow_Contract', 'transparent', true);
-testValid('InitializationOrder_MissingCall_UnsafeAllow_Function', 'transparent', true);
-testOverride('InitializationOrder_MissingCall_Bad', 'transparent', { unsafeAllow: ['missing-initializer-call'] }, true);
+testAccepts('InitializationOrder_MissingCall_UnsafeAllow_Contract', 'transparent');
+testAccepts('InitializationOrder_MissingCall_UnsafeAllow_Function', 'transparent');
+testOverride('InitializationOrder_MissingCall_Bad', 'transparent', { unsafeAllow: ['missing-initializer-call'] });
 
-testValid(
+testRejects(
   'InitializationOrder_Duplicate_Bad',
   'transparent',
-  false,
   'Contract has duplicate calls to parent initializer `__B_init` for contract `B`',
 );
-testValid('InitializationOrder_Duplicate_UnsafeAllow_Contract', 'transparent', true);
-testValid('InitializationOrder_Duplicate_UnsafeAllow_Function', 'transparent', true);
-testValid('InitializationOrder_Duplicate_UnsafeAllow_Call', 'transparent', true);
-testOverride('InitializationOrder_Duplicate_Bad', 'transparent', { unsafeAllow: ['duplicate-initializer-call'] }, true);
-
-testValid('Child_Of_ParentPrivateInitializer_Ok', 'transparent', true);
-testValid('Child_Of_ParentPublicInitializer_Ok', 'transparent', true);
-testValid('Child_Has_PrivateInitializer_Bad', 'transparent', false, 'Contract is missing an initializer');
+testAccepts('InitializationOrder_Duplicate_UnsafeAllow_Contract', 'transparent');
+testAccepts('InitializationOrder_Duplicate_UnsafeAllow_Function', 'transparent');
+testAccepts('InitializationOrder_Duplicate_UnsafeAllow_Call', 'transparent');
+testOverride('InitializationOrder_Duplicate_Bad', 'transparent', { unsafeAllow: ['duplicate-initializer-call'] });
+
+testAccepts('Child_Of_ParentPrivateInitializer_Ok', 'transparent');
+testAccepts('Child_Of_ParentPublicInitializer_Ok', 'transparent');
+testRejects('Child_Has_PrivateInitializer_Bad', 'transparent', 'Contract is missing an initializer');

From d7385c18db604c210d4f3bd65cca16547b0c0aa5 Mon Sep 17 00:00:00 2001
From: Eric Lau <ericglau@outlook.com>
Date: Tue, 17 Dec 2024 17:31:37 -0500
Subject: [PATCH 18/24] Apply review comments - simplify

---
 packages/core/src/validate/run.ts | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/packages/core/src/validate/run.ts b/packages/core/src/validate/run.ts
index 9189cf317..4890dc4ea 100644
--- a/packages/core/src/validate/run.ts
+++ b/packages/core/src/validate/run.ts
@@ -681,7 +681,7 @@ function* getInitializerErrors(
       baseContractDefs.map(base => [base.name, getPossibleInitializers(base, true)]),
     );
     const baseContractsWithInitializers = baseContractDefs
-      .filter(base => hasInitializers(base.name, baseContractsInitializersMap))
+      .filter(base => baseContractsInitializersMap.get(base.name)?.length)
       .map(base => base.name);
 
     if (baseContractsWithInitializers.length > 0) {
@@ -768,11 +768,6 @@ function* getInitializerErrors(
   }
 }
 
-function hasInitializers(baseName: string, baseContractsInitializersMap: Map<string, FunctionDefinition[]>) {
-  const initializers = baseContractsInitializersMap.get(baseName);
-  return initializers !== undefined && initializers.length > 0;
-}
-
 function getPossibleInitializers(contractDef: ContractDefinition, isParentContract: boolean) {
   const fns = [...findAll('FunctionDefinition', contractDef)];
   return fns.filter(

From 9f453018c6566f2ce91bc83144e26a7531685ad5 Mon Sep 17 00:00:00 2001
From: Eric Lau <ericglau@outlook.com>
Date: Wed, 18 Dec 2024 11:19:17 -0500
Subject: [PATCH 19/24] Clarify duplicate and incorrect order handling

---
 .../core/contracts/test/ValidationsInitializer.sol     | 10 ++++++++++
 packages/core/src/validate-initializers.test.ts        |  5 +++++
 packages/core/src/validate/run.ts                      |  6 +++---
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/packages/core/contracts/test/ValidationsInitializer.sol b/packages/core/contracts/test/ValidationsInitializer.sol
index b4a7c41f0..d06fa218c 100644
--- a/packages/core/contracts/test/ValidationsInitializer.sol
+++ b/packages/core/contracts/test/ValidationsInitializer.sol
@@ -222,6 +222,16 @@ contract InitializationOrder_Duplicate_UnsafeAllow_Call is A, B, C, Parent_NoIni
   }
 }
 
+/// @custom:oz-upgrades-unsafe-allow duplicate-initializer-call
+contract InitializationOrder_UnsafeAllowDuplicate_But_WrongOrder is A, B, C, Parent_NoInitializer {
+  function initialize() public {
+    __A_init();
+    __C_init();
+    __B_init();
+    __B_init();
+  }
+}
+
 // ==== Initializer visibility ====
 
 contract Parent_PrivateInitializer {
diff --git a/packages/core/src/validate-initializers.test.ts b/packages/core/src/validate-initializers.test.ts
index d46e1c49c..9360fa454 100644
--- a/packages/core/src/validate-initializers.test.ts
+++ b/packages/core/src/validate-initializers.test.ts
@@ -122,6 +122,11 @@ testAccepts('InitializationOrder_Duplicate_UnsafeAllow_Contract', 'transparent')
 testAccepts('InitializationOrder_Duplicate_UnsafeAllow_Function', 'transparent');
 testAccepts('InitializationOrder_Duplicate_UnsafeAllow_Call', 'transparent');
 testOverride('InitializationOrder_Duplicate_Bad', 'transparent', { unsafeAllow: ['duplicate-initializer-call'] });
+testRejects(
+  'InitializationOrder_UnsafeAllowDuplicate_But_WrongOrder',
+  'transparent',
+  'Contract has an incorrect order of parent initializer calls. Expected initializers to be called for parent contracts in the following order: A, B, C',
+);
 
 testAccepts('Child_Of_ParentPrivateInitializer_Ok', 'transparent');
 testAccepts('Child_Of_ParentPublicInitializer_Ok', 'transparent');
diff --git a/packages/core/src/validate/run.ts b/packages/core/src/validate/run.ts
index 4890dc4ea..2e6a82cf1 100644
--- a/packages/core/src/validate/run.ts
+++ b/packages/core/src/validate/run.ts
@@ -709,8 +709,8 @@ function* getInitializerErrors(
             const referencedFn = fnCall.expression.referencedDeclaration;
 
             // If this is a call to a parent initializer, then:
-            // - Check for duplicates
-            // - Check if the parent initializer is called in the correct order
+            // - Check if it was already called (duplicate call)
+            // - Otherwise, check if the parent initializer is called in the correct order
             for (const [baseName, initializers] of baseContractsInitializersMap) {
               const foundParentInitializer = initializers.find(init => init.id === referencedFn);
               if (referencedFn && foundParentInitializer) {
@@ -732,7 +732,7 @@ function* getInitializerErrors(
 
                 const index = uninitializedBaseContracts.indexOf(baseName);
                 if (
-                  !duplicate &&
+                  !duplicate && // Omit duplicate calls to avoid treating them as out of order. Duplicates are either reported above or they were skipped.
                   index !== 0 &&
                   !skipCheck('incorrect-initializer-order', contractDef) &&
                   !skipCheck('incorrect-initializer-order', contractInitializer)

From 4e8c4359d0513d6c0e26788665afa74efb755977 Mon Sep 17 00:00:00 2001
From: Eric Lau <ericglau@outlook.com>
Date: Thu, 19 Dec 2024 11:14:13 -0500
Subject: [PATCH 20/24] Only require calling non-empty parent initializers

---
 .../contracts/test/ValidationsInitializer.sol | 103 ++++++++++++++----
 packages/core/src/validate/run.ts             |   6 +-
 2 files changed, 88 insertions(+), 21 deletions(-)

diff --git a/packages/core/contracts/test/ValidationsInitializer.sol b/packages/core/contracts/test/ValidationsInitializer.sol
index d06fa218c..76d8f4e5f 100644
--- a/packages/core/contracts/test/ValidationsInitializer.sol
+++ b/packages/core/contracts/test/ValidationsInitializer.sol
@@ -8,103 +8,160 @@ import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
 // ==== Parent contracts ====
 
 contract Parent_NoInitializer {
-  function parentFn() internal {}
+  uint8 x;
+  function parentFn() internal {
+    x = 1;
+  }
 }
 
 contract Parent_InitializerModifier is Initializable {
-  function parentInit() initializer internal {}
+  uint8 x;
+  function parentInit() initializer internal {
+    x = 1;
+  }
 }
 
 contract Parent_ReinitializerModifier is Initializable {
-  function parentReinit() reinitializer(2) internal {}
+  uint8 x;
+  function parentReinit() reinitializer(2) internal {
+    x = 1;
+  }
 }
 
 contract Parent__OnlyInitializingModifier is Initializable {
-  function __Parent_init() onlyInitializing() internal {}
+  uint8 x;
+  function __Parent_init() onlyInitializing() internal {
+    x = 1;
+  }
 }
 
 contract Parent_InitializeName {
-  function initialize() internal virtual {}
+  uint8 x;
+  function initialize() internal virtual {
+    x = 1;
+  }
 }
 
 contract Parent_InitializerName {
-  function initializer() internal {}
+  uint8 x;
+  function initializer() internal {
+    x = 1;
+  }
 }
 
 contract Parent_ReinitializeName {
-  function reinitialize(uint64 version) internal {}
+  uint8 x;
+  function reinitialize(uint64 version) internal {
+    x = 1;
+  }
 }
 
 contract Parent_ReinitializerName {
-  function reinitializer(uint64 version) internal {}
+  uint8 x;
+  function reinitializer(uint64 version) internal {
+    x = 1;
+  }
 }
 
 // ==== Child contracts ====
 
 contract Child_Of_NoInitializer_Ok is Parent_NoInitializer {
-  function childFn() public {}
+  uint y;
+  function childFn() public {
+    y = 2;
+  }
 }
 
 contract Child_Of_InitializerModifier_Ok is Parent_InitializerModifier {
+  uint y;
   function initialize() public {
     parentInit();
+    y = 2;
   }
 }
 
 contract Child_Of_InitializerModifier_UsesSuper_Ok is Parent_InitializerModifier {
+  uint y;
   function initialize() public {
     super.parentInit();
+    y = 2;
   }
 }
 
 contract Child_Of_InitializerModifier_Bad is Parent_InitializerModifier {
-  function initialize() public {}
+  uint y;
+  function initialize() public {
+    y = 2;
+  }
 }
 
 contract Child_Of_ReinitializerModifier_Ok is Parent_ReinitializerModifier {
+  uint y;
   function initialize() public {
     parentReinit();
+    y = 2;
   }
 }
 
 contract Child_Of_ReinitializerModifier_Bad is Parent_ReinitializerModifier {
-  function initialize() public {}
+  uint y;
+  function initialize() public {
+    y = 2;
+  }
 }
 
 contract Child_Of_OnlyInitializingModifier_Ok is Parent__OnlyInitializingModifier {
+  uint y;
   function initialize() public {
     __Parent_init();
+    y = 2;
   }
 }
 
 contract Child_Of_OnlyInitializingModifier_Bad is Parent__OnlyInitializingModifier {
-  function initialize() public {}
+  uint y;
+  function initialize() public {
+    y = 2;
+  }
 }
 
 // This is considered to have a missing initializer because the `regularFn` function is not inferred as an intializer
 contract MissingInitializer_Bad is Parent_InitializerModifier {
+  uint y;
   function regularFn() public {
     parentInit();
+    y = 2;
   }
 }
 
 /// @custom:oz-upgrades-unsafe-allow missing-initializer
 contract MissingInitializer_UnsafeAllow_Contract is Parent_InitializerModifier {
+  uint y;
   function regularFn() public {
     parentInit();
+    y = 2;
   }
 }
 
 contract A is Initializable {
-  function __A_init() onlyInitializing internal {}
+  uint a;
+  function __A_init() onlyInitializing internal {
+    a = 2;
+  }
 }
 
 contract B is Initializable {
-  function __B_init() onlyInitializing internal {}
+  uint b;
+  function __B_init() onlyInitializing internal {
+    b = 2;
+  }
 }
 
 contract C is Initializable {
-  function __C_init() onlyInitializing internal {}
+  uint c;
+  function __C_init() onlyInitializing internal {
+    c = 2;
+  }
 }
 
 contract InitializationOrder_Ok is A, B, C, Parent_NoInitializer {
@@ -235,11 +292,17 @@ contract InitializationOrder_UnsafeAllowDuplicate_But_WrongOrder is A, B, C, Par
 // ==== Initializer visibility ====
 
 contract Parent_PrivateInitializer {
-  function initialize() private {} // not considered an initializer because it's private
+  uint x;
+  function initialize() private { // not considered an initializer because it's private
+    x = 1;
+  }
 }
 
 contract Parent_PublicInitializer {
-  function initialize() public {} // does not strictly need to be called by child
+  uint x;
+  function initialize() public { // does not strictly need to be called by child
+    x = 1;
+  }
 }
 
 contract Child_Of_ParentPrivateInitializer_Ok is Parent_PrivateInitializer { // no initializer required since parent initializer is private
@@ -249,5 +312,7 @@ contract Child_Of_ParentPublicInitializer_Ok is Parent_PublicInitializer { // no
 }
 
 contract Child_Has_PrivateInitializer_Bad is Parent__OnlyInitializingModifier { // parent has internal initializer, but child has private
-  function initialize() private {}
-}
\ No newline at end of file
+  function initialize() private {
+    __Parent_init();
+  }
+}
diff --git a/packages/core/src/validate/run.ts b/packages/core/src/validate/run.ts
index 2e6a82cf1..247d1a5fa 100644
--- a/packages/core/src/validate/run.ts
+++ b/packages/core/src/validate/run.ts
@@ -778,9 +778,11 @@ function getPossibleInitializers(contractDef: ContractDefinition, isParentContra
         ['initialize', 'initializer', 'reinitialize', 'reinitializer'].includes(fnDef.name)) &&
       // Skip virtual functions without a body, since that indicates an abstract function and is not itself an initializer
       !(fnDef.virtual && !fnDef.body) &&
-      // For parent contracts, only treat internal functions as initializers (since they MUST be called by the child)
+      // For parent contracts, only treat internal functions which contain statements as initializers (since they MUST be called by the child)
       // For child contracts, treat all non-private functions as initializers (since they can be called by another contract or externally)
-      (isParentContract ? fnDef.visibility === 'internal' : fnDef.visibility !== 'private'),
+      (isParentContract
+        ? fnDef.visibility === 'internal' && fnDef.body?.statements?.length
+        : fnDef.visibility !== 'private'),
   );
 }
 

From a8d06cf86b3fcdb115f3b3f550d677596c02e024 Mon Sep 17 00:00:00 2001
From: Eric Lau <ericglau@outlook.com>
Date: Thu, 19 Dec 2024 11:22:35 -0500
Subject: [PATCH 21/24] Add testcases for transitive initialization

---
 .../contracts/test/ValidationsInitializer.sol | 46 ++++++++++++++++++-
 .../core/src/validate-initializers.test.ts    |  8 ++++
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/packages/core/contracts/test/ValidationsInitializer.sol b/packages/core/contracts/test/ValidationsInitializer.sol
index 76d8f4e5f..63741d604 100644
--- a/packages/core/contracts/test/ValidationsInitializer.sol
+++ b/packages/core/contracts/test/ValidationsInitializer.sol
@@ -1,7 +1,11 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.0;
 
-import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
+import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
+import {ERC20Upgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";
+import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
+import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
+import {Ownable2StepUpgradeable} from "@openzeppelin/contracts-upgradeable/access/Ownable2StepUpgradeable.sol";
 
 // These contracts are for testing only. They are not safe for use in production, and do not represent best practices.
 
@@ -316,3 +320,43 @@ contract Child_Has_PrivateInitializer_Bad is Parent__OnlyInitializingModifier {
     __Parent_init();
   }
 }
+
+// ==== Transitive initialization ====
+
+contract Ownable_Ok is Initializable, ERC20Upgradeable, OwnableUpgradeable {
+    /// @custom:oz-upgrades-unsafe-allow constructor
+    constructor() {
+        _disableInitializers();
+    }
+
+    function initialize(address initialOwner) initializer public {
+        __ERC20_init("MyToken", "MTK");
+        __Ownable_init(initialOwner);
+    }
+}
+
+contract Ownable2Step_Ok is Initializable, ERC20Upgradeable, Ownable2StepUpgradeable {
+    /// @custom:oz-upgrades-unsafe-allow constructor
+    constructor() {
+        _disableInitializers();
+    }
+
+    function initialize(address initialOwner) initializer public {
+        __ERC20_init("MyToken", "MTK");
+        __Ownable_init(initialOwner); // Transitive dependency that needs to be initialized
+        __Ownable2Step_init();
+    }
+}
+
+contract Ownable2Step_Bad is Initializable, ERC20Upgradeable, Ownable2StepUpgradeable {
+    /// @custom:oz-upgrades-unsafe-allow constructor
+    constructor() {
+        _disableInitializers();
+    }
+
+    function initialize() initializer public {
+        __ERC20_init("MyToken", "MTK");
+        // Missing Ownable, which is a transitive dependency that needs to be initialized
+        __Ownable2Step_init();
+    }
+}
\ No newline at end of file
diff --git a/packages/core/src/validate-initializers.test.ts b/packages/core/src/validate-initializers.test.ts
index 9360fa454..53cd00c08 100644
--- a/packages/core/src/validate-initializers.test.ts
+++ b/packages/core/src/validate-initializers.test.ts
@@ -131,3 +131,11 @@ testRejects(
 testAccepts('Child_Of_ParentPrivateInitializer_Ok', 'transparent');
 testAccepts('Child_Of_ParentPublicInitializer_Ok', 'transparent');
 testRejects('Child_Has_PrivateInitializer_Bad', 'transparent', 'Contract is missing an initializer');
+
+testAccepts('Ownable_Ok', 'transparent');
+testAccepts('Ownable2Step_Ok', 'transparent');
+testRejects(
+  'Ownable2Step_Bad',
+  'transparent',
+  'Contract is missing initializer calls for one or more parent contracts: `OwnableUpgradeable`',
+);

From 330057ef1067d96e0bf3fee78016b6824db9be6d Mon Sep 17 00:00:00 2001
From: Eric Lau <ericglau@outlook.com>
Date: Fri, 20 Dec 2024 17:29:06 -0500
Subject: [PATCH 22/24] Check for missing calls to transitive parent
 initializers - WIP

---
 .../contracts/test/ValidationsInitializer.sol |  55 ++++++++
 .../core/src/validate-initializers.test.ts    | 133 +++++++++++-------
 packages/core/src/validate/report.ts          |   4 +-
 packages/core/src/validate/run.ts             | 128 ++++++++++++++---
 4 files changed, 250 insertions(+), 70 deletions(-)

diff --git a/packages/core/contracts/test/ValidationsInitializer.sol b/packages/core/contracts/test/ValidationsInitializer.sol
index 63741d604..4377e2019 100644
--- a/packages/core/contracts/test/ValidationsInitializer.sol
+++ b/packages/core/contracts/test/ValidationsInitializer.sol
@@ -323,6 +323,61 @@ contract Child_Has_PrivateInitializer_Bad is Parent__OnlyInitializingModifier {
 
 // ==== Transitive initialization ====
 
+abstract contract TransitiveGrandparent1 is Initializable {
+  uint x;
+  function __TransitiveGrandparent1_init() onlyInitializing internal {
+    x = 1;
+  }
+}
+
+abstract contract TransitiveGrandparent2 is Initializable {
+  uint y;
+  function __TransitiveGrandparent2_init() onlyInitializing internal {
+    y = 1;
+  }
+}
+
+contract TransitiveParent_Ok is TransitiveGrandparent1, TransitiveGrandparent2 {
+  function initializeParent() initializer public {
+    __TransitiveGrandparent1_init();
+    __TransitiveGrandparent2_init();
+  }
+}
+
+contract TransitiveParent_Bad is TransitiveGrandparent1, TransitiveGrandparent2 {
+  function initializeParent() initializer public {
+    __TransitiveGrandparent1_init();
+    // Does not call __TransitiveGrandparent2_init, and this contract is not abstract, so it is required
+  }
+}
+
+contract TransitiveChild_Bad_Parent is TransitiveParent_Bad { // this contract is ok but the parent is not
+  function initialize() initializer public {
+    initializeParent();
+  }
+}
+
+contract TransitiveChild_Bad_Order is TransitiveParent_Bad { // grandparent should be initialized first
+  function initialize() initializer public {
+    initializeParent();
+    __TransitiveGrandparent2_init();
+  }
+}
+
+contract TransitiveChild_Bad_Order2 is TransitiveParent_Bad { // this contract is ok but the parent is not
+  function initialize() initializer public {
+    __TransitiveGrandparent2_init();
+    initializeParent();
+  }
+}
+
+contract TransitiveDuplicate_Bad is TransitiveGrandparent1, TransitiveParent_Ok {
+  function initialize() initializer public {
+    __TransitiveGrandparent1_init();
+    initializeParent();
+  }
+}
+
 contract Ownable_Ok is Initializable, ERC20Upgradeable, OwnableUpgradeable {
     /// @custom:oz-upgrades-unsafe-allow constructor
     constructor() {
diff --git a/packages/core/src/validate-initializers.test.ts b/packages/core/src/validate-initializers.test.ts
index 53cd00c08..f0b32fdf5 100644
--- a/packages/core/src/validate-initializers.test.ts
+++ b/packages/core/src/validate-initializers.test.ts
@@ -37,17 +37,27 @@ function testAccepts(name: string, kind: ValidationOptions['kind']) {
   testOverride(name, kind, {}, undefined);
 }
 
-function testRejects(name: string, kind: ValidationOptions['kind'], expectedErrorContains: string) {
-  testOverride(name, kind, {}, expectedErrorContains);
+function testRejects(
+  name: string,
+  kind: ValidationOptions['kind'],
+  expectedError?: {
+    contains: string;
+    count: number;
+  },
+) {
+  testOverride(name, kind, {}, expectedError);
 }
 
 function testOverride(
   name: string,
   kind: ValidationOptions['kind'],
   opts: ValidationOptions,
-  expectErrorContains?: string,
+  expectedError?: {
+    contains: string;
+    count: number;
+  },
 ) {
-  const expectValid = expectErrorContains === undefined;
+  const expectValid = expectedError === undefined;
 
   const optKeys = Object.keys(opts);
   const describeOpts = optKeys.length > 0 ? '(' + optKeys.join(', ') + ')' : '';
@@ -59,7 +69,11 @@ function testOverride(
       t.notThrows(assertUpgSafe);
     } else {
       const error = t.throws(assertUpgSafe) as ValidationErrors;
-      t.true(error.message.includes(expectErrorContains), error.message);
+      t.true(
+        error.errors.length === expectedError.count,
+        `Expected ${expectedError.count} errors, got ${error.errors.length}:\n${error.message}`,
+      );
+      t.true(error.message.includes(expectedError.contains), error.message);
     }
   });
 }
@@ -67,75 +81,100 @@ function testOverride(
 testAccepts('Child_Of_NoInitializer_Ok', 'transparent');
 
 testAccepts('Child_Of_InitializerModifier_Ok', 'transparent');
-testRejects(
-  'Child_Of_InitializerModifier_Bad',
-  'transparent',
-  'Contract is missing initializer calls for one or more parent contracts: `Parent_InitializerModifier`',
-);
+testRejects('Child_Of_InitializerModifier_Bad', 'transparent', {
+  contains: 'Contract is missing initializer calls for one or more parent contracts: `Parent_InitializerModifier`',
+  count: 1,
+});
 testAccepts('Child_Of_InitializerModifier_UsesSuper_Ok', 'transparent');
 
 testAccepts('Child_Of_ReinitializerModifier_Ok', 'transparent');
-testRejects(
-  'Child_Of_ReinitializerModifier_Bad',
-  'transparent',
-  'Contract is missing initializer calls for one or more parent contracts: `Parent_ReinitializerModifier`',
-);
+testRejects('Child_Of_ReinitializerModifier_Bad', 'transparent', {
+  contains: 'Contract is missing initializer calls for one or more parent contracts: `Parent_ReinitializerModifier`',
+  count: 1,
+});
 
 testAccepts('Child_Of_OnlyInitializingModifier_Ok', 'transparent');
-testRejects(
-  'Child_Of_OnlyInitializingModifier_Bad',
-  'transparent',
-  'Contract is missing initializer calls for one or more parent contracts: `Parent__OnlyInitializingModifier`',
-);
+testRejects('Child_Of_OnlyInitializingModifier_Bad', 'transparent', {
+  contains:
+    'Contract is missing initializer calls for one or more parent contracts: `Parent__OnlyInitializingModifier`',
+  count: 1,
+});
+
+testRejects('MissingInitializer_Bad', 'transparent', {
+  contains: 'Contract is missing an initializer',
 
-testRejects('MissingInitializer_Bad', 'transparent', 'Contract is missing an initializer');
+  count: 1,
+});
 testAccepts('MissingInitializer_UnsafeAllow_Contract', 'transparent');
 testOverride('MissingInitializer_Bad', 'transparent', { unsafeAllow: ['missing-initializer'] });
 
 testAccepts('InitializationOrder_Ok', 'transparent');
 testAccepts('InitializationOrder_Ok_2', 'transparent');
 
-testRejects(
-  'InitializationOrder_WrongOrder_Bad',
-  'transparent',
-  'Contract has an incorrect order of parent initializer calls. Expected initializers to be called for parent contracts in the following order: A, B, C',
-);
+testRejects('InitializationOrder_WrongOrder_Bad', 'transparent', {
+  contains: 'Expected initializers to be called for parent contracts in the following order: A, B, C',
+  count: 1,
+});
 testAccepts('InitializationOrder_WrongOrder_UnsafeAllow_Contract', 'transparent');
 testAccepts('InitializationOrder_WrongOrder_UnsafeAllow_Function', 'transparent');
 testOverride('InitializationOrder_WrongOrder_Bad', 'transparent', { unsafeAllow: ['incorrect-initializer-order'] });
 
-testRejects(
-  'InitializationOrder_MissingCall_Bad',
-  'transparent',
-  'Contract is missing initializer calls for one or more parent contracts: `C`',
-);
+testRejects('InitializationOrder_MissingCall_Bad', 'transparent', {
+  contains: 'Contract is missing initializer calls for one or more parent contracts: `C`',
+  count: 1,
+});
 testAccepts('InitializationOrder_MissingCall_UnsafeAllow_Contract', 'transparent');
 testAccepts('InitializationOrder_MissingCall_UnsafeAllow_Function', 'transparent');
 testOverride('InitializationOrder_MissingCall_Bad', 'transparent', { unsafeAllow: ['missing-initializer-call'] });
 
-testRejects(
-  'InitializationOrder_Duplicate_Bad',
-  'transparent',
-  'Contract has duplicate calls to parent initializer `__B_init` for contract `B`',
-);
+testRejects('InitializationOrder_Duplicate_Bad', 'transparent', {
+  contains: 'Contract has duplicate calls to parent initializer `__B_init` for contract `B`',
+  count: 1,
+});
 testAccepts('InitializationOrder_Duplicate_UnsafeAllow_Contract', 'transparent');
 testAccepts('InitializationOrder_Duplicate_UnsafeAllow_Function', 'transparent');
 testAccepts('InitializationOrder_Duplicate_UnsafeAllow_Call', 'transparent');
 testOverride('InitializationOrder_Duplicate_Bad', 'transparent', { unsafeAllow: ['duplicate-initializer-call'] });
-testRejects(
-  'InitializationOrder_UnsafeAllowDuplicate_But_WrongOrder',
-  'transparent',
-  'Contract has an incorrect order of parent initializer calls. Expected initializers to be called for parent contracts in the following order: A, B, C',
-);
+testRejects('InitializationOrder_UnsafeAllowDuplicate_But_WrongOrder', 'transparent', {
+  contains: 'Expected initializers to be called for parent contracts in the following order: A, B, C',
+  count: 1,
+});
 
 testAccepts('Child_Of_ParentPrivateInitializer_Ok', 'transparent');
 testAccepts('Child_Of_ParentPublicInitializer_Ok', 'transparent');
-testRejects('Child_Has_PrivateInitializer_Bad', 'transparent', 'Contract is missing an initializer');
+testRejects('Child_Has_PrivateInitializer_Bad', 'transparent', {
+  contains: 'Contract is missing an initializer',
+  count: 1,
+});
+
+testAccepts('TransitiveParent_Ok', 'transparent');
+testRejects('TransitiveParent_Bad', 'transparent', {
+  contains: 'Contract is missing initializer calls for one or more parent contracts: `TransitiveGrandparent2`',
+  count: 1,
+});
+testRejects('TransitiveChild_Bad_Parent', 'transparent', {
+  contains: 'Contract is missing initializer calls for one or more parent contracts: `TransitiveGrandparent2`',
+  count: 3, // should be 2 if we ignore wrong order.  the errors are: missing for child, missing for parent
+});
+testRejects('TransitiveChild_Bad_Order', 'transparent', {
+  contains:
+    'Expected initializers to be called for parent contracts in the following order: TransitiveGrandparent2, TransitiveParent_Bad',
+  count: 2,
+}); // should have 2 errors: 'Expected initializers to be called for parent contracts in the following order: TransitiveGrandparent2, TransitiveParent_Bad', 'Contract is missing initializer calls for one or more parent contracts: `TransitiveGrandparent2`'
+// but 1 if we ignore wrong order
+testRejects('TransitiveChild_Bad_Order2', 'transparent', {
+  contains: 'Contract is missing initializer calls for one or more parent contracts: `TransitiveGrandparent2`',
+  count: 1,
+});
+testRejects('TransitiveDuplicate_Bad', 'transparent', {
+  contains: 'Contract has duplicate calls to parent',
+  count: 1,
+});
+// should allow this if we ignore duplicate calls transitively
 
 testAccepts('Ownable_Ok', 'transparent');
 testAccepts('Ownable2Step_Ok', 'transparent');
-testRejects(
-  'Ownable2Step_Bad',
-  'transparent',
-  'Contract is missing initializer calls for one or more parent contracts: `OwnableUpgradeable`',
-);
+testRejects('Ownable2Step_Bad', 'transparent', {
+  contains: 'Contract is missing initializer calls for one or more parent contracts: `OwnableUpgradeable`',
+  count: 1,
+});
diff --git a/packages/core/src/validate/report.ts b/packages/core/src/validate/report.ts
index ed598c1a7..ab518b214 100644
--- a/packages/core/src/validate/report.ts
+++ b/packages/core/src/validate/report.ts
@@ -91,7 +91,9 @@ const errorInfo: ErrorDescriptions<ValidationError> = {
   },
   'incorrect-initializer-order': {
     msg: e =>
-      `Contract has an incorrect order of parent initializer calls. Expected initializers to be called for parent contracts in the following order: ${e.expectedLinearization.join(', ')}`,
+      `Contract has an incorrect order of parent initializer calls.
+- Expected initializers to be called for parent contracts in the following order: ${e.expectedLinearization.join(', ')}
+- Found order: ${e.foundOrder.join(', ')}`,
     hint: () => `Call parent initializers in the order they are inherited`,
     link: 'https://zpl.in/upgrades/error-001',
   },
diff --git a/packages/core/src/validate/run.ts b/packages/core/src/validate/run.ts
index 247d1a5fa..1caf5f884 100644
--- a/packages/core/src/validate/run.ts
+++ b/packages/core/src/validate/run.ts
@@ -97,6 +97,7 @@ interface ValidationErrorDuplicateInitializerCall extends ValidationErrorBase {
 interface ValidationErrorIncorrectInitializerOrder extends ValidationErrorBase {
   kind: 'incorrect-initializer-order';
   expectedLinearization: string[];
+  foundOrder: string[];
 }
 
 type ValidationErrorInitializer =
@@ -662,7 +663,7 @@ function* getInternalFunctionStorageErrors(
 }
 
 /**
- * Reports an error if a parent contract has an initializer and any of the following are true:
+ * Reports an error this contract is non-abstract, a linearized parent contract has an initializer, and any of the following are true:
  * - 1. Missing initializer: This contract does not appear to have an initializer.
  * - 2. Missing initializer call: This contract's initializer is missing a call to a parent initializer.
  * - 3. Duplicate initializer call: This contract has duplicate calls to the same parent initializer function.
@@ -673,21 +674,97 @@ function* getInitializerErrors(
   deref: ASTDereferencer,
   decodeSrc: SrcDecoder,
 ): Generator<ValidationErrorInitializer> {
-  if (contractDef.baseContracts.length > 0) {
-    const baseContractDefs = contractDef.baseContracts.map(base =>
-      deref('ContractDefinition', base.baseName.referencedDeclaration),
+  if (contractDef.abstract) {
+    return;
+  }
+  if (contractDef.linearizedBaseContracts.length > 0) {
+    console.log('- Checking initializers for contract [' + contractDef.name + ']');
+
+    const linearizedBaseContractDefs = contractDef.linearizedBaseContracts.map(base =>
+      deref('ContractDefinition', base),
     );
+
+    // Remove the least derived contract from the list
+    linearizedBaseContractDefs.shift();
+    // Reverse the order to start from the most derived contract
+    linearizedBaseContractDefs.reverse();
+
     const baseContractsInitializersMap = new Map(
-      baseContractDefs.map(base => [base.name, getPossibleInitializers(base, true)]),
+      linearizedBaseContractDefs.map(base => [base.name, getPossibleInitializers(base, true)]),
     );
-    const baseContractsWithInitializers = baseContractDefs
+
+    console.log(' -> Before removing: ' + linearizedBaseContractDefs.map(base => base.name).join(', '));
+
+    // For each base contract, if its initializer calls any of the earlier base contracts' intializers, it can be removed from the list.
+    // Ignore whether the base contracts are calling their initializers in the correct order, because we only check the order of THIS contract's calls.
+    for (const base of linearizedBaseContractDefs) {
+      const baseInitializers = baseContractsInitializersMap.get(base.name)!;
+      for (const initializer of baseInitializers) {
+        const expressionStatements =
+          initializer.body?.statements?.filter(stmt => stmt.nodeType === 'ExpressionStatement') ?? [];
+        for (const stmt of expressionStatements) {
+          const fnCall = stmt.expression;
+          if (
+            fnCall.nodeType === 'FunctionCall' &&
+            (fnCall.expression.nodeType === 'Identifier' || fnCall.expression.nodeType === 'MemberAccess')
+          ) {
+            const referencedFn = fnCall.expression.referencedDeclaration;
+            if (referencedFn) {
+              const earlierBaseContractDefs = linearizedBaseContractDefs.slice(
+                0,
+                linearizedBaseContractDefs.indexOf(base),
+              );
+
+              const foundParentInitializer = earlierBaseContractDefs.find(base =>
+                baseContractsInitializersMap.get(base.name)!.some(init => init.id === referencedFn),
+              );
+              if (foundParentInitializer) {
+                const index = earlierBaseContractDefs.indexOf(foundParentInitializer);
+                if (index !== -1) {
+                  console.log(
+                    '  - Removing ' +
+                      foundParentInitializer.name +
+                      ' from linearizedBaseContractDefs because it is called by ' +
+                      base.name,
+                  );
+                  linearizedBaseContractDefs.splice(linearizedBaseContractDefs.indexOf(foundParentInitializer), 1);
+                }
+              }
+            }
+          }
+        }
+      }
+    }
+
+    console.log(' -> After removing: ' + linearizedBaseContractDefs.map(base => base.name).join(', '));
+
+    const baseContractsToInitialize = linearizedBaseContractDefs
       .filter(base => baseContractsInitializersMap.get(base.name)?.length)
       .map(base => base.name);
 
-    if (baseContractsWithInitializers.length > 0) {
+    console.log('    - baseContractsToInitialize: ' + baseContractsToInitialize);
+
+    if (baseContractsToInitialize.length > 0) {
       // Check for missing initializers
       const contractInitializers = getPossibleInitializers(contractDef, false);
-      if (contractInitializers.length === 0 && !skipCheck('missing-initializer', contractDef)) {
+
+      // If there are multiple parents with initializers, the contract must have its own initializer to call them.
+      // If there is one parent with possible initializers:
+      // - they are all internal, the contract must have its own initializer
+      // - otherwise the contract does not need its own initializer, since one of the parent's initializers can be called during deployment
+      const requiresParentInitializerCall =
+        baseContractsToInitialize.length > 1 ||
+        (baseContractsToInitialize.length === 1 &&
+          baseContractsInitializersMap.get(baseContractsToInitialize[0])!.length > 0 &&
+          baseContractsInitializersMap
+            .get(baseContractsToInitialize[0])!
+            .every(contractDef => contractDef.visibility === 'internal'));
+
+      if (
+        requiresParentInitializerCall &&
+        contractInitializers.length === 0 &&
+        !skipCheck('missing-initializer', contractDef)
+      ) {
         yield {
           kind: 'missing-initializer',
           src: decodeSrc(contractDef),
@@ -695,7 +772,8 @@ function* getInitializerErrors(
       }
 
       for (const contractInitializer of contractInitializers) {
-        const uninitializedBaseContracts = [...baseContractsWithInitializers];
+        const remaining: string[] = [...baseContractsToInitialize];
+        const foundOrder: string[] = [];
         const calledInitializerIds: number[] = [];
 
         const expressionStatements =
@@ -711,8 +789,9 @@ function* getInitializerErrors(
             // If this is a call to a parent initializer, then:
             // - Check if it was already called (duplicate call)
             // - Otherwise, check if the parent initializer is called in the correct order
-            for (const [baseName, initializers] of baseContractsInitializersMap) {
-              const foundParentInitializer = initializers.find(init => init.id === referencedFn);
+            for (const baseContractToInitialize of baseContractsToInitialize) {
+              const baseInitializers = baseContractsInitializersMap.get(baseContractToInitialize)!;
+              const foundParentInitializer = baseInitializers.find(init => init.id === referencedFn);
               if (referencedFn && foundParentInitializer) {
                 const duplicate = calledInitializerIds.includes(referencedFn);
                 if (
@@ -725,12 +804,14 @@ function* getInitializerErrors(
                     kind: 'duplicate-initializer-call',
                     src: decodeSrc(fnCall),
                     parentInitializer: foundParentInitializer.name,
-                    parentContract: baseName,
+                    parentContract: baseContractToInitialize,
                   };
                 }
                 calledInitializerIds.push(referencedFn);
 
-                const index = uninitializedBaseContracts.indexOf(baseName);
+                foundOrder.push(baseContractToInitialize);
+                // TODO handle linearized contracts
+                const index = remaining.indexOf(baseContractToInitialize);
                 if (
                   !duplicate && // Omit duplicate calls to avoid treating them as out of order. Duplicates are either reported above or they were skipped.
                   index !== 0 &&
@@ -740,11 +821,12 @@ function* getInitializerErrors(
                   yield {
                     kind: 'incorrect-initializer-order',
                     src: decodeSrc(fnCall),
-                    expectedLinearization: baseContractsWithInitializers,
+                    expectedLinearization: baseContractsToInitialize,
+                    foundOrder,
                   };
                 }
                 if (index !== -1) {
-                  uninitializedBaseContracts.splice(index, 1);
+                  remaining.splice(index, 1);
                 }
               }
             }
@@ -753,14 +835,14 @@ function* getInitializerErrors(
 
         // If there are any base contracts that were not initialized, report an error
         if (
-          uninitializedBaseContracts.length > 0 &&
+          remaining.length > 0 &&
           !skipCheck('missing-initializer-call', contractDef) &&
           !skipCheck('missing-initializer-call', contractInitializer)
         ) {
           yield {
             kind: 'missing-initializer-call',
             src: decodeSrc(contractInitializer),
-            parentContracts: uninitializedBaseContracts,
+            parentContracts: remaining,
           };
         }
       }
@@ -768,6 +850,9 @@ function* getInitializerErrors(
   }
 }
 
+/**
+ * Get all functions that could be initializers. Does not include private functions.
+ */
 function getPossibleInitializers(contractDef: ContractDefinition, isParentContract: boolean) {
   const fns = [...findAll('FunctionDefinition', contractDef)];
   return fns.filter(
@@ -778,11 +863,10 @@ function getPossibleInitializers(contractDef: ContractDefinition, isParentContra
         ['initialize', 'initializer', 'reinitialize', 'reinitializer'].includes(fnDef.name)) &&
       // Skip virtual functions without a body, since that indicates an abstract function and is not itself an initializer
       !(fnDef.virtual && !fnDef.body) &&
-      // For parent contracts, only treat internal functions which contain statements as initializers (since they MUST be called by the child)
-      // For child contracts, treat all non-private functions as initializers (since they can be called by another contract or externally)
-      (isParentContract
-        ? fnDef.visibility === 'internal' && fnDef.body?.statements?.length
-        : fnDef.visibility !== 'private'),
+      // Ignore private functions, since they cannot be called outside the contract
+      fnDef.visibility !== 'private' &&
+      // For parent contracts, only functions which contain statements need to be called
+      (isParentContract ? fnDef.body?.statements?.length : true),
   );
 }
 

From d1fce2c236eb8f2c4b2cb5233a4cd27396b82da7 Mon Sep 17 00:00:00 2001
From: Eric Lau <ericglau@outlook.com>
Date: Fri, 20 Dec 2024 17:58:32 -0500
Subject: [PATCH 23/24] Comment out console.log

---
 packages/core/src/validate/run.ts | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/packages/core/src/validate/run.ts b/packages/core/src/validate/run.ts
index 1caf5f884..4b7d08bcd 100644
--- a/packages/core/src/validate/run.ts
+++ b/packages/core/src/validate/run.ts
@@ -678,7 +678,7 @@ function* getInitializerErrors(
     return;
   }
   if (contractDef.linearizedBaseContracts.length > 0) {
-    console.log('- Checking initializers for contract [' + contractDef.name + ']');
+    // console.log('- Checking initializers for contract [' + contractDef.name + ']');
 
     const linearizedBaseContractDefs = contractDef.linearizedBaseContracts.map(base =>
       deref('ContractDefinition', base),
@@ -693,7 +693,7 @@ function* getInitializerErrors(
       linearizedBaseContractDefs.map(base => [base.name, getPossibleInitializers(base, true)]),
     );
 
-    console.log(' -> Before removing: ' + linearizedBaseContractDefs.map(base => base.name).join(', '));
+    // console.log(' -> Before removing: ' + linearizedBaseContractDefs.map(base => base.name).join(', '));
 
     // For each base contract, if its initializer calls any of the earlier base contracts' intializers, it can be removed from the list.
     // Ignore whether the base contracts are calling their initializers in the correct order, because we only check the order of THIS contract's calls.
@@ -721,12 +721,12 @@ function* getInitializerErrors(
               if (foundParentInitializer) {
                 const index = earlierBaseContractDefs.indexOf(foundParentInitializer);
                 if (index !== -1) {
-                  console.log(
-                    '  - Removing ' +
-                      foundParentInitializer.name +
-                      ' from linearizedBaseContractDefs because it is called by ' +
-                      base.name,
-                  );
+                  // console.log(
+                  //   '  - Removing ' +
+                  //     foundParentInitializer.name +
+                  //     ' from linearizedBaseContractDefs because it is called by ' +
+                  //     base.name,
+                  // );
                   linearizedBaseContractDefs.splice(linearizedBaseContractDefs.indexOf(foundParentInitializer), 1);
                 }
               }
@@ -736,13 +736,13 @@ function* getInitializerErrors(
       }
     }
 
-    console.log(' -> After removing: ' + linearizedBaseContractDefs.map(base => base.name).join(', '));
+    // console.log(' -> After removing: ' + linearizedBaseContractDefs.map(base => base.name).join(', '));
 
     const baseContractsToInitialize = linearizedBaseContractDefs
       .filter(base => baseContractsInitializersMap.get(base.name)?.length)
       .map(base => base.name);
 
-    console.log('    - baseContractsToInitialize: ' + baseContractsToInitialize);
+    // console.log('    - baseContractsToInitialize: ' + baseContractsToInitialize);
 
     if (baseContractsToInitialize.length > 0) {
       // Check for missing initializers

From b2ef161d56de39e29d4cd20ac17054329ed96a2a Mon Sep 17 00:00:00 2001
From: Eric Lau <ericglau@outlook.com>
Date: Tue, 7 Jan 2025 17:50:28 -0500
Subject: [PATCH 24/24] Detect duplicate calls in transitive dependencies

---
 .../contracts/test/ValidationsInitializer.sol | 22 +++++++++----------
 .../core/src/validate-initializers.test.ts    |  2 --
 packages/core/src/validate/run.ts             |  8 ++++---
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/packages/core/contracts/test/ValidationsInitializer.sol b/packages/core/contracts/test/ValidationsInitializer.sol
index 4377e2019..b0547faab 100644
--- a/packages/core/contracts/test/ValidationsInitializer.sol
+++ b/packages/core/contracts/test/ValidationsInitializer.sol
@@ -325,28 +325,28 @@ contract Child_Has_PrivateInitializer_Bad is Parent__OnlyInitializingModifier {
 
 abstract contract TransitiveGrandparent1 is Initializable {
   uint x;
-  function __TransitiveGrandparent1_init() onlyInitializing internal {
-    x = 1;
+  function __TransitiveGrandparent1_init(uint a) onlyInitializing internal {
+    x = a;
   }
 }
 
 abstract contract TransitiveGrandparent2 is Initializable {
   uint y;
-  function __TransitiveGrandparent2_init() onlyInitializing internal {
-    y = 1;
+  function __TransitiveGrandparent2_init(uint b) onlyInitializing internal {
+    y = b;
   }
 }
 
 contract TransitiveParent_Ok is TransitiveGrandparent1, TransitiveGrandparent2 {
   function initializeParent() initializer public {
-    __TransitiveGrandparent1_init();
-    __TransitiveGrandparent2_init();
+    __TransitiveGrandparent1_init(1);
+    __TransitiveGrandparent2_init(2);
   }
 }
 
 contract TransitiveParent_Bad is TransitiveGrandparent1, TransitiveGrandparent2 {
   function initializeParent() initializer public {
-    __TransitiveGrandparent1_init();
+    __TransitiveGrandparent1_init(1);
     // Does not call __TransitiveGrandparent2_init, and this contract is not abstract, so it is required
   }
 }
@@ -357,23 +357,23 @@ contract TransitiveChild_Bad_Parent is TransitiveParent_Bad { // this contract i
   }
 }
 
-contract TransitiveChild_Bad_Order is TransitiveParent_Bad { // grandparent should be initialized first
+contract TransitiveChild_Bad_Order is TransitiveParent_Bad {
   function initialize() initializer public {
     initializeParent();
-    __TransitiveGrandparent2_init();
+    __TransitiveGrandparent2_init(2); // grandparent should be initialized first
   }
 }
 
 contract TransitiveChild_Bad_Order2 is TransitiveParent_Bad { // this contract is ok but the parent is not
   function initialize() initializer public {
-    __TransitiveGrandparent2_init();
+    __TransitiveGrandparent2_init(2);
     initializeParent();
   }
 }
 
 contract TransitiveDuplicate_Bad is TransitiveGrandparent1, TransitiveParent_Ok {
   function initialize() initializer public {
-    __TransitiveGrandparent1_init();
+    __TransitiveGrandparent1_init(1000); // duplicate
     initializeParent();
   }
 }
diff --git a/packages/core/src/validate-initializers.test.ts b/packages/core/src/validate-initializers.test.ts
index f0b32fdf5..e78096470 100644
--- a/packages/core/src/validate-initializers.test.ts
+++ b/packages/core/src/validate-initializers.test.ts
@@ -161,7 +161,6 @@ testRejects('TransitiveChild_Bad_Order', 'transparent', {
     'Expected initializers to be called for parent contracts in the following order: TransitiveGrandparent2, TransitiveParent_Bad',
   count: 2,
 }); // should have 2 errors: 'Expected initializers to be called for parent contracts in the following order: TransitiveGrandparent2, TransitiveParent_Bad', 'Contract is missing initializer calls for one or more parent contracts: `TransitiveGrandparent2`'
-// but 1 if we ignore wrong order
 testRejects('TransitiveChild_Bad_Order2', 'transparent', {
   contains: 'Contract is missing initializer calls for one or more parent contracts: `TransitiveGrandparent2`',
   count: 1,
@@ -170,7 +169,6 @@ testRejects('TransitiveDuplicate_Bad', 'transparent', {
   contains: 'Contract has duplicate calls to parent',
   count: 1,
 });
-// should allow this if we ignore duplicate calls transitively
 
 testAccepts('Ownable_Ok', 'transparent');
 testAccepts('Ownable2Step_Ok', 'transparent');
diff --git a/packages/core/src/validate/run.ts b/packages/core/src/validate/run.ts
index 4b7d08bcd..5862d6670 100644
--- a/packages/core/src/validate/run.ts
+++ b/packages/core/src/validate/run.ts
@@ -695,6 +695,8 @@ function* getInitializerErrors(
 
     // console.log(' -> Before removing: ' + linearizedBaseContractDefs.map(base => base.name).join(', '));
 
+    const parentCalledInitializerIds: number[] = [];
+
     // For each base contract, if its initializer calls any of the earlier base contracts' intializers, it can be removed from the list.
     // Ignore whether the base contracts are calling their initializers in the correct order, because we only check the order of THIS contract's calls.
     for (const base of linearizedBaseContractDefs) {
@@ -727,6 +729,7 @@ function* getInitializerErrors(
                   //     ' from linearizedBaseContractDefs because it is called by ' +
                   //     base.name,
                   // );
+                  parentCalledInitializerIds.push(referencedFn);
                   linearizedBaseContractDefs.splice(linearizedBaseContractDefs.indexOf(foundParentInitializer), 1);
                 }
               }
@@ -774,7 +777,7 @@ function* getInitializerErrors(
       for (const contractInitializer of contractInitializers) {
         const remaining: string[] = [...baseContractsToInitialize];
         const foundOrder: string[] = [];
-        const calledInitializerIds: number[] = [];
+        const calledInitializerIds: number[] = [...parentCalledInitializerIds];
 
         const expressionStatements =
           contractInitializer.body?.statements?.filter(stmt => stmt.nodeType === 'ExpressionStatement') ?? [];
@@ -789,7 +792,7 @@ function* getInitializerErrors(
             // If this is a call to a parent initializer, then:
             // - Check if it was already called (duplicate call)
             // - Otherwise, check if the parent initializer is called in the correct order
-            for (const baseContractToInitialize of baseContractsToInitialize) {
+            for (const baseContractToInitialize of baseContractsInitializersMap.keys()) {
               const baseInitializers = baseContractsInitializersMap.get(baseContractToInitialize)!;
               const foundParentInitializer = baseInitializers.find(init => init.id === referencedFn);
               if (referencedFn && foundParentInitializer) {
@@ -810,7 +813,6 @@ function* getInitializerErrors(
                 calledInitializerIds.push(referencedFn);
 
                 foundOrder.push(baseContractToInitialize);
-                // TODO handle linearized contracts
                 const index = remaining.indexOf(baseContractToInitialize);
                 if (
                   !duplicate && // Omit duplicate calls to avoid treating them as out of order. Duplicates are either reported above or they were skipped.