Skip to content

Commit 9226f19

Browse files
authored
fix(simulator): clear the #145 review comments (#147)
* fix(simulator): pin simulated block time to zero compact-runtime 0.18 defaults `createCircuitContext`'s `time` to wall-clock, where the hand-rolled 0.16 `QueryContext` left it at 0. Any suite reading `kernel.blockTime()` silently became non-reproducible. Threads an explicit `time` from `BaseSimulatorOptions` through to the runtime factory, defaulting to 0 to restore the previous behaviour. * refactor(simulator): drop the dead circuit-context helpers `useCircuitContext` and `useCircuitContextSender` had no callers and were never exported from the barrel, so they were unreachable before this migration and got hand-ported to the 0.18 call-tree shape untested. * fix(simulator): guard context reads before init() 0.18 split construction from state building, so a caller that skipped `init()` got `Cannot read properties of undefined (reading 'callContext')`. Both `CircuitContextManager.context` and the simulator's `contractAddress` now throw a named error instead. * fix(simulator): declare initialState async on IMinimalContract The canonical interface still promised the sync-only 0.16 return type, so anything typed against it got the wrong shape and `CircuitContextManager` carried a private redeclaration to compensate. * fix(simulator): typecheck the test tree `tsconfig.types.json` compiled `src/**` only, and its `exclude` named a `tests` directory that does not exist, so nothing verified the test tree. Adding a project that covers it surfaced 74 errors, of which the load-bearing one is in `src/`: 0.18 circuits return `Promise<CircuitResults>`, which the `ContextlessCircuits` / `AsyncCircuits` conditionals did not match, collapsing every entry on `circuits.pure` and `circuits.impure` to `never`. * src: match the promised circuit shape; keep the anonymous class's address field underscore-public for declaration emit * test: give relative imports the `.js` extension nodenext requires * wiring: run both projects under `yarn types`, and let turbo see the test tree so a regression is not cached away * fix(simulator): build the setContext fixture as a real 0.18 context The replacement context was a hand-rolled literal in the pre-0.18 flat shape, so `setContext`'s only test asserted against a context the runtime would reject. `toEqual` compared two plain objects and passed anyway. Rebuilds it through `createCircuitContext` and asserts identity plus the two fields the test is about. Exporting `BackendDeps` is required for the same reason: it appears in `create`/`_create`, so an exported binding assigned from `createSimulator(...)` could not name it (TS4023). * fix(ci): make the prerelease compiler input functional and verified * `COMPACTC_VERSION` is now exported to `$GITHUB_ENV`. `VER` was step-scoped, so `test/setup.ts` never saw it: overriding the input installed one toolchain while the fixtures compiled with the hardcoded default. * The archive is checked against a pinned SHA-256 before it is unpacked and executed. The release publishes no checksum file, so the digests are pinned per platform here; overriding the version requires passing its digest. * fix(simulator): key the fixture cache on the compiler The staleness check compared artifact mtime against source mtime only, so an existing clone kept its 0.31-compiled artifacts and hit a load-time rejection against the 0.18 runtime. CI never saw it: the `test` turbo task is uncached and the checkout is clean. Artifacts now carry a stamp of the compiler version and the zkir flag, and recompile when it does not match. The flag also moves onto the fixture list, replacing a `secp256k1` regex over the source that a mention in a comment was enough to trip. * test(simulator): collapse the field-to-bytes helpers Two near-identical copies existed in the test tree. The one in `SampleZOwnable.test.ts` silently truncated where the runtime helper it replaced threw, so a wrong value would read as a passing assertion. Keeps one in the shared test utils, with the range check restored, and derives `zeroUint8Array` from it. `Signers.ts` keeps its own copy: `src` cannot import from `test`. * test(simulator): consume the generated secp256k1 types The `Ecdsa.compact` re-export is live: the artifact's `index.d.ts` declares both `Secp256k1EcdsaSignature` and `Secp256k1Point`. The simulator hand-rolled the signature struct and took the point from the runtime instead, so a change to the generated shape would not surface here. * docs(simulator): correct the sync-era comments The async migration left the docs describing a synchronous engine: the dry primitive called "synchronous", `DryBackend` said it wraps a synchronous result in a resolved promise and preserves dry behaviour "byte-for-byte", and the `SyncSimulator` name now describes nothing. Renaming the exported type is a breaking change, so it keeps the name with a `@remarks` note. Also records that `copyCircuitContext` is `@internal` upstream. * ci(turbo): declare COMPACTC_VERSION on the test task The setup action exports it and `test/setup.ts` reads it, but the task never declared it. Pass-through happens to work on turbo 2.9, so this only pins the behaviour against a stricter env mode. * fix(ci): compile fixtures before the type check `yarn types` now covers `test/**`, which imports the generated artifacts, but `checks.yml` runs it before `yarn test` — the only thing that compiled them. It passed locally only because the artifacts were already on disk. Lifts fixture compilation into its own turbo task that `types` depends on. `test/setup.ts` gains a direct entry point so the task does not have to go through vitest; vitest's `globalSetup` still calls the same function.
1 parent 6d0703f commit 9226f19

