Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix Hardhat compile error when overriding interface functions with public constant variables #1091

Merged
merged 6 commits into from
Oct 10, 2024
Merged

Fix Hardhat compile error when overriding interface functions with public constant variables #1091

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c492352
Convert functions to virtual where possible
ericglau Oct 8, 2024
6b20d76
Update other snapshots
ericglau Oct 8, 2024
5882f70
Add asserts
ericglau Oct 8, 2024
9bc25f0
Fix order of code edits, clarify logic
ericglau Oct 8, 2024
2dea4fc
Add changelog, test with modifier, fix comment order, add assert
ericglau Oct 8, 2024
ff7c8f0
Simplify tests
ericglau Oct 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/core/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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))
Expand Down
40 changes: 39 additions & 1 deletion packages/core/contracts/test/NamespacedToModify.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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");
}
}
Binary file modified packages/core/src/cli/cli.test.ts.snap
View file
Open in desktop
Binary file not shown.
Binary file modified packages/core/src/cli/validate/project-report.test.ts.snap
View file
Open in desktop
Binary file not shown.
Binary file modified packages/core/src/cli/validate/validate-upgrade-safety.test.ts.snap
View file
Open in desktop
Binary file not shown.
Binary file modified packages/core/src/cli/validate/validations.test.ts.snap
View file
Open in desktop
Binary file not shown.
Binary file modified packages/core/src/scripts/migrate-oz-cli-project.test.ts.snap
View file
Open in desktop
Binary file not shown.
Binary file modified packages/core/src/storage-0.8.test.ts.snap
View file
Open in desktop
Binary file not shown.
Binary file modified packages/core/src/storage-memory-0.5.test.ts.snap
View file
Open in desktop
Binary file not shown.
Binary file modified packages/core/src/storage-namespaced-conflicts-layout.test.ts.snap
View file
Open in desktop
Binary file not shown.
Binary file modified packages/core/src/storage-namespaced-conflicts.test.ts.snap
View file
Open in desktop
Binary file not shown.
Binary file modified packages/core/src/storage-namespaced-layout.test.ts.snap
View file
Open in desktop
Binary file not shown.
Binary file modified packages/core/src/storage-namespaced.test.ts.snap
View file
Open in desktop
Binary file not shown.
Binary file modified packages/core/src/storage.test.ts.snap
View file
Open in desktop
Binary file not shown.
Binary file modified packages/core/src/storage/report-gap.test.ts.snap
View file
Open in desktop
Binary file not shown.
Binary file modified packages/core/src/storage/report-rename-retype.test.ts.snap
View file
Open in desktop
Binary file not shown.
Binary file modified packages/core/src/storage/report.test.ts.snap
View file
Open in desktop
Binary file not shown.
126 changes: 89 additions & 37 deletions packages/core/src/utils/make-namespaced.test.ts.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));␊
Expand All @@ -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) ;␊
␊
␊
␊
Expand All @@ -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 }␊
Expand All @@ -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 {␊
Expand All @@ -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 {␊
Expand All @@ -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) ;␊
}␊
␊
␊
Expand Down Expand Up @@ -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) {}␊
}␊
␊
␊
Expand All @@ -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 {␊
Expand All @@ -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;␊
}␊
Expand All @@ -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␊
Expand Down Expand Up @@ -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));␊
Expand All @@ -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␊
␊
Expand All @@ -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␊
Expand All @@ -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 {␊
Expand All @@ -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 {␊
Expand Down Expand Up @@ -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) ;␊
}␊
␊
/**␊
Expand Down Expand Up @@ -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) {}␊
}␊
␊
/**␊
Expand All @@ -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 {␊
Expand All @@ -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;␊
}␊
Expand All @@ -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␊
Expand Down Expand Up @@ -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 {␊
Expand Down
Binary file modified packages/core/src/utils/make-namespaced.test.ts.snap
View file
Open in desktop
Binary file not shown.
Loading
Loading