OpenZeppelin / OpenZeppelin/compact-contracts
dev: live-harness follow-ups (node-rejection constants, orchestrator tests, env-down pkill)
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 55
- Forks
- 29
- Avg merge
- 5d 7h
- Merged PRs (30d)
- 25
Description
Follow-ups found while building the integration-live harness (branch
test/integration-live-harness). None of these block that branch; they were
deliberately left out to keep it scoped. Design notes live in
docs/design/integration-live-harness*.md.
Filed from the Codebase improvement template.
1. Centralize the node-rejection strings
Node error strings are hardcoded in 8 files, in three different roles:
| Role | Where |
|---|---|
| Live assertion | contracts/test/integration/specs/confidentialFungibleToken.spec.ts (exact phrase) |
| Live assertion | contracts/src/token/test/ConfidentialFungibleToken.test.ts:1188 (loose /block limits|exhaust the block/i) |
| Retry logic | contracts/test-utils/harness/LiveSimulatorBackend.ts:170 — DETERMINISTIC_REJECTION = /1010: Invalid Transaction/ |
| Prose only | 6 doc comments in live.globalSetup.ts, scripts/live/LiveOrchestrator.ts, test-utils/fixtures/nativeShieldedToken.ts |
The retry logic and the canary now encode different notions of the same
rejection: the canary demands the full phrase, isDeterministicRejection matches
only the 1010: Invalid Transaction prefix. That is how these drift apart.
Proposed contracts/test-utils/harness/nodeRejections.ts:
/** Verified against ledger-v8 8.1.0 / midnight-js 4.1.1. These are the NODE's
* words, not ours — a ledger bump can reword them. */
export const VERIFIED_AGAINST = { '@midnight-ntwrk/ledger-v8': '8.1.0' } as const;
export const RPC_INVALID_TRANSACTION = '1010: Invalid Transaction';
export const NODE_REJECTION = {
/** Tx IR exceeds the per-tx block byte budget. */
blockLimitExceeded: `${RPC_INVALID_TRANSACTION}: Transaction would exhaust the block limits`,
/** Coin spent against stale node state (re-spend / stale UTXO). */
staleCoinState: `${RPC_INVALID_TRANSACTION}: Custom error: 103`,
} as const;
On tying this to the ledger version: a Record<ledgerVersion, ...> map would be
speculative, since only one ledger version is pinned at a time and the unused
branches would rot. Prefer a VERIFIED_AGAINST constant plus a harness unit test
asserting the installed version still matches, so a ledger bump fails loudly and
forces someone to re-verify the strings rather than discovering it later as a
mystery red canary.
2. errorChainText — the shallow-walk bug
Both canaries collect the error text from message, one level of cause, and
String(error). The node's 1010 text can also sit:
- more than one level deep in a
causechain, - in
AggregateError.errors, - only in
toString(), for an effectFiberFailurethat keeps its cause behind a
Symbol.
isDeterministicRejection already walks all of these, and
test-utils/harness/test/LiveSimulatorBackend.test.ts:177-199 documents the
two-level and FiberFailure shapes explicitly. The shallow version would miss
those and fail on the wrong assertion.
It works today for the block-limit rejection (verified across three live runs), so
this is latent rather than broken. Fix it once, in the shared module, and have both
canaries plus isDeterministicRejection use it.
3. Tighten the base spec's matcher
contracts/src/token/test/ConfidentialFungibleToken.test.ts:1188 still uses
/block limits|exhaust the block/i. A broad matcher can be satisfied by an
unrelated deploy failure (funding, proving, a submission bounce) and report a false
green, which silently voids the scope claim the canary exists to prove. The
integration canary was tightened to the exact phrase; this one should match.
4. make env-down kills its own recipe shell
Every env-up and env-down prints:
make: [Makefile:21: env-down] Terminated (ignored)
Makefile:21 is @-pkill -f "docker compose -f $(COMPOSE_FILE) logs". pkill
excludes its own PID but not its parent — and the recipe's sh -c command line
contains the pattern, so pkill matches and kills the shell running it. The @-
prefix hides the failure.
Effect is mild: the line meant to kill the backgrounded docker compose logs -f
streamers is unreliable, but the following docker compose down stops the
containers, so the streamers exit anyway. Still, the line does not do what it was
written to do.
Fix is the standard bracket trick, so the pattern cannot match the shell that
contains it:
@-pkill -f "docker compose -f $(COMPOSE_FILE) [l]ogs" 2>/dev/null || true
5. Missing tests for the live orchestrator
The scripts vitest project added on the harness branch covers resolvePlan,
classify, and report naming. Still untested:
assertSoleLiveProject(live.globalSetup.ts) — belongs to theharness
project, notscripts, so the new project does not reach it. Already exported as
a pure function for this purpose.RunLockstaleness — injectpidAlivethe waylockHolderStatealready does.- The teardown policy — exactly-once, opt-out honoured, and teardown ordered before
the lock release.
6. No CI job runs the scripts project
It is green locally in ~200 ms and gates the orchestrator's only unit coverage.
7. Confirm CI gating parity for the integration-live job
scripts/test-live.ts --list now emits integration, and the workflow on
ci/live-test-workflow (draft #681) feeds --list into
fromJSON(needs.plan.outputs.categories), so the job is spawned automatically.
Unconfirmed: whether that job inherits the same node-up / 6-hour-budget /
live-tests-label gating as the per-category unit jobs.
8. Compile integration artifacts into their own directory
Split out into its own issue: #758. Raised by @andrew-fleming on #717.
Noted separately — not part of this issue
The dry integration project is red on main as installed: 5 failures in
contracts/test/integration/specs/initStateIsolation.spec.ts, because
test/integration/fixtures/* are written against an older
@openzeppelin/compact-simulator API. The installed 0.2.0 constructor takes
(deps), so new ComposedTokensSimulator(args, {}) hits deps.backend.kind on
undefined. Verified by running the suite with all harness-branch changes stashed.
This may already be tracked; if not it deserves its own issue, since fixing it
means updating three fixtures rather than the harness.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with docs/design/integration-live-harness*.md, then inspect the shared harness code, LiveSimulatorBackend tests, ConfidentialFungibleToken.test.ts, Makefile, and the scripts and harness Vitest configurations. Trace the existing orchestrator and CI workflow before changing anything. Done means the rejection handling, orchestrator coverage, env-down behavior, CI coverage and gating parity are verified, with the separately tracked artifact work excluded.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- docker-compose, typescript
- Domain
- build-system, ci-cd, devops, testing-qa
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100