Clarify duplicate and incorrect order handling

OpenZeppelin · Dec 18, 2024 · 9f45301 · 9f45301
1 parent d7385c1
commit 9f45301
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 3 deletions.
diff --git 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
@@ -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
@@ -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)