Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions .github/actions/setup/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ inputs:
description: "Skip Compact compiler installation"
required: false
default: "false"
compact-prerelease:
description: "Pre-release Compact compiler to fetch from GitHub releases (bridge until the toolchain ships stable). Keep in sync with setup.ts COMPACTC_VERSION."
required: false
default: "0.33.0-rc.2"

runs:
using: "composite"
Expand Down Expand Up @@ -44,3 +48,27 @@ runs:
uses: midnightntwrk/setup-compact-action@4130145456ad3f45934788dd4a65647eb283e658 # loose commit/not released
with:
compact-version: "0.31.0"

# The tests pin a pre-release compiler (secp256k1 / 0.18 runtime) that
# `compact update` does not serve, so fetch its release asset directly and
# drop it where `compact +<version>` looks. Remove once the toolchain ships
# a stable release (then just bump compact-version above).
- name: Install pre-release Compact toolchain
if: ${{ inputs.skip-compact != 'true' }}
shell: bash
env:
VER: ${{ inputs.compact-prerelease }}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 blocking: the compact-prerelease input is non-functional as written. VER is scoped to this step, and test/setup.ts:55 reads process.env.COMPACTC_VERSION, which nothing sets. Override the input and CI installs one toolchain while the tests compile with the hardcoded +0.33.0-rc.2.

Unblocks: export it, e.g. echo "COMPACTC_VERSION=$VER" >> "$GITHUB_ENV" in this step.

added by claude (dev3-midnight-basic-review)

run: |
set -euo pipefail
# Reuse the platform dir the CLI already created for the stable install
# (e.g. x86_64-unknown-linux-musl), so the pre-release lands where the
# `+<version>` selector resolves it; fall back to the host triple.
PLAT="$(basename "$(find "$HOME/.compact/versions" -mindepth 2 -maxdepth 2 -type d 2>/dev/null | head -1)" 2>/dev/null || true)"
[ -z "$PLAT" ] && PLAT="$(uname -m)-unknown-linux-musl"
DEST="$HOME/.compact/versions/$VER/$PLAT"
mkdir -p "$DEST"
curl -fsSL -o /tmp/compactc.zip \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 blocking (downgrade if the team disagrees): the pre-release compiler is downloaded, chmod +x'd and executed with no integrity check. Every other action in this file is pinned by commit SHA and the repo runs OpenSSF scorecard, so this is a step down in posture on the one artifact that runs arbitrary code in CI.

Unblocks: pin a sha256 next to compact-prerelease and sha256sum -c before unzip. If the release publishes no checksum, say so and I'll drop this to a followup.

added by claude (dev3-midnight-basic-review)

"https://github.com/LFDT-Minokawa/compact/releases/download/compactc-v$VER/compactc_v${VER}_${PLAT}.zip"
unzip -oq /tmp/compactc.zip -d "$DEST"
chmod +x "$DEST"/*
compact compile +"$VER" --version
13 changes: 7 additions & 6 deletions packages/simulator/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,21 +41,22 @@
"clean": "git clean -fXd"
},
"devDependencies": {
"@midnight-ntwrk/midnight-js-contracts": "^4.1.0",
"@midnight-ntwrk/midnight-js-types": "^4.1.0",
"@midnight-ntwrk/midnight-js-contracts": "5.0.0-beta.6",
"@midnight-ntwrk/midnight-js-types": "5.0.0-beta.6",
"@noble/curves": "^2.2.0",
"@noble/hashes": "^2.2.0",
"@tsconfig/node24": "^24.0.3",
"@types/node": "26.1.2",
"fast-check": "^4.5.2",
"typescript": "^6.0.3",
"vitest": "^4.1.9"
},
"dependencies": {
"@midnight-ntwrk/compact-runtime": "0.16.0",
"@midnight-ntwrk/ledger-v8": "8.1.0"
"@midnight-ntwrk/compact-runtime": "0.18.0-rc.1"
},
"peerDependencies": {
"@midnight-ntwrk/midnight-js-contracts": "^4.1.0",
"@midnight-ntwrk/midnight-js-types": "^4.1.0"
"@midnight-ntwrk/midnight-js-contracts": "^5.0.0-beta.6",
"@midnight-ntwrk/midnight-js-types": "^5.0.0-beta.6"
},
"peerDependenciesMeta": {
"@midnight-ntwrk/midnight-js-contracts": {
Expand Down
20 changes: 12 additions & 8 deletions packages/simulator/src/core/AbstractSimulator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ export abstract class AbstractSimulator<P, L>
* @returns The current private state of type P
*/
public getPrivateState(): P {
return this.circuitContext.currentPrivateState;
return this.circuitContext.callContext.currentPrivateState as P;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: as P casts over PS | undefined. Harmless once init() has run, but it is the same hole as the missing init guard: before init() this returns undefined typed as P.

added by claude (dev3-midnight-basic-review)

}

