From 9f453018c6566f2ce91bc83144e26a7531685ad5 Mon Sep 17 00:00:00 2001 From: Eric Lau Date: Wed, 18 Dec 2024 11:19:17 -0500 Subject: [PATCH] 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)