32 files changed

Lines changed: 346 additions & 235 deletions

.github/actions/setup/action.yml

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@ inputs:
1010
description: "Pre-release Compact compiler to fetch from GitHub releases (bridge until the toolchain ships stable). Keep in sync with setup.ts COMPACTC_VERSION."
1111
required: false
1212
default: "0.33.0-rc.2"
13+
compact-prerelease-sha256:
14+
description: "SHA-256 of the pre-release compiler archive. Required when overriding compact-prerelease; the default version's digests are built in."
15+
required: false
16+
default: ""
1317

1418
runs:
1519
using: "composite"
@@ -58,17 +62,51 @@ runs:
5862
shell: bash
5963
env:
6064
VER: ${{ inputs.compact-prerelease }}
65+
SHA_OVERRIDE: ${{ inputs.compact-prerelease-sha256 }}
6166
run: |
6267
set -euo pipefail
6368
# Reuse the platform dir the CLI already created for the stable install
6469
# (e.g. x86_64-unknown-linux-musl), so the pre-release lands where the
6570
# `+<version>` selector resolves it; fall back to the host triple.
6671
PLAT="$(basename "$(find "$HOME/.compact/versions" -mindepth 2 -maxdepth 2 -type d 2>/dev/null | head -1)" 2>/dev/null || true)"
6772
[ -z "$PLAT" ] && PLAT="$(uname -m)-unknown-linux-musl"
73+
74+
# The release publishes no checksum file, so the expected digests are
75+
# pinned here (computed from the published assets). Overriding the
76+
# version means supplying its digest too — this archive is unpacked and
77+
# executed, and every other action in this file is pinned by SHA.
78+
if [ -n "$SHA_OVERRIDE" ]; then
79+
WANT="$SHA_OVERRIDE"
80+
else
81+
case "$VER/$PLAT" in
82+
0.33.0-rc.2/x86_64-unknown-linux-musl)
83+
WANT=3055ab92bbc8d5bb0d6282b661b83761d2a0de2ee37e21cf7107e25aaf2a9aad ;;
84+
0.33.0-rc.2/aarch64-unknown-linux-musl)
85+
WANT=3aa23812b0b086dbce07da3931a40dcb01bec9676b1ceed7f2d0be370ab2dc46 ;;
86+
0.33.0-rc.2/x86_64-darwin)
87+
WANT=dce1a57d82ce06208fcc5d9de5343f18c48654a5ff3acf6bafabe3d17bf1ef18 ;;
88+
0.33.0-rc.2/aarch64-darwin)
89+
WANT=35a28009c9a57d20902e4fcfd12f0ca9ea94338208954cf8bcd335652e24f382 ;;
90+
*)
91+
echo "::error::no pinned digest for $VER/$PLAT; pass compact-prerelease-sha256"
92+
exit 1 ;;
93+
esac
94+
fi
95+
6896
DEST="$HOME/.compact/versions/$VER/$PLAT"
6997
mkdir -p "$DEST"
7098
curl -fsSL -o /tmp/compactc.zip \
7199
"https://github.com/LFDT-Minokawa/compact/releases/download/compactc-v$VER/compactc_v${VER}_${PLAT}.zip"
100+
# sha256sum on Linux runners, shasum on macOS.
101+
if command -v sha256sum >/dev/null; then
102+
echo "$WANT /tmp/compactc.zip" | sha256sum -c -
103+
else
104+
echo "$WANT /tmp/compactc.zip" | shasum -a 256 -c -
105+
fi
72106
unzip -oq /tmp/compactc.zip -d "$DEST"
73107
chmod +x "$DEST"/*
74108
compact compile +"$VER" --version
109+
110+
# `test/setup.ts` reads COMPACTC_VERSION to pick the toolchain it
111+
# compiles fixtures with; without this the input has no effect there.
112+
echo "COMPACTC_VERSION=$VER" >> "$GITHUB_ENV"

packages/simulator/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@
3535
},
3636
"scripts": {
3737
"build": "tsc -p .",
38-
"types": "tsc -p tsconfig.types.json",
38+
"compile:fixtures": "node test/setup.ts",
39+
"types": "tsc -p tsconfig.types.json && tsc -p tsconfig.test.json",
3940
"test": "MIDNIGHT_BACKEND=dry vitest run",
4041
"test:live": "MIDNIGHT_BACKEND=live vitest run",
4142
"clean": "git clean -fXd"

packages/simulator/src/backend/Backend.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export type CircuitKind = 'pure' | 'impure';
2626
* `createBackendSimulator` builds the async circuit proxies, caller helpers, and
2727
* state getters on top of this interface; the backend itself stays dumb. Every
2828
* operation is async so spec code is uniform `await` across both backends:
29-
* {@link DryBackend} wraps its synchronous results in `Promise.resolve`,
29+
* {@link DryBackend} resolves in memory,
3030
* the live adapter awaits the network.
3131
*
3232
* @template P - Private state type.
@@ -77,7 +77,7 @@ export interface Backend<P, L> {
7777
* implements `setPrivateState`; otherwise it throws
7878
* `PRIVATE_STATE_MUTATION_UNSUPPORTED` (from the live backend).
7979
*
80-
* Async across backends for uniform `await`: dry resolves synchronously,
80+
* Async across backends for uniform `await`: dry resolves in memory,
8181
* live awaits the provider write.
8282
*
8383
* @param privateState - The new private state `P`.

packages/simulator/src/backend/DryBackend.ts

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,14 @@ import type { Signers } from '../signers/Signers.js';
66
import type { Backend, BackendKind, CircuitKind } from './Backend.js';
77

88
/**
9-
* The slice of a synchronous `createSimulator` instance that {@link DryBackend}
9+
* The slice of a local `createSimulator` instance that {@link DryBackend}
1010
* drives. Kept structural so the dry backend reuses the existing simulator
1111
* machinery without coupling to its concrete (anonymous) class.
1212
*
13+
* @remarks Named `SyncSimulator` for the pre-0.18 runtime, whose circuits
14+
* returned their result directly. They return promises now; renaming the type
15+
* is a breaking change left for a later release.
16+
*
1317
* @template P - Private state type.
1418
* @template L - Public ledger state type.
1519
*/
@@ -30,14 +34,13 @@ export interface SyncSimulator<P, L> {
3034
}
3135

