Skip to content

Commit 49e7ae9

Browse files
ericglauericglau
authored
Remove non-struct NatSpec from namespaced storage layout compilation (#1051)
1 parent 13b6c12 commit 49e7ae9

File tree

12 files changed

+426
-37
lines changed

12 files changed

+426
-37
lines changed

‎packages/core/CHANGELOG.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
# Changelog
22

3-
## Unreleased
3+
## 1.35.0 (2024-07-31)
44

55
- Fix Hardhat compile error when public variables are used to implement interface functions. ([#1055](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/1055))
6+
- Remove non-struct NatSpec from Hardhat compilation step for namespaced storage layout validations. ([#1051](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/1051))
67

78
## 1.34.4 (2024-07-22)
89

‎packages/core/contracts/test/NamespacedToModify.sol‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,13 @@ contract Example {
1414
uint256 b;
1515
}
1616

17+
/// @custom:storage-location erc7201:example.with.following.comment
18+
// some comment
19+
struct StorageWithComment {
20+
uint256 a;
21+
uint256 b;
22+
}
23+
1724
/// @notice some natspec
1825
function foo() public {}
1926

‎packages/core/package.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@openzeppelin/upgrades-core",
3-
"version": "1.34.4",
3+
"version": "1.35.0",
44
"description": "",
55
"repository": "https://github.com/OpenZeppelin/openzeppelin-upgrades/tree/master/packages/core",
66
"license": "MIT",
@@ -61,6 +61,7 @@
6161
"typescript": "^5.0.0"
6262
},
6363
"dependencies": {
64+
"@nomicfoundation/slang": "^0.15.1",
6465
"cbor": "^9.0.0",
6566
"chalk": "^4.1.0",
6667
"compare-versions": "^6.0.0",

‎packages/core/src/utils/make-namespaced.test.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@ test('make namespaced input', async t => {
1616
await testMakeNamespaced(origBuildInfo, t, '0.8.20');
1717
});
1818

19+
test('make namespaced input - keep all natspec', async t => {
20+
const origBuildInfo = await artifacts.getBuildInfo('contracts/test/NamespacedToModifyImported.sol:Example');
21+
await testMakeNamespaced(origBuildInfo, t, '0.8.20', true);
22+
});
23+
1924
test('make namespaced input - solc 0.7', async t => {
2025
// The nameNamespacedInput function must work for different solc versions, since it is called before we check whether namespaces are used with solc >= 0.8.20
2126
const origBuildInfo = await artifacts.getBuildInfo('contracts/test/NamespacedToModify07.sol:HasFunction');
@@ -26,6 +31,7 @@ async function testMakeNamespaced(
2631
origBuildInfo: BuildInfo | undefined,
2732
t: ExecutionContext<unknown>,
2833
solcVersion: string,
34+
keepAllNatSpec = false,
2935
) {
3036
if (origBuildInfo === undefined) {
3137
throw new Error('Build info not found');
@@ -34,7 +40,11 @@ async function testMakeNamespaced(
3440
// Inefficient, but we want to test that we don't actually modify the original input object
3541
const origInput = JSON.parse(JSON.stringify(origBuildInfo.input));
3642

37-
const modifiedInput = makeNamespacedInput(origBuildInfo.input, origBuildInfo.output);
43+
const modifiedInput = makeNamespacedInput(
44+
origBuildInfo.input,
45+
origBuildInfo.output,
46+
keepAllNatSpec ? undefined : origBuildInfo.solcVersion,
47+
);
3848

3949
// Run hardhat compile on the modified input and make sure it has no errors
4050
const modifiedOutput = await hardhatCompile(modifiedInput, solcVersion);
@@ -53,6 +63,7 @@ function normalizeIdentifiers(input: SolcInput): void {
5363
source.content = source.content
5464
.replace(/\$MainStorage_\d{1,6}/g, '$MainStorage_random')
5565
.replace(/\$SecondaryStorage_\d{1,6}/g, '$SecondaryStorage_random')
66+
.replace(/\$StorageWithComment_\d{1,6}/g, '$StorageWithComment_random')
5667
.replace(/\$astId_\d+_\d{1,6}/g, '$astId_id_random');
5768
}
5869
}

‎packages/core/src/utils/make-namespaced.test.ts.md‎

Lines changed: 245 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,251 @@ Generated by [AVA](https://avajs.dev).
4444
uint256 a;␊
4545
uint256 b;␊
4646
} SecondaryStorage $SecondaryStorage_random;␊
47+
␊
48+
/// @custom:storage-location erc7201:example.with.following.comment␊
49+
// some comment␊
50+
struct StorageWithComment {␊
51+
uint256 a;␊
52+
uint256 b;␊
53+
} StorageWithComment $StorageWithComment_random;␊
54+
␊
55+
␊
56+
function foo() public {}␊
57+
␊
58+
␊
59+
function foo1(uint a) public {}␊
60+
␊
61+
␊
62+
function foo2(uint a) internal {}␊
63+
struct MyStruct { uint b; }␊
64+
␊
65+
// keccak256(abi.encode(uint256(keccak256("example.main")) - 1)) & ~bytes32(uint256(0xff));␊
66+
bytes32 private constant MAIN_STORAGE_LOCATION =␊
67+
0x183a6125c38840424c4a85fa12bab2ab606c4b6d0e7cc73c0c06ba5300eab500;␊
68+
␊
69+
function _getMainStorage() private pure returns (bool $) {}␊
70+
␊
71+
function _getXTimesY() internal view returns (bool) {}␊
72+
␊
73+
␊
74+
␊
75+
␊
76+
enum $astId_id_random { dummy }␊
77+
␊
78+
// standalone doc␊
79+
␊
80+
␊
81+
␊
82+
␊
83+
function foo3() public {}␊
84+
␊
85+
␊
86+
function foo4() public {}␊
87+
␊
88+
␊
89+
enum $astId_id_random { dummy }␊
90+
␊
91+
enum $astId_id_random { dummy }␊
92+
enum $astId_id_random { dummy }␊
93+
enum $astId_id_random { dummy }␊
94+
enum $astId_id_random { dummy }␊
95+
enum $astId_id_random { dummy }␊
96+
}␊
97+
␊
98+
abstract contract HasFunction {␊
99+
␊
100+
function foo() pure public returns (bool) {}␊
101+
}␊
102+
␊
103+
abstract contract UsingFunction is HasFunction {␊
104+
enum $astId_id_random { dummy }␊
105+
}␊
106+
␊
107+
function FreeFunctionUsingSelector() pure returns (bool) {}␊
108+
␊
109+
bytes4 constant CONSTANT_USING_SELECTOR = HasFunction.foo.selector;␊
110+
␊
111+
library Lib {␊
112+
function usingSelector() pure public returns (bool) {}␊
113+
␊
114+
function plusOne(uint x) pure public returns (bool) {}␊
115+
}␊
116+
␊
117+
abstract contract Consumer {␊
118+
enum $astId_id_random { dummy }␊
119+
␊
120+
function usingFreeFunction() pure public returns (bool) {}␊
121+
␊
122+
function usingConstant() pure public returns (bool) {}␊
123+
␊
124+
function usingLibraryFunction() pure public returns (bool) {}␊
125+
}␊
126+
␊
127+
abstract contract HasConstantWithSelector {␊
128+
bytes4 constant CONTRACT_CONSTANT_USING_SELECTOR = HasFunction.foo.selector;␊
129+
}␊
130+
␊
131+
function plusTwo(uint x) pure returns (bool) {}␊
132+
␊
133+
␊
134+
␊
135+
␊
136+
function plusThree(uint x) pure returns (bool) {}␊
137+
␊
138+
␊
139+
␊
140+
␊
141+
function plusThree(uint x, uint y) pure returns (bool) {}␊
142+
␊
143+
function originallyNoDocumentation() pure {}␊
144+
␊
145+
␊
146+
enum $astId_id_random { dummy }␊
147+
␊
148+
abstract contract UsingForDirectives {␊
149+
enum $astId_id_random { dummy }␊
150+
␊
151+
function usingFor(uint x) pure public returns (bool) {}␊
152+
}␊
153+
␊
154+
␊
155+
enum FreeEnum { MyEnum }␊
156+
␊
157+
␊
158+
enum CustomErrorOutsideContract { dummy }␊
159+
␊
160+
int8 constant MAX_SIZE_C = 2;␊
161+
␊
162+
abstract contract StructArrayUsesConstant {␊
163+
uint16 private constant MAX_SIZE = 10;␊
164+
␊
165+
struct NotNamespaced {␊
166+
uint16 a;␊
167+
uint256[MAX_SIZE] b;␊
168+
uint256[MAX_SIZE_C] c;␊
169+
}␊
170+
␊
171+
/// @custom:storage-location erc7201:uses.constant␊
172+
struct MainStorage {␊
173+
uint256 x;␊
174+
uint256[MAX_SIZE] y;␊
175+
uint256[MAX_SIZE_C] c;␊
176+
} MainStorage $MainStorage_random;␊
177+
}␊
178+
␊
179+
address constant MY_ADDRESS = address(0);␊
180+
uint constant CONVERTED_ADDRESS = uint160(MY_ADDRESS);␊
181+
␊
182+
interface IDummy {␊
183+
}␊
184+
␊
185+
abstract contract UsesAddress {␊
186+
IDummy public constant MY_CONTRACT = IDummy(MY_ADDRESS);␊
187+
}␊
188+
␊
189+
abstract contract HasFunctionWithRequiredReturn {␊
190+
struct S { uint x; }␊
191+
function foo(S calldata s) internal pure returns (bool) {}␊
192+
}␊
193+
␊
194+
␊
195+
function hasMultipleReturns() pure returns (bool, bool) {}␊
196+
␊
197+
␊
198+
function hasMultipleNamedReturns() pure returns (bool a, bool b) {}␊
199+
␊
200+
␊
201+
function hasReturnsDocumentedAsParams() pure returns (bool a, bool b) {}␊
202+
␊
203+
abstract contract HasNatSpecWithMultipleReturns {␊
204+
␊
205+
function hasMultipleReturnsInContract() public pure returns (bool, bool) {}␊
206+
␊
207+
␊
208+
function hasMultipleNamedReturnsInContract() public pure returns (bool a, bool b) {}␊
209+
␊
210+
␊
211+
function hasReturnsDocumentedAsParamsInContract() public pure returns (bool a, bool b) {}␊
212+
}␊
213+
␊
214+
interface IHasExternalViewFunction {␊
215+
function foo() external view returns (bool);␊
216+
}␊
217+
␊
218+
abstract contract HasExternalViewFunction is IHasExternalViewFunction {␊
219+
// This generates a getter function that conforms to the interface␊
220+
enum $astId_id_random { dummy }␊
221+
␊
222+
// References a selector in an interface␊
223+
bytes4 constant USING_INTERFACE_FUNCTION_SELECTOR = IHasExternalViewFunction.foo.selector;␊
224+
␊
225+
// References a getter generated for a public variable␊
226+
enum $astId_id_random { dummy }␊
227+
}␊
228+
␊
229+
abstract contract DeploysContractToImmutable {␊
230+
enum $astId_id_random { dummy }␊
231+
}`,
232+
},
233+
'contracts/test/NamespacedToModifyImported.sol': {
234+
content: `// SPDX-License-Identifier: MIT␊
235+
pragma solidity ^0.8.20;␊
236+
␊
237+
import {CONSTANT_USING_SELECTOR, plusTwo, plusThree, CustomErrorOutsideContract} from "./NamespacedToModify.sol";␊
238+
␊
239+
abstract contract Example {}␊
240+
`,
241+
},
242+
},
243+
}
244+
245+
## make namespaced input - keep all natspec
246+
247+
> Snapshot 1
248+
249+
{
250+
language: 'Solidity',
251+
settings: {
252+
evmVersion: 'paris',
253+
optimizer: {
254+
enabled: true,
255+
runs: 200,
256+
},
257+
outputSelection: {
258+
'*': {
259+
'': [
260+
'ast',
261+
],
262+
'*': [
263+
'storageLayout',
264+
],
265+
},
266+
},
267+
},
268+
sources: {
269+
'contracts/test/NamespacedToModify.sol': {
270+
content: `// SPDX-License-Identifier: MIT␊
271+
pragma solidity ^0.8.20;␊
272+
␊
273+
abstract contract Example {␊
274+
/// @custom:storage-location erc7201:example.main␊
275+
struct MainStorage {␊
276+
uint256 x;␊
277+
uint256 y;␊
278+
} MainStorage $MainStorage_random;␊
279+
␊
280+
/// @custom:storage-location erc7201:example.secondary␊
281+
struct SecondaryStorage {␊
282+
uint256 a;␊
283+
uint256 b;␊
284+
} SecondaryStorage $SecondaryStorage_random;␊
285+
␊
286+
/// @custom:storage-location erc7201:example.with.following.comment␊
287+
// some comment␊
288+
struct StorageWithComment {␊
289+
uint256 a;␊
290+
uint256 b;␊
291+
} StorageWithComment $StorageWithComment_random;␊
47292
␊
48293
/// @notice some natspec␊
49294
function foo() public {}␊

‎packages/core/src/utils/make-namespaced.test.ts.snap‎

256 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)