/**
Expand All @@ -102,7 +102,7 @@ export abstract class AbstractSimulator<P, L>
* @returns The current state value containing the ledger data
*/
public getContractState(): StateValue {
return this.circuitContext.currentQueryContext.state.state;
return this.circuitContext.callContext.currentQueryContext.state.state;
}

/**
Expand Down Expand Up @@ -130,12 +130,13 @@ export abstract class AbstractSimulator<P, L>
const original = Reflect.get(target, prop, receiver);
if (typeof original !== 'function') return original;

return (...args: unknown[]) => {
return async (...args: unknown[]) => {
const fn = original as (
ctx: CircuitContext<P>,
...args: unknown[]
) => { result: unknown };
const result = fn(context(), ...args).result;
) => { result: unknown } | Promise<{ result: unknown }>;
// 0.18 circuits are async; `await` also tolerates the older sync shape.
const { result } = await fn(context(), ...args);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: does 0.33-generated circuit code copy the context before executing? The pure proxy is wired with () => this.circuitContext (createDrySimulator.ts:105), so circuits receive the live base context, and 0.18's finalizeCallProofData pushes into callProofDataTrace in place. If generated code does not copyCircuitContext first, every circuits.pure.* call permanently appends trace, events and gas to the persistent context even though its result context is discarded. I could not get a 0.33-compiled artifact to check this.

added by claude (dev3-midnight-basic-review)


// Auto-reset single-use caller override
this.callerOverride = null;
Expand Down Expand Up @@ -172,13 +173,16 @@ export abstract class AbstractSimulator<P, L>
const original = Reflect.get(target, prop, receiver);
if (typeof original !== 'function') return original;

return (...args: unknown[]) => {
return async (...args: unknown[]) => {
const fn = original as (
ctx: CircuitContext<P>,
...args: unknown[]
) => { result: unknown; context: CircuitContext<P> };
) =>
| { result: unknown; context: CircuitContext<P> }
| Promise<{ result: unknown; context: CircuitContext<P> }>;

const { result, context: newCtx } = fn(context(), ...args);
// 0.18 circuits are async; `await` also tolerates the older sync shape.
const { result, context: newCtx } = await fn(context(), ...args);
updateContext(newCtx);

// Auto-reset single-use caller override
Expand Down
71 changes: 52 additions & 19 deletions packages/simulator/src/core/CircuitContextManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@ import {
type ConstructorContext,
type ContractAddress,
type ContractState,
CostModel,
createCircuitContext,
createConstructorContext,
type EncodedZswapLocalState,
QueryContext,
} from '@midnight-ntwrk/compact-runtime';

/**
Expand All @@ -16,12 +15,36 @@ import {
* Handles initialization and lifecycle management of the `CircuitContext`,
* which includes private state, public (ledger) state, zswap local state, and transaction context.
*/
/** Shape of a compiled contract's constructor result (sync or async in 0.18). */
type InitialStateResult<P> = {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 blocking: this local InitialStateResult exists because IMinimalContract.initialState (src/types/Contract.ts:22-32) still declares the sync-only return type. The canonical interface now lies about 0.18, and anything else typed against it gets the wrong shape.

Unblocks: move the T | Promise<T> union onto IMinimalContract and have this file reuse it rather than redeclaring.

added by claude (dev3-midnight-basic-review)

currentPrivateState: P;
currentContractState: ContractState;
currentZswapLocalState: EncodedZswapLocalState;
};