3236
/**
33-
* The in-memory backend: a thin async facade over the existing synchronous
34-
* `createSimulator` instance.
37+
* The in-memory backend: a thin facade over the local `createSimulator`
38+
* instance.
3539
*
36-
* Every operation delegates to the wrapped simulator and wraps the synchronous
37-
* result in a resolved promise, so a circuit never returns a bare value
38-
* on dry but a `Promise` on live. Because all real work routes through the
39-
* unchanged synchronous path, dry behavior is preserved byte-for-byte
40-
* and is the parity reference the live backend is measured against.
40+
* Every operation delegates to the wrapped simulator and resolves to a promise,
41+
* so a circuit never returns a bare value on dry but a `Promise` on live. All
42+
* real work routes through the in-memory path, which is the parity reference the
43+
* live backend is measured against.
4144
*
4245
* @template P - Private state type.
4346
* @template L - Public ledger state type.
@@ -49,7 +52,7 @@ export class DryBackend<P, L> implements Backend<P, L> {
4952
private readonly signers: Signers;
5053

5154
/**
52-
* @param sim - The wrapped synchronous simulator instance.
55+
* @param sim - The wrapped in-memory simulator instance.
5356
* @param signers - Resolver used to turn caller aliases into deterministic keys.
5457
*/
5558
constructor(sim: SyncSimulator<P, L>, signers: Signers) {

packages/simulator/src/core/CircuitContextManager.ts

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,29 +3,31 @@ import {
33
type CoinPublicKey,
44
type ConstructorContext,
55
type ContractAddress,
6-
type ContractState,
76
createCircuitContext,
87
createConstructorContext,
9-
type EncodedZswapLocalState,
108
} from '@midnight-ntwrk/compact-runtime';
9+
import type { InitialStateResult } from '../types/Contract.js';
1110

1211
/**
1312
* A composable utility class for managing Compact contract state in simulations.
1413
*
1514
* Handles initialization and lifecycle management of the `CircuitContext`,
1615
* which includes private state, public (ledger) state, zswap local state, and transaction context.
1716
*/
18-
/** Shape of a compiled contract's constructor result (sync or async in 0.18). */
19-
type InitialStateResult<P> = {
20-
currentPrivateState: P;
21-
currentContractState: ContractState;
22-
currentZswapLocalState: EncodedZswapLocalState;
23-
};
24-
2517
export class CircuitContextManager<P> {
26-
// Assigned by the async `init()`; the manager is always constructed and then
27-
// awaited (`init`) before any circuit call reads the context.
28-
public context!: CircuitContext<P>;
18+
private _context?: CircuitContext<P>;
19+
20+
/** The built context. Throws if read before {@link init} has been awaited. */
21+
get context(): CircuitContext<P> {
22+
if (!this._context) {
23+
throw new Error('CircuitContextManager: call init() before use');
24+
}
25+
return this._context;
26+
}
27+
28+
set context(newContext: CircuitContext<P>) {
29+
this._context = newContext;
30+
}
2931

3032
private readonly contract: {
3133
initialState: (
@@ -36,6 +38,7 @@ export class CircuitContextManager<P> {
3638
private readonly privateState: P;
3739
private readonly coinPK: CoinPublicKey;
3840
private readonly contractAddress: ContractAddress;
41+
private readonly time: number;
3942
private readonly contractArgs: any[];
4043

4144
/**
@@ -50,6 +53,8 @@ export class CircuitContextManager<P> {
5053
* @param privateState - The initial private state to inject into the contract
5154
* @param coinPK - The caller's coin public key
5255
* @param contractAddress - Optional override for the contract's address
56+
* @param time - Block time in seconds since the epoch, as the kernel's time
57+
* operations observe it
5358
* @param contractArgs - Additional arguments to pass to the contract constructor
5459
*/
5560
constructor(
@@ -62,12 +67,14 @@ export class CircuitContextManager<P> {
6267
privateState: P,
6368
coinPK: CoinPublicKey,
6469
contractAddress: ContractAddress,
70+
time: number,
6571
...contractArgs: any[]
6672
) {
6773
this.contract = contract;
6874
this.privateState = privateState;
6975
this.coinPK = coinPK;
7076
this.contractAddress = contractAddress;
77+
this.time = time;
7178
this.contractArgs = contractArgs;
7279
}
7380

@@ -94,6 +101,10 @@ export class CircuitContextManager<P> {
94101
currentZswapLocalState,
95102
currentContractState.data,
96103
currentPrivateState,
104+
undefined, // stateProvider: no cross-contract calls in the simulator
105+
undefined, // gasLimit: runtime default
106+
undefined, // costModel: runtime default
107+
this.time,
97108
);
98109
}
99110

packages/simulator/src/core/ContractSimulator.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@ export abstract class ContractSimulator<P, L> extends AbstractSimulator<P, L> {
5151
// compact-runtime 0.18: the caller-scoped fields live on `callContext`.
5252
// Copy (shallow-clones `callContext`) before overriding the Zswap local
5353
// state so the base context is left untouched.
54+
// `copyCircuitContext` is `@internal` upstream, so a patch release could
55+
// drop it; hand-rolling the copy is the worse option.
56+
// TODO: drop this note once the runtime makes it public.
5457
const ctx = copyCircuitContext(baseCtx) as CircuitContext<P>;
5558
ctx.callContext.currentZswapLocalState = emptyZswapLocalState(activeCaller);
5659
return ctx;

packages/simulator/src/create-contract.type-test.ts

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,12 @@
11
/**
22
* Type-level regression guard for the static `create` / `_create` contract.
33
*
4-
* `yarn types` compiles `src/**` but NOT `test/**` (the sample simulators import
5-
* generated, gitignored contract artifacts), and `vitest` strips types without
6-
* checking them — so a regression in the `create` / `_create` typing would
7-
* otherwise pass CI. This file reproduces the subclass-override pattern against a
8-
* synthetic contract type, so `yarn types` fails on a regression.
4+
* Reproduces the subclass-override pattern against a synthetic contract type, so
5+
* the guard holds without a compiled artifact. Carries the negative assertions
6+
* (`@ts-expect-error`) that the real simulators cannot express.
97
*
10-
* It exports nothing and is imported by nothing (inert at runtime). The build
11-
* config (`tsconfig.json`) excludes `*.type-test.ts` from emit, so it never
12-
* reaches `dist`; the type check runs it via `tsconfig.types.json`.
13-
*
14-
* Typechecking the real test simulators (which need the generated artifacts) is a
15-
* separate, larger effort; this is the minimal guard for the contract that
16-
* actually regressed.
8+
* Exports nothing and is imported by nothing. The build config excludes
9+
* `*.type-test.ts` from emit so it never reaches `dist`; `yarn types` compiles it.
1710
*/
1811
import { createSimulator } from './factory/createSimulator.js';
1912
import type { SimulatorConfig } from './factory/SimulatorConfig.js';

packages/simulator/src/factory/createDrySimulator.ts

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,11 @@ import type { BaseSimulatorOptions } from '../types/Options.js';
1212
import type { SimulatorConfig } from './SimulatorConfig.js';
1313

1414
/**
15-
* Internal synchronous simulator primitive.
15+
* Internal in-memory simulator primitive.
1616
*
17-
* This is the in-memory engine the public async {@link createSimulator} builds on:
18-
* the dry backend wraps an instance of this class, and the live backend uses one
19-
* locally to evaluate pure circuits. It is not the public testing API —
17+
* This is the engine the public {@link createSimulator} builds on: the dry
18+
* backend wraps an instance of this class, and the live backend uses one locally
19+
* to evaluate pure circuits. It is not the public testing API —
2020
* use {@link createSimulator} instead.
2121
*
2222
* Creates a class extending ContractSimulator with witness management, state
@@ -34,11 +34,22 @@ export function createDrySimulator<
3434
>(config: SimulatorConfig<P, L, W, TContract, TArgs>) {
3535
return class GeneratedSimulator extends ContractSimulator<P, L> {
3636
contract: TContract;
37-
// Assigned by the async `init()` (0.18 made `initialState` async, so the
38-
// address — read from the built context — is not known at construction).
39-
contractAddress!: string;
37+
// Underscore-public, not private: declaration emit rejects private members
38+
// on this exported anonymous class (TS4094). Treat as internal.
39+
public _contractAddress?: string;
4040
public _witnesses: W;
4141

42+
/**
43+
* The contract's address, read from the built context. Throws if read
44+
* before {@link init} has been awaited.
45+
*/
46+
get contractAddress(): string {
47+
if (!this._contractAddress) {
48+
throw new Error('Simulator: await init() before use');
49+
}
50+
return this._contractAddress;
51+
}
52+
4253
/**
4354
* Creates a new simulator instance with explicit contract args and options
4455
*/
@@ -53,6 +64,9 @@ export function createDrySimulator<
5364
witnesses = config.witnessesFactory(),
5465
coinPK = '0'.repeat(64),
5566
contractAddress = dummyContractAddress(),
67+
// Fixed rather than wall-clock, so a run that reads `kernel.blockTime()`
68+
// is reproducible. Override via `options.time`.
69+
time = 0,
5670
} = options;
5771

5872
this._witnesses = witnesses;
@@ -65,6 +79,7 @@ export function createDrySimulator<
6579
privateState,
6680
coinPK,
6781
contractAddress,
82+
time,
6883
...processedArgs,
6984
);
7085
}
@@ -76,7 +91,7 @@ export function createDrySimulator<
7691
*/
7792
async init(): Promise<this> {
7893
await this.circuitContextManager.init();
79-
this.contractAddress =
94+
this._contractAddress =
8095
this.circuitContext.callContext.currentQueryContext.address;
8196
return this;
8297
}

packages/simulator/src/factory/createSimulator.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,13 @@ import type { SimulatorOptions } from '../types/Options.js';
1414
import { createDrySimulator } from './createDrySimulator.js';
1515
import type { SimulatorConfig } from './SimulatorConfig.js';
1616

17-
/** Prepared backend wiring handed to the simulator constructor. */
18-
interface BackendDeps<P, L> {
17+
/**
18+
* Prepared backend wiring handed to the simulator constructor.
19+
*
20+
* Exported because it appears in `create` / `_create`, so a consumer that
21+
* assigns `createSimulator(...)` to an exported binding must be able to name it.
22+
*/
23+
export interface BackendDeps<P, L> {
1924
backend: Backend<P, L>;
2025
signers: Signers;
2126
pureNames: string[];
@@ -54,7 +59,7 @@ export function createSimulator<
5459
TContract extends IMinimalContract,
5560
TArgs extends readonly any[] = readonly any[],
5661
>(config: SimulatorConfig<P, L, W, TContract, TArgs>) {
57-
// Built once per factory; instances per `create()`. The synchronous primitive
62+
// Built once per factory; instances per `create()`. The in-memory primitive
5863
// is the whole dry path and the local JS artifact for pure-circuit eval.
5964
const DrySimClass = createDrySimulator<P, L, W, TContract, TArgs>(config);
6065

@@ -81,7 +86,7 @@ export function createSimulator<
8186
): Promise<BackendDeps<P, L>> => {
8287
const kind = resolveBackendKind(options.backend);
8388

84-
// The local synchronous simulator: the whole dry path, and the pure-circuit
89+
// The local in-memory simulator: the whole dry path, and the pure-circuit
8590
// evaluator in live (D2). In live this runs `initialState` in memory only —
8691
// it is never deployed on-chain.
8792
const localSim = new DrySimClass(contractArgs, options);

packages/simulator/src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ export { PrivateStateMutator } from './core/PrivateStateMutator.js';
1414
// `createSimulator`. `createLiveContext`/`registerLiveBackend` are
1515
// values, but their static graph is midnight-js-free (type-only + dynamic
1616
// import), so exporting them from the main barrel keeps the wall up.
17+
export type { BackendDeps } from './factory/createSimulator.js';
1718
export { createSimulator } from './factory/createSimulator.js';
1819
export type { SimulatorConfig } from './factory/SimulatorConfig.js';
1920
export type {
@@ -65,6 +66,7 @@ export type {
6566
ExtractPureCircuits,
6667
IContractSimulator,
6768
IMinimalContract,
69+
InitialStateResult,
6870
} from './types/index.js';
6971
export type {
7072
BaseSimulatorOptions,

0 commit comments

Comments
 (0)