Fix Hardhat compile error when overriding interface functions with pu…

…blic constant variables (#1091)
OpenZeppelin · Oct 10, 2024 · 52ae59e · 52ae59e
1 parent 8c42f06
commit 52ae59e
Show file tree

Hide file tree

Showing 21 changed files with 185 additions and 53 deletions.
diff --git a/packages/core/CHANGELOG.md b/packages/core/CHANGELOG.md
@@ -1,5 +1,9 @@
 # Changelog
 
+## Unreleased
+
+- Fix Hardhat compile error when overriding interface functions with public constant variables. ([#1091](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/1091))
+
 ## 1.39.0 (2024-10-02)
 
 - Fix Hardhat compile error when library or interface has struct with namespace annotation. ([#1086](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/1086))

diff --git a/packages/core/contracts/test/NamespacedToModify.sol b/packages/core/contracts/test/NamespacedToModify.sol
@@ -220,7 +220,20 @@ contract UsesAddress {
 
 contract HasFunctionWithRequiredReturn {
     struct S { uint x; }
-    function foo(S calldata s) internal pure returns (S calldata) {
+    modifier myModifier() {
+        _;
+    }
+    function foo(S calldata s) internal pure myModifier returns (S calldata) {
+        return s;
+    }
+}
+
+library LibWithRequiredReturn {
+    struct S { uint x; }
+    modifier myModifier() {
+        _;
+    }
+    function foo(S calldata s) myModifier internal pure returns (S calldata) {
         return s;
     }
 }
@@ -322,3 +335,28 @@ interface InterfaceWithNamespace {
         uint256 y;
     }
 }
+
+interface IHasConstantGetter {
+  function a() external view returns (bytes32);
+}
+
+contract HasConstantGetter is IHasConstantGetter {
+  bytes32 public override constant a = bytes32("foo");
+}
+
+abstract contract AbstractHasConstantGetter {
+  function a() virtual external pure returns (bytes32) {
+    // Virtual with default implementation
+    return bytes32("foo");
+  }
+}
+
+contract HasConstantGetterOverride is AbstractHasConstantGetter {
+  bytes32 public override constant a = bytes32("foo");
+}
+
+contract HasFunctionOverride is AbstractHasConstantGetter {
+  function a() override virtual external pure returns (bytes32) {
+    return bytes32("foo2");
+  }
+}
diff --git a/packages/core/src/cli/cli.test.ts.snap b/packages/core/src/cli/cli.test.ts.snap
diff --git a/packages/core/src/cli/validate/project-report.test.ts.snap b/packages/core/src/cli/validate/project-report.test.ts.snap
diff --git a/packages/core/src/cli/validate/validate-upgrade-safety.test.ts.snap b/packages/core/src/cli/validate/validate-upgrade-safety.test.ts.snap
diff --git a/packages/core/src/cli/validate/validations.test.ts.snap b/packages/core/src/cli/validate/validations.test.ts.snap
diff --git a/packages/core/src/scripts/migrate-oz-cli-project.test.ts.snap b/packages/core/src/scripts/migrate-oz-cli-project.test.ts.snap
diff --git a/packages/core/src/storage-0.8.test.ts.snap b/packages/core/src/storage-0.8.test.ts.snap
diff --git a/packages/core/src/storage-memory-0.5.test.ts.snap b/packages/core/src/storage-memory-0.5.test.ts.snap
diff --git a/packages/core/src/storage-namespaced-conflicts-layout.test.ts.snap b/packages/core/src/storage-namespaced-conflicts-layout.test.ts.snap
diff --git a/packages/core/src/storage-namespaced-conflicts.test.ts.snap b/packages/core/src/storage-namespaced-conflicts.test.ts.snap
diff --git a/packages/core/src/storage-namespaced-layout.test.ts.snap b/packages/core/src/storage-namespaced-layout.test.ts.snap
diff --git a/packages/core/src/storage-namespaced.test.ts.snap b/packages/core/src/storage-namespaced.test.ts.snap
diff --git a/packages/core/src/storage.test.ts.snap b/packages/core/src/storage.test.ts.snap
diff --git a/packages/core/src/storage/report-gap.test.ts.snap b/packages/core/src/storage/report-gap.test.ts.snap
diff --git a/packages/core/src/storage/report-rename-retype.test.ts.snap b/packages/core/src/storage/report-rename-retype.test.ts.snap
diff --git a/packages/core/src/storage/report.test.ts.snap b/packages/core/src/storage/report.test.ts.snap
diff --git a/packages/core/src/utils/make-namespaced.test.ts.md b/packages/core/src/utils/make-namespaced.test.ts.md
@@ -53,13 +53,13 @@ Generated by [AVA](https://avajs.dev).
               } StorageWithComment $StorageWithComment_random;␊
           ␊
               ␊
-              function foo() public {}␊
+              function foo() virtual  public ;␊
           ␊
               ␊
-              function foo1(uint a) public {}␊
+              function foo1(uint a) virtual  public ;␊
           ␊
               ␊
-              function foo2(uint a) internal {}␊
+              function foo2(uint a) virtual  internal ;␊
               struct MyStruct { uint b; }␊
           ␊
               // keccak256(abi.encode(uint256(keccak256("example.main")) - 1)) & ~bytes32(uint256(0xff));␊
@@ -68,7 +68,7 @@ Generated by [AVA](https://avajs.dev).
           ␊
               function _getMainStorage() private pure returns (bool $) {}␊
           ␊
-              function _getXTimesY() internal view returns (bool) {}␊
+              function _getXTimesY() virtual  internal view returns (uint256) ;␊
           ␊
               ␊
           ␊
@@ -80,10 +80,10 @@ Generated by [AVA](https://avajs.dev).
               ␊
           ␊
               ␊
-              function foo3() public {}␊
+              function foo3() virtual  public ;␊
           ␊
               ␊
-              function foo4() public {}␊
+              function foo4() virtual  public ;␊
           ␊
               ␊
               enum $astId_id_random { dummy }␊
@@ -99,7 +99,7 @@ Generated by [AVA](https://avajs.dev).
             ␊
             enum $astId_id_random { dummy }␊
           ␊
-            function foo() pure public returns (bool) {}␊
+            function foo() virtual  pure public returns (uint) ;␊
           }␊
           ␊
           abstract contract UsingFunction is HasFunction {␊
@@ -119,11 +119,11 @@ Generated by [AVA](https://avajs.dev).
           abstract contract Consumer {␊
             enum $astId_id_random { dummy }␊
           ␊
-            function usingFreeFunction() pure public returns (bool) {}␊
+            function usingFreeFunction() virtual  pure public returns (bytes4) ;␊
           ␊
-            function usingConstant() pure public returns (bool) {}␊
+            function usingConstant() virtual  pure public returns (bytes4) ;␊
           ␊
-            function usingLibraryFunction() pure public returns (bool) {}␊
+            function usingLibraryFunction() virtual  pure public returns (bytes4) ;␊
           }␊
           ␊
           abstract contract HasConstantWithSelector {␊
@@ -150,7 +150,7 @@ Generated by [AVA](https://avajs.dev).
           abstract contract UsingForDirectives {␊
             enum $astId_id_random { dummy }␊
           ␊
-            function usingFor(uint x) pure public returns (bool) {}␊
+            function usingFor(uint x) virtual  pure public returns (uint) ;␊
           }␊
           ␊
           ␊
@@ -190,7 +190,14 @@ Generated by [AVA](https://avajs.dev).
           ␊
           abstract contract HasFunctionWithRequiredReturn {␊
               struct S { uint x; }␊
-              function foo(S calldata s) internal pure returns (bool) {}␊
+              enum $astId_id_random { dummy }␊
+              function foo(S calldata s) virtual  internal pure  returns (S calldata) ;␊
+          }␊
+          ␊
+          library LibWithRequiredReturn {␊
+              struct S { uint x; }␊
+              enum $astId_id_random { dummy }␊
+              function foo(S calldata s)  internal pure returns (bool) {}␊
           }␊
           ␊
           ␊
@@ -204,17 +211,17 @@ Generated by [AVA](https://avajs.dev).
           ␊
           abstract contract HasNatSpecWithMultipleReturns {␊
               ␊
-              function hasMultipleReturnsInContract() public pure returns (bool, bool) {}␊
+              function hasMultipleReturnsInContract() virtual  public pure returns (uint, uint) ;␊
           ␊
               ␊
-              function hasMultipleNamedReturnsInContract() public pure returns (bool a, bool b) {}␊
+              function hasMultipleNamedReturnsInContract() virtual  public pure returns (uint a, uint b) ;␊
           ␊
               ␊
-              function hasReturnsDocumentedAsParamsInContract() public pure returns (bool a, bool b) {}␊
+              function hasReturnsDocumentedAsParamsInContract() virtual  public pure returns (uint a, uint b) ;␊
           }␊
           ␊
           interface IHasExternalViewFunction {␊
-              function foo() external view returns (bool);␊
+              function foo() external view returns (uint256);␊
           }␊
           ␊
           abstract contract HasExternalViewFunction is IHasExternalViewFunction {␊
@@ -240,7 +247,7 @@ Generated by [AVA](https://avajs.dev).
               enum $astId_id_random { dummy }␊
           ␊
               // Regular function but payable␊
-              function hasPayable() public payable {}␊
+              function hasPayable() virtual  public payable ;␊
           ␊
               bytes4 constant PAYABLE_SELECTOR = this.hasPayable.selector;␊
           }␊
@@ -261,7 +268,26 @@ Generated by [AVA](https://avajs.dev).
                   uint256 y;␊
               }␊
           }␊
-          `,
+          ␊
+          interface IHasConstantGetter {␊
+            function a() external view returns (bytes32);␊
+          }␊
+          ␊
+          abstract contract HasConstantGetter is IHasConstantGetter {␊
+            bytes32 public override constant a = bytes32("foo");␊
+          }␊
+          ␊
+          abstract contract AbstractHasConstantGetter {␊
+            function a() virtual external pure returns (bytes32) ;␊
+          }␊
+          ␊
+          abstract contract HasConstantGetterOverride is AbstractHasConstantGetter {␊
+            bytes32 public override constant a = bytes32("foo");␊
+          }␊
+          ␊
+          abstract contract HasFunctionOverride is AbstractHasConstantGetter {␊
+            function a() override virtual external pure returns (bytes32) ;␊
+          }`,
         },
         'contracts/test/NamespacedToModifyImported.sol': {
           content: `// SPDX-License-Identifier: MIT␊
@@ -324,13 +350,13 @@ Generated by [AVA](https://avajs.dev).
               } StorageWithComment $StorageWithComment_random;␊
           ␊
               /// @notice some natspec␊
-              function foo() public {}␊
+              function foo() virtual  public ;␊
           ␊
               /// @param a docs␊
-              function foo1(uint a) public {}␊
+              function foo1(uint a) virtual  public ;␊
           ␊
               /// @param a docs␊
-              function foo2(uint a) internal {}␊
+              function foo2(uint a) virtual  internal ;␊
               struct MyStruct { uint b; }␊
           ␊
               // keccak256(abi.encode(uint256(keccak256("example.main")) - 1)) & ~bytes32(uint256(0xff));␊
@@ -339,7 +365,7 @@ Generated by [AVA](https://avajs.dev).
           ␊
               function _getMainStorage() private pure returns (bool $) {}␊
           ␊
-              function _getXTimesY() internal view returns (bool) {}␊
+              function _getXTimesY() virtual  internal view returns (uint256) ;␊
           ␊
               /// @notice standlone natspec␊
           ␊
@@ -355,14 +381,14 @@ Generated by [AVA](https://avajs.dev).
               /**␊
                * doc block without natspec␊
                */␊
-              function foo3() public {}␊
+              function foo3() virtual  public ;␊
           ␊
               /**␊
                * doc block with natspec␊
                *␊
                * @notice some natspec␊
                */␊
-              function foo4() public {}␊
+              function foo4() virtual  public ;␊
           ␊
               /**␊
                * @dev a custom error inside a contract␊
@@ -380,7 +406,7 @@ Generated by [AVA](https://avajs.dev).
             /// @param myInt an integer␊
             enum $astId_id_random { dummy }␊
           ␊
-            function foo() pure public returns (bool) {}␊
+            function foo() virtual  pure public returns (uint) ;␊
           }␊
           ␊
           abstract contract UsingFunction is HasFunction {␊
@@ -400,11 +426,11 @@ Generated by [AVA](https://avajs.dev).
           abstract contract Consumer {␊
             enum $astId_id_random { dummy }␊
           ␊
-            function usingFreeFunction() pure public returns (bool) {}␊
+            function usingFreeFunction() virtual  pure public returns (bytes4) ;␊
           ␊
-            function usingConstant() pure public returns (bool) {}␊
+            function usingConstant() virtual  pure public returns (bytes4) ;␊
           ␊
-            function usingLibraryFunction() pure public returns (bool) {}␊
+            function usingLibraryFunction() virtual  pure public returns (bytes4) ;␊
           }␊
           ␊
           abstract contract HasConstantWithSelector {␊
@@ -442,7 +468,7 @@ Generated by [AVA](https://avajs.dev).
           abstract contract UsingForDirectives {␊
             enum $astId_id_random { dummy }␊
           ␊
-            function usingFor(uint x) pure public returns (bool) {}␊
+            function usingFor(uint x) virtual  pure public returns (uint) ;␊
           }␊
           ␊
           /**␊
@@ -494,7 +520,14 @@ Generated by [AVA](https://avajs.dev).
           ␊
           abstract contract HasFunctionWithRequiredReturn {␊
               struct S { uint x; }␊
-              function foo(S calldata s) internal pure returns (bool) {}␊
+              enum $astId_id_random { dummy }␊
+              function foo(S calldata s) virtual  internal pure  returns (S calldata) ;␊
+          }␊
+          ␊
+          library LibWithRequiredReturn {␊
+              struct S { uint x; }␊
+              enum $astId_id_random { dummy }␊
+              function foo(S calldata s)  internal pure returns (bool) {}␊
           }␊
           ␊
           /**␊
@@ -520,23 +553,23 @@ Generated by [AVA](https://avajs.dev).
                * @return uint 1␊
                * @return uint 2␊
                */␊
-              function hasMultipleReturnsInContract() public pure returns (bool, bool) {}␊
+              function hasMultipleReturnsInContract() virtual  public pure returns (uint, uint) ;␊
           ␊
               /**␊
                * @return a first␊
                * @return b second␊
                */␊
-              function hasMultipleNamedReturnsInContract() public pure returns (bool a, bool b) {}␊
+              function hasMultipleNamedReturnsInContract() virtual  public pure returns (uint a, uint b) ;␊
           ␊
               /**␊
                * @param a first␊
                * @param b second␊
                */␊
-              function hasReturnsDocumentedAsParamsInContract() public pure returns (bool a, bool b) {}␊
+              function hasReturnsDocumentedAsParamsInContract() virtual  public pure returns (uint a, uint b) ;␊
           }␊
           ␊
           interface IHasExternalViewFunction {␊
-              function foo() external view returns (bool);␊
+              function foo() external view returns (uint256);␊
           }␊
           ␊
           abstract contract HasExternalViewFunction is IHasExternalViewFunction {␊
@@ -562,7 +595,7 @@ Generated by [AVA](https://avajs.dev).
               enum $astId_id_random { dummy }␊
           ␊
               // Regular function but payable␊
-              function hasPayable() public payable {}␊
+              function hasPayable() virtual  public payable ;␊
           ␊
               bytes4 constant PAYABLE_SELECTOR = this.hasPayable.selector;␊
           }␊
@@ -583,7 +616,26 @@ Generated by [AVA](https://avajs.dev).
                   uint256 y;␊
               }␊
           }␊
-          `,
+          ␊
+          interface IHasConstantGetter {␊
+            function a() external view returns (bytes32);␊
+          }␊
+          ␊
+          abstract contract HasConstantGetter is IHasConstantGetter {␊
+            bytes32 public override constant a = bytes32("foo");␊
+          }␊
+          ␊
+          abstract contract AbstractHasConstantGetter {␊
+            function a() virtual external pure returns (bytes32) ;␊
+          }␊
+          ␊
+          abstract contract HasConstantGetterOverride is AbstractHasConstantGetter {␊
+            bytes32 public override constant a = bytes32("foo");␊
+          }␊
+          ␊
+          abstract contract HasFunctionOverride is AbstractHasConstantGetter {␊
+            function a() override virtual external pure returns (bytes32) ;␊
+          }`,
         },
         'contracts/test/NamespacedToModifyImported.sol': {
           content: `// SPDX-License-Identifier: MIT␊
@@ -626,7 +678,7 @@ Generated by [AVA](https://avajs.dev).
            ␊
           abstract contract HasFunction {␊
             enum $astId_id_random { dummy }␊
-            function foo() pure public returns (bool) {}␊
+            function foo() virtual  pure public returns (uint) ;␊
           }␊
           ␊
           abstract contract UsingFunction is HasFunction {␊

diff --git a/packages/core/src/utils/make-namespaced.test.ts.snap b/packages/core/src/utils/make-namespaced.test.ts.snap