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.

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

Upgrade slang to 0.18.3 #1102

Merged
merged 7 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions packages/core/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# Changelog

## 1.41.0 (2024-11-25)

- Update Slang dependency to 0.18.3. ([#1102](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/1102))
- Improves reliability of Hardhat compilation step for namespaced storage layout validations when using Solidity versions 0.8.27 and 0.8.28.

## 1.40.0 (2024-10-10)

- Fix Hardhat compile error when overriding interface functions with public constant variables. ([#1091](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/1091))
Expand Down
4 changes: 2 additions & 2 deletions packages/core/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@openzeppelin/upgrades-core",
"version": "1.40.0",
"version": "1.41.0",
"description": "",
"repository": "https://github.com/OpenZeppelin/openzeppelin-upgrades/tree/master/packages/core",
"license": "MIT",
Expand Down Expand Up @@ -61,7 +61,7 @@
"typescript": "^5.0.0"
},
"dependencies": {
"@nomicfoundation/slang": "^0.17.0",
"@nomicfoundation/slang": "^0.18.3",
"cbor": "^9.0.0",
"chalk": "^4.1.0",
"compare-versions": "^6.0.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export {
export { ValidateUpgradeSafetyOptions, validateUpgradeSafety, ProjectReport, ReferenceContractNotFound } from './cli';

export { getUpgradeInterfaceVersion } from './upgrade-interface-version';
export { makeNamespacedInput } from './utils/make-namespaced';
export { makeNamespacedInput, trySanitizeNatSpec } from './utils/make-namespaced';
export { isNamespaceSupported } from './storage/namespace';
export { inferProxyAdmin } from './infer-proxy-admin';
export { assertUnreachable } from './utils/assert';
11 changes: 5 additions & 6 deletions packages/core/src/utils/make-namespaced.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
TASK_COMPILE_SOLIDITY_RUN_SOLCJS,
} from 'hardhat/builtin-tasks/task-names';

import { makeNamespacedInput } from './make-namespaced';
import { makeNamespacedInput, trySanitizeNatSpec } from './make-namespaced';
import { SolcBuild } from 'hardhat/types/builtin-tasks';
import { SolcInput, SolcOutput } from '../solc-api';
import { BuildInfo } from 'hardhat/types';
Expand Down Expand Up @@ -40,11 +40,10 @@ async function testMakeNamespaced(
// Inefficient, but we want to test that we don't actually modify the original input object
const origInput = JSON.parse(JSON.stringify(origBuildInfo.input));

const modifiedInput = makeNamespacedInput(
origBuildInfo.input,
origBuildInfo.output,
keepAllNatSpec ? undefined : origBuildInfo.solcVersion,
);
let modifiedInput = makeNamespacedInput(origBuildInfo.input, origBuildInfo.output, origBuildInfo.solcVersion);
if (!keepAllNatSpec) {
modifiedInput = await trySanitizeNatSpec(modifiedInput, origBuildInfo.solcVersion);
}

// Run hardhat compile on the modified input and make sure it has no errors
const modifiedOutput = await hardhatCompile(modifiedInput, solcVersion);
Expand Down
105 changes: 54 additions & 51 deletions packages/core/src/utils/make-namespaced.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ const OUTPUT_SELECTION = {
*
* @param input The original solc input.
* @param output The original solc output.
* @param solcVersion The version of the solc compiler that was originally used to compile the input.
* @param _solcVersion The version of the solc compiler that was originally used to compile the input. This argument is no longer used and is kept for backwards compatibility.
* @returns The modified solc input with storage layout that includes namespaced type information.
*/
export function makeNamespacedInput(input: SolcInput, output: SolcOutput, solcVersion?: string): SolcInput {
export function makeNamespacedInput(input: SolcInput, output: SolcOutput, _solcVersion?: string): SolcInput {
const modifiedSources: Record<string, { content?: string }> = {};

for (const [sourcePath] of Object.entries(input.sources)) {
Expand Down Expand Up @@ -163,74 +163,77 @@ export function makeNamespacedInput(input: SolcInput, output: SolcOutput, solcVe
}
}

const modifiedSource = tryRemoveNonStructNatSpec(getModifiedSource(orig, modifications), solcVersion);

modifiedSources[sourcePath] = { ...source, content: modifiedSource };
modifiedSources[sourcePath] = { ...source, content: getModifiedSource(orig, modifications) };
}

return { ...input, sources: modifiedSources, settings: { ...input.settings, outputSelection: OUTPUT_SELECTION } };
}

/**
* If we have the compiler version available and Slang is supported for the current platform and compiler version,
* use Slang to parse and remove all NatSpec comments that do not precede a struct definition and return the modified content.
* Attempts to remove all NatSpec comments that do not precede a struct definition from the input source contents.
* Directly modifies the input source contents, and also returns the modified input.
*
* If the solc version is not supported by the parser, the original content is kept.
*
* @param solcInput Solc input.
* @param solcVersion The version of the solc compiler that was originally used to compile the input.
* @returns The modified solc input with NatSpec comments removed where they do not precede a struct definition.
*/
export async function trySanitizeNatSpec(solcInput: SolcInput, solcVersion: string): Promise<SolcInput> {
for (const [sourcePath, source] of Object.entries(solcInput.sources)) {
if (source.content !== undefined) {
solcInput.sources[sourcePath].content = await tryRemoveNonStructNatSpec(source.content, solcVersion);
}
}
return solcInput;
}

/**
* If Slang is supported for the current compiler version, use Slang to parse and remove all NatSpec comments
* that do not precede a struct definition and return the modified content.
*
* Otherwise, return the original content.
*/
function tryRemoveNonStructNatSpec(origContent: string, solcVersion: string | undefined): string {
const natSpecRemovals: Modification[] = [];
async function tryRemoveNonStructNatSpec(origContent: string, solcVersion: string): Promise<string> {
if (solcVersion === undefined) {
return origContent;
}

if (solcVersion !== undefined && tryRequire('@nomicfoundation/slang') && slangSupportsVersion(solcVersion)) {
/* eslint-disable @typescript-eslint/no-var-requires */
const { Language } = require('@nomicfoundation/slang/language');
const { NonterminalKind, TerminalKind } = require('@nomicfoundation/slang/kinds');
const { TerminalNode } = require('@nomicfoundation/slang/cst');
const { isTrivia } = require('./slang/trivia');
/* eslint-enable @typescript-eslint/no-var-requires */
const { Parser } = await import('@nomicfoundation/slang/parser');
if (!Parser.supportedVersions().includes(solcVersion)) {
ericglau marked this conversation as resolved.
Show resolved Hide resolved
return origContent;
}

const { TerminalKind, TerminalKindExtensions } = await import('@nomicfoundation/slang/cst');

const language = new Language(solcVersion);
const parseOutput = language.parse(NonterminalKind.SourceUnit, origContent);
const cursor = parseOutput.createTreeCursor();
const parser = Parser.create(solcVersion);
const parseOutput = parser.parse(Parser.rootKind(), origContent);
const cursor = parseOutput.createTreeCursor();

const natSpecRemovals: Modification[] = [];

while (
cursor.goToNextTerminalWithKinds([TerminalKind.MultiLineNatSpecComment, TerminalKind.SingleLineNatSpecComment])
) {
// Lookahead to determine if the next non-trivia node is the struct keyword.
// If so, this NatSpec is part of the struct definition and should not be removed.
const lookaheadCursor = cursor.clone();
while (
cursor.goToNextTerminalWithKinds([TerminalKind.MultiLineNatSpecComment, TerminalKind.SingleLineNatSpecComment])
lookaheadCursor.goToNextTerminal() &&
lookaheadCursor.node.isTerminalNode() &&
TerminalKindExtensions.isTrivia(lookaheadCursor.node.kind)
) {
// Lookahead to determine if the next non-trivia node is the struct keyword.
// If so, this NatSpec is part of the struct definition and should not be removed.
const lookaheadCursor = cursor.clone();
while (lookaheadCursor.goToNextTerminal() && isTrivia(lookaheadCursor.node())) {
// skip over trivia nodes
}
if (lookaheadCursor.node().kind === TerminalKind.StructKeyword) {
continue;
}
// skip over trivia nodes
}

const triviaNode = cursor.node();
assert(triviaNode instanceof TerminalNode);
natSpecRemovals.push(makeDeleteRange(cursor.textRange.start.utf8, cursor.textRange.end.utf8));
if (lookaheadCursor.node.kind === TerminalKind.StructKeyword) {
continue;
}
return getModifiedSource(Buffer.from(origContent, 'utf8'), natSpecRemovals);
} else {
return origContent;
}
}

function tryRequire(id: string) {
try {
require(id);
return true;
} catch (e: any) {
// do nothing
natSpecRemovals.push(makeDeleteRange(cursor.textRange.start.utf8, cursor.textRange.end.utf8));
}
return false;
}

function slangSupportsVersion(solcVersion: string): boolean {
/* eslint-disable @typescript-eslint/no-var-requires */
const { Language } = require('@nomicfoundation/slang/language');
/* eslint-enable @typescript-eslint/no-var-requires */

return Language.supportedVersions().includes(solcVersion);
return getModifiedSource(Buffer.from(origContent, 'utf8'), natSpecRemovals);
}

interface Modification {
Expand Down
23 changes: 0 additions & 23 deletions packages/core/src/utils/slang/trivia.ts

This file was deleted.

5 changes: 5 additions & 0 deletions packages/plugin-hardhat/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# Changelog

## 3.6.0 (2024-11-25)

- Update Slang dependency to 0.18.3. ([#1102](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/1102))
- Improves reliability of Hardhat compilation step for namespaced storage layout validations when using Solidity versions 0.8.27 and 0.8.28.

## 3.5.0 (2024-10-10)

- Support ignoring Hardhat compile errors when extracting detailed namespaced storage layouts for validations. ([#1090](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/1090))
Expand Down
4 changes: 2 additions & 2 deletions packages/plugin-hardhat/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@openzeppelin/hardhat-upgrades",
"version": "3.5.0",
"version": "3.6.0",
"description": "",
"repository": "https://github.com/OpenZeppelin/openzeppelin-upgrades/tree/master/packages/plugin-hardhat",
"license": "MIT",
Expand Down Expand Up @@ -38,7 +38,7 @@
"@openzeppelin/defender-sdk-base-client": "^1.14.4",
"@openzeppelin/defender-sdk-deploy-client": "^1.14.4",
"@openzeppelin/defender-sdk-network-client": "^1.14.4",
"@openzeppelin/upgrades-core": "^1.40.0",
"@openzeppelin/upgrades-core": "^1.41.0",
"chalk": "^4.1.0",
"debug": "^4.1.1",
"ethereumjs-util": "^7.1.5",
Expand Down
18 changes: 10 additions & 8 deletions packages/plugin-hardhat/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,9 @@ interface RunCompilerArgs {
}

subtask(TASK_COMPILE_SOLIDITY, async (args: { force: boolean }, hre, runSuper) => {
const { readValidations, ValidationsCacheOutdated, ValidationsCacheNotFound } = await import('./utils/validations');
const { readValidations, ValidationsCacheOutdated, ValidationsCacheNotFound } = await import(
'./utils/validations.js'
ernestognw marked this conversation as resolved.
Show resolved Hide resolved
);

try {
await readValidations(hre);
Expand All @@ -85,21 +87,21 @@ subtask(TASK_COMPILE_SOLIDITY, async (args: { force: boolean }, hre, runSuper) =
});

subtask(TASK_COMPILE_SOLIDITY_COMPILE, async (args: RunCompilerArgs, hre, runSuper) => {
const { isNamespaceSupported, validate, solcInputOutputDecoder, makeNamespacedInput } = await import(
'@openzeppelin/upgrades-core'
);
const { writeValidations } = await import('./utils/validations');
const { isNamespaceSupported, validate, solcInputOutputDecoder, makeNamespacedInput, trySanitizeNatSpec } =
await import('@openzeppelin/upgrades-core');
const { writeValidations } = await import('./utils/validations.js');

// TODO: patch input
const { output, solcBuild } = await runSuper();

const { isFullSolcOutput } = await import('./utils/is-full-solc-output');
const { isFullSolcOutput } = await import('./utils/is-full-solc-output.js');
if (isFullSolcOutput(output)) {
const decodeSrc = solcInputOutputDecoder(args.input, output);

let namespacedOutput = undefined;
if (isNamespaceSupported(args.solcVersion)) {
const namespacedInput = makeNamespacedInput(args.input, output, args.solcVersion);
let namespacedInput = makeNamespacedInput(args.input, output, args.solcVersion);
namespacedInput = await trySanitizeNatSpec(namespacedInput, args.solcVersion);
namespacedOutput = (await runSuper({ ...args, quiet: true, input: namespacedInput })).output;

const namespacedCompileErrors = getNamespacedCompileErrors(namespacedOutput);
Expand Down Expand Up @@ -210,7 +212,7 @@ extendConfig((config: HardhatConfig) => {

if (tryRequire('@nomicfoundation/hardhat-verify')) {
subtask('verify:etherscan').setAction(async (args, hre, runSuper) => {
const { verify } = await import('./verify-proxy');
const { verify } = await import('./verify-proxy.js');
return await verify(args, hre, runSuper);
});
}
Expand Down
4 changes: 2 additions & 2 deletions tsconfig.base.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
"compilerOptions": {
"incremental": true,
"target": "es2020",
"module": "commonjs",
"module": "nodenext",
"strict": true,
"moduleResolution": "node",
"moduleResolution": "nodenext",
"esModuleInterop": true,
"sourceMap": true,
"declaration": true,
Expand Down
Loading
Loading