Upgrade slang to 0.18.3 (#1102)

Co-authored-by: OmarTawfik <15987992+OmarTawfik@users.noreply.github.com>

packages/core/CHANGELOG.md

@@ -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))

packages/core/package.json

@@ -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",
@@ -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",

packages/core/src/index.ts

@@ -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';

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

@@ -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';
@@ -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);

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

@@ -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)) {
@@ -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)) {
+    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 {

packages/plugin-hardhat/CHANGELOG.md

@@ -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))

packages/plugin-hardhat/package.json

@@ -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",
@@ -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",

packages/plugin-hardhat/src/index.ts

@@ -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'
+  );
 
   try {
     await readValidations(hre);
@@ -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);
@@ -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);
   });
 }

tsconfig.base.json

@@ -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,