export class CircuitContextManager<P> {
public context: CircuitContext<P>;
// Assigned by the async `init()`; the manager is always constructed and then
// awaited (`init`) before any circuit call reads the context.
public context!: CircuitContext<P>;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 blocking: two-phase construction with no guard. CircuitContextManager is a barrel export, so an external caller who skips init() gets Cannot read properties of undefined (reading 'callContext') instead of a useful message. Pre-PR the constructor always returned a usable object; this PR is what makes it constructible-but-unusable.

Unblocks: back context with a getter that throws "CircuitContextManager: call init() before use". Same applies to contractAddress! in createDrySimulator.ts.

added by claude (dev3-midnight-basic-review)


private readonly contract: {
initialState: (
ctx: ConstructorContext<P>,
...args: any[]
) => InitialStateResult<P> | Promise<InitialStateResult<P>>;
};
private readonly privateState: P;
private readonly coinPK: CoinPublicKey;
private readonly contractAddress: ContractAddress;
private readonly contractArgs: any[];

/**
* Creates an instance of `CircuitContextManager`.
*
* @remarks compact-runtime 0.18 made `initialState` (and every circuit) async,
* so the constructor only records inputs; the context is built by the async
* {@link init}, which callers must await before using the manager.
*
* @param contract - A compiled Compact contract instance exposing `initialState()`
* @param contract.initialState - Function that initializes contract state given a constructor context
* @param privateState - The initial private state to inject into the contract
Expand All @@ -34,34 +57,44 @@ export class CircuitContextManager<P> {
initialState: (
ctx: ConstructorContext<P>,
...args: any[]
) => {
currentPrivateState: P;
currentContractState: ContractState;
currentZswapLocalState: EncodedZswapLocalState;
};
) => InitialStateResult<P> | Promise<InitialStateResult<P>>;
},
privateState: P,
coinPK: CoinPublicKey,
contractAddress: ContractAddress,
...contractArgs: any[]
) {
const initCtx = createConstructorContext(privateState, coinPK);
this.contract = contract;
this.privateState = privateState;
this.coinPK = coinPK;
this.contractAddress = contractAddress;
this.contractArgs = contractArgs;
}

/**
* Runs the contract constructor and builds the initial `CircuitContext`.
* Must be awaited once, after construction, before any circuit call.
*/
async init(): Promise<void> {
const initCtx = createConstructorContext(this.privateState, this.coinPK);

const {
currentPrivateState,
currentContractState,
currentZswapLocalState,
} = contract.initialState(initCtx, ...contractArgs);

// Extract ChargedState from the compiler-generated ContractState
const chargedState = currentContractState.data;
} = await this.contract.initialState(initCtx, ...this.contractArgs);

this.context = {
currentPrivateState,
// compact-runtime 0.18 restructured `CircuitContext` into a call-tree

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question (non-blocking): heads-up on an upstream quirk this comment glosses. createCircuitContext calls createCallContext twice and seeds queryContexts[addr] / gasCosts[addr] from the first while callContext is the second, so context.queryContexts[addr] !== context.callContext.currentQueryContext from birth. Nothing here reads the former today. Worth confirming that still holds if the simulator grows a cross-contract path.

added by claude (dev3-midnight-basic-review)

// (`callContext` + per-contract `queryContexts`/`gasCosts`). Build it via
// the runtime's `createCircuitContext` factory rather than a hand-rolled
// literal so every required field is populated correctly.
this.context = createCircuitContext<P>(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 blocking: this drops the optional time arg, so simulated block time changes from a fixed 0 to wall-clock. Upstream defaults it to Math.floor(Date.now() / 1000) and stamps it into initialQueryContext.block.secondsSinceEpoch; under 0.16 the hand-rolled new QueryContext(...) left it at 0n (verified against the installed 0.16). Every downstream suite reading kernel.blockTime() flips to a non-reproducible clock, with no option, no test and no note in the PR body. useCircuitContextSender compounds it: it drops the existing callContext.time and re-stamps mid-flow.

Unblocks, any one of:

  • pass time explicitly (0 preserves today's behaviour),
  • add SimulatorOptions.time defaulting to 0, thread it through init() and useCircuitContextSender, and pin it with a test,
  • or confirm wall-clock is intended and document it as a breaking behaviour change for consumers.

added by claude (dev3-midnight-basic-review)

'circuit',
this.contractAddress,
currentZswapLocalState,
currentQueryContext: new QueryContext(chargedState, contractAddress),
costModel: CostModel.initialCostModel(),
};
currentContractState.data,
currentPrivateState,
);
}

/**
Expand All @@ -88,6 +121,6 @@ export class CircuitContextManager<P> {
* @param newPrivateState - The new private state to set in the current context
*/
updatePrivateState(newPrivateState: P) {
this.context.currentPrivateState = newPrivateState;
this.context.callContext.currentPrivateState = newPrivateState;
}
}
20 changes: 11 additions & 9 deletions packages/simulator/src/core/ContractSimulator.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
type CircuitContext,
copyCircuitContext,
emptyZswapLocalState,
} from '@midnight-ntwrk/compact-runtime';
import { AbstractSimulator } from './AbstractSimulator.js';
Expand Down Expand Up @@ -43,15 +44,16 @@ export abstract class ContractSimulator<P, L> extends AbstractSimulator<P, L> {
const activeCaller = this.callerOverride || this.persistentCallerOverride;
const baseCtx = this.circuitContext;

return {
currentPrivateState: baseCtx.currentPrivateState,
currentQueryContext: baseCtx.currentQueryContext,
currentZswapLocalState: activeCaller
? emptyZswapLocalState(activeCaller)
: baseCtx.currentZswapLocalState,
costModel: baseCtx.costModel,
gasLimit: baseCtx.gasLimit,
};
if (!activeCaller) {
return baseCtx;
}

// compact-runtime 0.18: the caller-scoped fields live on `callContext`.
// Copy (shallow-clones `callContext`) before overriding the Zswap local
// state so the base context is left untouched.
const ctx = copyCircuitContext(baseCtx) as CircuitContext<P>;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 followup (non-blocking): copyCircuitContext is marked @internal in the runtime's typings, so a patch release could drop it without notice. Nothing better to do here (hand-rolling the copy is worse). Worth a note in the comment above and an upstream ask to make it public.

added by claude (dev3-midnight-basic-review)

ctx.callContext.currentZswapLocalState = emptyZswapLocalState(activeCaller);
return ctx;
}

