From f1c95039cf4541296ed1ad8eda180855565a9c2c Mon Sep 17 00:00:00 2001 From: Branimir Rakic Date: Tue, 5 May 2026 17:59:24 +0200 Subject: [PATCH 1/2] fix(node-update): never run hardhat compile on node hosts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Node updates routinely failed on small / ARM64 VPS because the auto-updater ran `hardhat compile` whenever a release touched `.sol` sources. The compile is the heaviest and most fragile step in the slot build (cold solc OOMs, hardhat-deploy quirks), and any failure aborted the entire slot swap. Nothing on the runtime path actually needs hardhat: `packages/chain` loads contract ABIs from the COMMITTED `packages/evm-module/abi/*.json` via `require()`. Treat those committed JSONs as the runtime contract surface and move the freshness check to CI, where it runs once per PR on a beefy runner instead of on every node, every update. Changes ------- - install.sh: switch both blue-green slots from `pnpm build` to `pnpm build:runtime`. Mirrors what the auto-updater already picks via `dkgBuild.releaseRuntimeBuildScript`, and skips the ~2 min hardhat compile per slot (so first install is ~4 min faster on small hosts). - packages/cli/src/daemon/auto-update.ts: remove the conditional `pnpm --filter dkg-evm-module build` block and the now-unused `shouldRebuildContracts` helper. Replace with a comment documenting the new contract: committed ABIs are authoritative, CI enforces freshness. - packages/cli/src/config.ts: mark `AutoUpdateBuildTimeouts.contracts` as `@deprecated`. Field is preserved on the type so existing user configs don't fail to parse, but the value is no longer read. - .github/workflows/ci.yml: add an `abi-freshness` job (TORNADO tier). Runs only when `paths-filter` reports `contracts == true`, executes `pnpm --filter dkg-evm-module build`, then `git diff --exit-code -- packages/evm-module/abi/`. On drift it fails with a copy-pasteable remediation message so the contributor can fix and push without guessing. - packages/cli/test/auto-update.test.ts: drop the four `shouldRebuildContracts`-era tests (timeout, fails-closed, hardhat-clean ordering, fetch-retry) and replace with one regression guard asserting the auto-updater never invokes any `hardhat` / evm-module command, even when the diff would have shown changed `.sol` files. - RELEASE_PROCESS.md, packages/evm-module/README.md: document the new contract — contributors regenerate ABIs alongside `.sol` changes, CI enforces it, nodes never recompile. Operational impact ------------------ - Node updates: zero hardhat invocations on the host. The slowest, most failure-prone step is gone. - Releases: unchanged. Release workflows still run unfiltered `pnpm build` (CI lane uses `DKG_SKIP_EVM_BUILD=1`). - Contributors: if you change `.sol`, also commit regenerated ABIs. CI prints the exact `pnpm --filter ... build && git add packages/evm-module/abi/` command on failure. Test plan --------- - `pnpm vitest run test/auto-update.test.ts` (cli): 73/73 passing. - `pnpm vitest run test/migration.test.ts test/node-ui-static.test.ts` (cli): 38/38 passing. - `DKG_SKIP_EVM_BUILD=1 pnpm build` at repo root: 20/20 turbo tasks OK. - `bash -n install.sh`: clean. - ci.yml YAML parses; new `abi-freshness` job listed. Co-authored-by: Cursor --- .github/workflows/ci.yml | 63 ++++++++++ RELEASE_PROCESS.md | 2 + install.sh | 10 +- packages/cli/src/config.ts | 8 +- packages/cli/src/daemon/auto-update.ts | 157 ++----------------------- packages/cli/test/auto-update.test.ts | 128 +++++--------------- packages/evm-module/README.md | 22 ++++ 7 files changed, 142 insertions(+), 248 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ecfa7b5f2..e4e1393ee 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -413,6 +413,69 @@ jobs: --filter @origintrail-official/dkg-adapter-openclaw \ run test + # ------------------------------------------------------------------ + # ABI freshness gate — guards the contract `auto-update.ts` deliberately + # never invokes `hardhat compile` on node hosts (would OOM small VPS, + # cold-solc on ARM64 trips the build timeout). It relies on the + # COMMITTED `packages/evm-module/abi/*.json` being the runtime + # contract surface (consumed by `packages/chain` via require()). + # + # This job catches the only way that contract can break: a contributor + # changes a `.sol` file and forgets to commit the regenerated ABIs. + # Runs only when `changes.outputs.contracts == 'true'` so PRs that + # don't touch contracts pay zero cost. On failure prints an explicit + # remediation command so the fix is one copy-paste away. + # ------------------------------------------------------------------ + abi-freshness: + name: "Tornado: ABI freshness (committed abi/ vs hardhat compile)" + needs: changes + runs-on: ubuntu-latest + timeout-minutes: 10 + if: needs.changes.outputs.contracts == 'true' + steps: + - uses: actions/checkout@v4 + - uses: pnpm/action-setup@v4 + - uses: actions/setup-node@v4 + with: + node-version-file: .nvmrc + cache: pnpm + - name: Install dependencies + run: pnpm install --frozen-lockfile + - name: Cache Hardhat artifacts + uses: actions/cache@v4 + with: + path: | + packages/evm-module/artifacts + packages/evm-module/cache + packages/evm-module/typechain + key: hardhat-${{ runner.os }}-${{ hashFiles('packages/evm-module/contracts/**/*.sol', 'packages/evm-module/hardhat.node.config.ts') }} + restore-keys: | + hardhat-${{ runner.os }}- + - name: Compile contracts (regenerates abi/*.json) + run: pnpm --filter @origintrail-official/dkg-evm-module build + - name: Verify committed abi/ matches regenerated abi/ + # `git diff --exit-code` returns 1 if anything under abi/ changed + # after compile. If so, the contributor forgot to commit the + # regenerated ABIs; the auto-updater would then activate code + # that loads stale ABIs against new contracts. Block the PR. + run: | + set -euo pipefail + if ! git diff --exit-code -- packages/evm-module/abi/; then + echo "::error title=ABI drift::Committed packages/evm-module/abi/*.json does not match the output of \`pnpm --filter @origintrail-official/dkg-evm-module build\`." + echo "" + echo "The auto-updater no longer runs hardhat compile on node hosts (see comment in packages/cli/src/daemon/auto-update.ts), so committed ABIs ARE the runtime contract surface. Drift here would silently activate stale ABIs against new contracts on every node." + echo "" + echo "To fix: regenerate and commit the ABIs locally:" + echo "" + echo " pnpm --filter @origintrail-official/dkg-evm-module build" + echo " git add packages/evm-module/abi/" + echo " git commit --amend --no-edit # or a fresh commit, your call" + echo " git push --force-with-lease" + echo "" + exit 1 + fi + echo "OK: committed packages/evm-module/abi/ matches hardhat output." + # ------------------------------------------------------------------ # Tornado Solidity lane — `packages/evm-module` is TORNADO-tier # (`dkgv10-spec/CRITICALITY_CATEGORIZATION.md` §1): bugs in the diff --git a/RELEASE_PROCESS.md b/RELEASE_PROCESS.md index 964396ba1..846b9b093 100644 --- a/RELEASE_PROCESS.md +++ b/RELEASE_PROCESS.md @@ -130,6 +130,8 @@ dkg update 9.0.0-beta.2 --allow-prerelease --no-verify-tag Git-based blue-green updates run runtime packages and the Node UI static bundle as separate timed build steps, then verify `packages/node-ui/dist-ui/index.html` before activation. `build:runtime` remains a UI-inclusive compatibility wrapper so nodes updating from an older updater still prepare the UI through the target ref's build script. +> **Note on EVM contracts**: nodes never run `hardhat compile` during install or auto-update. The committed `packages/evm-module/abi/*.json` files are the runtime contract surface (consumed by `packages/chain` via `require()`). The `abi-freshness` CI job (`.github/workflows/ci.yml`) blocks any PR that changes Solidity sources without committing the regenerated ABIs, so by the time a tag exists on `main` the committed ABIs are guaranteed to match. **Release implication**: when contract source changes are part of a release, the contributor MUST regenerate ABIs (`pnpm --filter @origintrail-official/dkg-evm-module build && git add packages/evm-module/abi/`) and commit them with the source change. CI enforces this; releases cut from `main` cannot ship with stale ABIs. + After each update: ```bash diff --git a/install.sh b/install.sh index 5e1ddeca1..762ea7475 100755 --- a/install.sh +++ b/install.sh @@ -77,7 +77,13 @@ else info "Installing dependencies in slot a ..." (cd "$SLOT_A" && pnpm install --frozen-lockfile) info "Building slot a ..." - (cd "$SLOT_A" && pnpm build) + # Runtime build only — skips evm-module's hardhat compile. The committed + # `packages/evm-module/abi/*.json` files are the runtime contract surface + # consumed by `packages/chain`; CI enforces they stay in sync with the + # Solidity sources, so nodes never need to invoke `hardhat compile`. + # Mirrors what the auto-updater does (see node-ui-static.ts: + # runtimeBuildCommandFromPackageJson). + (cd "$SLOT_A" && pnpm build:runtime) fi stage_markitdown "$SLOT_A" "a" @@ -90,7 +96,7 @@ else info "Installing dependencies in slot b ..." (cd "$SLOT_B" && pnpm install --frozen-lockfile) info "Building slot b ..." - (cd "$SLOT_B" && pnpm build) + (cd "$SLOT_B" && pnpm build:runtime) fi stage_markitdown "$SLOT_B" "b" diff --git a/packages/cli/src/config.ts b/packages/cli/src/config.ts index e63d73571..cad6eaeb4 100644 --- a/packages/cli/src/config.ts +++ b/packages/cli/src/config.ts @@ -22,7 +22,13 @@ export interface AutoUpdateBuildTimeouts { install?: number; /** `pnpm build:runtime` / `pnpm build` (default 180_000). */ build?: number; - /** `pnpm --filter dkg-evm-module build` (default 300_000; bump to 900_000 on ARM64). */ + /** + * @deprecated Ignored since the auto-updater stopped invoking `hardhat + * compile` on node hosts. Committed `packages/evm-module/abi/*.json` are + * now the runtime contract surface, and CI enforces freshness (see + * `abi-freshness` job in `.github/workflows/ci.yml`). The field is + * retained on the type so existing user configs don't fail to parse. + */ contracts?: number; /** MarkItDown bundling step (default 900_000). */ markitdown?: number; diff --git a/packages/cli/src/daemon/auto-update.ts b/packages/cli/src/daemon/auto-update.ts index 46d275eae..ec09a3f8b 100644 --- a/packages/cli/src/daemon/auto-update.ts +++ b/packages/cli/src/daemon/auto-update.ts @@ -909,84 +909,6 @@ async function cleanGeneratedOutputs( } } -/** - * Decide whether to rebuild Solidity contracts. Same semantics as the original - * inline check (skip on terminal diff failure) plus one robustness improvement: - * if the parent commit isn't reachable in the slot's pack files (most common - * cause is a shallow clone or upstream force-push rebase), try a single - * `git fetch --depth=1 origin ` and retry the diff once before - * giving up. We've never observed an ABI/JS mismatch from this skipping in - * practice, so we err toward "less work" rather than "build to be safe". - */ -async function shouldRebuildContracts(args: { - au: ResolvedAutoUpdateConfig; - fetchUrl: string; - currentCommit: string; - checkedOutCommit: string; - targetDir: string; - execFileAsync: (file: string, args: string[], opts: any) => Promise; - log: (m: string) => void; -}): Promise { - const { au, fetchUrl, currentCommit, checkedOutCommit, targetDir, execFileAsync, log } = args; - if ( - !/^[0-9a-f]{6,40}$/i.test(currentCommit) || - !/^[0-9a-f]{6,40}$/i.test(checkedOutCommit) - ) { - log('Auto-update: contract-change check skipped (commit SHAs invalid); skipping contract build.'); - return false; - } - const tryDiff = async (): Promise<{ ok: boolean; stdout?: string; err?: any }> => { - try { - const result = await execFileAsync( - 'git', - ['diff', '--name-only', `${currentCommit}..${checkedOutCommit}`], - { cwd: targetDir, encoding: 'utf-8', timeout: 30_000 }, - ); - return { ok: true, stdout: String(result?.stdout ?? '') }; - } catch (err: any) { - return { ok: false, err }; - } - }; - let diff = await tryDiff(); - if (!diff.ok) { - // Most common cause: the parent commit isn't in the slot's pack files. - // Fetch it explicitly (depth=1 on the SHA), then retry once. The slots - // are initialized with bare `git init` and fetched via direct URL — no - // `origin` remote is configured — so we must mirror the main fetch and - // pass the URL + auth args explicitly. Best-effort: if the fetch itself - // errors, skip the build (legacy behaviour); we've never observed a - // real ABI/JS mismatch from this path. - try { - log(`Auto-update: contract-diff failed; fetching parent commit ${currentCommit.slice(0, 8)} to retry.`); - await execFileAsync( - 'git', - [...gitCommandArgs(fetchUrl, au), 'fetch', '--depth=1', fetchUrl, currentCommit], - { - cwd: targetDir, - encoding: 'utf-8', - timeout: 30_000, - env: gitCommandEnv(au), - }, - ); - diff = await tryDiff(); - } catch (fetchErr: any) { - log(`Auto-update: parent-commit fetch failed (${fetchErr?.message ?? fetchErr}); skipping contract build.`); - return false; - } - } - if (!diff.ok) { - log( - `Auto-update: contract-change check failed (${diff.err?.message ?? diff.err}); skipping contract build.`, - ); - return false; - } - const changedPaths = String(diff.stdout ?? '') - .split('\n') - .map((line) => line.trim()) - .filter(Boolean); - return changedPaths.some((p) => p.startsWith('packages/evm-module/contracts/')); -} - /** * Core blue-green update logic. Builds the new version in the inactive slot, * then atomically swaps the `releases/current` symlink. @@ -1252,7 +1174,6 @@ async function _performUpdateInner( label: "pnpm install", log, }); - let usedFullBuildFallback = false; let runtimeBuildCommand = FULL_BUILD_COMMAND; try { const rootPkgRaw = await readFile( @@ -1281,78 +1202,16 @@ async function _performUpdateInner( label: FULL_BUILD_COMMAND, log, }); - usedFullBuildFallback = true; } - if (usedFullBuildFallback) { - log( - "Auto-update: contract build check skipped (full build fallback already executed).", - ); - } else { - const shouldBuildContracts = await shouldRebuildContracts({ - au, - fetchUrl, - currentCommit, - checkedOutCommit, - targetDir, - execFileAsync, - log, - }); - - if (shouldBuildContracts) { - log( - "Auto-update: contract folder changes detected; building @origintrail-official/dkg-evm-module...", - ); - // Run `hardhat clean` first so stale artifacts/, abi/, and typechain - // outputs from a deleted/renamed contract don't survive into the - // inactive slot. We deliberately scope this to the - // `shouldBuildContracts` branch: - // - the no-change branch keeps the Hardhat compile cache intact, - // which is what saves us from the cold-solc / ARM64 build - // timeout that the rest of this helper exists to prevent; - // - when contract sources actually changed we're already paying - // for a recompile, so wiping the cache here is essentially free - // and guarantees the swap doesn't activate ghost ABIs/types. - // Best-effort: a clean failure must not abort an otherwise-valid - // contract rebuild — `hardhat compile` will still recreate every - // artifact that the new source tree references; only stale outputs - // for *deleted* contracts would be missed, which is a strict - // improvement over today's behaviour anyway. - try { - await runBuildStep( - execAsync, - "pnpm --filter @origintrail-official/dkg-evm-module clean", - { - cwd: targetDir, - timeoutMs: timeouts.contracts, - label: "pnpm --filter dkg-evm-module clean", - log, - }, - ); - } catch (cleanErr: any) { - log( - `Auto-update: hardhat clean failed (${cleanErr?.message ?? String(cleanErr)}); proceeding with rebuild — stale artifacts for renamed/deleted contracts may persist.`, - ); - } - await runBuildStep( - execAsync, - "pnpm --filter @origintrail-official/dkg-evm-module build", - { - cwd: targetDir, - timeoutMs: timeouts.contracts, - label: "pnpm --filter dkg-evm-module build", - log, - }, - ); - log( - "Auto-update: @origintrail-official/dkg-evm-module build completed.", - ); - } else { - log( - "Auto-update: no contract folder changes detected; skipping @origintrail-official/dkg-evm-module build.", - ); - } - } + // NOTE: the auto-updater intentionally never invokes `hardhat compile` on + // node hosts. The committed `packages/evm-module/abi/*.json` files are the + // runtime contract surface (consumed by `packages/chain` via require()), + // and a CI gate (`abi-freshness` job in ci.yml) blocks any PR that changes + // `.sol` sources without committing the regenerated ABIs. This removes the + // single most failure-prone step from the update flow — hardhat compile + // routinely OOMs / times out on resource-constrained nodes (cold solc on + // ARM64, in particular) and any failure here would abort the slot swap. let nodeUiPackageNames = NODE_UI_PACKAGE_NAME_FALLBACKS; try { diff --git a/packages/cli/test/auto-update.test.ts b/packages/cli/test/auto-update.test.ts index 8e23a5331..7024837f0 100644 --- a/packages/cli/test/auto-update.test.ts +++ b/packages/cli/test/auto-update.test.ts @@ -1578,69 +1578,43 @@ describe('autoupdater hardening', () => { expect(installTimeout).toBe(600_000); }); - it('honours autoUpdate.buildTimeoutMs.contracts when contracts rebuild', async () => { + // ─── Contract build is OFF the node update path ───────────────────── + // + // The auto-updater intentionally never invokes `hardhat compile` on node + // hosts. The committed `packages/evm-module/abi/*.json` files are the + // runtime contract surface (consumed by `packages/chain` via require()), + // and the `abi-freshness` CI job blocks any PR that changes `.sol` + // sources without committing the regenerated ABIs. This removes the + // most failure-prone step from the update flow — `hardhat compile` + // routinely OOMs / times out on resource-constrained nodes (cold solc + // on ARM64), and any failure there used to abort the slot swap. + // + // The previous tests in this slot exercised: + // - autoUpdate.buildTimeoutMs.contracts honouring (contracts no longer rebuilt) + // - contract-diff fails closed (no diff is performed) + // - hardhat clean ordering before rebuild (no rebuild) + // - contract-diff retry via `git fetch --depth=1` (no diff) + // + // Replaced with one explicit assertion: hardhat is never invoked on the + // node update path, even when the diff between commits lists changed + // .sol sources. + it('never invokes hardhat / evm-module build on the auto-update path, even when contracts changed between commits', async () => { mockGitUpdateReadFile(); makeFetchOk('bbb222'); - let contractsTimeout: number | undefined; - execImpl = async (cmd: string, opts?: any) => { - if (cmd.includes('pnpm --filter @origintrail-official/dkg-evm-module build')) { - contractsTimeout = opts?.timeout; - } - return { stdout: '', stderr: '' }; - }; - // Force a runtime build path + contract rebuild trigger via diff. - execFileImpl = async (file: string, args: string[]) => { - if (file === 'git' && args[0] === 'diff') { - return { stdout: 'packages/evm-module/contracts/Foo.sol\n', stderr: '' }; - } - return { stdout: '', stderr: '' }; - }; - const auWithTimeout: AutoUpdateConfig = { - ...AU, - buildTimeoutMs: { contracts: 1_200_000 }, - }; - await performUpdate(auWithTimeout as any, () => {}); - expect(contractsTimeout).toBe(1_200_000); - }); - - it('contract-diff fails closed: skips contract build when diff errors and parent fetch also errors (matches legacy behaviour)', async () => { - readFileImpl = async () => 'aaa111'; - makeFetchOk('bbb222'); - let contractsBuilt = false; + const evmCommands: string[] = []; execImpl = async (cmd: string) => { - if (cmd.includes('pnpm --filter @origintrail-official/dkg-evm-module build')) { - contractsBuilt = true; + if ( + cmd.includes('@origintrail-official/dkg-evm-module') || + cmd.includes('hardhat') + ) { + evmCommands.push(cmd); } return { stdout: '', stderr: '' }; }; - execFileImpl = async (file: string, args: string[]) => { - if (file === 'git' && args[0] === 'diff') { - throw new Error('fatal: bad revision aaa111..bbb222'); - } - if (file === 'git' && args[0] === 'fetch' && args.includes('--depth=1')) { - throw new Error('fatal: remote unreachable'); - } - return { stdout: '', stderr: '' }; - }; - await performUpdate(AU, () => {}); - expect(contractsBuilt).toBe(false); - }); - - it('runs `hardhat clean` before the contract rebuild so stale artifacts/abi/typechain from renamed/deleted contracts do not survive into the slot', async () => { - // Default path skips `git clean -fdx` (cold-solc on ARM64 trips the - // build timeout) and cleanGeneratedOutputs intentionally spares - // evm-module/{cache,artifacts}/. So when contract sources actually - // change we run `hardhat clean` first to drop ghost outputs from - // deleted contracts. Scoped to the same trigger as the rebuild so - // no-change updates still benefit from the Hardhat compile cache. - mockGitUpdateReadFile(); - makeFetchOk('bbb222'); - const order: string[] = []; - execImpl = async (cmd: string) => { - if (cmd.includes('pnpm --filter @origintrail-official/dkg-evm-module clean')) order.push('clean'); - if (cmd.includes('pnpm --filter @origintrail-official/dkg-evm-module build')) order.push('build'); - return { stdout: '', stderr: '' }; - }; + // Even if the diff would have shown a .sol change, the new + // implementation must not consult it — and must not invoke any + // evm-module / hardhat command. Stub diff anyway to make this test + // a regression guard if the conditional is ever re-introduced. execFileImpl = async (file: string, args: string[]) => { if (file === 'git' && args[0] === 'diff') { return { stdout: 'packages/evm-module/contracts/Foo.sol\n', stderr: '' }; @@ -1648,45 +1622,7 @@ describe('autoupdater hardening', () => { return { stdout: '', stderr: '' }; }; await performUpdate(AU, () => {}); - expect(order).toEqual(['clean', 'build']); - }); - - it('contract-diff retries via `git fetch --depth=1` for the missing parent commit before giving up', async () => { - mockGitUpdateReadFile(); - makeFetchOk('bbb222'); - let firstDiffSeen = false; - let retryFetchArgs: string[] | null = null; - let secondDiffSeen = false; - execFileImpl = async (file: string, args: string[]) => { - if (file === 'git' && args[0] === 'diff') { - if (!firstDiffSeen) { - firstDiffSeen = true; - throw new Error('fatal: bad revision'); - } - secondDiffSeen = true; - return { stdout: 'packages/evm-module/contracts/Foo.sol\n', stderr: '' }; - } - if (file === 'git' && args.includes('fetch') && args.includes('--depth=1')) { - retryFetchArgs = args; - } - return { stdout: '', stderr: '' }; - }; - let contractsBuilt = false; - execImpl = async (cmd: string) => { - if (cmd.includes('pnpm --filter @origintrail-official/dkg-evm-module build')) { - contractsBuilt = true; - } - return { stdout: '', stderr: '' }; - }; - await performUpdate({ ...AU, repo: 'owner/repo' }, () => {}); - expect(firstDiffSeen).toBe(true); - expect(retryFetchArgs).toBeTruthy(); - expect(secondDiffSeen).toBe(true); - expect(contractsBuilt).toBe(true); - // Slots are initialized with bare `git init` and have no `origin` remote; - // the retry must use the explicit fetch URL, not the literal 'origin'. - expect(retryFetchArgs!.includes('origin')).toBe(false); - expect(retryFetchArgs!.some(a => a.includes('github.com/owner/repo'))).toBe(true); + expect(evmCommands).toEqual([]); }); it('atomic bookkeeping writes go through a temp path then rename to final', async () => { diff --git a/packages/evm-module/README.md b/packages/evm-module/README.md index 7e5fe6158..c2858aae8 100644 --- a/packages/evm-module/README.md +++ b/packages/evm-module/README.md @@ -34,6 +34,28 @@ import HubAbi from '@origintrail-official/dkg-evm-module/abi/Hub.json'; import ParanetAbi from '@origintrail-official/dkg-evm-module/abi/Paranet.json'; ``` +## Committed ABIs are the runtime contract surface + +The files under `abi/*.json` are checked into git and consumed at runtime by +`@origintrail-official/dkg-chain`. **Nodes never run `hardhat compile` during +install or auto-update** — neither `install.sh` nor `packages/cli/src/daemon/auto-update.ts` +invoke any `hardhat` command. This makes node updates fast and removes the most +failure-prone step (cold solc on small VPS / ARM64 used to OOM and abort the +slot swap). + +That contract is enforced by the `abi-freshness` job in +`.github/workflows/ci.yml`: every PR that touches `contracts/`, `hardhat.*`, +or `package.json` runs `pnpm --filter @origintrail-official/dkg-evm-module build` +and then `git diff --exit-code -- packages/evm-module/abi/`. Any drift fails +the PR with an explicit remediation message. + +If you change a `.sol` file, regenerate and commit ABIs in the same change: + +```bash +pnpm --filter @origintrail-official/dkg-evm-module build +git add packages/evm-module/abi/ +``` + ## Internal Dependencies None — standalone Solidity/Hardhat project. Consumed by `@origintrail-official/dkg-chain`. From 9234261e4d0d274277620d0813ae742aea9a010d Mon Sep 17 00:00:00 2001 From: Branimir Rakic Date: Tue, 5 May 2026 18:18:29 +0200 Subject: [PATCH 2/2] fix(ci,install): address codex review on PR #415 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs from automated review on the previous commit: 1. abi-freshness gate was a no-op `pnpm --filter dkg-evm-module build` shells out to `hardhat compile --config hardhat.node.config.ts`, but the `hardhat-abi-exporter` plugin (with `runOnCompile: true`) is only wired into the *default* `hardhat.config.ts`. The node config never touches `abi/`, so the diff check would always pass even when committed ABIs were stale — silently activating the wrong contract surface on every node. Switched the CI step to `npx hardhat compile` (no `--config`, picks up the abi-exporting default), expanded the cache key to cover `hardhat.config.ts` too, and updated the remediation message + README + auto-update.ts comment to point at the same command. Verified locally: regenerating with the fixed command yields no diff on a clean main checkout (positive smoke), and corrupting one ABI file makes `git diff --exit-code -- abi/` return 1 (negative smoke). 2. install.sh hard-coded `pnpm build:runtime` The script supports arbitrary `DKG_BRANCH` / `DKG_REPO`, so installing an older tag that predates `build:runtime` would now fail on first install. Added `runtime_build_script()` that mirrors the JS `runtimeBuildCommandFromPackageJson` fallback chain (`dkgBuild .releaseRuntimeBuildScript` -> `build:runtime` -> `build:runtime :packages` -> `build`) using the node binary that the script already requires. Validates the dkgBuild override against `[A-Za-z0-9:_-]+` so a malicious package.json can't inject shell. Smoke-tested all 6 fallback cases including malformed JSON and unsafe override values. Tests: cli/auto-update.test.ts still 73/73 green, ci.yml still parses. Co-authored-by: Cursor --- .github/workflows/ci.yml | 22 +++++++++--- install.sh | 50 ++++++++++++++++++++------ packages/cli/src/daemon/auto-update.ts | 12 ++++--- packages/evm-module/README.md | 13 ++++--- 4 files changed, 73 insertions(+), 24 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e4e1393ee..ee477a8d2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -448,11 +448,23 @@ jobs: packages/evm-module/artifacts packages/evm-module/cache packages/evm-module/typechain - key: hardhat-${{ runner.os }}-${{ hashFiles('packages/evm-module/contracts/**/*.sol', 'packages/evm-module/hardhat.node.config.ts') }} + # Hash both configs because abi/ regen depends on hardhat.config.ts + # (which is what loads hardhat-abi-exporter; see the next step). + key: hardhat-${{ runner.os }}-abi-${{ hashFiles('packages/evm-module/contracts/**/*.sol', 'packages/evm-module/hardhat.config.ts', 'packages/evm-module/hardhat.node.config.ts') }} restore-keys: | + hardhat-${{ runner.os }}-abi- hardhat-${{ runner.os }}- - - name: Compile contracts (regenerates abi/*.json) - run: pnpm --filter @origintrail-official/dkg-evm-module build + - name: Compile contracts with the default config (regenerates abi/*.json) + # Subtle: `pnpm --filter dkg-evm-module build` ultimately calls + # `hardhat compile --config hardhat.node.config.ts`, which does NOT + # have `hardhat-abi-exporter` wired in. Only the default + # `hardhat.config.ts` imports the plugin and sets `abiExporter: + # { runOnCompile: true, ... }`. Running `npx hardhat compile` + # without a `--config` flag (so it picks up the default config) + # is what actually regenerates `abi/*.json`. Without this, the + # diff check below would always pass even with stale ABIs. + run: npx hardhat compile + working-directory: packages/evm-module - name: Verify committed abi/ matches regenerated abi/ # `git diff --exit-code` returns 1 if anything under abi/ changed # after compile. If so, the contributor forgot to commit the @@ -461,13 +473,13 @@ jobs: run: | set -euo pipefail if ! git diff --exit-code -- packages/evm-module/abi/; then - echo "::error title=ABI drift::Committed packages/evm-module/abi/*.json does not match the output of \`pnpm --filter @origintrail-official/dkg-evm-module build\`." + echo "::error title=ABI drift::Committed packages/evm-module/abi/*.json does not match the output of \`hardhat compile\` (default config)." echo "" echo "The auto-updater no longer runs hardhat compile on node hosts (see comment in packages/cli/src/daemon/auto-update.ts), so committed ABIs ARE the runtime contract surface. Drift here would silently activate stale ABIs against new contracts on every node." echo "" echo "To fix: regenerate and commit the ABIs locally:" echo "" - echo " pnpm --filter @origintrail-official/dkg-evm-module build" + echo " cd packages/evm-module && npx hardhat compile && cd -" echo " git add packages/evm-module/abi/" echo " git commit --amend --no-edit # or a fresh commit, your call" echo " git push --force-with-lease" diff --git a/install.sh b/install.sh index 762ea7475..33af1daa6 100755 --- a/install.sh +++ b/install.sh @@ -40,6 +40,34 @@ slot_ready() { [ -d "$slot_path/.git" ] && [ -f "$entry_path" ] } +# Mirror packages/cli/src/node-ui-static.ts:runtimeBuildCommandFromPackageJson. +# Honour `dkgBuild.releaseRuntimeBuildScript` first, then fall back through +# `build:runtime` and `build:runtime:packages`, finally `build`. This keeps +# the install path compatible with arbitrary $DKG_BRANCH / $DKG_REPO targets +# (e.g. older tags that predate the runtime-build split). +runtime_build_script() { + slot_path="$1" + node -e " + try { + const fs = require('fs'); + const pkg = JSON.parse(fs.readFileSync('$slot_path/package.json', 'utf-8')); + const isSafe = (s) => typeof s === 'string' && /^[A-Za-z0-9:_-]+\$/.test(s); + const rrbs = pkg.dkgBuild && pkg.dkgBuild.releaseRuntimeBuildScript; + if (isSafe(rrbs) && pkg.scripts && typeof pkg.scripts[rrbs] === 'string') { + process.stdout.write(rrbs); + } else if (pkg.scripts && typeof pkg.scripts['build:runtime'] === 'string') { + process.stdout.write('build:runtime'); + } else if (pkg.scripts && typeof pkg.scripts['build:runtime:packages'] === 'string') { + process.stdout.write('build:runtime:packages'); + } else { + process.stdout.write('build'); + } + } catch (e) { + process.stdout.write('build'); + } + " +} + stage_markitdown() { slot_path="$1" slot_name="$2" @@ -76,14 +104,15 @@ else git clone --branch "$BRANCH" "$REPO_URL" "$SLOT_A" info "Installing dependencies in slot a ..." (cd "$SLOT_A" && pnpm install --frozen-lockfile) - info "Building slot a ..." - # Runtime build only — skips evm-module's hardhat compile. The committed - # `packages/evm-module/abi/*.json` files are the runtime contract surface - # consumed by `packages/chain`; CI enforces they stay in sync with the - # Solidity sources, so nodes never need to invoke `hardhat compile`. - # Mirrors what the auto-updater does (see node-ui-static.ts: - # runtimeBuildCommandFromPackageJson). - (cd "$SLOT_A" && pnpm build:runtime) + # Runtime build — picks the best available script in the target ref + # (mirrors the auto-updater's fallback so older tags still install + # correctly). On current main this resolves to `build:runtime:packages` + # via `dkgBuild.releaseRuntimeBuildScript`, which skips evm-module's + # hardhat compile. The committed `packages/evm-module/abi/*.json` files + # are the runtime contract surface; CI enforces they stay in sync. + SLOT_A_BUILD_SCRIPT=$(runtime_build_script "$SLOT_A") + info "Building slot a (pnpm run $SLOT_A_BUILD_SCRIPT) ..." + (cd "$SLOT_A" && pnpm run "$SLOT_A_BUILD_SCRIPT") fi stage_markitdown "$SLOT_A" "a" @@ -95,8 +124,9 @@ else git clone --reference "$SLOT_A" --dissociate --branch "$BRANCH" "$REPO_URL" "$SLOT_B" info "Installing dependencies in slot b ..." (cd "$SLOT_B" && pnpm install --frozen-lockfile) - info "Building slot b ..." - (cd "$SLOT_B" && pnpm build:runtime) + SLOT_B_BUILD_SCRIPT=$(runtime_build_script "$SLOT_B") + info "Building slot b (pnpm run $SLOT_B_BUILD_SCRIPT) ..." + (cd "$SLOT_B" && pnpm run "$SLOT_B_BUILD_SCRIPT") fi stage_markitdown "$SLOT_B" "b" diff --git a/packages/cli/src/daemon/auto-update.ts b/packages/cli/src/daemon/auto-update.ts index ec09a3f8b..157c3dcda 100644 --- a/packages/cli/src/daemon/auto-update.ts +++ b/packages/cli/src/daemon/auto-update.ts @@ -1207,11 +1207,13 @@ async function _performUpdateInner( // NOTE: the auto-updater intentionally never invokes `hardhat compile` on // node hosts. The committed `packages/evm-module/abi/*.json` files are the // runtime contract surface (consumed by `packages/chain` via require()), - // and a CI gate (`abi-freshness` job in ci.yml) blocks any PR that changes - // `.sol` sources without committing the regenerated ABIs. This removes the - // single most failure-prone step from the update flow — hardhat compile - // routinely OOMs / times out on resource-constrained nodes (cold solc on - // ARM64, in particular) and any failure here would abort the slot swap. + // and a CI gate (`abi-freshness` job in ci.yml) runs `npx hardhat compile` + // (default config, the one that loads `hardhat-abi-exporter`) on every + // contract-touching PR and blocks merge if the regenerated `abi/` differs + // from what was committed. This removes the single most failure-prone + // step from the update flow — hardhat compile routinely OOMs / times out + // on resource-constrained nodes (cold solc on ARM64, in particular) and + // any failure here would abort the slot swap. let nodeUiPackageNames = NODE_UI_PACKAGE_NAME_FALLBACKS; try { diff --git a/packages/evm-module/README.md b/packages/evm-module/README.md index c2858aae8..d0b22f4dc 100644 --- a/packages/evm-module/README.md +++ b/packages/evm-module/README.md @@ -45,14 +45,19 @@ slot swap). That contract is enforced by the `abi-freshness` job in `.github/workflows/ci.yml`: every PR that touches `contracts/`, `hardhat.*`, -or `package.json` runs `pnpm --filter @origintrail-official/dkg-evm-module build` -and then `git diff --exit-code -- packages/evm-module/abi/`. Any drift fails -the PR with an explicit remediation message. +or `package.json` runs `npx hardhat compile` (default config — picks up +`hardhat-abi-exporter`) and then `git diff --exit-code -- packages/evm-module/abi/`. +Any drift fails the PR with an explicit remediation message. + +> Note: the package's `pnpm build` script intentionally runs Hardhat with +> `--config hardhat.node.config.ts`, which does **not** load +> `hardhat-abi-exporter`. Use `npx hardhat compile` (no `--config`) when you +> need to regenerate `abi/*.json`. If you change a `.sol` file, regenerate and commit ABIs in the same change: ```bash -pnpm --filter @origintrail-official/dkg-evm-module build +cd packages/evm-module && npx hardhat compile && cd - git add packages/evm-module/abi/ ```