Migration of hardhat-upgrade to plugin to use hardhat v3#1194
Open
Migration of hardhat-upgrade to plugin to use hardhat v3#1194
Conversation
ericglau
reviewed
Oct 21, 2025
packages/plugin-hardhat/src/index.ts
Outdated
| @@ -1,29 +1,37 @@ | |||
| /* eslint-disable @typescript-eslint/no-var-requires */ | |||
| // TODO figure out what is the newer, TASK_COMPILE_SOLIDITY and TASK_COMPILE_SOLIDITY_COMPILE | |||
Member
There was a problem hiding this comment.
These are Hardhat internal subtasks, where Hardhat calls TASK_COMPILE_SOLIDITY -> TASK_COMPILE_SOLIDITY_COMPILE_JOBS -> TASK_COMPILE_SOLIDITY_COMPILE_JOB -> TASK_COMPILE_SOLIDITY_COMPILE. And only certain parts of those need to be overridden in our plugin. See https://github.com/NomicFoundation/hardhat/blob/09951afd78e49465f7949478919c9217a12e8338/packages/hardhat-core/src/builtin-tasks/compile.ts#L1366 for the v2 internal subtasks.
We may need to find the equivalents in v3.
Author
There was a problem hiding this comment.
On it already.
it seems that there is no such function named subtask anymore, so I'm trying to find what is the alternative right now, so we can figure out how to do what we did before but in hardhat3.
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. Ignoring alerts on:
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis pull request migrates the OpenZeppelin Hardhat Upgrades plugin from Hardhat 2 to Hardhat 3, upgrading Node.js from 20 to 22, converting the codebase to ES modules, introducing a connection-based API architecture, adding Hardhat hooks for config and Solidity compilation, and including comprehensive examples and test fixtures for upgrade workflows. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes The PR represents a major architectural migration with high heterogeneity: ESM conversion across 100+ files, connection-based API refactoring throughout core modules, new hook system implementation, Defender integration updates, comprehensive example projects, and extensive test file migrations. While many test file changes follow repetitive patterns (reducing per-file review burden), the core plugin logic changes, new hooks system, and API factory require careful analysis of control flow and backward compatibility implications. Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
afa7789
force-pushed
the
migrate/hardhat-3
branch
from
7c7e87d to
5a86473
Compare
packages changes and initial config in accord to migration documentation.
…rrors for ESM structure.
importing errors + a few todos, change of Imports + new functions to use hardhat3 library in plugin files.
…ig for NodeNext module resolution
…xtensions for Hardhat compatibility ( still have to test it )
…nhance artifact cleanup process
…out support in Solidity hooks
…ns for defender integration
…entation address tests
- Updated deploy and upgrade functions across multiple files to accept a NetworkConnection parameter.
…solidity, etherscan-api, and verify-proxy files
…for better context management
Member
|
@SocketSecurity ignore-all |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/plugin-hardhat/test/import-with-deploy-v5.js (1)
91-91:⚠️ Potential issue | 🟡 MinorIncorrect assertion: comparing boolean to string.
The expression
implAddrUpgraded2 !== implAddrUpgradedevaluates to a boolean, sot.not(boolean, implAddrUpgraded2)compares a boolean against a string address—this will always pass regardless of the actual values.The intended assertion is likely to verify the implementation address changed after upgrading to a different contract.
🐛 Proposed fix
- t.not(implAddrUpgraded2 !== implAddrUpgraded, implAddrUpgraded2); + t.not(implAddrUpgraded2, implAddrUpgraded, 'implementation address should change after upgrading to different contract');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/import-with-deploy-v5.js` at line 91, The assertion currently compares a boolean to a string (t.not(implAddrUpgraded2 !== implAddrUpgraded, implAddrUpgraded2)); change it to directly assert the two addresses are different by calling t.not(implAddrUpgraded2, implAddrUpgraded) (or the equivalent t.notEqual) so the test actually verifies that implAddrUpgraded2 and implAddrUpgraded are not the same.packages/plugin-hardhat/test/propose-upgrade-with-approval-uups.js (1)
95-98:⚠️ Potential issue | 🟡 MinorUse
proposal2.txResponse.wait()for the second assertion path.At Line 97, the test waits on
proposal.txResponseagain, so it doesn’t validate that the second call’s response object is usable.✅ Suggested fix
- const txReceipt2 = await proposal.txResponse.wait(); + const txReceipt2 = await proposal2.txResponse.wait(); t.is(txReceipt2.contractAddress, txReceipt.contractAddress);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/propose-upgrade-with-approval-uups.js` around lines 95 - 98, The test currently waits on proposal.txResponse twice instead of waiting on the second proposal's response; change the second wait to call proposal2.txResponse.wait() so the assertion verifies proposal2.txResponse is usable (update the code around the t.is(proposal2.txResponse.hash, proposal.txResponse.hash) block to call await proposal2.txResponse.wait() when assigning txReceipt2 and then assert txReceipt2.contractAddress against txReceipt.contractAddress).packages/plugin-hardhat/test/beacon-upgrade-validation.js (1)
25-28:⚠️ Potential issue | 🔴 CriticalThe test expects the wrong error;
upgradeProxyis not intended for beacon proxies.The test calls
upgradeProxy(greeter, Invalid)on a beacon proxy and expects the error'Contract Invalid is not upgrade safe'. However, based on thebeacon-upgrade-block-proxy.jstest suite,upgradeProxyshould reject beacon proxies outright with the error'Beacon proxies are not supported with the current function'before validating the implementation.The test should either:
- Use
upgradeBeacon(beacon, Invalid)instead to properly test beacon upgrades (consistent with all other beacon tests), or- Change the expected error message to match what
upgradeProxyactually throws for beacon proxiesAll other beacon upgrade tests use
upgradeBeaconfor beacon upgrades andupgradeProxyis only called inbeacon-upgrade-block-proxy.jsto verify it explicitly rejects beacon proxies.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/beacon-upgrade-validation.js` around lines 25 - 28, The test is calling upgrades.upgradeProxy(greeter, Invalid) on a beacon proxy but should use the beacon upgrade flow; replace the call to upgrades.upgradeProxy with upgrades.upgradeBeacon(beacon, Invalid) (keep the greeter/beacon/Invalid identifiers) and update the t.throwsAsync expectation to assert the implementation-safety error ('Contract Invalid is not upgrade safe') as in other beacon tests; alternatively, if you prefer to assert upgradeProxy rejecting beacon proxies, change only the expected error string to 'Beacon proxies are not supported with the current function' when calling upgradeProxy(greeter, Invalid).packages/plugin-hardhat/src/verify-proxy.ts (1)
514-523:⚠️ Potential issue | 🟠 MajorAdd a timeout/cap to proxy-link polling loop.
The
while (responseBody.result === 'Pending in queue')loop has no max attempts or timeout. If Etherscan stays pending, the command can hang indefinitely.🛠️ Suggested reliability guard
- while (responseBody.result === 'Pending in queue') { - await delay(3000); - responseBody = await checkProxyVerificationStatus(etherscan, guid); - } + const maxAttempts = 40; + for (let attempts = 0; responseBody.result === 'Pending in queue' && attempts < maxAttempts; attempts++) { + await delay(3000); + responseBody = await checkProxyVerificationStatus(etherscan, guid); + } + if (responseBody.result === 'Pending in queue') { + recordError(`Timed out while linking proxy ${proxyAddress} with implementation`, errorReport); + return; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/src/verify-proxy.ts` around lines 514 - 523, The polling loop that checks proxy verification status (starting after callEtherscanApi returns RESPONSE_OK and guid is set, using checkProxyVerificationStatus and delay) has no max attempts or timeout and can hang indefinitely; modify the loop to include a retry limit and/or elapsed-time timeout (e.g., MAX_ATTEMPTS or MAX_POLL_MS), increment a counter or track startTime before the while (responseBody.result === 'Pending in queue') loop, break and throw a clear error via the same error-handling path if the cap is exceeded, and ensure any resources or promises are cleaned up—update references in this block that use guid, responseBody, checkProxyVerificationStatus, delay, and RESPONSE_OK.
♻️ Duplicate comments (1)
packages/plugin-hardhat/test/prepare-upgrade-txresponse.js (1)
4-6:⚠️ Potential issue | 🔴 CriticalMove
upgradesFactoryimport above executable code (ESM syntax blocker).
importon Line 6 appears after top-level executable statements on Lines 4-5. In ESM, this can fail module parsing/loading.🐛 Proposed fix
import test from 'ava'; import hre from 'hardhat'; +import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades'; const connection = await hre.network.connect(); const { ethers } = connection; -import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades';#!/bin/bash set -euo pipefail target="packages/plugin-hardhat/test/prepare-upgrade-txresponse.js" # Verifies no static import appears after first executable top-level statement. awk ' BEGIN { seen_code=0; bad=0 } { line=$0 if (line ~ /^[[:space:]]*$/) next if (line ~ /^[[:space:]]*\/\/.*/) next if (line ~ /^[[:space:]]*import[[:space:]].*;[[:space:]]*$/) { if (seen_code==1) { printf("Invalid import order at line %d: %s\n", NR, line) bad=1 } next } seen_code=1 } END { exit bad } ' "$target"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/prepare-upgrade-txresponse.js` around lines 4 - 6, The file has a static ESM import ("upgradesFactory" from '@openzeppelin/hardhat-upgrades') placed after top-level executable statements (the calls creating const connection and const { ethers } using hre.network.connect), which breaks ESM parsing; move the import line for upgradesFactory to the top of the module before any top-level execution (i.e., place the import above the code that calls hre.network.connect and before const connection / const { ethers } are defined) so that import order follows ESM rules and the module parses correctly.
🧹 Nitpick comments (30)
packages/plugin-hardhat/test/transparent-v4-change-admin-happy-path.js (1)
15-25: Prefer test context over module-scoped mutableupgrades.This works, but storing
upgradesint.contextimproves isolation and future-proofs for additional concurrent tests.Suggested refactor
-let upgrades; - test.before(async t => { - upgrades = await upgradesFactory(hre, connection); + t.context.upgrades = await upgradesFactory(hre, connection); t.context.Greeter = await ethers.getContractFactory('Greeter'); t.context.ProxyAdmin = await ethers.getContractFactory(ProxyAdmin.abi, ProxyAdmin.bytecode); t.context.TransparentUpgradableProxy = await ethers.getContractFactory( TransparentUpgradableProxy.abi, TransparentUpgradableProxy.bytecode, ); });test('changeProxyAdmin', async t => { - const { Greeter, ProxyAdmin, TransparentUpgradableProxy } = t.context; + const { Greeter, ProxyAdmin, TransparentUpgradableProxy, upgrades } = t.context;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/transparent-v4-change-admin-happy-path.js` around lines 15 - 25, The module-scoped mutable variable `upgrades` should be moved into the per-test context to improve isolation: in the `test.before` hook replace assigning to the top-level `upgrades` with `t.context.upgrades = await upgradesFactory(hre, connection)` (and keep `t.context.Greeter = await ethers.getContractFactory('Greeter')`), then update any references elsewhere in the file to use `t.context.upgrades` instead of the module-scoped `upgrades` (search for uses of the `upgrades` symbol and the initialization call to `upgradesFactory`).packages/plugin-hardhat/test/transparent-linked-libraries.js (3)
14-20: Reorder lifecycle hooks to match execution order.
test.after.alwaysis defined beforetest.before, which is counterintuitive. Placingtest.beforefirst improves readability by reflecting the actual execution sequence.♻️ Proposed fix to reorder hooks
-test.after.always(async () => { - await connection.close(); -}); - test.before(async () => { upgrades = await upgradesFactory(hre, connection); }); + +test.after.always(async () => { + await connection.close(); +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/transparent-linked-libraries.js` around lines 14 - 20, Move the test lifecycle hooks so their declarations reflect execution order: place the test.before(async () => { upgrades = await upgradesFactory(hre, connection); }); block before the test.after.always(async () => { await connection.close(); }); block; this keeps test.before, test.after.always and the use of upgradesFactory/connection organized and improves readability.
1-10: Move import statement to the top of the file with other imports.The
import { upgrades as upgradesFactory }on line 6 is placed after executable statements (lines 4-5). While ESM hoists imports, mixing them with top-level await statements reduces readability and violates conventional module structure.♻️ Proposed fix to group imports at the top
import test from 'ava'; import hre from 'hardhat'; +import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades'; const connection = await hre.network.connect(); const { ethers } = connection; -import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades'; const artifacts = hre.artifacts;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/transparent-linked-libraries.js` around lines 1 - 10, Move the import statement for upgrades so all imports are grouped at the top of the module: relocate "import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades';" to be before any executable top-level code (before the top-level await on "const connection = await hre.network.connect();" and before "const { ethers } = connection;") to keep module imports together and restore conventional module structure.
116-119: Replace deprecatedsubstrwithsubstring.
String.prototype.substris deprecated. Usesubstringfor the same behavior with start and end indices.♻️ Proposed fix to use substring
for (const fixup of fixups) { bytecode = - bytecode.substr(0, 2 + fixup.start * 2) + - addr.substr(2) + - bytecode.substr(2 + (fixup.start + fixup.length) * 2); + bytecode.substring(0, 2 + fixup.start * 2) + + addr.substring(2) + + bytecode.substring(2 + (fixup.start + fixup.length) * 2); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/transparent-linked-libraries.js` around lines 116 - 119, Replace deprecated String.prototype.substr calls in the bytecode assembly expression by using String.prototype.substring: change bytecode.substr(0, 2 + fixup.start * 2) to bytecode.substring(0, 2 + fixup.start * 2), replace addr.substr(2) with addr.substring(2), and replace bytecode.substr(2 + (fixup.start + fixup.length) * 2) with bytecode.substring(2 + (fixup.start + fixup.length) * 2). Ensure you reference the existing variables bytecode, addr, fixup, and fixup.length when making these substitutions so the start/end indices preserve the same slicing semantics.packages/plugin-hardhat/test/beacon-initializers.js (1)
35-38: Consider whetherkind: 'beacon'is necessary here.When using
deployBeaconProxy, specifyingkind: 'beacon'appears redundant since the function already implies beacon deployment. The key option here isinitializerto resolve the overloaded function. If this is intentional for explicitness or required by the API, feel free to keep it.♻️ Optional simplification
const instance = await upgrades.deployBeaconProxy(beacon, InitializerOverloaded, [42], { - kind: 'beacon', initializer: 'initialize(uint256)', });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/beacon-initializers.js` around lines 35 - 38, The options object passed to upgrades.deployBeaconProxy includes a redundant kind: 'beacon' entry; update the call to rely on upgrades.deployBeaconProxy's beacon semantics and remove the kind property, keeping only the initializer: 'initialize(uint256)' option to disambiguate the overloaded InitializerOverloaded initializer.packages/plugin-hardhat/test/transparent-happy-path.js (1)
1-9: Consider moving the import statement before executable code for conventional ordering.ESM hoists imports regardless of source position, so the code works correctly. However, placing the import on line 6 after the top-level
awaitstatements (lines 4-5) is unconventional and may confuse readers about execution order.♻️ Suggested reordering for clarity
import test from 'ava'; import hre from 'hardhat'; +import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades'; const connection = await hre.network.connect(); const { ethers } = connection; -import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades'; /** `@type` {import('@openzeppelin/hardhat-upgrades').HardhatUpgrades} */ let upgrades;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/transparent-happy-path.js` around lines 1 - 9, The import of upgradesFactory from '@openzeppelin/hardhat-upgrades' appears after top-level executable code (the await hre.network.connect() and const { ethers } = connection), which is unconventional; move the statement "import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades';" (and any other imports) above the top-level await and any executable lines so imports are grouped at the top (keeping the declarations like "const connection = await hre.network.connect();" and "let upgrades;" below them) to clarify execution order and reader expectations.packages/plugin-hardhat/test/constructor.js (1)
23-23: Optional: deduplicate repeated proxy option literals.The repeated
{ constructorArgs: [...], initializer: false }blocks make future edits noisier than necessary. Consider extracting small helpers/constants for readability and consistency.Proposed refactor
+const noInitializer = { initializer: false }; + +function deployWithConstructorArg(WithConstructor, arg) { + return upgrades.deployProxy(WithConstructor, [], { constructorArgs: [arg], ...noInitializer }); +} + +function upgradeWithConstructorArg(proxy, WithConstructor, arg) { + return upgrades.upgradeProxy(proxy, WithConstructor, { constructorArgs: [arg], ...noInitializer }); +}Also applies to: 27-27, 37-37, 41-41, 51-51, 55-55, 65-65, 69-69
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/constructor.js` at line 23, Multiple tests call upgrades.deployProxy(WithConstructor, ..., { constructorArgs: [...], initializer: false }) which duplicates the same literal; refactor by extracting a small helper or constant (e.g., PROXY_CONSTRUCTOR_OPTIONS or a function proxyOptions(constructorArgs)) and use it in all calls (references: upgrades.deployProxy and WithConstructor) to avoid repetition and make edits simpler and consistent across the test file.packages/plugin-hardhat/test/transparent-deploy-overload.js (1)
1-6: Move the import statement to the top of the file for clarity.Line 6's
importstatement appears after the top-levelawaiton lines 4-5. While ESM hoists imports so this works correctly, the unconventional ordering harms readability. Group all imports together at the top.♻️ Suggested reordering
import test from 'ava'; import hre from 'hardhat'; +import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades'; const connection = await hre.network.connect(); const { ethers } = connection; -import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/transparent-deploy-overload.js` around lines 1 - 6, Move the trailing import of "upgrades as upgradesFactory" from the bottom of the top-level statements into the import group at the top of the file so all imports are together; specifically, relocate the import of '@openzeppelin/hardhat-upgrades' (exports renamed as upgradesFactory) to sit above the top-level await that creates connection (the expression using hre.network.connect and const { ethers } = connection) to improve readability and maintain conventional ordering.packages/plugin-hardhat/test/implementation-functions.js (2)
1-13: Import statement placed after executable code.Line 6 has an import statement after the top-level
awaiton lines 4-5. While valid ESM, grouping all imports at the top improves readability and aligns with common conventions.Suggested reordering
import test from 'ava'; import hre from 'hardhat'; +import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades'; const connection = await hre.network.connect(); const { ethers } = connection; -import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/implementation-functions.js` around lines 1 - 13, Top-level imports must be grouped before any executable code: move the import of "upgrades as upgradesFactory" (from '@openzeppelin/hardhat-upgrades') so it's declared with the other imports at the top of the file, then run the top-level await that creates "connection" and destructures "ethers"; ensure "upgrades" remains declared as the typed variable but only initialized later if needed. This keeps identifiers like connection, hre, ethers, upgradesFactory and the upgrades declaration in the same logical order and removes the import-after-execution pattern.
169-216: Unusedsignervariable in multiple tests.The
signervariable is declared but never used in five tests:
- Line 169 (
validate upgrade uups - happy path)- Line 179 (
validate upgrade uups - incompatible storage)- Line 191 (
validate upgrade uups - incompatible storage - forced)- Line 201 (
validate upgrade uups - wrong kind)- Line 211 (
validate upgrade uups - no upgrade function)Either remove these unused declarations or use the signer if it was intended for the
deployProxycalls.Remove unused signer declarations
test('validate upgrade uups - happy path', async t => { const { GreeterProxiable, GreeterV2Proxiable } = t.context; - const signer = await ethers.provider.getSigner(); const greeter = await upgrades.deployProxy(GreeterProxiable, ['Hola mundo!'], { kind: 'uups', }); await upgrades.validateUpgrade(greeter, GreeterV2Proxiable); }); test('validate upgrade uups - incompatible storage', async t => { const { GreeterProxiable, GreeterStorageConflictProxiable } = t.context; - const signer = await ethers.provider.getSigner(); const greeter = await upgrades.deployProxy(GreeterProxiable, ['Hola mundo!'], { kind: 'uups', }); await t.throwsAsync(() => upgrades.validateUpgrade(greeter, GreeterStorageConflictProxiable), { message: /(New storage layout is incompatible)/, }); }); test('validate upgrade uups - incompatible storage - forced', async t => { const { GreeterProxiable, GreeterStorageConflictProxiable } = t.context; - const signer = await ethers.provider.getSigner(); const greeter = await upgrades.deployProxy(GreeterProxiable, ['Hola mundo!'], { kind: 'uups', }); await upgrades.validateUpgrade(greeter, GreeterStorageConflictProxiable, { unsafeSkipStorageCheck: true }); }); test('validate upgrade uups - wrong kind', async t => { const { GreeterProxiable, GreeterV2 } = t.context; - const signer = await ethers.provider.getSigner(); const greeter = await upgrades.deployProxy(GreeterProxiable, ['Hola mundo!']); await t.throwsAsync(() => upgrades.validateUpgrade(greeter, GreeterV2), { message: /(Requested an upgrade of kind transparent but proxy is uups)/, }); }); test('validate upgrade uups - no upgrade function', async t => { const { GreeterProxiable, GreeterV2 } = t.context; - const signer = await ethers.provider.getSigner(); const greeter = await upgrades.deployProxy(GreeterProxiable, ['Hola mundo!']); await t.throwsAsync(() => upgrades.validateUpgrade(greeter, GreeterV2, { kind: 'uups' }), { message: getUpgradeUnsafeRegex('GreeterV2'), }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/implementation-functions.js` around lines 169 - 216, Remove the unused signer local in the affected tests or actually use it when calling deployProxy; specifically, in the tests referencing GreeterProxiable/GreeterV2/GreeterStorageConflictProxiable (tests named "validate upgrade uups - happy path", "validate upgrade uups - incompatible storage", "validate upgrade uups - incompatible storage - forced", "validate upgrade uups - wrong kind", and "validate upgrade uups - no upgrade function") either delete the line that declares const signer = await ethers.provider.getSigner(); or pass that signer into upgrades.deployProxy(...) if the deploy was intended to use an explicit signer, ensuring references to the variable (signer) are removed or used consistently across the functions deployProxy and validateUpgrade calls.packages/plugin-hardhat/test/transparent-transfer-admin-ownership-happy-path.js (1)
31-31: Prefer removing the temporary mixed-language inline note.Line [31] includes a migration/history comment in Portuguese; this can be simplified to keep test code neutral and maintainable.
Proposed cleanup
- const admin = await ethers.getContractAt(OWNABLE_ABI, adminAddress); // ← mudou aqui de hre.ethers para ethers + const admin = await ethers.getContractAt(OWNABLE_ABI, adminAddress);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/transparent-transfer-admin-ownership-happy-path.js` at line 31, Remove the temporary mixed-language inline note after the statement creating the admin contract instance: the comment following "const admin = await ethers.getContractAt(OWNABLE_ABI, adminAddress);" should be deleted (no replacement comment needed) so the test remains language-neutral and maintainable; locate this in the test where variables/admin, ethers, OWNABLE_ABI, and adminAddress are used and simply remove the Portuguese remark.packages/plugin-hardhat/test/uups-initial-owner.js (2)
4-6: Consider grouping all imports at the top of the file.The import on line 6 is placed after the top-level await statements. While ESM hoists imports so this works correctly, it reduces readability. Moving the import to lines 1-2 with other imports would follow conventional module organization.
♻️ Suggested reordering
import test from 'ava'; import hre from 'hardhat'; +import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades'; const connection = await hre.network.connect(); const { ethers } = connection; -import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/uups-initial-owner.js` around lines 4 - 6, Move the import of upgrades (import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades') to the top of the module alongside other imports so it appears before any top-level awaits; specifically, place that import above the call to hre.network.connect() (which invokes hre.network.connect() and uses ethers) to improve readability and follow conventional module organization while keeping the existing symbols unchanged (hre.network.connect, upgradesFactory).
23-24: Unused variablesigner.The
signervariable on line 24 is retrieved but never used in this test. If it's not needed for future assertions, consider removing it to keep the test focused.♻️ Suggested removal
test('uups with initialOwner option', async t => { const { Greeter } = t.context; const initialOwner = await ethers.provider.getSigner(1); - const signer = await ethers.provider.getSigner(); await t.throwsAsync(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/uups-initial-owner.js` around lines 23 - 24, The test defines an unused variable signer (const signer = await ethers.provider.getSigner();) which should be removed if not used; delete the unused signer declaration or, if intended for future assertions, actually use signer in the test assertions. Locate the pair of declarations (initialOwner and signer) and remove the signer line (or replace its usage accordingly) to eliminate the unused-variable warning.packages/plugin-hardhat/test/uups-unknown-upgrade-interface.js (1)
1-9: Move import statement before top-level await for clarity.Line 6's import is placed after the top-level await on lines 4-5. While ESM hoists imports (so it executes correctly), this ordering is misleading—readers may think the connection is established before the import runs. Group all imports together at the top for readability.
🔧 Suggested reordering
import test from 'ava'; import hre from 'hardhat'; +import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades'; const connection = await hre.network.connect(); const { ethers } = connection; -import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/uups-unknown-upgrade-interface.js` around lines 1 - 9, Move the import for upgradesFactory (and any other imports) above the top-level await so all imports are grouped at the top; specifically, place "import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades';" before using "await hre.network.connect()" (which creates "connection" and "ethers") to avoid the misleading ordering—ensure imports appear before the top-level await that instantiates "connection" and before the "let upgrades" declaration.packages/plugin-hardhat/test/transparent-happy-path-with-call.js (1)
8-9: Remove duplicated JSDoc annotation.The
HardhatUpgradestype annotation is repeated twice; keep a single declaration to reduce noise.Proposed cleanup
-/** `@type` {import('@openzeppelin/hardhat-upgrades').HardhatUpgrades} */ /** `@type` {import('@openzeppelin/hardhat-upgrades').HardhatUpgrades} */ let upgrades;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/transparent-happy-path-with-call.js` around lines 8 - 9, There are two identical JSDoc type annotations for HardhatUpgrades (/** `@type` {import('@openzeppelin/hardhat-upgrades').HardhatUpgrades} */) duplicated; remove the extra duplicate so only a single JSDoc declaration remains at the top of the test file to avoid noise and redundancy.packages/plugin-hardhat/test/uups-initializers.js (1)
1-9: Move import statement to top with other imports.The
importon line 6 is placed after theawait hre.network.connect()code, but ESM hoists all static imports to execute before any module code. The current placement is misleading—group it with the other imports at lines 1-2 for clarity.✨ Suggested reordering
import test from 'ava'; import hre from 'hardhat'; +import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades'; const connection = await hre.network.connect(); const { ethers } = connection; -import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/uups-initializers.js` around lines 1 - 9, Move the misplaced static import so it's grouped with the other top-of-file imports: relocate "import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades';" to the top alongside "import test from 'ava';" and "import hre from 'hardhat';", leaving the await hre.network.connect() call and the subsequent "const { ethers } = connection;" line unchanged; this fixes the incorrect ordering relative to the runtime call to hre.network.connect() and avoids confusing ESM hoisting behavior.packages/plugin-hardhat/test/transparent-happy-path-with-enums.js (1)
1-9: Consider moving import statement to the top with other imports.Line 6's import statement appears after the top-level
awaitexpressions, which is misleading. While ESM hoists imports (so they execute before lines 4-5 regardless), placing the import after executable code can confuse readers who may assume sequential execution.📝 Suggested reordering
import test from 'ava'; import hre from 'hardhat'; +import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades'; const connection = await hre.network.connect(); const { ethers } = connection; -import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades'; /** `@type` {import('@openzeppelin/hardhat-upgrades').HardhatUpgrades} */ let upgrades;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/transparent-happy-path-with-enums.js` around lines 1 - 9, Move the import "import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades';" so it sits with the other top-level imports before any executable statements (e.g., before the top-level await connecting to hre.network); update the file to import upgradesFactory at the top alongside test, hre and any other imports to avoid placing an import after "const connection = await hre.network.connect()" and make the import order clear.packages/plugin-hardhat/test/uups-happy-path.js (1)
26-35: Strengthen upgrade-path assertions for better regression safety.The current flow executes correctly, but adding address-level assertions would make the happy-path test more robust (proxy address unchanged after
upgradeProxy, implementation address different afterprepareUpgrade).Suggested test hardening diff
test('happy path', async t => { const { Greeter, GreeterV2, GreeterV3 } = t.context; const signer = await ethers.provider.getSigner(); const greeter = await upgrades.deployProxy(Greeter, ['Hello, Hardhat!'], { kind: 'uups' }); + const proxyAddr = await greeter.getAddress(); const greeter2 = await upgrades.upgradeProxy(greeter, GreeterV2); await greeter2.waitForDeployment(); + t.is(await greeter2.getAddress(), proxyAddr); await greeter2.resetGreeting(); - const greeter3ImplAddr = await upgrades.prepareUpgrade(await greeter.getAddress(), GreeterV3); + const greeter3ImplAddr = await upgrades.prepareUpgrade(proxyAddr, GreeterV3); + t.not(greeter3ImplAddr, proxyAddr); const greeter3 = GreeterV3.attach(greeter3ImplAddr); const version3 = await greeter3.version(); t.is(version3, 'V3'); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/uups-happy-path.js` around lines 26 - 35, Add address-level assertions to ensure the proxy address remains the same after upgrades and the implementation address changes after prepareUpgrade: after deploying via upgrades.deployProxy (greeter) assert greeter.getAddress() equals greeter2.getAddress(); after calling upgrades.prepareUpgrade(store the returned greeter3ImplAddr) assert greeter3ImplAddr is not equal to the current implementation address of greeter (or the implementation address before prepareUpgrade); use the existing variables greeter, greeter2, greeter3ImplAddr and the upgrade helpers upgrades.upgradeProxy and upgrades.prepareUpgrade to locate where to insert these assertions.packages/plugin-hardhat/test/namespaced.js (1)
4-5: Move connection creation into the hook to avoid module-load side effects.Line 4 opens the network connection during module evaluation. Prefer doing this in
test.beforeso setup/teardown lifecycle is fully controlled by AVA hooks.♻️ Suggested refactor
-const connection = await hre.network.connect(); -const { ethers } = connection; +let connection; +let ethers; import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades'; @@ test.after.always(async () => { - await connection.close(); + if (connection) { + await connection.close(); + } }); @@ test.before(async t => { + connection = await hre.network.connect(); + ({ ethers } = connection); upgrades = await upgradesFactory(hre, connection);Also applies to: 11-13, 16-16
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/namespaced.js` around lines 4 - 5, The module currently opens the network connection at load time by calling hre.network.connect() and destructuring ethers from connection; move that async connection setup into an AVA lifecycle hook (test.before) and store the resulting connection/ethers in a variable accessible to tests so setup/teardown are controlled; update any other top-level calls to hre.network.connect()/destructuring (the occurrences around the connection, ethers and any direct uses at lines referenced) to instead read from the variables initialized in test.before and ensure disconnection/cleanup (e.g., in test.after) is performed.packages/plugin-hardhat/test/defender-contract-instance.js (1)
78-90: Consider cleaning up inner connections created by the stubbed connect.The
connectStubcreates a new connection viaoriginalConnect()each time it's called (line 81). These inner connections are not explicitly closed when the test completes—only the top-levelconnectionis closed intest.after.always. For a local Hardhat network in tests this is likely harmless, but if you want to be thorough about resource cleanup:♻️ Optional: Track and close inner connections
+ const innerConnections = []; const connectStub = sinon.stub(hre.network, 'connect').callsFake(async () => { const connection = await originalConnect(); + innerConnections.push(connection); const originalGetTransaction = connection.ethers.provider.getTransaction.bind(connection.ethers.provider); sinon.stub(connection.ethers.provider, 'getTransaction').callsFake(async hash => { if (hash === second.deploymentTransaction().hash) { return second.deploymentTransaction(); } return originalGetTransaction(hash); }); return connection; }); // ... rest of test ... connectStub.restore(); + for (const conn of innerConnections) { + await conn.close(); + } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/defender-contract-instance.js` around lines 78 - 90, connectStub creates inner connections by calling originalConnect() that aren't closed; record each inner connection (push to an array inside the connectStub when you create the connection) and then in the existing test.after.always teardown iterate that array and call the same close method used for the top-level connection (e.g., connection.close()) and clear the array; update the connectStub restore code to still restore the stub (sinon.restore or connectStub.restore()) so both stubbing and inner-connection resources are cleaned up (referencing connectStub, originalConnect, connection, and test.after.always).packages/plugin-hardhat/test/transparent-transfer-admin-ownership-wrong-signer.js (1)
1-6: Move import statement above top-level await for clarity.The import on line 6 is placed after the top-level
awaitstatements on lines 4-5. While ESM hoists imports (so this executes correctly), the ordering is unconventional and visually suggests a dependency that doesn't exist. Group all imports together at the top for readability.🧹 Suggested reordering
import test from 'ava'; import hre from 'hardhat'; +import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades'; const connection = await hre.network.connect(); const { ethers } = connection; -import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/transparent-transfer-admin-ownership-wrong-signer.js` around lines 1 - 6, Move the import for "upgrades as upgradesFactory" to join the other top-level imports so all import statements are grouped before any top-level await; specifically, reorder the file so the imports (import test, import hre, import { upgrades as upgradesFactory }) appear before the top-level await that calls hre.network.connect() and destructures ethers from connection to improve clarity and conventional ordering.packages/plugin-hardhat/test/propose-upgrade-with-approval-beacon.js (1)
39-48: Consider aligning mock setup with other proposal tests.This esmock setup only provides local mocks and only mocks
getDeployClient. Other similar tests (e.g.,propose-upgrade-with-approval.js) also mockgetNetworkClientand use global mocks to ensure consistent mocking throughout the module tree. While this may work for tests that verify blocking behavior, consider aligning the mock strategy for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/propose-upgrade-with-approval-beacon.js` around lines 39 - 48, The esmock setup only overrides getNetwork and getDeployClient; update it to mirror other proposal tests by also mocking getNetworkClient (from '../dist/defender/utils.js' or '../dist/defender/client.js' as appropriate) and enable global mocking so mocks apply consistently across the module tree; specifically, when creating the module via esmock('./dist/defender/propose-upgrade-with-approval.js', {...}) include a getNetworkClient mock alongside getDeployClient and getNetwork (reference symbols: otherDefenderUtils, getNetwork, getDeployClient, getNetworkClient, and module) and switch to the same global/mock strategy used in the other test file so behavior is consistent.packages/plugin-hardhat/test/defender-deploy-proxy.js (1)
4-4: Remove stale commented-out defender scaffolding.Lines 4, 16, and 25 are leftover comments from the old path and add noise in the migrated test.
🧹 Suggested cleanup
-// defender as defenderFactory @@ -/** `@type` {import('@openzeppelin/hardhat-upgrades').DefenderHardhatUpgrades} */ -// let defender; @@ test.before(async () => { upgrades = await upgradesFactory(hre, connection); - // defender = await defenderFactory(hre, connection); });Also applies to: 16-16, 25-25
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/defender-deploy-proxy.js` at line 4, Remove the stale commented-out Defender scaffolding comments (e.g. lines containing "// defender as defenderFactory" and other leftover comments referencing the old defender path) from the test file; locate the occurrences of that exact comment text and similar defender-related commented lines in the test (they appear in multiple places) and delete them so the migrated test no longer contains noisy leftover comments.packages/plugin-hardhat/test/beacon-upgrade-validation.js (1)
27-31: Consider using expectations object to verify error message content.The third argument to
t.throwsAsyncis the assertion failure message, not the expected error message. If the intent is to verify the thrown error contains "ContractInvalidis not upgrade safe", use the expectations object instead:💡 Optional improvement to verify error message
await t.throwsAsync( () => upgrades.upgradeProxy(greeter, Invalid), - undefined, - 'Contract `Invalid` is not upgrade safe', + { message: /Contract `Invalid` is not upgrade safe/ }, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/beacon-upgrade-validation.js` around lines 27 - 31, The test currently passes the expected error message as the third argument to t.throwsAsync (which is the assertion failure message) instead of as an expectations object; update the call to t.throwsAsync that wraps upgrades.upgradeProxy(greeter, Invalid) so the second parameter is an expectations object verifying the thrown error message (e.g. { message: /Contract `Invalid` is not upgrade safe/ }) and remove the current third-argument string; locate the call by the symbols t.throwsAsync, upgrades.upgradeProxy, greeter, and Invalid.packages/core/src/cli/validate/build-info-file.test.ts (1)
386-401: Add a HH3 test for malformed (present-but-empty).output.json.Current coverage checks missing output file, but not a present file missing compiler output payload.
✅ Suggested test case
+test.serial('Hardhat 3 format with malformed .output.json reports missing Solidity compiler output', async t => { + await fs.mkdir('hh3-format-invalid-output', { recursive: true }); + + await fs.writeFile( + 'hh3-format-invalid-output/solc-0_8_0-abc123.json', + JSON.stringify({ + _format: 'hh3-sol-build-info-1', + input: BUILD_INFO.input, + solcVersion: '0.8.9', + }), + ); + + await fs.writeFile('hh3-format-invalid-output/solc-0_8_0-abc123.output.json', JSON.stringify({})); + + const error = await t.throwsAsync(getBuildInfoFiles('hh3-format-invalid-output')); + t.true(error?.message.includes('missing Solidity compiler output')); +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/cli/validate/build-info-file.test.ts` around lines 386 - 401, Add a new test similar to "Hardhat 3 format with missing .output.json suggests hardhat compile" that verifies a present-but-empty HH3 output file triggers the same error: create a directory (e.g., "hh3-format-empty-output"), write the hh3 build-info file (with _format: 'hh3-sol-build-info-1') and then write the corresponding ".output.json" file containing an empty object or a JSON lacking the compiler output payload, then call getBuildInfoFiles('hh3-format-empty-output') and assert it throws with a message that includes either "could not be read" or "missing Solidity compiler output" and also includes "hardhat compile"; use the same test harness (test.serial) and error assertions as the existing test to keep behavior consistent with the missing-file case.packages/plugin-hardhat/test/uups-upgrade-storage.js (1)
17-20: Bind signer explicitly in test setup to consolidate account configuration.
getSigner()is fetched in each test (lines 25, 36) but never used. Meanwhile,getContractFactory()relies on implicit default signer selection. Move signer initialization totest.beforeand pass it explicitly to both factory calls, eliminating the redundant fetches and making account assignment clear.♻️ Proposed refactor
test.before(async t => { upgrades = await upgradesFactory(hre, connection); + const signer = await ethers.getSigners().then(signers => signers[0]); - t.context.Greeter = await ethers.getContractFactory('contracts/Greeter.sol:GreeterProxiable'); + t.context.Greeter = await ethers.getContractFactory('contracts/Greeter.sol:GreeterProxiable', signer); t.context.GreeterStorageConflict = await ethers.getContractFactory( 'contracts/InvalidGreeter.sol:GreeterStorageConflictProxiable', + signer, ); }); @@ - const signer = await ethers.provider.getSigner(); const greeter = await upgrades.deployProxy(Greeter, ['Hola mundo!'], { kind: 'uups' }); @@ - const signer = await ethers.provider.getSigner(); const greeter = await upgrades.deployProxy(Greeter, ['Hola mundo!'], { kind: 'uups' });Also applies to: 25-25, 36-36
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/uups-upgrade-storage.js` around lines 17 - 20, Move signer initialization into the shared setup (test.before) and use it explicitly when creating contract factories so account assignment is deterministic: call getSigner() once in test.before (save as e.g. signer) and pass that signer to ethers.getContractFactory for both 'GreeterProxiable' and 'GreeterStorageConflictProxiable' instead of relying on implicit default signers; remove the redundant per-test getSigner() calls (lines where getSigner() is fetched but unused) so tests use the single explicit signer for deployments and interactions.packages/plugin-hardhat/examples/BoxUUPS/tsconfig.json (1)
1-13: Consider a shared base tsconfig for both examples.This config matches the BoxTransparent config closely. A shared base (extended by each example) would reduce future drift.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/examples/BoxUUPS/tsconfig.json` around lines 1 - 13, The two example tsconfig.json files (e.g., the BoxUUPS tsconfig.json with "compilerOptions" like "target":"ES2020", "module":"Node16", "moduleResolution":"node16" and includes ["./scripts","./test","./hardhat.config.ts"]) should be refactored to use a shared base config to avoid drift: create a root/shared base tsconfig.json containing the common "compilerOptions" and common exclude/include defaults, then update the example tsconfig.json files to remove duplicated compilerOptions and instead use "extends": "<path-to-shared-tsconfig>" and only override any example-specific keys (like "include") in the example configs; ensure the symbol names to change are the example tsconfig.json files and the shared base tsconfig that they extend..github/actions/setup/action.yml (1)
17-19: Cache key is broader than necessary and may hurt hit rate.Line 17 hashes every
package.json, so unrelated workspace edits can invalidate the Solidity compiler cache. Consider keying primarily on lockfile + Hardhat version inputs.💡 Suggested key refinement
- key: hardhat-compilers-${{ runner.os }}-${{ hashFiles('**/package.json', '**/yarn.lock') }} + key: hardhat-compilers-${{ runner.os }}-${{ hashFiles('**/yarn.lock') }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/setup/action.yml around lines 17 - 19, The cache key is too broad because it hashes every package.json; update the cache key string (the YAML "key" entry value) to hash only lockfiles and include the Hardhat version input (e.g. use hashFiles('**/yarn.lock','**/package-lock.json') and append ${{ inputs.hardhat-version }}), and update the "restore-keys" entry to match the more specific prefix (e.g. hardhat-compilers-${{ runner.os }}-). Ensure both the key and restore-keys lines in .github/actions/setup/action.yml are adjusted accordingly.packages/plugin-hardhat/src/index.ts (1)
12-17: Consider deduplicating plugin-module wrapper logic.The repeated
then(m => ({ default: m.default }))pattern is easy to drift over time.♻️ Proposed refactor
+const toPluginModule = <T extends { default: HardhatPlugin }>(loader: () => Promise<T>) => + loader().then(m => ({ default: m.default })); + const plugin: HardhatPlugin = { id: '@openzeppelin/hardhat-upgrades', @@ - dependencies: () => [import('@nomicfoundation/hardhat-ethers').then(m => ({ default: m.default }))], + dependencies: () => [toPluginModule(() => import('@nomicfoundation/hardhat-ethers'))], @@ - condition: () => [import('@nomicfoundation/hardhat-verify').then(m => ({ default: m.default }))], + condition: () => [toPluginModule(() => import('@nomicfoundation/hardhat-verify'))], plugin: () => import('./verify-plugin.js'),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/src/index.ts` around lines 12 - 17, The duplicated module-wrapper pattern (then(m => ({ default: m.default })) used inside dependencies and conditionalDependencies) should be extracted into a small helper to avoid drift: add a utility function (e.g., wrapDefaultImport or importDefault) and replace the inline then(...) usages in the dependencies array and the conditionalDependencies[*].condition and plugin entries to call that helper instead; update references in packages/plugin-hardhat/src/index.ts where dependencies, conditionalDependencies, and plugin currently inline the wrapper so they now call the shared function.packages/plugin-hardhat/test/beacon-000-externally-deployed.js (1)
33-47: Prefer removing ad-hoc console logs from tests.These logs add noise in CI output; assertions already capture failure context.
🧹 Proposed cleanup
- console.log('Deployed Greeter at:', await greeter.getAddress()); @@ - console.log('Deployed Beacon at:', await beacon.getAddress()); @@ - console.log('Upgrade error message:', e.message);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/plugin-hardhat/test/beacon-000-externally-deployed.js` around lines 33 - 47, Remove the ad-hoc console.log statements added to the test: delete the console.log('Deployed Greeter at:', await greeter.getAddress()); console.log('Deployed Beacon at:', await beacon.getAddress()); and console.log('Upgrade error message:', e.message); keep the existing logic around Beacon.deploy, beacon.waitForDeployment, and the try/catch that calls upgrades.upgradeBeacon and asserts t.true(e.message.includes(IS_NOT_REGISTERED)) so the test remains informative without noisy CI output.
Comment on lines
+418
to
+421
| t.true( | ||
| error?.message.includes( | ||
| 'Build info from invalid-hh2-build-info/invalid.json must include Solidity compiler input, output, and solcVersion', | ||
| ), |
There was a problem hiding this comment.
This assertion is path-separator brittle.
The test hardcodes invalid-hh2-build-info/invalid.json, which can be unstable across environments. Assert on stable substrings (message core + format token) instead.
🔧 Proposed fix
- t.true(
- error?.message.includes(
- 'Build info from invalid-hh2-build-info/invalid.json must include Solidity compiler input, output, and solcVersion',
- ),
- );
+ t.true(error?.message.includes('must include Solidity compiler input, output, and solcVersion'));
+ t.true(error?.message.includes('Got format: hh-sol-build-info-1'));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| t.true( | |
| error?.message.includes( | |
| 'Build info from invalid-hh2-build-info/invalid.json must include Solidity compiler input, output, and solcVersion', | |
| ), | |
| t.true(error?.message.includes('must include Solidity compiler input, output, and solcVersion')); | |
| t.true(error?.message.includes('Got format: hh-sol-build-info-1')); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/cli/validate/build-info-file.test.ts` around lines 418 -
421, The test currently asserts error?.message.includes('Build info from
invalid-hh2-build-info/invalid.json must include Solidity compiler input,
output, and solcVersion'), which is brittle due to path separators; update the
assertion around error?.message.includes(...) to check for stable substrings
instead (e.g., the core message "must include Solidity compiler input, output,
and solcVersion" plus a format token like "invalid.json" or the filename only)
rather than the full path; locate the assertion that uses
error?.message.includes in build-info-file.test.ts and replace the hardcoded
path check with two contains checks (core message and filename) or normalize the
path before comparing.
Comment on lines
+216
to
+230
| const outputJson = await readJSON(outputFilePath); | ||
| outputData = outputJson.output ?? outputJson; | ||
| } catch (error) { | ||
| throw new ValidateCommandError( | ||
| `Build info file ${buildInfoFilePath} does not contain output, and output file ${outputFilePath} could not be read.`, | ||
| () => HH3_BUILD_INFO_HELP, | ||
| ); | ||
| } | ||
|
|
||
| if (outputData === undefined) { | ||
| const { dir, name } = path.parse(buildInfoFilePath); | ||
| const outputFilePath = path.join(dir, `${name}.output.json`); | ||
|
|
||
| try { | ||
| const outputJson = await readJSON(outputFilePath); | ||
| outputData = outputJson.output ?? outputJson; | ||
| } catch (error) { | ||
| throw new ValidateCommandError( | ||
| `Build info file ${buildInfoFilePath} does not contain output, and output file ${outputFilePath} could not be read.`, | ||
| ); | ||
| } | ||
|
|
||
| if (outputData === undefined) { | ||
| throw new ValidateCommandError( | ||
| `Build info file ${buildInfoFilePath} does not contain output, and output file ${outputFilePath} is missing Solidity compiler output.`, | ||
| ); | ||
| } | ||
| throw new ValidateCommandError( | ||
| `Build info file ${buildInfoFilePath} does not contain output, and output file ${outputFilePath} is missing Solidity compiler output.`, | ||
| () => HH3_BUILD_INFO_HELP, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Malformed HH3 .output.json can be accepted as valid output.
outputData = outputJson.output ?? outputJson treats {} (or other non-SolcOutput objects) as valid output, so invalid output files can bypass this check and fail later with less actionable errors.
🔧 Proposed fix
try {
const outputJson = await readJSON(outputFilePath);
- outputData = outputJson.output ?? outputJson;
+ const candidateOutput = outputJson.output ?? outputJson;
+ if (
+ candidateOutput === null ||
+ typeof candidateOutput !== 'object' ||
+ (!('sources' in candidateOutput) && !('contracts' in candidateOutput))
+ ) {
+ throw new ValidateCommandError(
+ `Build info file ${buildInfoFilePath} does not contain output, and output file ${outputFilePath} is missing Solidity compiler output.`,
+ () => HH3_BUILD_INFO_HELP,
+ );
+ }
+ outputData = candidateOutput as SolcOutput;
} catch (error) {
throw new ValidateCommandError(
`Build info file ${buildInfoFilePath} does not contain output, and output file ${outputFilePath} could not be read.`,
() => HH3_BUILD_INFO_HELP,
);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/cli/validate/build-info-file.ts` around lines 216 - 230,
The current assignment outputData = outputJson.output ?? outputJson lets empty
or malformed JSON (e.g., {}) pass as valid; update the logic in the block that
reads outputFilePath so that after reading outputJson you verify the
shape/contents before assigning to outputData (check that either
outputJson.output exists and is a non-empty object with expected SolcOutput keys
like "contracts" or "sources", or if falling back to outputJson itself it
likewise contains those keys); if the validation fails, throw the existing
ValidateCommandError (using buildInfoFilePath, outputFilePath and
HH3_BUILD_INFO_HELP) instead of accepting the malformed object—look for the
readJSON call, the outputData assignment, and the two places that throw
ValidateCommandError to implement this guard.
Comment on lines
+233
to
+237
| const userSourceNameMap: Record<string, string> | undefined = buildInfoJson.userSourceNameMap; | ||
| if (userSourceNameMap !== undefined) { | ||
| const canonicalToUser: Record<string, string> = {}; | ||
| for (const [userSource, canonicalSource] of Object.entries(userSourceNameMap)) { | ||
| canonicalToUser[canonicalSource] = userSource; |
There was a problem hiding this comment.
Guard userSourceNameMap shape before iterating it.
If userSourceNameMap is null or malformed, Object.entries can throw a raw runtime error instead of returning a ValidateCommandError.
🔧 Proposed fix
- const userSourceNameMap: Record<string, string> | undefined = buildInfoJson.userSourceNameMap;
+ const userSourceNameMap = buildInfoJson.userSourceNameMap as unknown;
if (userSourceNameMap !== undefined) {
+ if (userSourceNameMap === null || typeof userSourceNameMap !== 'object') {
+ throw new ValidateCommandError(
+ `Build info file ${buildInfoFilePath} has an invalid userSourceNameMap.`,
+ () => HH3_BUILD_INFO_HELP,
+ );
+ }
const canonicalToUser: Record<string, string> = {};
- for (const [userSource, canonicalSource] of Object.entries(userSourceNameMap)) {
+ for (const [userSource, canonicalSource] of Object.entries(userSourceNameMap as Record<string, string>)) {
canonicalToUser[canonicalSource] = userSource;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/cli/validate/build-info-file.ts` around lines 233 - 237,
Guard the shape of buildInfoJson.userSourceNameMap before iterating: ensure
userSourceNameMap is a non-null plain object (e.g. typeof userSourceNameMap ===
"object" && userSourceNameMap !== null && !Array.isArray(userSourceNameMap)) and
that each entry's key/value are strings; if the check fails, return or throw a
ValidateCommandError with a clear message instead of proceeding to build
canonicalToUser and using Object.entries. Locate the block referencing
userSourceNameMap, canonicalToUser and buildInfoJson and add the shape checks
and error creation early to prevent Object.entries from throwing on
null/malformed data.
Comment on lines
+12
to
+21
| const connection = await hre.network.connect(); | ||
| const { ethers } = connection; | ||
| const upgradesApi = await upgrades(hre, connection); | ||
|
|
||
| const BoxV2 = await ethers.getContractFactory('BoxV2'); | ||
| const box = await upgradesApi.upgradeProxy(PROXY_ADDRESS, BoxV2); | ||
| await box.waitForDeployment(); | ||
|
|
||
| console.log('Box upgraded at:', await box.getAddress()); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's examine the actual file
echo "=== File: 2-upgrade.ts ==="
cat -n packages/plugin-hardhat/examples/BoxTransparent/scripts/2-upgrade.tsRepository: OpenZeppelin/openzeppelin-upgrades
Length of output: 1048
🏁 Script executed:
#!/bin/bash
# Check connection lifecycle patterns across examples
echo "=== hre.network.connect() usage ==="
rg -nP --type=ts -C2 'hre\.network\.connect\s*\(' packages/plugin-hardhat/examples
echo ""
echo "=== connection.close() usage ==="
rg -nP --type=ts -C2 'connection\.close\s*\(' packages/plugin-hardhat/examplesRepository: OpenZeppelin/openzeppelin-upgrades
Length of output: 4217
🏁 Script executed:
#!/bin/bash
# Let's check all files in the BoxTransparent examples directory
echo "=== Files in BoxTransparent/scripts ==="
find packages/plugin-hardhat/examples/BoxTransparent/scripts -type fRepository: OpenZeppelin/openzeppelin-upgrades
Length of output: 254
Close the Hardhat connection in a finally block.
The script opens a NetworkConnection but never closes it. This pattern is missing across all script examples; test files properly close connections using connection.close(), and scripts should follow the same resource-cleanup discipline to avoid leaving open handles.
♻️ Proposed fix
async function main() {
if (PROXY_ADDRESS === '0x...') {
console.error('Please set PROXY_ADDRESS to your deployed proxy address');
process.exit(1);
}
- const connection = await hre.network.connect();
- const { ethers } = connection;
- const upgradesApi = await upgrades(hre, connection);
-
- const BoxV2 = await ethers.getContractFactory('BoxV2');
- const box = await upgradesApi.upgradeProxy(PROXY_ADDRESS, BoxV2);
- await box.waitForDeployment();
-
- console.log('Box upgraded at:', await box.getAddress());
+ const connection = await hre.network.connect();
+ try {
+ const { ethers } = connection;
+ const upgradesApi = await upgrades(hre, connection);
+
+ const BoxV2 = await ethers.getContractFactory('BoxV2');
+ const box = await upgradesApi.upgradeProxy(PROXY_ADDRESS, BoxV2);
+ await box.waitForDeployment();
+
+ console.log('Box upgraded at:', await box.getAddress());
+ } finally {
+ await connection.close();
+ }Note: The same issue affects 1-deploy.ts and both scripts in the BoxUUPS example. Consider applying this fix consistently across all script files.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const connection = await hre.network.connect(); | |
| const { ethers } = connection; | |
| const upgradesApi = await upgrades(hre, connection); | |
| const BoxV2 = await ethers.getContractFactory('BoxV2'); | |
| const box = await upgradesApi.upgradeProxy(PROXY_ADDRESS, BoxV2); | |
| await box.waitForDeployment(); | |
| console.log('Box upgraded at:', await box.getAddress()); | |
| } | |
| const connection = await hre.network.connect(); | |
| try { | |
| const { ethers } = connection; | |
| const upgradesApi = await upgrades(hre, connection); | |
| const BoxV2 = await ethers.getContractFactory('BoxV2'); | |
| const box = await upgradesApi.upgradeProxy(PROXY_ADDRESS, BoxV2); | |
| await box.waitForDeployment(); | |
| console.log('Box upgraded at:', await box.getAddress()); | |
| } finally { | |
| await connection.close(); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/plugin-hardhat/examples/BoxTransparent/scripts/2-upgrade.ts` around
lines 12 - 21, The script opens a Hardhat NetworkConnection via
hre.network.connect() (variable connection) but never closes it; wrap the main
upgrade logic (getting ethers, calling upgrades(hre, connection),
ethers.getContractFactory('BoxV2'), upgradesApi.upgradeProxy(...),
box.waitForDeployment(), console.log) in a try/finally and call
connection.close() from the finally block to ensure the NetworkConnection is
always closed (apply the same pattern to 1-deploy.ts and the BoxUUPS scripts);
reference the connection variable and ensure any thrown errors still let finally
run to call connection.close().
Comment on lines
+16
to
+17
| "ethers": "^6.8.1", | ||
| "hardhat": "^3.0.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Hardhat/Ethers versions across package.json files =="
fd '^package\.json$' -t f | while read -r f; do
python - <<'PY' "$f"
import json,sys
p=sys.argv[1]
with open(p) as fh:
j=json.load(fh)
deps={}
for k in ("dependencies","devDependencies","peerDependencies"):
deps.update(j.get(k,{}))
h=deps.get("hardhat")
e=deps.get("ethers")
oz=deps.get("@openzeppelin/hardhat-upgrades")
if h or e or oz:
print(f"{p}: hardhat={h} ethers={e} oz-upgrades={oz}")
PY
done
echo
echo "== plugin-hardhat peerDependencies (if present) =="
fd '^package\.json$' -t f -x grep -l "plugin-hardhat" {} \; | head -1 | while read -r f; do
python - <<'PY' "$f"
import json,sys
p=sys.argv[1]
with open(p) as fh:
j=json.load(fh)
print(f"File: {p}")
print(json.dumps(j.get("peerDependencies",{}), indent=2))
PY
doneRepository: OpenZeppelin/openzeppelin-upgrades
Length of output: 691
🏁 Script executed:
# Verify the target file exists and read lines 16-17
echo "=== Target file content ==="
if [ -f "packages/plugin-hardhat/examples/BoxUUPS/package.json" ]; then
echo "File found. Content around lines 16-17:"
sed -n '14,19p' "packages/plugin-hardhat/examples/BoxUUPS/package.json" | cat -n
else
echo "File not found at expected path"
fd 'BoxUUPS.*package.json'
fiRepository: OpenZeppelin/openzeppelin-upgrades
Length of output: 391
🏁 Script executed:
#!/bin/bash
echo "=== Checking lock files for baseline versions ==="
if [ -f "package-lock.json" ]; then
echo "Root package-lock.json - hardhat entries:"
jq '.packages[] | select(.name == "hardhat") | .version' package-lock.json 2>/dev/null | head -5 || rg 'hardhat.*3\.0\.' package-lock.json | head -3
fi
if [ -f "pnpm-lock.yaml" ]; then
echo "pnpm-lock.yaml found - checking hardhat versions:"
rg 'hardhat@' pnpm-lock.yaml | head -5
fi
echo
echo "=== Checking main plugin-hardhat package.json details ==="
cat packages/plugin-hardhat/package.json | head -30
echo
echo "=== Comparing both example directories ==="
echo "BoxTransparent package.json (lines 16-17):"
sed -n '14,19p' packages/plugin-hardhat/examples/BoxTransparent/package.json | cat -n
echo
echo "BoxUUPS package.json (lines 16-17):"
sed -n '14,19p' packages/plugin-hardhat/examples/BoxUUPS/package.json | cat -nRepository: OpenZeppelin/openzeppelin-upgrades
Length of output: 1722
🏁 Script executed:
# Check if there's any documentation or CI config mentioning baseline versions
echo "=== Searching for version baseline references ==="
rg -i "baseline|migration.*version|tested.*with" --type md --type json --type yaml --type txt . 2>/dev/null | head -20Repository: OpenZeppelin/openzeppelin-upgrades
Length of output: 819
Example dependencies have outdated ethers version compared to main plugin.
The examples at lines 16-17 use ethers ^6.8.1, which lags behind the main plugin-hardhat (^6.14.0) by 6 minor versions. This can cause drift and inconsistent behavior.
The hardhat version suggestion to ^3.0.16 could not be verified against the codebase baseline; all packages consistently use hardhat ^3.0.0. Recommend updating ethers to align with the main plugin:
🔧 Suggested fix
- "ethers": "^6.8.1",
+ "ethers": "^6.14.0",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "ethers": "^6.8.1", | |
| "hardhat": "^3.0.0", | |
| "ethers": "^6.14.0", | |
| "hardhat": "^3.0.0", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/plugin-hardhat/examples/BoxUUPS/package.json` around lines 16 - 17,
Update the example's package.json dependency for ethers to match the main plugin
version: replace the current "ethers": "^6.8.1" entry with the same version used
by the plugin ("^6.14.0") so the example and main package remain aligned; leave
the existing hardhat entry as-is ("^3.0.0") to maintain consistency across
packages.
Comment on lines
+12
to
+18
| import { createRequire } from 'node:module'; | ||
|
|
||
| const require = createRequire(import.meta.url); | ||
|
|
||
| const artifactsBuildInfo = require('@openzeppelin/upgrades-core/artifacts/build-info-v5.json'); | ||
| import { AbiCoder } from 'ethers'; | ||
| import * as defenderUtils from '../dist/defender/utils.js'; |
There was a problem hiding this comment.
Fix ESM import ordering: static imports must precede all module-level code.
The import { AbiCoder } and import * as defenderUtils statements (lines 17-18) appear after the const require = createRequire(...) and require(...) calls (lines 14, 16). In ESM, all static imports must come before any other module-level statements.
Proposed fix
import { getCombinedBuildInfo } from '../dist/utils/artifacts.js';
-import { createRequire } from 'node:module';
-
-const require = createRequire(import.meta.url);
-
-const artifactsBuildInfo = require('@openzeppelin/upgrades-core/artifacts/build-info-v5.json');
import { AbiCoder } from 'ethers';
import * as defenderUtils from '../dist/defender/utils.js';
+import { createRequire } from 'node:module';
+
+const require = createRequire(import.meta.url);
+
+const artifactsBuildInfo = require('@openzeppelin/upgrades-core/artifacts/build-info-v5.json');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/plugin-hardhat/test/defender-deploy.js` around lines 12 - 18, Move
all static import statements to the top of the module so they precede any
module-level code: ensure the lines "import { createRequire } from
'node:module';", "import { AbiCoder } from 'ethers';" and "import * as
defenderUtils from '../dist/defender/utils.js';" appear before the "const
require = createRequire(import.meta.url);" and the "const artifactsBuildInfo =
require(...)" call; alternatively replace the JSON require with an ESM import if
preferred, but at minimum reorder to place imports (createRequire, AbiCoder,
defenderUtils) before createRequire/require usage.
Comment on lines
+4
to
+9
| const connection = await hre.network.connect(); | ||
| const { ethers } = connection; | ||
| // import { defender as defenderFactory } from '@openzeppelin/hardhat-upgrades'; | ||
| import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades'; | ||
| import sinon from 'sinon'; | ||
| import esmock from 'esmock'; |
There was a problem hiding this comment.
Fix ESM import ordering: static imports must precede top-level await.
Lines 4-5 execute top-level await before the static imports on lines 7-9. Move the connection setup below all imports.
Proposed fix
import test from 'ava';
import hre from 'hardhat';
-
-const connection = await hre.network.connect();
-const { ethers } = connection;
-// import { defender as defenderFactory } from '@openzeppelin/hardhat-upgrades';
import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades';
import sinon from 'sinon';
import esmock from 'esmock';
+
+const connection = await hre.network.connect();
+const { ethers } = connection;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const connection = await hre.network.connect(); | |
| const { ethers } = connection; | |
| // import { defender as defenderFactory } from '@openzeppelin/hardhat-upgrades'; | |
| import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades'; | |
| import sinon from 'sinon'; | |
| import esmock from 'esmock'; | |
| import test from 'ava'; | |
| import hre from 'hardhat'; | |
| import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades'; | |
| import sinon from 'sinon'; | |
| import esmock from 'esmock'; | |
| const connection = await hre.network.connect(); | |
| const { ethers } = connection; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/plugin-hardhat/test/propose-upgrade-with-approval-beacon.js` around
lines 4 - 9, Top-level await is executed before static imports; move the dynamic
connection setup so that all static imports (the lines importing
upgradesFactory, sinon, esmock and any other imports) appear first, then call
const connection = await hre.network.connect(); const { ethers } = connection; —
i.e., relocate the connection/ethers assignment below the import statements
(ensure symbols connection, hre.network.connect and ethers remain unchanged and
referenced after the imports).
|
|
||
| const proposalId = 'mocked proposal id'; | ||
| const proposalUrl = 'https://example.com'; | ||
| const approvalProcessId = '123'; // ← ADICIONEI |
There was a problem hiding this comment.
Remove development comments before merging.
Lines 16 and 69 contain Portuguese comments (← ADICIONEI, ← MUDEI AQUI: usar transparent proxy ao invés de beacon) that appear to be development notes. These should be removed or translated to English before merging.
Proposed fix
-const approvalProcessId = '123'; // ← ADICIONEI
+const approvalProcessId = '123';- // ← MUDEI AQUI: usar transparent proxy ao invés de beacon
t.context.greeter = await upgrades.deployProxy(t.context.Greeter, { kind: 'transparent' });Also applies to: 69-69
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/plugin-hardhat/test/propose-upgrade-with-approval-transparent.js` at
line 16, Remove the leftover Portuguese development comments from the test by
deleting the inline comment text after the approvalProcessId constant (the "←
ADICIONEI" note) and the other Portuguese note near the proxy change (the "←
MUDEI AQUI: usar transparent proxy ao invés de beacon") so the file contains
only code or English comments; locate the statements by the variable name
approvalProcessId and the nearby proxy-change comment and either remove or
replace them with a concise English comment before merging.
Comment on lines
+4
to
+9
| const connection = await hre.network.connect(); | ||
| const { ethers } = connection; | ||
| import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades'; | ||
| import sinon from 'sinon'; | ||
| import esmock from 'esmock'; | ||
| import { mockDeploy } from '../dist/test-utils/mock-deploy.js'; |
There was a problem hiding this comment.
Fix ESM import ordering: static imports must precede top-level await.
Lines 4-5 execute top-level await before the static imports on lines 6-9. Move the connection setup below all imports.
Proposed fix
import test from 'ava';
import hre from 'hardhat';
-
-const connection = await hre.network.connect();
-const { ethers } = connection;
import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades';
import sinon from 'sinon';
import esmock from 'esmock';
import { mockDeploy } from '../dist/test-utils/mock-deploy.js';
+
+const connection = await hre.network.connect();
+const { ethers } = connection;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/plugin-hardhat/test/propose-upgrade-with-approval-unsafeAllow.js`
around lines 4 - 9, The file currently calls top-level await via const
connection = await hre.network.connect() and const { ethers } = connection
before static imports (e.g., imports of upgradesFactory, sinon, esmock,
mockDeploy); move the top-level await lines (the connection and ethers
extraction using hre.network.connect) to after all static import statements so
that static imports (imports of '@openzeppelin/hardhat-upgrades', 'sinon',
'esmock', and '../dist/test-utils/mock-deploy.js') appear first and the
asynchronous network connection is performed subsequently.
Comment on lines
+4
to
+6
| const connection = await hre.network.connect(); | ||
| const { ethers } = connection; | ||
| import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades'; |
There was a problem hiding this comment.
Fix ESM import ordering: static imports must precede top-level await.
In ESM, all import declarations must appear at the top of the module before any other code, including top-level await. Move the connection setup below all imports.
Proposed fix
import test from 'ava';
import hre from 'hardhat';
-
-const connection = await hre.network.connect();
-const { ethers } = connection;
import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades';
+
+const connection = await hre.network.connect();
+const { ethers } = connection;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const connection = await hre.network.connect(); | |
| const { ethers } = connection; | |
| import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades'; | |
| import test from 'ava'; | |
| import hre from 'hardhat'; | |
| import { upgrades as upgradesFactory } from '@openzeppelin/hardhat-upgrades'; | |
| const connection = await hre.network.connect(); | |
| const { ethers } = connection; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/plugin-hardhat/test/uups-deploy-overload.js` around lines 4 - 6, The
file uses top-level await before static imports: move all import declarations
(e.g., "import { upgrades as upgradesFactory } from
'@openzeppelin/hardhat-upgrades';" and any other import statements including
ethers if using static import) to the very top of the module, then place the
top-level await call that sets up the Hardhat connection (const connection =
await hre.network.connect();) and the subsequent const { ethers } = connection;
after those imports; ensure no top-level code runs before the import
declarations so ESM ordering rules are satisfied.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Migration to Hardhat 3: ESM & Plugin Hooks
This PR migrates
@openzeppelin/hardhat-upgradesto Hardhat 3, converting the package to ESM and adopting the new plugin hooks architecture. This is a major breaking change that replaces the legacyextendEnvironmentpattern withHardhatPluginhook handlers for config and Solidity initialization. Users must now utilize explicit async factory functions—await upgrades(hre, connection)anddefender()—and manage network connectivity viaawait hre.network.connect(), asethersis now obtained through the connection rather than the HRE.The package has been fully converted to ESM (
type: module) usingNode16resolution. To ensure compatibility with Babel and NYC coverage,import...with { type: 'json' }statements were replaced withcreateRequire()across 15 files. Dependencies are updated tohardhat@^3.0.16,ethers v6, and the latest@nomicfoundationplugins.Testing infrastructure now includes Solidity test support via the
foundry-upgradesrepository. To stabilize the environment, example projects were isolated usingpnpmlink protocols and local AVA configurations. CI/CD workflows were modified to initialize git submodules before installation and now includefoundry-upgradesinnpmFilesToBuild.A comprehensive
MIGRATION.mdhas been added with before/after examples, and the README now reflects Hardhat 3 patterns. Please note that fullhardhat-verifyintegration and certain namespace layouts are currently pending an upstream fix in Hardhat ([PR #7744](NomicFoundation/hardhat#7744)), which also impacts the provided example projects. While core functionality is tested and passing, some edge cases in verification may require further refinement following the upstream merge. We also need a new feature from the nomic team to allow access to their internal libraries for us to use them like in hh2, at the verify part.Summary by CodeRabbit
New Features
Documentation
Chores