Introduce standalone ops CLI - #2998
Conversation
7845f1a to
e0f5d70
Compare
e0f5d70 to
1908fb1
Compare
1908fb1 to
b6a74f2
Compare
| const { parseUnits } = require("ethers").utils; | ||
|
|
||
| const isFork = process.env.FORK === "true"; | ||
| const isMainnet = hre.network.name === "mainnet"; |
There was a problem hiding this comment.
pnpm ops never hands --network to Hardhat, so the require("hardhat") above boots the default in-process network and hre.network.name is hardhat. isMainnet is therefore always false under the new CLI (node -e 'require("./utils/hardhat-task-helpers")' in this tree prints {isFork:false,isMainnet:false}). Two consumers depend on it: pnpm ops capital --symbol OUSD --pause true --network mainnet skips the print-a-governance-proposal branch (tasks/vault.js:241-248) and calls vault.connect(signer).pauseCapital() directly, and pnpm ops execute --network mainnet loses its "can not be used on mainnet" guard (tasks/governance.js:31-33). tasks/test/governance.js:5 mocks this module, so the unit test cannot see it. Talos schedules are unaffected, run.ts never loads tasks.js.
This is fixed in #3000, which replaces this file with utils/runtime-helpers.js (derives isMainnet from the standalone getNetworkName()) and pins both states in tasks/test/runtime-helpers.js. Since each layer is meant to be reviewable on its own, could you hoist that file and its test into this PR? It has no Hardhat dependency, so nothing else needs to move. Otherwise a line in the PR body saying pnpm ops is not mainnet-safe until #3000 would do.
| getContractAt: typeof contracts.getContractAt; | ||
| getContractFactory: typeof contracts.getContractFactory; | ||
| }; | ||
| network: { |
There was a problem hiding this comment.
mockBeaconRoot cannot run through this context: utils/hardhat.js#replaceContractAt (reached via tasks/beaconTesting.js:112) calls hre.ethers.getSigners() and hre.network.provider.request({ method: "hardhat_setCode" }), and neither ethers.getSigners nor network.provider.request exists here (provider is an ethers JsonRpcProvider, which has .send). It throws TypeError at first use, so the catalogue test's "every registry-backed command has a live handler" only holds in the typeof handler === "function" sense. Dev-only command, but the stated invariant does not hold on this branch.
Fixed in #3000, which replaces utils/hardhat.js with utils/anvil.js (getProvider().send("anvil_setCode", …)). Same ask as for isMainnet: hoisting that file into this PR keeps the layer self-consistent.
| type CatalogueEntry = Omit<CommandDefinition, "handler">; | ||
| const catalogue = JSON.parse( | ||
| readFileSync( | ||
| join(__dirname, "test", "fixtures", "ops-command-catalog.json"), |
There was a problem hiding this comment.
This makes the JSON under test/fixtures/ the only source of truth for every command's params: the makeDefinition shim in command.ts (~156-185) turns addParam / addOptionalParam / addFlag into no-ops, so the .addParam(...) chains that still decorate every task() in tasks.js are dead text. It is correct today because the JSON was generated from those declarations (diffed all 75 against master's hre.tasks: zero drift), but the first edit in the obvious place fails silently: an optional param added in tasks.js reaches the handler as undefined instead of its default (coerceParams only iterates the JSON specs), a changed default in tasks.js is ignored and the JSON one wins (e.g. depositToStrategy symbol), a removed param is still accepted. Only a new required param is loud (Unknown option). command-catalog.js will not catch any of it: it compares names from tasks.js against the fixture, then the runtime commands against the fixture, which is tautological since the runtime is the fixture. Talos is unaffected (actions use lib/action.ts's ParamBuilder), so this is about pnpm ops.
Proposal: have the registry record the declarations instead of dropping them, in task-registry.ts once #3000 lands (the same shim lives here in command.ts). addParam(name, description, defaultValue, type), addOptionalParam(...) and addFlag(name, description) push { name, description, type: type?.name ?? "string", optional, flag, default } onto the task entry (~15 lines; types.* already carry .name), commands.ts builds the catalogue from registeredTasks() plus the inline accounts, and the JSON becomes a pinned snapshot that command-catalog.js asserts deep-equality against. A param edit in tasks.js then either updates the snapshot or fails the test, as it did under Hardhat; tasks.js stays the single source of truth and the JSON can move out of test/fixtures/.
| const next = rest[index + 1]; | ||
| if (next === undefined || next.startsWith("--")) flags[key] = true; | ||
| else { | ||
| flags[key] = next; |
There was a problem hiding this comment.
A repeated option silently last-wins here: pnpm ops rebase --symbol OUSD --network mainnet --symbol OETH ran with OETH. Hardhat rejected that (HH308). Cheap to keep: throw if key is already in flags.
|
|
||
| export async function main(argv = process.argv.slice(2)): Promise<void> { | ||
| const { name, network, flags } = parseCli(argv); | ||
| if (!name || name === "help" || flags.help === true) { |
There was a problem hiding this comment.
Two small parity gaps with npx hardhat <task> --help:
--helpafter a command name prints the global command list, not that command's params; there is no per-command help at all, which matters more now that the param definitions are only readable in the JSON catalogue (see thecommands.tscomment). Printingcommand.params(name, type, optional/default, description) whennameis set andflags.helpis true would cover it.--network=mainnetfails with "--network is required" (line 25) becauseparseClionly understands the space-separated form. Hardhat also rejected the=form, so this is parity, but the message misleads; either accept--key=valueinparseClior say "use --network ".
| handler: | ||
| entry.name === "accounts" | ||
| ? async (_args, context) => { | ||
| const accounts = await context.ethers.provider.listAccounts(); |
There was a problem hiding this comment.
provider.listAccounts() is eth_accounts, which a live RPC answers with [], so pnpm ops accounts --network mainnet now prints nothing. On master the task printed the configured signer addresses (DEPLOYER_PK / GOVERNOR_PK, tasks/account.js:5-19, together with the keys, which we can happily stop printing). It is only useful against a local node now. Either print the address the standalone signer would use (getOptionalSigner() → getAddress()), or drop the command; tasks/account.js is left without an importer once #3000 deletes hardhat.config.js, so it can go together with it. This is also the only description drift in the catalogue: "Prints the list of accounts" → "Prints the list of configured accounts".
2 participants
Plain-English summary
Hardhat is currently acting as a command launcher for operational tasks. The useful logic already lives in ordinary TypeScript handlers, but operators must still enter through
hardhatto reach it.This PR adds a small standalone CLI that calls those same handlers directly. The commands, parameters, signers, roles, and deployment data stay the same; only the entry point changes.
The local/forked node still uses Hardhat in this layer. That is replaced separately in #2999.
Before and after
This is a runtime CLI migration, not a contract compilation or deployment migration. Foundry already handles those jobs.
What changes in this PR
pnpm opswith the complete 75-command public catalogue;What does not change yet
Stack and merge order
Base: #2997 (
chore/remove-obsolete-historical-artifacts)Merge in cascade: #3000 → #2999 → #2998 → #2997 →
master.What to review
Please focus on behavioral parity:
Validation
pnpm install --frozen-lockfilepnpm test:tasks— 38 passingpnpm test:scripts— 66 passingpnpm test:layouts— 11 passingpnpm typecheckpnpm lint:js,pnpm lint:ts,pnpm prettier:checkforge build contracts/ -j 1