feat: compact-deployer tool (Part 1/2)#86
Open
0xisk wants to merge 2 commits into
Open
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds a deployer package, CLI entrypoints, a fungible-token example workspace, and integration test harnesses/specs. The changes also introduce deployer configuration, wallet/proof-server handling, deployment persistence, and coverage/test-runner wiring. ChangesDeploy tooling and integration stack
Sequence Diagram(s)sequenceDiagram
participant RunDeploy
participant Deployer
participant ProofServer
participant WalletHandler
participant Deployments
RunDeploy->>Deployer: prepare(...)
Deployer->>ProofServer: start(...)
Deployer->>WalletHandler: build(...)
Deployer->>Deployer: deploy() or dryRun()
Deployer->>Deployments: record(...)
Deployer-->>RunDeploy: DeployResult
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![boostsecurity-io[bot]](https://avatars.githubusercontent.com/in/192984?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ 9 New Security Findings
The latest commit contains 9 new security findings.
| Findings | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Dependency: yarn / brace-expansion@ 2.0.2 SUMMARY Direct Dependency: brace-expansion Location : yarn.lock OCCURRENCE
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
Remediation :
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Dependency: yarn / ip-address@ 10.0.1 SUMMARY Direct Dependency: ip-address Location : yarn.lock OCCURRENCE
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
Remediation :
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Dependency: yarn / minimatch@ 9.0.5 SUMMARY Direct Dependency: minimatch Location : yarn.lock OCCURRENCES
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
Remediation :
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Dependency: yarn / picomatch@ 4.0.3 SUMMARY Direct Dependency: picomatch Location : yarn.lock OCCURRENCES
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
Remediation :
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Dependency: yarn / postcss@ 8.5.6 SUMMARY Direct Dependency: postcss Location : yarn.lock OCCURRENCE
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
Remediation :
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Dependency: yarn / rollup@ 4.52.5 SUMMARY Direct Dependency: rollup Location : yarn.lock OCCURRENCE
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
Remediation :
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Dependency: yarn / tar@ 7.5.3 SUMMARY Direct Dependency: tar Location : yarn.lock OCCURRENCES
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
Remediation :
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Dependency: yarn / undici@ 5.29.0 SUMMARY Direct Dependency: undici Location : yarn.lock OCCURRENCES
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
Remediation :
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Dependency: yarn / vite@ 7.1.12 SUMMARY Direct Dependency: vite Location : yarn.lock OCCURRENCES
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
Remediation :
|
Not a finding? Ignore it by adding a comment on the line with just the word noboost.
Scanner: boostsecurity - Trivy (Filesystem scanning)
There was a problem hiding this comment.
⚠️ 10 New Security Findings
The latest commit contains 10 new security findings.
| Findings | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Dependency: npm / @substrate/connect@ 0.8.11 SUMMARY Dependency: @substrate/connect Transitive through:
Location : yarn.lock OCCURRENCE
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Remediation :
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Dependency: npm / glob@ 10.5.0 SUMMARY Dependency: glob Transitive through:
Location : yarn.lock OCCURRENCE
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Remediation :
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Dependency: npm / minimatch@ 9.0.5 SUMMARY Dependency: minimatch Transitive through:
Location : yarn.lock OCCURRENCES
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Remediation :
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Dependency: npm / node-domexception@ 1.0.0 SUMMARY Dependency: node-domexception Transitive through:
Location : yarn.lock OCCURRENCE
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Remediation :
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Dependency: npm / picomatch@ 4.0.3 SUMMARY Dependency: picomatch Transitive through:
Location : yarn.lock OCCURRENCE
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Remediation :
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Dependency: npm / rollup@ 4.52.5 SUMMARY Dependency: rollup Transitive through:
Location : yarn.lock OCCURRENCE
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Remediation :
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Dependency: npm / tar@ 7.5.3 SUMMARY Dependency: tar Transitive through:
Location : yarn.lock OCCURRENCES
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Remediation :
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Dependency: npm / undici@ 5.29.0 SUMMARY Dependency: undici Transitive through:
Location : yarn.lock OCCURRENCES
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Remediation :
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Dependency: npm / uuid@ 10.0.0 SUMMARY Dependency: uuid Transitive through:
Location : yarn.lock OCCURRENCE
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Remediation :
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Dependency: npm / vite@ 7.1.12 SUMMARY Dependency: vite Transitive through:
Location : yarn.lock OCCURRENCES
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Remediation :
|
Not a finding? Ignore it by adding a comment on the line with just the word noboost.
Scanner: boostsecurity - BoostSecurity SCA
0xisk
force-pushed
the
feat/compact-deployer
branch
from
958d227 to
26aa42b
Compare
0xisk
added a commit
that referenced
this pull request
May 19, 2026
…ions Addresses the BoostSecurity findings on PR #86. Eight of the originally flagged packages auto-upgraded when yarn.lock regenerated post-rebase (brace-expansion, ip-address, minimatch, picomatch, postcss, rollup, tar, vite). The remaining three are forced via root resolutions: - undici ^6.24.0 (was 5.29.0 via testcontainers) Fixes CVE-2026-1525/1526/1527/22036/2229 — HTTP smuggling, WebSocket memory exhaustion, CRLF injection, decompression DoS. Major bump but testcontainers' usage stays within the v6 API. - glob ^11.0.0 (was 10.5.0 via archiver-utils, cacache) Replaces the EOL v10 line. - uuid ^13.0.0 (was 10.0.0 via dockerode) Replaces the EOL v10 line. Two findings left unaddressed: - @substrate/connect@0.8.11 (EOL warning). Pinned exactly by `@polkadot/rpc-provider@16.5.6` (constraint is `0.8.11`, not a range), so resolutions can't override it without breaking that transitive. Would need to wait for @PolkaDot to publish a release that drops the EOL dep. - node-domexception@1.0.0 (EOL warning). Pulled by `fetch-blob@3.2.0`. Modern Node has a native DOMException; the package only matters until fetch-blob/undici upstream drops the polyfill import. No safe override exists. Both are warning-level (EOL classification, no active CVE). Build + 50 unit tests + CLI typecheck still pass after resolutions.
![semgrep-code-openzeppelin[bot]](https://avatars.githubusercontent.com/u/20820676?s=60&v=4){kind=small}
![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
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (14)
packages/deployer/src/wallet/keystore.ts-196-201 (1)
196-201:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEnforce seed length constraints at keystore creation time.
seedFromHexcurrently accepts any even-length hex, so users can create keystores that later failclassifySeed(which expects 64/128 hex chars). Validate length here to fail fast.Suggested fix
function seedFromHex(hex: string): Buffer { const stripped = hex.startsWith('0x') ? hex.slice(2) : hex; - if (!/^[0-9a-fA-F]+$/.test(stripped) || stripped.length % 2 !== 0) { - throw new WalletError('Seed must be hex-encoded'); + const isValidHex = /^[0-9a-fA-F]+$/.test(stripped); + const isSupportedLength = stripped.length === 64 || stripped.length === 128; + if (!isValidHex || !isSupportedLength) { + throw new WalletError('Seed must be a 64- or 128-char hex string'); } return Buffer.from(stripped, 'hex'); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/deployer/src/wallet/keystore.ts` around lines 196 - 201, seedFromHex currently accepts any even-length hex and should instead enforce the expected seed lengths so keystores fail fast: in seedFromHex, after stripping the 0x prefix and validating hex characters, check that stripped.length is either 64 or 128 (matching classifySeed expectations) and throw a WalletError with a clear message if not; otherwise continue to return Buffer.from(stripped, 'hex'). Reference: seedFromHex and classifySeed.tests/integrations/README.md-7-25 (1)
7-25:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a language to the fenced layout block.
Line 7 opens a fenced code block without a language tag, which triggers markdownlint MD040.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integrations/README.md` around lines 7 - 25, The README's fenced code block starting with ``` lacks a language tag (markdownlint MD040); update the opening fence from ``` to include a language specifier (for example ```text or ```bash) so the block is properly tagged—modify the README's fenced block that contains the directory tree (the ``` fence and its closing ``` fence) to add the chosen language tag.Source: Linters/SAST tools
tests/integrations/_harness/paths.ts-20-37 (1)
20-37:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStrengthen fixture artifact preflight checks beyond directory existence.
existsSync(...)alone accepts empty/stale directories, so tests can pass preflight and fail later with less clear errors. Validate that the artifact directory contains expected compiled outputs (or at least is non-empty).Suggested patch
-import { existsSync, rmSync } from 'node:fs'; +import { existsSync, readdirSync, rmSync } from 'node:fs'; @@ +function hasCompiledArtifact(dir: string): boolean { + return existsSync(dir) && readdirSync(dir).length > 0; +} + export function requireFixtureArtifact(): void { - if (existsSync(ARTIFACT_DIR)) return; + if (hasCompiledArtifact(ARTIFACT_DIR)) return; @@ export function requirePrivateCounterArtifact(): void { - if (existsSync(PRIVATE_COUNTER_ARTIFACT_DIR)) return; + if (hasCompiledArtifact(PRIVATE_COUNTER_ARTIFACT_DIR)) return;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integrations/_harness/paths.ts` around lines 20 - 37, The current preflight helpers requireFixtureArtifact and requirePrivateCounterArtifact only call existsSync(ARTIFACT_DIR) / existsSync(PRIVATE_COUNTER_ARTIFACT_DIR), which permits empty/stale directories; update both functions to ensure the artifact directory is not only present but contains expected compiled outputs (e.g., check readdirSync(...) yields at least one file and/or assert presence of expected filenames/extensions such as .wasm/.json); if the directory is empty or missing expected files, throw the same Error with the helpful "Run `make -C tests/integrations compile` first." message so failures surface earlier and with clearer diagnostics.packages/deployer/src/loaders/constructor-meta.ts-34-46 (1)
34-46:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse own-property checks when validating named args.
Using
incan match prototype properties. UseObject.hasOwn/hasOwnPropertyto validate only explicit user-provided keys.Proposed fix
export function reorderNamedArgs( named: Record<string, unknown>, paramNames: readonly string[], ): unknown[] { - const missing = paramNames.filter((n) => !(n in named)); + const hasOwn = Object.prototype.hasOwnProperty; + const missing = paramNames.filter((n) => !hasOwn.call(named, n)); if (missing.length > 0) { throw new ConfigError( `args object is missing constructor parameter(s): ${missing.join(', ')}`, ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/deployer/src/loaders/constructor-meta.ts` around lines 34 - 46, The validation uses the `in` operator which matches prototype properties; update the missing-parameter check in the constructor args validation to only consider own properties by replacing `(n in named)` with an own-property check such as `Object.hasOwn(named, n)` (or `Object.prototype.hasOwnProperty.call(named, n)` for older runtimes), keeping the rest of the logic (throwing ConfigError, computing extra via Object.keys(named), and returning `paramNames.map((n) => named[n])`) unchanged.packages/deployer/src/providers/private-state-password.test.ts-38-60 (1)
38-60:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEnsure mocked
node:cryptois always restored.Cleanup at Line 58-59 won’t run if the test fails earlier, which can contaminate subsequent tests in the same worker. Wrap the mock scope in
try/finally.Suggested fix
it('should throw after 1024 rounds when every hash has 4+ identical chars', async () => { - // Force every round to produce a string that hits the `(.)\1{3,}` guard so - // the loop exhausts its retry budget and reaches the explicit throw. - vi.resetModules(); - vi.doMock('node:crypto', async () => { - const actual = - await vi.importActual<typeof import('node:crypto')>('node:crypto'); - return { - ...actual, - createHash: () => ({ - update: () => ({ - digest: () => 'aaaaBBBBccccDDDD', - }), - }), - }; - }); - const { derivePrivateStatePassword: derive } = await import( - './private-state-password.ts' - ); - expect(() => derive('anything')).toThrow(/unable to find a hash/); - vi.doUnmock('node:crypto'); - vi.resetModules(); + vi.resetModules(); + try { + vi.doMock('node:crypto', async () => { + const actual = + await vi.importActual<typeof import('node:crypto')>('node:crypto'); + return { + ...actual, + createHash: () => ({ + update: () => ({ + digest: () => 'aaaaBBBBccccDDDD', + }), + }), + }; + }); + const { derivePrivateStatePassword: derive } = await import( + './private-state-password.ts' + ); + expect(() => derive('anything')).toThrow(/unable to find a hash/); + } finally { + vi.doUnmock('node:crypto'); + vi.resetModules(); + } });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/deployer/src/providers/private-state-password.test.ts` around lines 38 - 60, The test mocks node:crypto but restores it after assertions, which won't run if the test throws; wrap the mock/require/assert/unmock sequence in a try/finally so node:crypto is always restored: call vi.resetModules(); vi.doMock('node:crypto', ...) then import { derivePrivateStatePassword as derive } and run the expect inside the try block, and in finally call vi.doUnmock('node:crypto') and vi.resetModules(); reference derivePrivateStatePassword / derive and the vi.doMock/vi.doUnmock calls so the mock is reliably cleaned up even when the test throws.packages/deployer/README.md-23-37 (1)
23-37:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd languages to fenced code blocks for lint compliance.
Lines 23 and 64 use unlabeled fenced blocks, which triggers MD040.
Suggested fix
-``` +```text compact-deploy <Contract> --network <name> required unless [profile].default_network is set --config <path> default: walk up from CWD for compact.toml @@ -v, --verbose pino debug logs to .compact/logs/<timestamp>.log -h, --help --version@@
-+bash
compact-deploy --network preprod
--seed-cache-from-dust /path/to/state.json
--seed-cache-from-shielded /path/to/shielded.json # optionalAlso applies to: 64-68
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/deployer/README.md` around lines 23 - 37, The README has unlabeled fenced code blocks causing MD040; update the two unlabeled blocks that show the CLI usage and example (the blocks beginning with "compact-deploy <Contract>" and the example "compact-deploy <Contract> --network preprod \") by adding appropriate language identifiers: use "text" for the CLI usage block and "bash" (or "sh") for the example block. Ensure both fenced blocks are updated so the first starts with ```text and the second with ```bash to satisfy the linter.Source: Linters/SAST tools
packages/deployer/README.md-7-7 (1)
7-7:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winBroken roadmap anchor link.
Line 7 links to
#roadmap--todo, but this README has no matching section, so the link is dead.Suggested fix
-> **Status: developer-preview, testnet only.** Verified on local devnet + preview. Preprod blocked ([Known issues](`#known-issues-may-2026`)). Mainnet unsupported: unaudited, no hardware signer, no multisig, no tx retry, no upgrade tooling. See [Roadmap](`#roadmap--todo`). +> **Status: developer-preview, testnet only.** Verified on local devnet + preview. Preprod blocked ([Known issues](`#known-issues-may-2026`)). Mainnet unsupported: unaudited, no hardware signer, no multisig, no tx retry, no upgrade tooling.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/deployer/README.md` at line 7, The README contains a dead anchor link `#roadmap--todo`; update the markdown to point to the actual Roadmap heading or create a matching section: either change the link target in the Status line from `#roadmap--todo` to the real anchor (e.g., `#roadmap--todo` -> `#roadmap` or `#roadmap--todo` -> `#roadmap-and-todo` depending on the existing heading text), or add a "Roadmap / TODO" heading that generates the `#roadmap--todo` anchor so the link resolves; ensure the visible link text ("Roadmap") and the heading slug match.Source: Linters/SAST tools
packages/deployer/src/config/compact-config.test.ts-48-50 (1)
48-50:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTest can pass for the wrong failure path.
Line 49 creates a second
[profile]table, which can fail at TOML parsing before thedefault_networksemantic check runs. This weakens what this test is asserting.💡 Suggested fix
- const dir = tmpRepo(`${MIN_VALID}\n[profile]\ndefault_network = "ghost"\n`); + const invalid = MIN_VALID.replace( + 'default_network = "local"', + 'default_network = "ghost"', + ); + const dir = tmpRepo(invalid);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/deployer/src/config/compact-config.test.ts` around lines 48 - 50, The test is creating a second [profile] table which can trigger a TOML parse error before the semantic check; change the tmpRepo content so it does not duplicate the [profile] header but instead adds default_network = "ghost" inside the already-existing profile from MIN_VALID (i.e. append 'default_network = "ghost"' under the existing [profile] rather than adding a new '[profile]' line) so CompactConfig.load(...) exercises the semantic validation for default_network missing.packages/deployer/src/deployer.ts-580-583 (1)
580-583:⚠️ Potential issue | 🟡 MinorAbsolute sync timeout concern is overstated for this pipeline
timeout({ each: timeoutMs })does restart on every emission, but becauseRx.filter((s: FacadeState) => s.isSynced)is upstream, onlyisSynced=trueemissions can reset the timer—progress emissions whereisSyncedis false won’t extend it. Switching tofirstis only necessary if the code can emit multipleisSynced=truevalues without terminating (e.g., it does not stop after the first synced emission).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/deployer/src/deployer.ts` around lines 580 - 583, The current pipeline uses Rx.timeout({ each: timeoutMs }) after Rx.filter((s: FacadeState) => s.isSynced) so only emissions with s.isSynced === true will reset the timer; if you want any progress emission to extend the timeout, move Rx.timeout before the Rx.filter so all FacadeState emissions reset the timer, otherwise if your intent is to bound time until the first synced emission (and not allow repeated isSynced=true to restart it) replace the timeout call with Rx.timeout({ first: timeoutMs }); update the pipeline around Rx.timeout and Rx.filter accordingly.examples/fungible-token/README.md-16-20 (1)
16-20:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winConstructor type table conflicts with the Uint mapping.
The table lists
_decimals/_feeBpsasnumber, but later this README statesUint<N>maps tobigintand examples use bigint literals. Please make the table consistent.Suggested fix
-| `_decimals` | `Uint<8>` | `number` | +| `_decimals` | `Uint<8>` | `bigint` | ... -| `_feeBps` | `Uint<32>` | `number` | +| `_feeBps` | `Uint<32>` | `bigint` |Also applies to: 143-144
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/fungible-token/README.md` around lines 16 - 20, The constructor type table incorrectly lists `_decimals` and `_feeBps` as `number` while `Uint<N>` is mapped to `bigint` elsewhere; update the table rows for `_decimals` and `_feeBps` to use `BigInt`/`bigint` (consistent with `_maxSupply`/`_quorum` and the README examples), and apply the same correction to the duplicated entries at lines referenced (around 143-144). Ensure the table consistently reflects that `Uint<N>` maps to `bigint`.examples/README.md-16-16 (1)
16-16:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlign artifact policy docs across example READMEs.
This says artifacts are committed, but the fungible-token README describes
artifacts/as gitignored and locally generated. Please make both docs consistent to avoid broken setup expectations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/README.md` at line 16, The README currently states "Compiled artifacts ship committed..." which conflicts with the fungible-token README that says artifacts/ is gitignored and generated locally; make the artifact policy consistent across both READMEs by choosing one policy and updating the sentence that references "artifacts/" (the line currently claiming compiled artifacts are committed) to match the chosen policy, and ensure the fungible-token README has the same wording and instructions for setup, generation, or deployment so both docs present the same expectations.packages/cli/src/runDeploy.ts-106-112 (1)
106-112:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFail fast on unknown short flags and extra positional args.
Current parsing treats unknown single-dash args as positional, and silently ignores extra positionals after the first contract. That produces confusing downstream errors instead of actionable CLI feedback.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/runDeploy.ts` around lines 106 - 112, The parser currently treats unknown single-dash tokens as positional and allows extra positionals; update the default branch in the argument loop in runDeploy.ts so that if arg startsWith('-') (single-dash short flag) you throw an Error(`Unknown flag: ${arg}`) instead of pushing to out.positional, and after parsing validate out.positional: if out.positional.length === 0 throw a missing contract error, and if out.positional.length > 1 throw an Error indicating extra positional args (mention the unexpected tokens) before assigning out.contract = out.positional[0]; reference the variables arg, out.positional, and out.contract when locating where to change behavior.packages/cli/src/runDeploy.ts-95-103 (1)
95-103:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHarden
--sync-timeoutparsing to reject malformed values.
parseInt()accepts strings like10abc. This should fail validation instead of being treated as10.Suggested fix
- const seconds = Number.parseInt(raw, 10); - if (!Number.isFinite(seconds) || seconds <= 0) { + if (!/^\d+$/.test(raw)) { + throw new Error( + `--sync-timeout requires a positive integer (seconds); got "${raw}"`, + ); + } + const seconds = Number(raw); + if (!Number.isSafeInteger(seconds) || seconds <= 0) { throw new Error( `--sync-timeout requires a positive integer (seconds); got "${raw}"`, ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/runDeploy.ts` around lines 95 - 103, The --sync-timeout branch currently uses Number.parseInt(raw, 10) which permits inputs like "10abc"; update the parsing in the case '--sync-timeout' (around expectValue and out.syncTimeoutSec) to reject malformed values by ensuring the raw string is an exact integer representation before assigning out.syncTimeoutSec — e.g., parse the number and verify String(seconds) === raw (or use a /^\d+$/ check) and then confirm Number.isFinite(seconds) && seconds > 0, otherwise throw the existing error with the original raw value.examples/fungible-token/README.md-26-43 (1)
26-43:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a language tag to the fenced tree block.
The block is missing a fence language and triggers MD040.
Suggested fix
-``` +```text fungible-token/ contracts/ ... -``` +```🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/fungible-token/README.md` around lines 26 - 43, The fenced tree block starting with "fungible-token/" in examples/fungible-token/README.md is missing a language tag (triggers MD040); update the opening fence from ``` to ```text so the block becomes a text-fenced code block (i.e., change the opening triple-backticks that precede "fungible-token/" to include the "text" tag).Source: Linters/SAST tools
🧹 Nitpick comments (3)
tests/integrations/specs/deploy/historyIsolation.spec.ts (1)
26-38: Shared deployment-state wipes—race unlikely under current single-fork config, but shared state still couples specs.
tests/integrations/vitest.config.tsuses Vitest forks withpoolOptions.forks.singleFork: true, which should prevent concurrent execution across spec files, reducing the flakiness risk from wipingtests/integrations/deployments/compactinhistoryIsolation.spec.ts(beforeAll/afterAll).Still, all specs share the same
DEPLOYMENTS_DIRand rely on wipes; if parallelism settings change, this becomes order-dependent. Consider isolating the deployments directory per spec/worker to avoid future nondeterminism.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integrations/specs/deploy/historyIsolation.spec.ts` around lines 26 - 38, The test currently calls wipeDeployments() in beforeAll/afterAll and uses shared DEPLOYMENTS_DIR (via requireFixtureArtifact()/deployFixture()), coupling specs; change the test setup to use an isolated deployments directory per spec/worker by deriving a unique DEPLOYMENTS_DIR (e.g. from Vitest worker id or a temp UUID) and pass it into requireFixtureArtifact()/deployFixture() or make wipeDeployments() operate on that per-test directory; update historyIsolation.spec.ts to set the unique directory before calling requireFixtureArtifact(), deployFixture(), and wipeDeployments() so each spec uses its own deployments folder and no longer shares global state.tests/integrations/specs/deploy/deploy.spec.ts (1)
33-35: ⚡ Quick winTighten hex ID assertions to catch truncation regressions.
/^[0-9a-f]+$/iis permissive and accepts malformed short values. Use length-constrained patterns (matching the deployer contract for address/tx IDs) so persistence and return-shape regressions fail fast.Suggested patch
+const HEX_64 = /^[0-9a-f]{64}$/i; @@ - expect(result.address).toMatch(/^[0-9a-f]+$/i); - expect(result.txId).toMatch(/^[0-9a-f]+$/i); - expect(result.txHash).toMatch(/^[0-9a-f]+$/i); + expect(result.address).toMatch(HEX_64); + expect(result.txId).toMatch(HEX_64); + expect(result.txHash).toMatch(HEX_64); @@ - expect(head.Counter.address).toMatch(/^[0-9a-f]+$/i); - expect(head.Counter.txHash).toMatch(/^[0-9a-f]+$/i); + expect(head.Counter.address).toMatch(HEX_64); + expect(head.Counter.txHash).toMatch(HEX_64);Also applies to: 47-49
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integrations/specs/deploy/deploy.spec.ts` around lines 33 - 35, The hex assertions are too permissive; update the regexes for result.address, result.txId and result.txHash to enforce exact lengths used by the deployer contract (e.g., address = 40 hex chars, txId/txHash = 64 hex chars) so truncated values fail; replace /^[0-9a-f]+$/i with length-constrained patterns (e.g., /^[0-9a-f]{40}$/i and /^[0-9a-f]{64}$/i as appropriate) and make the same change for the other duplicate assertions further down in the spec.packages/cli/test/prompt.test.ts (1)
79-149: ⚡ Quick winAdd regression tests for stdin
endanderrorpaths.Please add cases asserting promise settlement when the stream ends/errors, so prompt termination behavior stays stable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/test/prompt.test.ts` around lines 79 - 149, Add two regression tests to prompt.test.ts that exercise stdin 'end' and 'error' paths: use promptPassphrase(...) to start the promise, trigger mockStdin.emit('end') in one test and assert the promise settles (rejects) and raw mode is turned off and stdin.pause() called; in the other test emit mockStdin.emit('error', new Error('boom')) and assert the promise rejects with that error (or the propagated error) and cleanup (setRawMode(false) and pause) occurs; locate tests near the existing describe blocks and reuse mockStdin.emit, mockStdin.setRawMode, and mockStdin.pause checks already used in other cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/src/logger.ts`:
- Around line 18-20: The logger currently returns pino() to STDOUT when
createLogger({ json: true }) is used, which can interleave with the single JSON
result and prompt output; update createLogger to route pino logs to STDERR (or
disable them) when opts.json is true by providing an explicit
destination/transport to STDERR so all logger.* output goes to stderr, and also
update promptPassphrase (or any prompt printing) to write its prompt text to
STDERR when json mode is enabled; locate createLogger in
packages/cli/src/logger.ts and promptPassphrase (or the passphrase prompt
function) and change their output targets accordingly so only the final JSON
result is written to STDOUT.
In `@packages/cli/src/prompt.ts`:
- Around line 9-47: readMaskedLine currently can hang if stdin closes without a
newline; add handlers for stdin 'end' and 'error' events so the Promise always
settles: attach stdin.once('end', ...) to cleanup and resolve with the current
buffer, attach stdin.once('error', ...) to cleanup and reject with the error,
and ensure cleanup removes these listeners (and still restores raw mode, pauses,
removes 'data' listener and writes newline) so readMaskedLine always resolves or
rejects when input closes or errors; update references in the function
(readMaskedLine, onData, cleanup) accordingly.
In `@packages/deployer/src/config/schema.ts`:
- Around line 43-49: The union allows ambiguous objects because z.object strips
unknown keys by default; update fileRefSchema and moduleRefSchema to be strict
(e.g., call .strict() on each z.object) so that an input containing both file
and module will fail validation and therefore fileOrModuleRefSchema will reject
ambiguous refs; modify the definitions of fileRefSchema and moduleRefSchema
(used by fileOrModuleRefSchema) accordingly.
In `@packages/deployer/src/deployments.ts`:
- Around line 111-113: The writeJson function currently writes directly to the
target file which can corrupt the JSON if interrupted; make it atomic by writing
to a temporary file and then renaming it into place: open a temp path (e.g.
`${path}.tmp-${process.pid}-${Date.now()}`) with fs.promises.open, write the
JSON payload, call handle.sync() (fsync) and close the handle, then fs.rename
the temp to the final path; ensure any errors clean up the temp file. Update
writeJson to use this temp-write+fsync+rename sequence to guarantee crash-safe
durable writes.
In `@packages/deployer/src/loaders/artifact.ts`:
- Around line 65-67: The code currently hardcodes contractDir =
resolve(artifactPath, 'contract') but then calls findEntry(artifactPath) which
may return a top-level entry; fix by computing the actual compiled-asset
directory from the resolved entry returned by findEntry instead of the hardcoded
'contract' path. Concretely, call findEntry(artifactPath) first, then derive
contractDir from the entry (e.g., the entry's parent/resolved directory) and use
that contractDir everywhere assets are bound (the same places where contractDir
is passed later around entry/asset binding), replacing any remaining uses of the
original hardcoded resolve(artifactPath, 'contract').
In `@packages/deployer/src/loaders/constructor-meta.ts`:
- Around line 21-24: The current check throws a ConfigError when names.length
=== 0 even if the caller passed an empty named-args object; modify the logic so
that when names.length === 0 you detect an empty plain object (e.g. args is an
object and Object.keys(args).length === 0) and treat it as an empty positional
args array (resolve to []), otherwise keep throwing the ConfigError referencing
artifactPath; update the branch that currently throws the ConfigError (the
symbols names, artifactPath, and ConfigError) to perform this empty-object check
and return [] when appropriate.
In `@packages/deployer/src/loaders/contract-resolve.ts`:
- Around line 88-91: The current absolute-path check in resolveUnderRoot uses
artifact.startsWith('/'), which fails on Windows; change it to use
path.isAbsolute(artifact) (i.e., import/qualify isAbsolute from the path module)
so Windows-style absolute paths like C:\... and UNC paths are detected
correctly; keep the rest of the logic (resolve(rootDir, artifact), existsSync
check, fallback to resolve(rootDir, artifactsDir, artifact)) unchanged and
reference resolveUnderRoot, artifact, rootDir, artifactsDir when making the
edit.
In `@packages/deployer/src/providers/network.ts`:
- Around line 12-20: KNOWN_NETWORK_IDS currently includes 'mainnet' which
contradicts the intended testnet-only scope; update the set named
KNOWN_NETWORK_IDS to remove 'mainnet' so the deployer will no longer whitelist
or accept mainnet. Locate the constant KNOWN_NETWORK_IDS in
packages/deployer/src/providers/network.ts and delete the 'mainnet' entry (or
replace the set with a new ReadonlySet that omits 'mainnet') and ensure any code
relying on KNOWN_NETWORK_IDS (e.g., validation logic elsewhere) will now reject
mainnet networks accordingly.
In `@packages/deployer/src/providers/proof-server.ts`:
- Around line 65-69: The PROOF_SERVER_PORT parsing currently uses
Number.parseInt which allows partial numeric strings and doesn't check bounds;
update the validation around where port is parsed (the block using
Number.parseInt/Number.isNaN) to first ensure the string fully matches /^\d+$/
(reject anything like "6300abc" or empty/whitespace), then parse with
Number.parseInt or Number() and verify the resulting integer is between 1 and
65535 inclusive; if validation fails throw a ConfigError with a clear message
including the invalid value.
In `@packages/deployer/src/wallet/handler.ts`:
- Around line 448-455: Replace the manual '/' parsing in pathDir and pathBase
with Node's path utilities to handle platform-specific separators: import the
Node 'path' module and implement pathDir using path.dirname and pathBase using
path.basename so Windows backslashes are handled correctly and cache
directory/file resolution works across OSes; update any callers if necessary to
expect the same string behavior returned by path.dirname/path.basename
(functions: pathDir, pathBase).
In `@packages/deployer/src/wallet/keystore.ts`:
- Around line 125-141: fromJSON currently assumes data.crypto exists and will
throw a raw TypeError for malformed JSON; instead validate the shape and types
before accessing nested properties and throw a WalletError for bad input. In the
Keystore.fromJSON method (handling MidnightKeystore), add defensive checks that
data is an object, data.version exists, data.crypto is a non-null object, and
that data.crypto.kdf and data.crypto.cipher are strings; if any check fails,
throw a WalletError with a clear message about malformed keystore JSON; then
continue to validate values (kdf === 'scrypt', cipher === 'aes-128-ctr') and
return new Keystore(data). Ensure this protects callers like readFromFile from
leaking raw TypeErrors.
In `@tests/integrations/_harness/walletPool.ts`:
- Around line 56-66: The in-flight promise is being cached via
this.cache.set(alias, built) before the async build completes, so if
WalletHandler.build(...) or owned.provider.start(true) rejects the rejected
promise remains cached and signerFor(alias) will keep failing; change the flow
so you only cache a successfully-resolved wallet: either await the built result
before calling this.cache.set(alias, ...) or attach a .then(...) that stores the
resolved owned value and a .catch(...) that removes the cache entry on failure
(referencing built, WalletHandler.build, owned.provider.start, this.cache.set
and signerFor(alias) to locate the code).
In `@tests/integrations/local-env.yml`:
- Line 4: Replace floating :latest tags with pinned, deterministic tags for the
container images referenced by the image: keys — specifically update
midnightntwrk/proof-server and midnightntwrk/indexer-standalone to explicit
version tags or digests (e.g., midnightntwrk/proof-server:<version> and
midnightntwrk/indexer-standalone:<version> or use an IMAGE_* variable with a
pinned default) instead of :latest so integration runs are reproducible; ensure
both image entries match the existing pinned approach used for the node image
(node:0.22.2) and update any CI docs or env variables that control these image
values.
---
Minor comments:
In `@examples/fungible-token/README.md`:
- Around line 16-20: The constructor type table incorrectly lists `_decimals`
and `_feeBps` as `number` while `Uint<N>` is mapped to `bigint` elsewhere;
update the table rows for `_decimals` and `_feeBps` to use `BigInt`/`bigint`
(consistent with `_maxSupply`/`_quorum` and the README examples), and apply the
same correction to the duplicated entries at lines referenced (around 143-144).
Ensure the table consistently reflects that `Uint<N>` maps to `bigint`.
- Around line 26-43: The fenced tree block starting with "fungible-token/" in
examples/fungible-token/README.md is missing a language tag (triggers MD040);
update the opening fence from ``` to ```text so the block becomes a text-fenced
code block (i.e., change the opening triple-backticks that precede
"fungible-token/" to include the "text" tag).
In `@examples/README.md`:
- Line 16: The README currently states "Compiled artifacts ship committed..."
which conflicts with the fungible-token README that says artifacts/ is
gitignored and generated locally; make the artifact policy consistent across
both READMEs by choosing one policy and updating the sentence that references
"artifacts/" (the line currently claiming compiled artifacts are committed) to
match the chosen policy, and ensure the fungible-token README has the same
wording and instructions for setup, generation, or deployment so both docs
present the same expectations.
In `@packages/cli/src/runDeploy.ts`:
- Around line 106-112: The parser currently treats unknown single-dash tokens as
positional and allows extra positionals; update the default branch in the
argument loop in runDeploy.ts so that if arg startsWith('-') (single-dash short
flag) you throw an Error(`Unknown flag: ${arg}`) instead of pushing to
out.positional, and after parsing validate out.positional: if
out.positional.length === 0 throw a missing contract error, and if
out.positional.length > 1 throw an Error indicating extra positional args
(mention the unexpected tokens) before assigning out.contract =
out.positional[0]; reference the variables arg, out.positional, and out.contract
when locating where to change behavior.
- Around line 95-103: The --sync-timeout branch currently uses
Number.parseInt(raw, 10) which permits inputs like "10abc"; update the parsing
in the case '--sync-timeout' (around expectValue and out.syncTimeoutSec) to
reject malformed values by ensuring the raw string is an exact integer
representation before assigning out.syncTimeoutSec — e.g., parse the number and
verify String(seconds) === raw (or use a /^\d+$/ check) and then confirm
Number.isFinite(seconds) && seconds > 0, otherwise throw the existing error with
the original raw value.
In `@packages/deployer/README.md`:
- Around line 23-37: The README has unlabeled fenced code blocks causing MD040;
update the two unlabeled blocks that show the CLI usage and example (the blocks
beginning with "compact-deploy <Contract>" and the example "compact-deploy
<Contract> --network preprod \") by adding appropriate language identifiers: use
"text" for the CLI usage block and "bash" (or "sh") for the example block.
Ensure both fenced blocks are updated so the first starts with ```text and the
second with ```bash to satisfy the linter.
- Line 7: The README contains a dead anchor link `#roadmap--todo`; update the
markdown to point to the actual Roadmap heading or create a matching section:
either change the link target in the Status line from `#roadmap--todo` to the
real anchor (e.g., `#roadmap--todo` -> `#roadmap` or `#roadmap--todo` ->
`#roadmap-and-todo` depending on the existing heading text), or add a "Roadmap /
TODO" heading that generates the `#roadmap--todo` anchor so the link resolves;
ensure the visible link text ("Roadmap") and the heading slug match.
In `@packages/deployer/src/config/compact-config.test.ts`:
- Around line 48-50: The test is creating a second [profile] table which can
trigger a TOML parse error before the semantic check; change the tmpRepo content
so it does not duplicate the [profile] header but instead adds default_network =
"ghost" inside the already-existing profile from MIN_VALID (i.e. append
'default_network = "ghost"' under the existing [profile] rather than adding a
new '[profile]' line) so CompactConfig.load(...) exercises the semantic
validation for default_network missing.
In `@packages/deployer/src/deployer.ts`:
- Around line 580-583: The current pipeline uses Rx.timeout({ each: timeoutMs })
after Rx.filter((s: FacadeState) => s.isSynced) so only emissions with
s.isSynced === true will reset the timer; if you want any progress emission to
extend the timeout, move Rx.timeout before the Rx.filter so all FacadeState
emissions reset the timer, otherwise if your intent is to bound time until the
first synced emission (and not allow repeated isSynced=true to restart it)
replace the timeout call with Rx.timeout({ first: timeoutMs }); update the
pipeline around Rx.timeout and Rx.filter accordingly.
In `@packages/deployer/src/loaders/constructor-meta.ts`:
- Around line 34-46: The validation uses the `in` operator which matches
prototype properties; update the missing-parameter check in the constructor args
validation to only consider own properties by replacing `(n in named)` with an
own-property check such as `Object.hasOwn(named, n)` (or
`Object.prototype.hasOwnProperty.call(named, n)` for older runtimes), keeping
the rest of the logic (throwing ConfigError, computing extra via
Object.keys(named), and returning `paramNames.map((n) => named[n])`) unchanged.
In `@packages/deployer/src/providers/private-state-password.test.ts`:
- Around line 38-60: The test mocks node:crypto but restores it after
assertions, which won't run if the test throws; wrap the
mock/require/assert/unmock sequence in a try/finally so node:crypto is always
restored: call vi.resetModules(); vi.doMock('node:crypto', ...) then import {
derivePrivateStatePassword as derive } and run the expect inside the try block,
and in finally call vi.doUnmock('node:crypto') and vi.resetModules(); reference
derivePrivateStatePassword / derive and the vi.doMock/vi.doUnmock calls so the
mock is reliably cleaned up even when the test throws.
In `@packages/deployer/src/wallet/keystore.ts`:
- Around line 196-201: seedFromHex currently accepts any even-length hex and
should instead enforce the expected seed lengths so keystores fail fast: in
seedFromHex, after stripping the 0x prefix and validating hex characters, check
that stripped.length is either 64 or 128 (matching classifySeed expectations)
and throw a WalletError with a clear message if not; otherwise continue to
return Buffer.from(stripped, 'hex'). Reference: seedFromHex and classifySeed.
In `@tests/integrations/_harness/paths.ts`:
- Around line 20-37: The current preflight helpers requireFixtureArtifact and
requirePrivateCounterArtifact only call existsSync(ARTIFACT_DIR) /
existsSync(PRIVATE_COUNTER_ARTIFACT_DIR), which permits empty/stale directories;
update both functions to ensure the artifact directory is not only present but
contains expected compiled outputs (e.g., check readdirSync(...) yields at least
one file and/or assert presence of expected filenames/extensions such as
.wasm/.json); if the directory is empty or missing expected files, throw the
same Error with the helpful "Run `make -C tests/integrations compile` first."
message so failures surface earlier and with clearer diagnostics.
In `@tests/integrations/README.md`:
- Around line 7-25: The README's fenced code block starting with ``` lacks a
language tag (markdownlint MD040); update the opening fence from ``` to include
a language specifier (for example ```text or ```bash) so the block is properly
tagged—modify the README's fenced block that contains the directory tree (the
``` fence and its closing ``` fence) to add the chosen language tag.
---
Nitpick comments:
In `@packages/cli/test/prompt.test.ts`:
- Around line 79-149: Add two regression tests to prompt.test.ts that exercise
stdin 'end' and 'error' paths: use promptPassphrase(...) to start the promise,
trigger mockStdin.emit('end') in one test and assert the promise settles
(rejects) and raw mode is turned off and stdin.pause() called; in the other test
emit mockStdin.emit('error', new Error('boom')) and assert the promise rejects
with that error (or the propagated error) and cleanup (setRawMode(false) and
pause) occurs; locate tests near the existing describe blocks and reuse
mockStdin.emit, mockStdin.setRawMode, and mockStdin.pause checks already used in
other cases.
In `@tests/integrations/specs/deploy/deploy.spec.ts`:
- Around line 33-35: The hex assertions are too permissive; update the regexes
for result.address, result.txId and result.txHash to enforce exact lengths used
by the deployer contract (e.g., address = 40 hex chars, txId/txHash = 64 hex
chars) so truncated values fail; replace /^[0-9a-f]+$/i with length-constrained
patterns (e.g., /^[0-9a-f]{40}$/i and /^[0-9a-f]{64}$/i as appropriate) and make
the same change for the other duplicate assertions further down in the spec.
In `@tests/integrations/specs/deploy/historyIsolation.spec.ts`:
- Around line 26-38: The test currently calls wipeDeployments() in
beforeAll/afterAll and uses shared DEPLOYMENTS_DIR (via
requireFixtureArtifact()/deployFixture()), coupling specs; change the test setup
to use an isolated deployments directory per spec/worker by deriving a unique
DEPLOYMENTS_DIR (e.g. from Vitest worker id or a temp UUID) and pass it into
requireFixtureArtifact()/deployFixture() or make wipeDeployments() operate on
that per-test directory; update historyIsolation.spec.ts to set the unique
directory before calling requireFixtureArtifact(), deployFixture(), and
wipeDeployments() so each spec uses its own deployments folder and no longer
shares global state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2121ac35-31dc-4364-ad61-5db4afc670a7
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (102)
.gitignoreMakefilecompact.tomlexamples/README.mdexamples/fungible-token/README.mdexamples/fungible-token/compact.tomlexamples/fungible-token/contracts/TokenExample.compactexamples/fungible-token/contracts/security/Initializable.compactexamples/fungible-token/contracts/token/FungibleToken.compactexamples/fungible-token/contracts/utils/Utils.compactexamples/fungible-token/deploy/TokenExample.args.mjsexamples/fungible-token/deploy/deployTokenExample.tsexamples/fungible-token/package.jsonpackage.jsonpackages/cli/package.jsonpackages/cli/src/logger.tspackages/cli/src/prompt.tspackages/cli/src/runBuilder.tspackages/cli/src/runCompiler.tspackages/cli/src/runDeploy.tspackages/cli/test/logger.test.tspackages/cli/test/prompt.test.tspackages/cli/test/runBuilder.test.tspackages/cli/test/runDeploy.test.tspackages/cli/vitest.config.tspackages/deployer/README.mdpackages/deployer/package.jsonpackages/deployer/src/config/compact-config.test.tspackages/deployer/src/config/compact-config.tspackages/deployer/src/config/schema.test.tspackages/deployer/src/config/schema.tspackages/deployer/src/deployer.test.tspackages/deployer/src/deployer.tspackages/deployer/src/deployments.test.tspackages/deployer/src/deployments.tspackages/deployer/src/errors.test.tspackages/deployer/src/errors.tspackages/deployer/src/index.tspackages/deployer/src/loaders/args.test.tspackages/deployer/src/loaders/args.tspackages/deployer/src/loaders/artifact.test.tspackages/deployer/src/loaders/artifact.tspackages/deployer/src/loaders/constructor-meta.test.tspackages/deployer/src/loaders/constructor-meta.tspackages/deployer/src/loaders/context.test.tspackages/deployer/src/loaders/context.tspackages/deployer/src/loaders/contract-resolve.test.tspackages/deployer/src/loaders/contract-resolve.tspackages/deployer/src/loaders/init-state.test.tspackages/deployer/src/loaders/init-state.tspackages/deployer/src/loaders/ref-resolver.test.tspackages/deployer/src/loaders/ref-resolver.tspackages/deployer/src/loaders/signing-key.test.tspackages/deployer/src/loaders/signing-key.tspackages/deployer/src/providers/build.test.tspackages/deployer/src/providers/build.tspackages/deployer/src/providers/network.test.tspackages/deployer/src/providers/network.tspackages/deployer/src/providers/private-state-password.test.tspackages/deployer/src/providers/private-state-password.tspackages/deployer/src/providers/proof-server.test.tspackages/deployer/src/providers/proof-server.tspackages/deployer/src/runDeploy.test.tspackages/deployer/src/runDeploy.tspackages/deployer/src/wallet/handler.test.tspackages/deployer/src/wallet/handler.tspackages/deployer/src/wallet/keystore.test.tspackages/deployer/src/wallet/keystore.tspackages/deployer/src/wallet/seeds.test.tspackages/deployer/src/wallet/seeds.tspackages/deployer/tsconfig.jsonpackages/deployer/vitest.config.tstests/integrations/.gitignoretests/integrations/README.mdtests/integrations/_harness/deployer.tstests/integrations/_harness/logger.tstests/integrations/_harness/network.tstests/integrations/_harness/paths.tstests/integrations/_harness/teardown.tstests/integrations/_harness/walletPool.tstests/integrations/compact.tomltests/integrations/fixtures/Counter.compacttests/integrations/fixtures/PrivateCounter.compacttests/integrations/fixtures/initstates/PrivateCounter.jsontests/integrations/fixtures/signingkeys/Counter.signingkeytests/integrations/fixtures/signingkeys/PrivateCounter.signingkeytests/integrations/fixtures/signingkeys/SecondaryCounter.signingkeytests/integrations/fixtures/witnesses/PrivateCounter.witness.tstests/integrations/local-env.ymltests/integrations/specs/deploy/asyncDisposeCleanup.spec.tstests/integrations/specs/deploy/deploy.spec.tstests/integrations/specs/deploy/dryRun.spec.tstests/integrations/specs/deploy/historyIsolation.spec.tstests/integrations/specs/deploy/historyRotation.spec.tstests/integrations/specs/deploy/privateCounter.spec.tstests/integrations/specs/deploy/proofServerAuto.spec.tstests/integrations/specs/errors/errors.spec.tstests/integrations/specs/wallet/keystorePassphrase.spec.tstests/integrations/specs/wallet/walletLifecycle.spec.tstests/integrations/specs/wallet/walletPool.spec.tstests/integrations/vitest.config.tsturbo.json
This was referenced Jun 8, 2026
0xisk
force-pushed
the
feat/compact-deployer
branch
from
ff82d7b to
04760d1
Compare
0xisk
changed the title
compact-deployer tool
0xisk
added a commit
that referenced
this pull request
Jun 8, 2026
The deployer (#86) moved to the midnight-js 4.1 / ledger-v8 / compact-runtime 0.16 stack, but the simulator and the repo-wide compiler pin were left behind on three different versions. * simulator: bump @midnight-ntwrk/compact-runtime 0.14.0 -> 0.16.0 and replace @midnight-ntwrk/ledger-v7 with ledger-v8 8.1.0. The v7->v8 API is signature-identical for every symbol the simulator uses, so the only source change is one test-fixture import path. * CI: bump the compiler pin 0.28.0 -> 0.31.0 in the setup action so the freshly compiled fixtures target runtime 0.16.0. * README: bump the compiler badge 0.29.0 -> 0.31.0. Verified locally: fixtures recompile to runtime-version 0.16.0 / compiler-version 0.31.0, and build, types, lint, and all 56 simulator tests pass. Refs: #107
0xisk
added a commit
that referenced
this pull request
Jun 8, 2026
Harden the deployer tool and CLI against the issues raised in the #86 review. Each fix verified against current code and covered by tests. * cli/logger: route `--json` pino logs to STDERR (fd 2) so STDOUT carries only the single result object. * cli/prompt: write the passphrase prompt + newline to STDERR, and settle the promise on stdin `end`/`error` so non-interactive input (closed/newline-less pipe) can no longer hang the CLI. * config/schema: mark the file/module ref union members `.strict()` so an ambiguous `{ file, module }` is rejected instead of silently dropping a key. * deployments: write the ledger atomically (temp file + rename) so a crash mid-write can't truncate `*.json`. * loaders/artifact: bind compiled assets to `dirname(entry)` rather than a hardcoded `contract/`, fixing the top-level-`index.js` case. * loaders/constructor-meta: allow an empty named-args object for a no-arg constructor (resolves to `[]`); `reorderNamedArgs` still rejects unexpected keys. * loaders/contract-resolve: use `path.isAbsolute` instead of `startsWith('/')` for OS-correct absolute-path detection. * providers/network: drop `mainnet` from the allow-list while the deployer is testnet/preview-only. * providers/proof-server: reject partial-numeric and out-of-range `PROOF_SERVER_PORT` values (full `\d+` match + 1..65535 bounds). * wallet/handler: use `path.dirname`/`basename` instead of manual `/` splitting so cache paths work on Windows. * wallet/keystore: validate shape in `fromJSON` and throw `WalletError` (not a raw `TypeError`) on malformed keystore JSON. Refs: #86
0xisk
added a commit
that referenced
this pull request
Jun 8, 2026
* integrations/walletPool: evict a wallet build from the cache if it rejects, so a failed `WalletHandler.build`/`provider.start` no longer poisons every later `signerFor(alias)` call with the same rejection. * integrations/local-env: pin proof-server and indexer-standalone by digest (resolved from `:latest`) for deterministic runs, matching the already-pinned node image. Both digests are multi-arch (amd64 + arm64) manifests and identical to the current `:latest`, so behaviour is unchanged. Refs: #86
Introduce the `@openzeppelin/compact-deployer` package: a programmatic and CLI deployer for Compact contracts. Covers config loading, artifact and constructor-arg resolution, wallet/keystore handling, network and proof-server providers, and deployment-record bookkeeping. Wire the deployer into `@openzeppelin/compact-cli` via the new `compact-deploy` command (runDeploy), with passphrase prompting and JSON/verbose output modes. * Add root resolutions for the midnight stack and a `coverage` turbo task so the new package can report coverage. Part 1 of 2 splitting the original deployer PR. Examples and the integration-test suite follow in a stacked PR that builds on this one. Refs: #86
Harden the deployer tool and CLI against the issues raised in the #86 review. Each fix verified against current code and covered by tests. * cli/logger: route `--json` pino logs to STDERR (fd 2) so STDOUT carries only the single result object. * cli/prompt: write the passphrase prompt + newline to STDERR, and settle the promise on stdin `end`/`error` so non-interactive input (closed/newline-less pipe) can no longer hang the CLI. * config/schema: mark the file/module ref union members `.strict()` so an ambiguous `{ file, module }` is rejected instead of silently dropping a key. * deployments: write the ledger atomically (temp file + rename) so a crash mid-write can't truncate `*.json`. * loaders/artifact: bind compiled assets to `dirname(entry)` rather than a hardcoded `contract/`, fixing the top-level-`index.js` case. * loaders/constructor-meta: allow an empty named-args object for a no-arg constructor (resolves to `[]`); `reorderNamedArgs` still rejects unexpected keys. * loaders/contract-resolve: use `path.isAbsolute` instead of `startsWith('/')` for OS-correct absolute-path detection. * providers/network: drop `mainnet` from the allow-list while the deployer is testnet/preview-only. * providers/proof-server: reject partial-numeric and out-of-range `PROOF_SERVER_PORT` values (full `\d+` match + 1..65535 bounds). * wallet/handler: use `path.dirname`/`basename` instead of manual `/` splitting so cache paths work on Windows. * wallet/keystore: validate shape in `fromJSON` and throw `WalletError` (not a raw `TypeError`) on malformed keystore JSON. Refs: #86
0xisk
force-pushed
the
feat/compact-deployer
branch
from
475c4ac to
15619e6
Compare
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
1 participant
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.
What
Part 1 of 2 (originally one large PR, now split for reviewability).
Introduces the
@openzeppelin/compact-deployerpackage: a programmatic and CLI deployer for Compact contracts.packages/deployer/— config loading (compact.toml+ schema), artifact and constructor-arg resolution, init-state and signing-key loaders, wallet/keystore handling, network + proof-server + build providers, deployment-record bookkeeping, and therunDeployentry point.packages/cli/— wires the deployer intocompact-clias thecompact-deploycommand, with passphrase prompting and JSON/verbose output.resolutionsand acoverageturbo task.Why split
The original PR was 103 files / +16.7k. This half is the tool; the consumers (examples + integration tests) live in the stacked PR below.
Stacked PR
➡️ #109 — examples + integration-test suite, based on this branch. Merge this first.
Verification
Build, types, and lint pass.
compact-deployer247 unit tests andcompact-cli67 unit tests green.