Conversation
🦋 Changeset detectedLatest commit: eaef43d The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
hardhatTotal size of the bundle: List of dependencies (sorted by size) |
75299f0 to
e59d2bb
Compare
| ...input, | ||
| settings: { | ||
| ...input.settings, | ||
| ...this.#extraSettings, |
There was a problem hiding this comment.
I don't know where to validate this: are the extra settings top-level like this?
adc48a5 to
7056461
Compare
There was a problem hiding this comment.
Pull request overview
Adds a new @nomicfoundation/hardhat-solx plugin package to enable using the experimental solx compiler (via a dedicated build profile) in Hardhat 3, including config/type extensions, download orchestration, and tests/docs.
Changes:
- Introduce the
hardhat-solxplugin with config + solidity hook handlers, a solx compiler wrapper, and a binary downloader. - Extend Hardhat config/types to register
solxas a compiler type and addsolxplugin configuration. - Add unit/integration tests, fixture project, and documentation; add new
hardhat-errorsdescriptors for solx-related errors.
Reviewed changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| v-next/hardhat-solx/tsconfig.json | New TS project config for the plugin package build. |
| v-next/hardhat-solx/test/solx-compiler.ts | Unit tests for the SolxCompiler wrapper behavior. |
| v-next/hardhat-solx/test/platform.ts | Unit tests for platform/asset naming helpers. |
| v-next/hardhat-solx/test/integration.ts | Basic integration coverage via creating an HRE with the plugin. |
| v-next/hardhat-solx/test/hook-handlers/solidity.ts | Tests for solidity hook handler behavior (download/getCompiler). |
| v-next/hardhat-solx/test/fixture-projects/simple/package.json | Fixture project package metadata for integration tests. |
| v-next/hardhat-solx/test/fixture-projects/simple/hardhat.config.ts | Fixture Hardhat config using a solx build profile. |
| v-next/hardhat-solx/test/fixture-projects/simple/contracts/Greeter.sol | Simple Solidity contract for fixture compilation context. |
| v-next/hardhat-solx/test/config.ts | Tests for plugin config validation/resolution behavior. |
| v-next/hardhat-solx/src/type-extensions.ts | Module augmentation adding solx compiler type + plugin config typing. |
| v-next/hardhat-solx/src/internal/solx-compiler.ts | Compiler implementation calling spawnCompile with solx binary. |
| v-next/hardhat-solx/src/internal/platform.ts | Platform/arch -> solx asset naming and unsupported-platform error. |
| v-next/hardhat-solx/src/internal/hook-handlers/solidity.ts | Hook handlers to download solx binaries and provide a solx Compiler. |
| v-next/hardhat-solx/src/internal/hook-handlers/config.ts | Hook handlers to validate/resolve config and enforce solx profile rules. |
| v-next/hardhat-solx/src/internal/downloader.ts | Binary cache path + download-with-retries implementation. |
| v-next/hardhat-solx/src/internal/constants.ts | Solx constants: supported versions/EVMs, defaults, and version map. |
| v-next/hardhat-solx/src/index.ts | Plugin entrypoint registering hook handlers and type extensions. |
| v-next/hardhat-solx/package.json | New publishable package manifest, scripts, and dependencies. |
| v-next/hardhat-solx/eslint.config.js | Package-level ESLint configuration wiring to repo config. |
| v-next/hardhat-solx/README.md | User documentation for installation/configuration/usage. |
| v-next/hardhat-solx/LICENSE | License for the new package. |
| v-next/hardhat-solx/.prettierignore | Prettier ignore rules for generated outputs and fixtures. |
| v-next/hardhat-solx/.gitignore | Git ignore rules for build/test outputs and fixtures. |
| v-next/hardhat-errors/src/descriptors.ts | New HARDHAT_SOLX error category + errors for platform/download failures. |
| pnpm-lock.yaml | Workspace lockfile updates adding the new package dependencies. |
| .peer-bumps.json | Record that the new plugin depends on new Hardhat hook APIs. |
| .changeset/warm-solx-plugin.md | Changeset publishing hardhat-solx and bumping hardhat-errors. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| try { | ||
| await hooks.downloadCompilers!(context, configs, true); | ||
| } catch { | ||
| // Expected — download fails in test environment | ||
| } |
There was a problem hiding this comment.
This test calls hooks.downloadCompilers with a solx config, which triggers a real network download attempt (3 retries + request timeouts) and then swallows the error. This will make CI flaky/slow and may even download binaries during tests. Prefer mocking downloadSolx/download (e.g. via dependency injection or an undici MockAgent) so the test is deterministic and offline.
| try { | |
| await hooks.downloadCompilers!(context, configs, true); | |
| } catch { | |
| // Expected — download fails in test environment | |
| } | |
| await assert.rejects( | |
| hooks.downloadCompilers!(context, configs, true), | |
| ); |
There was a problem hiding this comment.
will look at fixing this where it makes sense, to follow the same pattern like solc downloader tests.
| return { | ||
| ...resolvedConfig, | ||
| solidity: { | ||
| ...resolvedConfig.solidity, | ||
| registeredCompilerTypes: [ | ||
| ...resolvedConfig.solidity.registeredCompilerTypes, | ||
| SOLX_COMPILER_TYPE, | ||
| ], |
There was a problem hiding this comment.
registeredCompilerTypes is always appended with "solx" here, which can introduce duplicates if the plugin is registered more than once (or if another plugin also registers the same type). Consider deduping (e.g. check includes first or build a Set) before returning the resolved config.
| return { | |
| ...resolvedConfig, | |
| solidity: { | |
| ...resolvedConfig.solidity, | |
| registeredCompilerTypes: [ | |
| ...resolvedConfig.solidity.registeredCompilerTypes, | |
| SOLX_COMPILER_TYPE, | |
| ], | |
| const registeredCompilerTypes = resolvedConfig.solidity | |
| .registeredCompilerTypes.includes(SOLX_COMPILER_TYPE) | |
| ? resolvedConfig.solidity.registeredCompilerTypes | |
| : [ | |
| ...resolvedConfig.solidity.registeredCompilerTypes, | |
| SOLX_COMPILER_TYPE, | |
| ]; | |
| return { | |
| ...resolvedConfig, | |
| solidity: { | |
| ...resolvedConfig.solidity, | |
| registeredCompilerTypes, |
There was a problem hiding this comment.
i can take a look at fixing this.
7056461 to
12e7a3e
Compare
| .refine( | ||
| (data) => "path" in data || SUPPORTED_VERSIONS.includes(data.version), | ||
| { | ||
| message: `Solx only supports versions: ${SUPPORTED_VERSIONS.join(", ")}`, | ||
| path: ["version"], | ||
| }, | ||
| ); |
There was a problem hiding this comment.
The solx compiler version validation can be bypassed by setting a path property that is present but undefined (e.g. path: process.env.SOLX_PATH when the env var isn’t set). Because the refine uses "path" in data, any config with a path key will skip the supported-version check, which can lead to an unsupported Solidity version making it past validation and later triggering an internal invariant in the Solidity hook handler. Consider changing this refine to require a non-empty string path (e.g. typeof data.path === "string" && data.path.length > 0) before skipping the supported-versions check.
There was a problem hiding this comment.
I thin this doesn't make a ton of sense
There was a problem hiding this comment.
I actually went ahead and implemented this suggestion, i'll explain below on your other comment why i think this makes sense.
| assertHardhatInvariant( | ||
| await exists(compilerConfig.path), | ||
| `solx binary not found at ${compilerConfig.path} — the configured path does not exist`, | ||
| ); |
There was a problem hiding this comment.
Missing/invalid compilerConfig.path is a user configuration error, but it’s currently reported via assertHardhatInvariant, which surfaces as a CORE.INTERNAL.ASSERTION_ERROR. This makes a common misconfiguration look like an internal bug. Prefer throwing a dedicated HardhatError (ideally under HARDHAT_SOLX) or returning a resolved-config validation error so the user gets a proper config error category.
| assertHardhatInvariant( | |
| await exists(compilerConfig.path), | |
| `solx binary not found at ${compilerConfig.path} — the configured path does not exist`, | |
| ); | |
| const pathExists = await exists(compilerConfig.path); | |
| if (!pathExists) { | |
| throw new Error( | |
| `solx binary not found at ${compilerConfig.path} — the configured path does not exist`, | |
| ); | |
| } |
There was a problem hiding this comment.
Agreed with this comment, this should throw a HardhatError, as it can be a user mistake, more than an assertion violation.
| websiteDescription: `The solx compiler binary could not be downloaded from GitHub releases. | ||
|
|
||
| Check your internet connection and verify that the requested solx version exists.`, |
There was a problem hiding this comment.
The HARDHAT_SOLX.GENERAL.DOWNLOAD_FAILED website description says the binary couldn’t be downloaded from GitHub releases, but the implementation downloads from https://solx-releases-mirror.hardhat.org. Updating this text would avoid sending users to the wrong place when troubleshooting download failures.
| websiteDescription: `The solx compiler binary could not be downloaded from GitHub releases. | |
| Check your internet connection and verify that the requested solx version exists.`, | |
| websiteDescription: `The solx compiler binary could not be downloaded from the solx releases server used by Hardhat. | |
| Check your internet connection, ensure that the solx releases mirror (https://solx-releases-mirror.hardhat.org) is reachable from your environment, and verify that the requested solx version exists.`, |
| version: solxVersion, | ||
| attempts: DOWNLOAD_RETRY_COUNT.toString(), | ||
| reason: lastError?.message ?? "unknown error", | ||
| }, |
There was a problem hiding this comment.
| }, | |
| }, | |
| lastError, |
this keeps the error.cause chain, making errors easier to debug
| }) | ||
| .passthrough() | ||
| .refine( | ||
| (data) => "path" in data || SUPPORTED_VERSIONS.includes(data.version), |
There was a problem hiding this comment.
| (data) => "path" in data || SUPPORTED_VERSIONS.includes(data.version), | |
| (data) => SUPPORTED_VERSIONS.includes(data.version), |
I believe this is what you wanted, because later you have code to explicitly handle the custom path.
There was a problem hiding this comment.
I actually intentionally want us to not check supported versions when the compiler custom path is used , and i've added an additional check to make sure the path is not a string.Empty path . the reason is that for custom paths, users might be using a latest nightly build which supports a higher solidity version than the current solx plugin map. e.g. while 0.8.35 is in development in the nightly build and not released yet. i actually have unit tests specifically checking that this works, i think it is a legit use case (using a custom path to a compiler that supports a solidity version that's not officially in the supported map). thoughts?
alcuadrado
left a comment
There was a problem hiding this comment.
I left some minor comments, but LGTM
| const allPackageNames = (await readdir(packagesDir)).filter( | ||
| (file) => | ||
| ![ | ||
| "config", | ||
| "example-project", | ||
| "template-package", | ||
| "hardhat-test-utils", | ||
| "hardhat-solx", | ||
| ].includes(file), |
There was a problem hiding this comment.
readAllReleasablePackages is documented as returning packages that are released to npm, but this change adds hardhat-solx to the explicit exclusion list. That makes it impossible for release tooling that relies on this list to detect/version/publish the new plugin. If @nomicfoundation/hardhat-solx is intended to be published, remove it from this exclusion list; if it’s intended to stay internal, consider removing the changeset/peer-bump entry and updating the README installation instructions accordingly.
| { | ||
| "name": "@nomicfoundation/hardhat-solx", | ||
| "private": true, | ||
| "version": "2.0.0", | ||
| "description": "Hardhat plugin for using solx compiler in test builds", | ||
| "homepage": "https://github.com/NomicFoundation/hardhat/tree/main/packages/hardhat-solx", |
There was a problem hiding this comment.
This package is marked "private": true, which prevents publishing to npm, but the PR introduces a changeset and README instructions for npm install --save-dev @nomicfoundation/hardhat-solx. Please align these: either remove private (and ensure release tooling includes the package), or adjust docs/changesets to reflect that the plugin is not meant to be installed from npm.
| ```bash | ||
| npm install --save-dev @nomicfoundation/hardhat-solx | ||
| ``` | ||
|
|
||
| Then add the plugin to your `hardhat.config.ts` and create a `solx` build profile. You must use the build profiles config format, which requires both a `default` and a `solx` profile: | ||
|
|
||
| ```typescript | ||
| import { defineConfig } from "hardhat/config"; | ||
| import hardhatSolx from "@nomicfoundation/hardhat-solx"; |
There was a problem hiding this comment.
The Installation section suggests installing @nomicfoundation/hardhat-solx from npm, but the new package is currently marked private and is excluded from release tooling (scripts/lib/packages.ts). Either make the package publishable, or update these instructions to match the intended distribution method.
| ```bash | |
| npm install --save-dev @nomicfoundation/hardhat-solx | |
| ``` | |
| Then add the plugin to your `hardhat.config.ts` and create a `solx` build profile. You must use the build profiles config format, which requires both a `default` and a `solx` profile: | |
| ```typescript | |
| import { defineConfig } from "hardhat/config"; | |
| import hardhatSolx from "@nomicfoundation/hardhat-solx"; | |
| This plugin is currently experimental and is not published to npm. It is intended to be used from within the Hardhat monorepo or from source. | |
| Once the plugin is available in your project, add it to your `hardhat.config.ts` and create a `solx` build profile. You must use the build profiles config format, which requires both a `default` and a `solx` profile: | |
| ```typescript | |
| import { defineConfig } from "hardhat/config"; | |
| import hardhatSolx from "@nomicfoundation/hardhat-solx"; | |
| import { defineConfig } from "hardhat/config"; | |
| import hardhatSolx from "@nomicfoundation/hardhat-solx"; |
|
|
||
| it("does not mutate compiler config paths", async () => { | ||
| const hookHandlerModule = await import( | ||
| "../../src/internal/hook-handlers/solidity.js" | ||
| ); | ||
| const hooks = await hookHandlerModule.default(); | ||
|
|
||
| const context = { config: {} } as any; | ||
|
|
||
| const configs: SolidityCompilerConfig[] = [ | ||
| createSolidityCompilerConfig({ type: "solx", version: "0.8.33" }), | ||
| ]; | ||
|
|
||
| // This will fail to download (no network in tests), but we can | ||
| // verify via the error that it tries and that path is not mutated. | ||
| // For a true unit test we'd mock downloadSolx, but for now just | ||
| // check the path isn't set before the download attempt. | ||
| const originalPath = configs[0].path; | ||
|
|
||
| try { | ||
| await hooks.downloadCompilers!(context, configs, true); | ||
| } catch { | ||
| // Expected — download fails in test environment | ||
| } | ||
|
|
||
| assert.equal( | ||
| configs[0].path, | ||
| originalPath, | ||
| "compiler config path should not be mutated", | ||
| ); | ||
| }); |
There was a problem hiding this comment.
This test triggers a real downloadCompilers run for a solx config (which attempts network downloads with retries) and then swallows any error. That makes the test suite slow/flaky and can unexpectedly download binaries in CI. Prefer mocking downloadSolx/download (e.g. via dependency injection or a request MockAgent) and asserting on the expected behavior without doing real I/O.
| it("does not mutate compiler config paths", async () => { | |
| const hookHandlerModule = await import( | |
| "../../src/internal/hook-handlers/solidity.js" | |
| ); | |
| const hooks = await hookHandlerModule.default(); | |
| const context = { config: {} } as any; | |
| const configs: SolidityCompilerConfig[] = [ | |
| createSolidityCompilerConfig({ type: "solx", version: "0.8.33" }), | |
| ]; | |
| // This will fail to download (no network in tests), but we can | |
| // verify via the error that it tries and that path is not mutated. | |
| // For a true unit test we'd mock downloadSolx, but for now just | |
| // check the path isn't set before the download attempt. | |
| const originalPath = configs[0].path; | |
| try { | |
| await hooks.downloadCompilers!(context, configs, true); | |
| } catch { | |
| // Expected — download fails in test environment | |
| } | |
| assert.equal( | |
| configs[0].path, | |
| originalPath, | |
| "compiler config path should not be mutated", | |
| ); | |
| }); |
| const { getSolxBinaryPath } = await import( | ||
| "../../src/internal/downloader.js" | ||
| ); | ||
| const { exists } = await import("@nomicfoundation/hardhat-utils/fs"); | ||
|
|
||
| // Use the cached solx binary if available, skip otherwise | ||
| const cachedPath = await getSolxBinaryPath("0.1.3"); | ||
| if (!(await exists(cachedPath))) { | ||
| return; | ||
| } | ||
|
|
||
| const hookHandlerModule = await import( | ||
| "../../src/internal/hook-handlers/solidity.js" | ||
| ); | ||
| const hooks = await hookHandlerModule.default(); | ||
|
|
||
| const context = { config: {} } as any; | ||
| const compilerConfig = createSolidityCompilerConfig({ | ||
| type: "solx", | ||
| version: "0.8.33", | ||
| path: cachedPath, | ||
| }); | ||
| const mockNext = createGetCompilerMockNext(); | ||
|
|
||
| const compiler = await hooks.getCompiler!( | ||
| context, | ||
| compilerConfig, | ||
| mockNext.next, | ||
| ); | ||
|
|
||
| assert.ok( | ||
| !mockNext.wasCalled(), | ||
| "next should NOT have been called for solx type", | ||
| ); | ||
| assert.equal(compiler.compilerPath, cachedPath); | ||
| // Version should be parsed from the binary, not from config | ||
| assert.equal(compiler.version, "0.1.3"); | ||
| assert.equal(compiler.longVersion, "0.1.3+solx"); |
There was a problem hiding this comment.
This test becomes a no-op when the solx binary isn’t already present in the global cache (return if the cached path doesn’t exist), which means CI may not exercise the custom-path code path at all. Consider making the test deterministic by stubbing exists/execFile (or injecting a version lookup function) so it always validates the behavior without relying on external cache state.
| const { getSolxBinaryPath } = await import( | |
| "../../src/internal/downloader.js" | |
| ); | |
| const { exists } = await import("@nomicfoundation/hardhat-utils/fs"); | |
| // Use the cached solx binary if available, skip otherwise | |
| const cachedPath = await getSolxBinaryPath("0.1.3"); | |
| if (!(await exists(cachedPath))) { | |
| return; | |
| } | |
| const hookHandlerModule = await import( | |
| "../../src/internal/hook-handlers/solidity.js" | |
| ); | |
| const hooks = await hookHandlerModule.default(); | |
| const context = { config: {} } as any; | |
| const compilerConfig = createSolidityCompilerConfig({ | |
| type: "solx", | |
| version: "0.8.33", | |
| path: cachedPath, | |
| }); | |
| const mockNext = createGetCompilerMockNext(); | |
| const compiler = await hooks.getCompiler!( | |
| context, | |
| compilerConfig, | |
| mockNext.next, | |
| ); | |
| assert.ok( | |
| !mockNext.wasCalled(), | |
| "next should NOT have been called for solx type", | |
| ); | |
| assert.equal(compiler.compilerPath, cachedPath); | |
| // Version should be parsed from the binary, not from config | |
| assert.equal(compiler.version, "0.1.3"); | |
| assert.equal(compiler.longVersion, "0.1.3+solx"); | |
| const downloaderModule = await import( | |
| "../../src/internal/downloader.js" | |
| ); | |
| const fsModule = await import("@nomicfoundation/hardhat-utils/fs"); | |
| const childProcessModule = await import("node:child_process"); | |
| const originalGetSolxBinaryPath = (downloaderModule as any) | |
| .getSolxBinaryPath; | |
| const originalExists = (fsModule as any).exists; | |
| const originalExecFile = (childProcessModule as any).execFile; | |
| const fakePath = "/fake/path/to/solx"; | |
| (downloaderModule as any).getSolxBinaryPath = async () => fakePath; | |
| (fsModule as any).exists = async () => true; | |
| (childProcessModule as any).execFile = ( | |
| _file: string, | |
| _args: string[] | undefined, | |
| callback: (error: Error | null, stdout: string, stderr: string) => void, | |
| ) => { | |
| callback(null, "0.1.3+solx", ""); | |
| }; | |
| try { | |
| const hookHandlerModule = await import( | |
| "../../src/internal/hook-handlers/solidity.js" | |
| ); | |
| const hooks = await hookHandlerModule.default(); | |
| const context = { config: {} } as any; | |
| const compilerConfig = createSolidityCompilerConfig({ | |
| type: "solx", | |
| version: "0.8.33", | |
| path: fakePath, | |
| }); | |
| const mockNext = createGetCompilerMockNext(); | |
| const compiler = await hooks.getCompiler!( | |
| context, | |
| compilerConfig, | |
| mockNext.next, | |
| ); | |
| assert.ok( | |
| !mockNext.wasCalled(), | |
| "next should NOT have been called for solx type", | |
| ); | |
| assert.equal(compiler.compilerPath, fakePath); | |
| // Version should be parsed from the binary, not from config | |
| assert.equal(compiler.version, "0.1.3"); | |
| assert.equal(compiler.longVersion, "0.1.3+solx"); | |
| } finally { | |
| (downloaderModule as any).getSolxBinaryPath = originalGetSolxBinaryPath; | |
| (fsModule as any).exists = originalExists; | |
| (childProcessModule as any).execFile = originalExecFile; | |
| } |
Summary
Adds
@nomicfoundation/hardhat-solxplugin that enables adding solx for test builds in Hardhat 3.The plugin extends
SolidityCompilerType,SolidityCompilerConfigaddingsolxtypes, and hooks intovalidateUserConfig,downloadCompilersandgetCompilerto orchestrate the compilation flow for solx.The plugin handles supported versions and supported EVM versions, and validates solx configs.
It requires adding a
solxbuild profile that can be used when running build or tests.Downloads from
solx-releases-mirror.hardhat.orgwith retry logic matching the solc downloader patternBuilt on top of #8008 and #8009.