/**
Expand Down
22 changes: 17 additions & 5 deletions packages/simulator/src/factory/createDrySimulator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ export function createDrySimulator<
>(config: SimulatorConfig<P, L, W, TContract, TArgs>) {
return class GeneratedSimulator extends ContractSimulator<P, L> {
contract: TContract;
readonly contractAddress: string;
// Assigned by the async `init()` (0.18 made `initialState` async, so the
// address — read from the built context — is not known at construction).
contractAddress!: string;
public _witnesses: W;

/**
Expand Down Expand Up @@ -65,8 +67,18 @@ export function createDrySimulator<
contractAddress,
...processedArgs,
);
}

this.contractAddress = this.circuitContext.currentQueryContext.address;
/**
* Runs the contract constructor and finalizes state. Must be awaited once,
* after construction, before any circuit call. Split out from the
* constructor because compact-runtime 0.18 made `initialState` async.
*/
async init(): Promise<this> {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: docs went stale with the async migration. Line 15 still calls this the "Internal synchronous simulator primitive"; DryBackend.ts:38-42 still says it "wraps the synchronous result in a resolved promise" and that dry behaviour is preserved "byte-for-byte"; the exported SyncSimulator type name and its (...args) => unknown circuit signature are both misleading now.

added by claude (dev3-midnight-basic-review)

await this.circuitContextManager.init();
this.contractAddress =
this.circuitContext.callContext.currentQueryContext.address;
return this;
}

public _pureCircuitProxy?: ContextlessCircuits<
Expand Down Expand Up @@ -144,7 +156,7 @@ export function createDrySimulator<
*/
getPublicState(): L {
return config.ledgerExtractor(
this.circuitContext.currentQueryContext.state.state,
this.circuitContext.callContext.currentQueryContext.state.state,
);
}

Expand Down Expand Up @@ -191,8 +203,8 @@ export function createDrySimulator<
const circuitCtx = this.circuitContext;
return {
ledger: this.getPublicState(),
privateState: circuitCtx.currentPrivateState,
contractAddress: circuitCtx.currentQueryContext.address,
privateState: circuitCtx.callContext.currentPrivateState as P,
contractAddress: circuitCtx.callContext.currentQueryContext.address,
};
}
};
Expand Down
12 changes: 12 additions & 0 deletions packages/simulator/src/factory/createSimulator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ export function createSimulator<
// evaluator in live (D2). In live this runs `initialState` in memory only —
// it is never deployed on-chain.
const localSim = new DrySimClass(contractArgs, options);
// 0.18: `initialState` is async, so the constructor defers the constructor
// run to `init()`. Await it before deriving names / wiring any backend.
await localSim.init();
const contract = localSim.contract;
const impureNames = Object.keys(contract.impureCircuits);
const impureSet = new Set(impureNames);
Expand Down Expand Up @@ -246,6 +249,15 @@ export function createSimulator<
return this._signers;
}

/**
* The deployed contract's address. Needed by callers that must reconstruct
* a circuit's message digest off-chain (e.g. signing an operation bound to
* `kernel.self()`).
*/
get contractAddress(): string {
return this._backend.contractAddress;
}

/**
* Sets the caller for the next call only, then reverts.
*
Expand Down
6 changes: 4 additions & 2 deletions packages/simulator/src/signers/Signers.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import {
type CoinPublicKey,
convertFieldToBytes,
encodeCoinPublicKey,
} from '@midnight-ntwrk/compact-runtime';
import type { BackendKind } from '../backend/Backend.js';
Expand Down Expand Up @@ -39,7 +38,10 @@ export type Either<L, R> = {
const aliasToHex = (alias: string): CoinPublicKey =>
Buffer.from(alias, 'ascii').toString('hex').padStart(64, '0');

const zeroBytes = (): Uint8Array => convertFieldToBytes(32, 0n, '');
// A 32-byte zero array. Previously derived via the runtime's
// `convertFieldToBytes(32, 0n, '')`, which was removed in compact-runtime 0.18;
// the value is identical (the byte encoding of field element 0).
const zeroBytes = (): Uint8Array => new Uint8Array(32);

/**
* Configuration for {@link Signers}.
Expand Down
Loading
Loading