fix(web-client): resolve string AuthScheme for AuthGuardedMultisigConfig - #367
Open
kutluhaneth46 wants to merge 41 commits into
Open
fix(web-client): resolve string AuthScheme for AuthGuardedMultisigConfig#367kutluhaneth46 wants to merge 41 commits into
kutluhaneth46 wants to merge 41 commits into
Conversation
* fix: prevent overwrite on db * chore: add atomicity and deduplication * ci: fix linting * docs: fix changelog
* chore: update rust-sdk deps * chore: re-pin rust-sdk version * fix(web-client): re-import the note against a genesis-bearing store snapshot The test wiped IndexedDB under the live client and imported into the empty store, which cannot resolve any block header. Restoring a pre-mint snapshot keeps the scenario (note gone, chain known) without relying on client state that a store wipe invalidates. * chore: support encrypted tx inputs (0xMiden#252) * ci: cache the test node build dir per node rev only Cross-rev seeding via the restore-keys prefix lets cargo reuse stale build-script outputs when the same crate versions come from two git revs, which fails the node compile with phantom type errors. * chore: point the client patch and test infra at rust-sdk next The encrypted-tx-inputs client branch merged into rust-sdk next (cf510e39f), whose lock pins the node rev serving GetTransactionEncryptionKey and whose test-node scripts handle the validator's genesis/bootstrap CLI split and storage-key requirements. * chore: depend on the rust-sdk next branch directly Replaces the crates-io patch with git dependencies on rust-sdk next. Revert to registry versions when the next alpha is published. * chore: taplo fmt --------- Co-authored-by: Ignacio Amigo <ignacio.amigo@lambdaclass.com>
…Miden#255) The nightly MSRV job verifies every workspace package against wasm32-unknown-unknown. miden-mobile-prover is a native-only crate (cdylib/staticlib C ABI for iOS/Android, shipped via the native release job) and is never built for wasm. Verifying its MSRV against wasm is a category error: it pulls in the tonic->hyper->tokio->mio gRPC stack, and mio 1.2.0 — which declares no rust-version — fails to compile on our 1.96.1 MSRV. This has failed the nightly every day for weeks. - Exclude miden-mobile-prover from the generated package matrix. - Set fail-fast: false so each remaining crate reports its own MSRV status instead of the first failure cancelling and masking the rest.
* fix(ci): publish linked-client-pr gate as a Check Run to escape the 1000-status cap (0xMiden#187) * release: 0.15.1 (0xMiden#198) * chore(deps): upgrade miden-client to 0.15.2 Bumps miden-client + miden-client-sqlite-store 0.15.0 -> 0.15.2 (pulls miden-note-transport-proto-build 0.4.1; miden-protocol resolves to 0.15.3). Updates the CI node-builder ref (MIDEN_CLIENT_REF) to the v0.15.2 tag commit so the node's miden-protocol matches the bundled client. 0.15.2 deprecates the no-hint Client::send_private_note; the web-client keeps the no-hint path (allow(deprecated)) since the JS API does not expose a block hint yet. * release: 0.15.1 * fix(ci): make Rust crates publishable to crates.io (0xMiden#199) The "Publish Rust Crates on Release" job has failed since v0.15.0: - web-client depends on js-export-macro via a bare path dep (no version), so cargo publish refuses it, and js-export-macro was never in the publish list. - The Rust crates share the workspace version, which releases don't bump (only the npm versions move), so cargo publish hits "already exists". Fixes: - Bump the workspace version + internal dep version refs to 0.15.1. - Add js-export-macro as a workspace dep (path + version); web-client uses it via workspace = true so the published manifest carries a version. - Publish in dependency order (js-export-macro, idxdb-store, web-client, mobile-prover) and add the missing js-export-macro step. - web-client publishes with --no-verify: it's a wasm cdylib that can't build for the host target; CI verifies it for wasm32 instead. - Document the dual npm/Rust version bump in CONTRIBUTING. Verified: js-export-macro/idxdb-store/mobile-prover dry-run-publish cleanly at 0.15.1; web-client's manifest now resolves (its deps publish first in the workflow). * ci: allow crates-publish to be dispatched manually (0xMiden#200) Add a workflow_dispatch trigger (with an optional ref input) to publish-crates-release.yml so the crates can be (re-)published off a chosen ref — e.g. to catch crates.io up to a fix that landed after a release tag, or to recover when a release's publish failed partway. The checkout ref falls back from the release tag to the dispatched ref. * feat(pswap): track PSWAP order lineages and expose reads + cancel-by-order (0xMiden#176) * feat(web-client,react-sdk): track PSWAP order lineages + cancel-by-order Persist a lineage per partially-fillable swap order — the chain of remainder notes a PSWAP leaves behind as it is filled round by round — keyed by a stable orderId. Adds a `pswap` resource on MidenClient (lineages / lineagesFor / lineage / cancelByOrder) and four React hooks (usePswapLineages, usePswapLineagesFor, usePswapLineage, usePswapCancelByOrder). - pswap.rs: lineage reads + build_pswap_cancel_by_order with a terminal-state guard (only Active lineages can be cancelled), covering the raw binding, the pswap.js resource, and the React hook through the one shared layer. - PswapLineageRecord model exposes remainingOffered/remainingRequested as FungibleAsset (faucet + amount), depth, tip, state, and block numbers. - applyTransaction routes through the high-level apply so registered tx observers (PSWAP tracking) fire. - Tracks inicio-labs/miden-client vaibhav/pswap until the PSWAP API releases. * fix(pswap): drop redundant JS terminal-state guard, binding is authoritative The cancelByOrder resource duplicated the FullyFilled/Reclaimed check that buildPswapCancelByOrder already enforces in Rust, and its comment described the pre-guard behavior (request reaching the kernel). Remove the JS state block so the binding is the single guard, matching usePswapCancelByOrder; keep the lineage fetch for the creator account. * chore(deps): adopt published miden-client 0.15.2; drop the fork pin 0.15.2 ships the PSWAP API on crates.io, so pin the version and drop the inicio-labs/vaibhav/pswap git dependency. Adapt to 0.15.2's PswapLineageRecord: remainingOffered/remainingRequested are now plain amounts (the record no longer carries the faucet — it's recovered from the original note when needed), and the created/updated block accessors are gone (fields dropped upstream). Suppress the new send_private_note deprecation; the block-hint variant can be wired later. * refactor: remove protocol crates (0xMiden#197) * feat(web-client): expose advice map accessors on TransactionRequest (0xMiden#203) * feat(web-client): expose advice map accessors on TransactionRequest Add TransactionRequest.adviceMap() (returns a copy of the request's advice map) and TransactionRequest.extendAdviceMap(adviceMap) (merges entries into an already-built request and returns a new request, with last-write-wins semantics on key collisions). wasm-bindgen cannot return the native &mut AdviceMap, so this exposes the immutable builder-style shape instead. It lets a signer/guardian flow inject advice (e.g. a signature) that only becomes available after the request object is constructed, without going back through the builder. The pinned miden-client 0.15.2 already exposes the native advice_map()/advice_map_mut() methods, so no upstream change is needed. Closes 0xMiden#202 * docs(changelog): reference PR 0xMiden#203 for advice map entry * fix(react): add advice map methods to TransactionRequest mock The new TransactionRequest.adviceMap() / extendAdviceMap() methods made the hand-rolled mock in the react-sdk tests structurally incompatible with the TransactionRequest type, failing the typecheck. Add both to the shared createMockTransactionRequest factory. * release: 0.15.2 (0xMiden#205) * fix: declare MSRV for js-export-macro to fix nightly (0xMiden#209) * fix(web): export full public surface from the node entry (0xMiden#206) * fix(web): export full public surface from the node entry The node export entry (js/node-index.js, used via the "node" condition for SSR / Next.js server / Vitest) re-exported only a hand-curated subset of the WASM classes. The browser entry exports the full surface and the .d.ts types advertise it, so importing an omitted class (e.g. BasicFungibleFaucetComponent, TransactionRequest, InputNoteRecord) — or a @miden-sdk/react hook that imports one — threw "X is not exported from '@miden-sdk/miden-sdk'" under node resolution. Add _reexport lines for every public napi class (71 were missing), plus the JS-layer helpers react needs (CompilerResource, getWasmOrThrow). ESM can't re-export a native addon's members dynamically, so the names are listed explicitly; node_export_parity.node.test.ts enforces completeness against the napi module so the two can't silently drift again. * docs(test): tighten node export parity comment per review * refactor(web): generate node re-exports instead of hand-maintaining them Per review: replace the hand-written napi re-export list (and the runtime parity test) with a codegen script that derives the _reexport block from the native module's exports, written into a <generated:napi-reexports> marker region. `gen:node-reexports` regenerates it; `check:node-reexports` (run in the node CI job, which already builds the napi binary) fails if it drifts. The hand-written remaps (WebClient -> WasmWebClient, the AccountType/AuthScheme enum shadows) and JS-layer helpers (CompilerResource, getWasmOrThrow) stay manual. * release: 0.15.3 (0xMiden#210) * feat: add batch builder implementation (0xMiden#31) * feat(web,react): add AggLayer bridge-out (B2AGG) note support (0xMiden#211) * feat(web,react): add AggLayer bridge-out (B2AGG) note support Enables creating and submitting a B2AGG (Bridge-to-AggLayer) note entirely within web-sdk by consuming the agglayer functionality already re-exported by the bundled miden-client (miden_client::agglayer) — no new dependency and no miden-client change. - EthAddress JS model (20-byte Ethereum address) - Note.createB2AggNote(...) and WebClient.newB2AggTransactionRequest(...) - client.transactions.bridge(...) resource method (+ preview support) - @miden-sdk/react useBridge() hook - docs (CHANGELOG, READMEs, react-sdk guide) and unit tests Closes 0xMiden#173 * test(web): pin B2AGG resource argument order; drop changelog version floor Review follow-up: - transactions.test.js now asserts the resolved sender/bridge/faucet/destination ids by position, so a sender<->bridge<->faucet swap in #buildB2AggRequest would fail the test (the most important correctness property of this change). - CHANGELOG: drop the "(0.15.1+)" miden-client floor — unverifiable for the exact B2AggNote::create surface; the consumer-relevant fact is "no new dependency". * fix(web): use as_chunks for nightly clippy chunks_exact_to_as_chunks lint Pre-existing line surfaced by nightly clippy drift (the lint denies a constant-size chunks_exact under -D warnings); not related to the B2AGG feature, but it blocks this PR's Clippy WASM gate. Applies clippy's own suggestion; as_chunks is stable at the project MSRV (1.93). * fix(web-client): forward toU64s() on StorageResult (0xMiden#194) * fix(web-client): forward toU64s() on StorageResult StorageResult (returned by StorageView.getItem/getMapItem) is a Word-like wrapper that forwarded toFelts()/toHex()/toBigInt() but not toU64s(), while being typed as Word. Reading raw u64 elements off a storage value — e.g. `account.storage().getItem(slot).toU64s()` — threw "toU64s is not a function" at runtime even though Word.toU64s() is declared on the type. This blocked the OpenZeppelin multisig client's AccountInspector (which inspects every multisig account on load), and thus every guardian transaction. Add the missing pass-through (mirroring toFelts) plus the .d.ts declaration and a test assertion. * docs(changelog): add entry for StorageResult.toU64s() fix * feat(web): expose full faucet metadata on BasicFungibleFaucetComponent (0xMiden#204) * feat(web): expose full faucet metadata on BasicFungibleFaucetComponent Add tokenName(), tokenSupply(), description(), logoUri(), and externalLink() to BasicFungibleFaucetComponent so consumers can extract the complete token metadata of any fungible-faucet account (basic or network-style). In miden-standards 0.15.x the separate BasicFungibleFaucet and NetworkFungibleFaucet types were unified into a single FungibleFaucet component (the basic-vs-network distinction is now account configuration, not a component type), so the existing binding already works on network faucet accounts -- this just surfaces the metadata it wasn't exposing. A dedicated NetworkFungibleFaucet binding (as 0xMiden#162 originally proposed) would not compile against the pinned deps. Also adds BasicFungibleFaucetComponent to the typedoc curated exports. * fix(test): normalize null to undefined for faucet metadata getters on Node The new BasicFungibleFaucetComponent.description() / logoUri() / externalLink() getters return Rust Option<String>, which napi maps to null on Node.js (wasm-bindgen maps None to undefined on the browser). The shared Playwright test asserts toBeUndefined(), so the Node project failed. Register the three getters with the node-adapter's existing patchNullToUndefined shim, matching AccountStorage / NoteConsumability. * fix(test): normalize faucet metadata getters in the active node sdk path The previous attempt patched node-adapter.ts, but the per-test sdk is built by test-setup.ts's createNodeSdkWrapper -> patchNapiPrototypes, so that patch never ran on the test path (description() still returned napi null, failing toBeUndefined() on the Node project). Register the three BasicFungibleFaucetComponent getters in patchNapiPrototypes instead, and revert the node-adapter.ts change. Verified locally: the nodejs project's basic_fungible_faucet_component tests pass. * release: 0.15.4 (0xMiden#213) * chore: prepare 0.15.4 * chore: drop empty Unreleased header from 0.15.4 changelog * fix(release): bump wallet example @miden-sdk/miden-sdk dep to ^0.15.4 * chore(release): bump wallet example @miden-sdk/react dep to ^0.15.4 * feat(web,react): create custom-script network notes (NetworkAccountTarget attachments) (0xMiden#230) * feat(web): add NetworkAccountTarget WASM binding * feat(web): add Note.withAttachments/attachments/isNetworkNote bindings * feat(web): add NoteRecipient.fromScript (random serial) * fix(web): register NoteAttachment and NoteExecutionHint for napi by-value params * feat(web): export NetworkAccountTarget on node + typedoc surfaces * feat(web): declare NetworkNoteOptions/Result + createNetworkNote/buildNetworkNote types * feat(web): add standalone buildNetworkNote builder * fix(web): reject recipient+script and pin buildNetworkNote test assertions buildNetworkNote silently dropped `script` when both `recipient` and `script` were passed (via `??`); now throws instead, matching the "exactly one of recipient/script" contract. Also strengthens the NetworkAccountTarget construction test to pin executionHint as the second constructor arg, and adds coverage for the pre-built-target and assets-provided branches. * feat(web): add transactions.createNetworkNote resource method * fix(web): wrap script-recipient inputs in FeltArray for network notes buildNetworkNote and createNetworkNote passed a plain JS array straight into `new wasm.NoteStorage(...)`, which works under the napi array polyfill but throws `expected instance of FeltArray` against the real browser WASM bindings, breaking the script + inputs path outside of mocked unit tests. * test(web): integration gate for network-note attachment survives submit * feat(react): add CreateNetworkNoteOptions/NetworkNoteResult types * feat(react): export CreateNetworkNoteOptions/NetworkNoteResult from package barrel * feat(react): add useCreateNetworkNote hook * docs: document network-note creation (web + react) * fix(web): type NetworkNoteOptions.attachment as bigint[] to match runtime * refactor(react): reuse target.targetId() in useCreateNetworkNote * docs: link network-note CHANGELOG entries to web-sdk#230 * docs: link CHANGELOG entries to 0xMiden#230 and amend stale NetworkAccountTarget comment - CHANGELOG: use the linked ([0xMiden#230](url)) form to match repo convention (per @juan518munoz). - note_attachment.rs: NetworkAccountTarget is re-exposed by this PR, so drop the 'type does not exist on this surface' clause (per @igamigo / 0xMiden#228). * fix(web): wrap network-note `inputs` bigints into Felt before FeltArray createNetworkNote / buildNetworkNote passed raw `inputs` into `new FeltArray(...)`, which throws `expected instance of Felt` against real WASM for any non-empty inputs. The React hook already wrapped each value in `new Felt(v)`; the web-client resource + standalone builders did not, and the unit tests only passed because `FeltArray` was mocked (the exact marshaling gap this feature's integration test was meant to close). - transactions.js / standalone.js: map `inputs` through `new wasm.Felt(v)`. - api-types.d.ts: `inputs?: Felt[]` -> `bigint[]`, matching the React surface and the sibling `attachment` field. - unit tests: assert the Felt-wrapping (Felt mock + FeltArray/NoteStorage args). - network_note integration test: exercise non-empty `inputs` marshaling end to end against real WASM, not a mock. - useCreateNetworkNote: reuse `senderId` for submit instead of re-parsing. Found by independent review of 0xMiden#230. * chore: prepare 0.15.5 (0xMiden#232) * feat: re-add removed ntx functionality (0xMiden#236) * chore: upgrade to client `0.15.4` (0xMiden#238) * feat(web): expose AssetCallbackFlag on FungibleAsset (0xMiden#240) * feat: expose manual transaction lifecycle on TransactionsResource (0xMiden#235) * feat(web): expose manual transaction lifecycle on TransactionsResource Adds the four stages that submit() runs in one call as individual public methods, so each step can be benchmarked and error-handled independently (closes 0xMiden#233): const result = await client.transactions.executeRequest(account, request); const proven = await client.transactions.prove(result, { prover? }); const { blockNumber } = await client.transactions.submitProven(proven, result); await client.transactions.apply(result, blockNumber); - executeRequest: execute only — nothing proven, submitted, or persisted - prove: per-call prover override, falls back to the client default - submitProven: network submission, returns { blockNumber } - apply: persists to the local store and fires transaction observers Docs: api-types JSDoc (+ ProveOptions / SubmitProvenResult types, ProvenTransaction / TransactionResult / TransactionStoreUpdate typedoc re-exports), Docusaurus transactions page, web-client README, CHANGELOG under 0.15.6 (TBD). Covered by resource unit tests and a mock-chain integration test driving the four stages end-to-end. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(web): staged handles for manual transaction lifecycle Replace the flat executeRequest/prove/submitProven/apply methods with a staged pipeline: executeRequest() returns a TransactionExecution handle advanced via .prove() -> .submit() -> .apply(). Each stage carries its own context (result, proof, blockNumber), so callers never re-thread state and out-of-order calls are unrepresentable. - submit() now shares the prover-fallback helper (proveResult) with the staged path, so the two can no longer drift. - submitProven(proof, result) kept as the detached-proving escape hatch, now returning a TransactionSubmission handle. - TransactionSubmission.waitForConfirmation() added. - JSDoc documents the prover single-use hazard and the non-atomicity of the stages as a group. Docs updated across CHANGELOG, README, Docusaurus, and api-types JSDoc. Unit + mock-chain integration tests drive the staged handles. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Wiktor Starczewski <poszerny@gmail.com> * release: 0.15.7 * feat(web): add BasicFungibleFaucetComponent.fromAccountStorage (0xMiden#244) * feat: add BasicFungibleFaucetComponent.fromAccountStorage with tests * docs: changelog entry for fromAccountStorage * fix: address review — unwrap StorageView in fromAccountStorage facade, add 0xMiden#243 differentiator test * fix: patch fromAccountStorage via defineProperty — napi statics are read-only * fix: accept StorageView in napi fromAccountStorage via FromNapiValue fallback * feat(web): expose fungible asset vault key helpers (0xMiden#247) * feat(web): expose fungible asset vault key helpers * docs: add fungible asset vault key changelog * test(web): normalize fungible asset value arrays * refactor(web): reshape vault round-trip to FungibleAsset.fromVaultEntry(key, value) Addresses the API-shape review of the vault-key helpers. - fromVaultKey(key, amount) -> fromVaultEntry(key, value): take the (key, value) word pair the vault actually stores. This mirrors the native from_key_value_words primitive and pairs 1:1 with the model's own getters, so `FungibleAsset.fromVaultEntry(a.vaultKey(), a.intoWord())` round-trips an asset read from vault data with zero decoding — previously a caller holding both words had to decompose the value word back to a scalar amount. Deletes the internal value-word re-encoding (which duplicated native to_value_word) and the redundant AssetAmount re-validation (from_key_value_words already validates), and the name no longer understates its inputs. - Make the key/value duality legible: vaultKey() (key word — faucet id + callback flag) and intoWord() (value word — amount) now cross-reference each other and fromVaultEntry; fromVaultEntry documents that the callback flag comes from the key. intoWord() is left in place (released API), documented as the value half. - Tests round-trip via the two getters and reject an oversized amount encoded into the value word plus an invalid key. Verified: cargo check --package miden-client-web --target wasm32-unknown-unknown (clean). * feat(web): add FungibleAsset.fromVaultKey(key, amount) convenience Restores the key + scalar amount ergonomic as a distinctly-named convenience alongside the symmetric fromVaultEntry(key, value): use fromVaultEntry when you already hold both vault words, fromVaultKey when you have the key word and the amount as a number. The key supplies the faucet id + callback flag; the amount is encoded into the value word (layout mirrors native to_value_word, noted so the two stay in lockstep). Test asserts fromVaultKey(key, amount) yields the same asset as fromVaultEntry(key, value). Verified: cargo check --package miden-client-web --target wasm32-unknown-unknown and cargo fmt --check (clean). * test(web): cover value-word rejection paths + correct max-amount docs Review follow-ups: - Correct the documented amount ceiling from 2^63 - 1 to the real AssetAmount::MAX (2^63 - 2^31) in the fromVaultEntry / fromVaultKey docs and the test comment (the enforced max is 2^63 - 2^31, not 2^63 - 1). - Add rejection tests for the two public error branches that were uncovered: fromVaultKey's own AssetAmount::new guard (over-max scalar amount), and a value word with non-zero upper limbs fed through fromVaultEntry. Verified: cargo check --package miden-client-web --target wasm32-unknown-unknown and cargo fmt --check (clean). * docs(changelog): file vault-entry feature under 0.15.8, not released 0.15.7 v0.15.7 is already tagged/published and did not contain these APIs, so the fromVaultEntry/fromVaultKey/vaultKey entry was misfiled under its section. Merged main (which carries the `## 0.15.8 (TBA)` staging section) and moved the entry there. package.json stays at 0.15.7, matching the repo's release flow (the version is bumped at release time, not when staging changelog entries). * style: prettier-format fungible asset test --------- Co-authored-by: Wiktor Starczewski <poszerny@gmail.com> * release: 0.15.8 (0xMiden#248) * docs(readme): document single-threaded vs multi-threaded WASM builds (0xMiden#250) Co-authored-by: WiktorStarczewski <wiktor.s@miden.team> --------- Co-authored-by: igamigo <ignacio.amigo@lambdaclass.com> Co-authored-by: Wiktor Starczewski <poszerny@gmail.com> Co-authored-by: VaibhavJindal <vaibhavjindal29@gmail.com> Co-authored-by: Utkarsh Sharma <114555115+0xnullifier@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: WiktorStarczewski <wiktor.s@miden.team>
* refactor: idempotent publishing * Update .github/workflows/publish-crates-release.yml Co-authored-by: Santiago Pittella <87827390+SantiagoPittella@users.noreply.github.com> --------- Co-authored-by: Santiago Pittella <87827390+SantiagoPittella@users.noreply.github.com>
…ateOutput (0.16/next) (0xMiden#264)
* release: 0.16.0-alpha.2 * chore(idxdb-store): allow clone_on_copy on wasm-bindgen getter_with_clone getters
* refactor: remove binary size * refactor: simplify the MASP strip tool's package reader Replace the hand-rolled CountingReader with std::io::Cursor (winter-utils implements ByteReader for it) and note the fresh-memory invariant that --zero-filled-memory relies on. Strip output is byte-identical. * chore: bump crate and package versions to `0.16.0-rc.2` * docs: fill in the changelog PR link * chore: format workspace members list * ci: harden WASM release pipeline * ci: register MT release verifier with Knip
…int gap) (0xMiden#300) * fix(strip-masp-debug): make pad_to_len reach every target length A section frames both its id and its data length as vint64s, so for a fixed padding-section id the reachable serialized lengths skip the value just past each 7-bit boundary (deficits of exactly 128, 16385, 2097154, ... bytes). A package whose original length lands on one is un-paddable — and since strip_file now treats an un-paddable package as a hard error, that would break the release build with a confusing "could not be padded back to N bytes". Vary the padding-section id length by a byte to step off the boundary, so every reachable target is exact. Strengthen padding_handles_varint_boundaries to sweep a contiguous range across the +128 and +16384 boundaries; the previous hand-picked set never landed on the gap (e.g. deficit +145 fails on the old single-id padding). * test(strip-masp-debug): harden padding boundary coverage per review - Make the second varint-boundary region a contiguous sweep (16_380..=16_420) instead of 5 hand-picked points. The discrete bracket had only one point of margin above the gap, so lengthening PAD_SECTION_ID by >=2 bytes would slide the gap out of it and silently stop covering the 2->3-byte boundary while the test stayed green. - Match the padding-section id by prefix (starts_with) instead of an exact string in strips_multiple_packages_...: pad_to_len can legitimately append '-' bytes to the id when a package's length lands on a gap, so the exact-match assertion would false-fail if a fixture ever needed the lengthened-id path. - Assert the padding section is present in the boundary test. * chore: changelog --------- Co-authored-by: igamigo <ignacio.amigo@lambdaclass.com>
* feat: chain-anchored transaction execution
Since protocol 0.16 a signed transaction summary binds the reference block
commitment, so signatures collected over a summary only authorize an execution
whose reference block is the one the summary was built at. Every client
executes at its own sync height, which makes any flow that collects signatures
and executes later — multisig proposals, offline co-signing — impossible to
complete: the proposer, each co-signer, and the executor are at different
heights and none can reproduce the summary the others signed.
A ChainAnchor pins execution to a specific reference block so the summary
reproduces on any client. Exposed as explicit `...At` methods at the wasm
boundary, mirroring rust-sdk, and as an `anchor` option on the MidenClient
methods that take a caller-built request. `send`, `mint`, and friends build
their request internally, so an anchor could never have been captured for one
and the option is deliberately absent there.
`executeForSummaryAt` has no rust-sdk counterpart and is what makes the flow
completable: without it a co-signer cannot re-derive the summary at the
proposer's anchor, and comparing against a locally-derived one always fails.
For the same reason the React SDK gains `usePreview`, its first summary
surface, alongside `useChainAnchor`.
Client PR: #2421
* fix: address review findings on chain-anchored execution
- Export ChainAnchor from node-index.js. The generated napi re-export region
is what lets Node consumers reach the class at all, so without it
ChainAnchor.deserialize was unreachable from Node — exactly the call a
co-signer makes first — and check:node-reexports failed.
- Reject an anchor on preview operations other than "custom". Those build
their request internally, so an anchor could only have been captured for a
different one; the guard turns a confusing error from deep in the executor
into one at the API boundary. TypeScript already prevented this, plain JS
did not.
- Tag ChainAnchorError as INVALID_CHAIN_ANCHOR rather than folding it into a
generic message, so callers can distinguish "recapture the anchor" from a
real execution failure without matching on strings.
Also adapts submitNewTransactionBatch to BatchBuilder::push now returning
&mut Self upstream, which is what broke the build against the current head
of the linked client PR.
* fix: correct anchor error mapping and add failure-path coverage
Review caught that INVALID_CHAIN_ANCHOR could never fire where it was wired.
Upstream stringifies execution-time anchor rejections into
DataStoreError::other, so ClientError::ChainAnchorError only ever surfaces
from capture, where the anchor's three store reads are validated for mutual
consistency. Route chainAnchorForRequest through the mapper — a concurrent
sync mid-capture is the real "retry this" case — and rewrite the comment,
which described a scenario that does not reach it.
Docs told co-signers to import ChainAnchor from @miden-sdk/react, which
re-exports it as a type only, so their first call would not compile.
Reset the captured anchor when the client changes: an anchor is bound to one
chain, so carrying it across a network switch replays against a foreign
header and fails far from the cause.
Adds the missing failure-path test. It asserts the rejection names the
anchor's block as the reference block, which is what distinguishes anchoring
being honored from it being silently ignored.
* fix: don't publish an anchor or summary captured on a client we left
Two reviewers independently caught that the client-swap reset added in the
previous commit only handled settled state. A capture already in flight
resolves after the effect runs and publishes an anchor built against the
previous chain — the exact thing the reset exists to prevent. Compare against
a ref at the publish point and reject with STALE_CLIENT instead, since the
returned value is what a proposer ships to co-signers and must not reach them
either.
usePreview gets the same treatment: a summary binds the reference block
commitment, so it is no more portable across chains than the anchor.
Documents INVALID_CHAIN_ANCHOR on captureAnchor, the one path that can emit
it, and fixes two doc slips: the react hook table named a `transact` method
that does not exist, and the README co-signer snippet compared against a
`proposed` summary it never derived.
* fix: declare STALE_CLIENT and keep it out of hook error state
MidenErrorCode is a closed union, so the STALE_CLIENT code added in the last
commit did not typecheck. Local verification missed it because vitest strips
types rather than checking them, so a full green test run says nothing about
whether the branch builds.
A stale rejection was also writing into the error state the client-swap effect
had just cleared, leaving the user looking at an error about a chain they had
already navigated away from. The rejection is the channel that matters, since
the point is to keep the value from reaching co-signers, so skip the state
write on that path only.
Adds the client-swap tests usePreview was missing entirely — both the settled
reset and the in-flight rejection — and pins the code rather than the message
in both hooks' tests, which is what would have surfaced the union gap here
instead of in CI.
* fix: gate hook error state on client identity rather than error shape
The previous guard fingerprinted the error to recognize a stale result, which
only covered the case where the abandoned client succeeded. When it failed
instead, an error about the chain the user had already left still landed in
state. Comparing the client directly is both simpler and strictly broader: it
covers the rejection path too, and drops a dependency on instanceof surviving
future build-target changes.
Documents that these rejections bypass error state, since a consumer rendering
from it would otherwise see nothing, and softens the STALE_CLIENT wording — it
is the code only when the call would have succeeded.
* test: cover the abandoned-client rejection path in usePreview
usePreview took the same error-state guard as useChainAnchor but only the
latter got a test for a plain failure arriving after a swap, so the specific
case the guard exists for was unverified on this side.
* fix(rev): round 1 — anchor nullish handling and error guidance
F-006 P1 anchored summary path told callers to resubmit with `execute`,
which is the unanchored path, so following the message defeats the
anchoring the caller had just set up
F-010 P1 `{ anchor: null }` fell through every truthiness check and executed
at the tip — the one outcome anchoring exists to prevent, and easy
to hit since `useChainAnchor().anchor` is null until it resolves
F-007 P2 map_anchor_err dropped its context argument on the tagged arm
F-009 P2 the preview anchor guard shadowed the unknown-operation diagnostic
F-011 P2 a request factory returning null reached wasm as a null pointer,
which reads like a consumed handle
F-012 P2 new captureAnchor docs promised an error code without the Node
caveat its neighbours carry
F-013 P2 reset() cleared isCapturing without cancelling, so a UI keyed on
that flag re-enabled a button the busy guard still rejects
F-014 P2 React docs omitted INVALID_CHAIN_ANCHOR
F-015 P3 the request-or-factory union was spelled out four times
Reviewed by claude-opus-5-thinking-high (x2), gpt-5.6-sol-medium,
gpt-5.6-terra-medium, cursor-grok-4.6-high-fast.
* fix(rev): round 2 — anchor option semantics and untrusted-bytes decoding
F-021 P1 round 1's guard rejected `{ anchor: undefined }`, which is how an
optional property spells "absent" — a regression on submit,
executeRequest, preview, useTransaction and usePreview. Now only
non-undefined falsy values throw, which also closes the
`cond && anchor` case the first guard missed
F-022 P2 ChainAnchor.deserialize ignored trailing bytes, so unboundedly many
blobs decoded to one anchor
F-023 P2 it also read attacker-supplied bytes through the reader upstream
documents as trusted-input-only; now budgeted to the input length
F-025 P2 docs claimed the summary "reproduces on any client", but an anchor
pins chain data only — account state still comes from each party's
local store, which is the likeliest reason a multisig flow fails
F-028 P2 docs told readers to compare anchor.commitment() against a value
TransactionSummary does not expose; the workable check is
re-deriving the summary at the anchor
F-026 P2 the preview operation set could drift from the switch it mirrors
F-029 P2 no test pinned byte-level round-trip stability
F-024 P3 documented that the encoding carries no version tag
Reviewed by gpt-5.6-terra-medium, claude-opus-5-thinking-high,
gpt-5.6-sol-medium, and the security-review subagent (no medium+ findings).
* fix(rev): round 3 — remove the allocation budget that rejected real anchors
F-031 P0 round 2's `read_from_bytes_with_budget(bytes, bytes.len())` rejects
every legitimate anchor whose partial blockchain tracks a block. The
budget bounds a collection's length by `remaining / min_serialized_size()`,
and that default is `size_of::<Self>()`, which exceeds the on-wire size
for every type in the anchor — BlockHeader omits its derived commitments
from the encoding. Anchored execution of a note-consuming request was
broken end to end, including locally, since the worker round-trips the
anchor through deserialize on every call. Allocation is bounded anyway
by the reader, which reserves nothing up front
F-032 P2 the only anchors under test came from mint requests, which track zero
blocks — the one shape that still worked. Added a consume-request
round-trip, which is what should have caught the above
F-033 P2 the trailing-byte check no longer re-encodes the whole value; it reads
the reader position instead, removing a full second serialization from
every anchored execution
F-034 P2 round 2's doc fixes left the unqualified "reproduces on any client"
claim standing in four places and the superseded commitment guidance
in the rustdoc that wasm-bindgen emits
F-035 P2 the drift test matched cases by regex over a range spanning three
methods; now bounded to preview and asserted for exact equality,
and verified to actually fail when a case is added
F-036 P2 the worker never freed the anchor it rebuilds on every execution
F-037 P3 JSON.stringify in the guard message throws on BigInt and renders NaN
as "null"; the raw executeTransactionAt had no anchor guard at all
F-038 P3 the reset assertions could not fail, since the flag was already clear
Reviewed by claude-opus-5-thinking-high, cursor-grok-4.6-high-fast,
gpt-5.6-sol-medium, gpt-5.6-terra-medium, and the bugbot subagent (no findings).
* fix(rev): round 4 — API contract, diagnosability, and a test that could flake
F-042 P1 the React README still told readers to verify an untrusted anchor by
comparing it against a commitment TransactionSummary does not expose.
Third incomplete doc fix in a row for this claim; swept the whole repo
this time, no occurrences remain
F-043 P2 the resource-layer anchor spec fired two proveBlock calls without
awaiting them, then asserted the tip had moved past the anchor
F-044 P2 the round-3 test comment claimed to cover what the worker rebuilds,
but the harness terminates the worker. Comment now states what is
actually covered (the codec) and what is not (the postMessage wiring)
F-046 P2 hooks documented branching on error.code while typing error as Error,
so the documented contract needed a cast. Raised independently by
three reviewers across three rounds. Added CodedError and WasmErrorCode
and verified the discriminator typechecks without one
F-047 P2 js_error_with_context attaches remediation text as `help`, but the
worker dropped it, making a worker-backed failure less diagnosable
than the same failure on the main thread — and anchored execution is
the worker-proxied path
F-049 P2 INVALID_CHAIN_ANCHOR named the cause but not the remedy
F-052 P2 usePreview got round 3's reset change without round 3's test
F-045 P2 noted the Node resource-layer gap the way this file already notes it,
rather than reaching into a private method to fake coverage
F-039 resolved as NOT A BUG. Two reviewers disagreed on whether the client-swap
guard is sound. Settled empirically: a SyncLane commit flushes its own passive
effects synchronously, so the ref is current before any continuation can run.
Both setClient call sites are passive effects, where the window has zero width.
The proposed alternative — writing the ref during render — was shown to be
actively unsafe: a discarded render leaves the ref pointing at a client the
committed tree never adopted. Left unchanged.
Reviewed by claude-opus-5-thinking-high, cursor-grok-4.6-high-fast,
gpt-5.6-sol-medium, and gpt-5.6-terra-medium.
* fix(rev): round 5 — export blockCommitment, close mutation-surviving gaps
Round 5 mutation-tested the suite: 24 mutations, 18 caught, 6 survived.
F-054 P1 wasm TransactionSummary never exported blockCommitment(), though the
protocol type has it. That absence is why rounds 2-4 kept rewriting the
untrusted-anchor guidance: the check the Rust client documents was
unimplementable in JS, so the docs kept working around it. Exported it
(and expirationDelta), and the guidance is now the upstream one — the
cheap commitment comparison first, re-derivation as the stronger check
F-055 P1 useTransaction's anchor guard could be deleted with all 879 tests
green. It is the hook that submits, so an unanchored fallthrough there
spends against a different reference block than was signed for
F-056 P1 the round-3 test written to cover tracked-block anchors asserted
executedBlock === anchorBlock without advancing the tip, so it held
whether or not the anchor was honored — the same vacuity that hid the
round-3 P0, in the test added to prevent it
F-057 P1 docs snippets called ChainAnchor.deserialize without importing it
F-058 P1 React docs and hook JSDoc promised error.code unconditionally; on Node
client-originated codes prefix the message instead
F-059 P2 the multisig flow never showed signatures returning to the executor,
and never said a co-signer must already track the account
F-060 P2 direct unit tests for assertAnchorValueUsable and
resolveTransactionRequest, covering all three hooks in one place
F-061 P2 the !isReady half of the readiness guard, and the documented
OPERATION_BUSY code, were both untested
F-062 P3 0n and NaN now covered — the two values that motivated String() over
JSON.stringify in the round-2 error path
F-063 P2 React docs now state that anchored execution skips the recency check
Verified by re-running the surviving mutations: both are now caught.
Deliberately not documented: the advice-map key formula for signature
attachment. No example exists anywhere in the repo, so it would be inferred —
and unverified instructions are what needed correcting three rounds running.
Reviewed by claude-opus-5-thinking-high (mutation testing),
cursor-grok-4.6-high-fast (upstream fidelity), gpt-5.6-sol-medium (end-to-end
flow), gpt-5.6-terra-medium (docs).
* fix(rev): round 6 — red team; verification guidance was unsound
F-065 P1 the documented co-signer verification told readers a self-consistency
check was sufficient to sign. The request, anchor and summary all come
from the proposer, so "re-derive at the anchor and compare
toCommitment()" holds by construction for any request they chose —
including one that drains the account. The checks detect a corrupted
or substituted component; they say nothing about intent. Rewritten
across three surfaces to say so, and to direct co-signers at the
effects (accountDelta, inputNotes, outputNotes, expirationDelta).
Also noted that ChainAnchor's two invariants relate attacker-chosen
fields to each other, so a well-formed anchor for a block that never
existed is constructible — a reviewer built one in 715 bytes. Harmless
(the node rejects it) but not what "can never be malformed" implied
F-066 P1 send/mint/consume/swap/execute/batch/submitBatch all accepted an
`anchor` option and silently ignored it, executing at the tip while
the caller believed the transaction was pinned. preview already threw
for this; the others never learned. `execute` vs `executeRequest` and
`submitBatch` vs `submit` are the easy confusions. Now all reject
F-067 P2 the trailing-byte check was documented as keeping the encoding
canonical. It rejects suffixes; a non-minimal length prefix still
decodes. Corrected rather than paying a full re-encode per call
F-068 P2 an anchor keeps a summary reproducible but does not extend the
transaction's lifetime. Since anchored execution skips the recency
check, an over-long signing round executes and proves locally, then
is rejected at submission. Documented with expirationDelta
F-069 P2 "executing a different request against it fails" overstated: it fails
only if a needed block is untracked, which is never true for a request
with no authenticated input notes
F-070 P3 the min_serialized_size comment blamed the collection types, which
override it to 1; BlockHeader is the one that inherits the default
Kept as settled: `{ anchor: undefined }` meaning absent (round 2 resolved this
on evidence — rejecting it regressed five existing methods).
Out of scope: worker death leaves pending calls unrejected. Real, but it
predates this change and affects every worker method equally.
Reviewed by claude-opus-5-thinking-high (malicious counterparty),
cursor-grok-4.6-high-fast (hostile integrator), gpt-5.6-sol-medium (attacking
prior rounds' conclusions), gpt-5.6-terra-medium (hostile environment).
* fix: guard every request-building method against a stray anchor
The methods that build their own request accepted an `anchor` option and
executed at the tip anyway, so a caller who passed one got the outcome
anchoring exists to prevent, silently. Six were still unguarded.
Derive the guarded set from the source in the test rather than listing it,
so a method added later cannot skip the guard without an explicit exemption.
Three methods are exempt on purpose: executeProgram, prove and
waitForConfirmation never execute a transaction against a reference block,
so there is no tip to fall back to.
Also switch TransactionSummary::deserialize to the untrusted path, matching
ChainAnchor: a summary crosses the same wire from the same counterparty.
Nothing persists these bytes, so rejecting trailing bytes breaks no caller.
* fix: pin the anchored request, and derive guard coverage from the AST
A request factory resolves to a new object per call, and any builder that
creates an output note draws a fresh serial number from the client's RNG.
captureAnchor discarded the request it resolved, so passing the same factory
on to preview or execute built a different transaction than the anchor
pinned — the summary co-signers verified would not match the one submitted.
useChainAnchor now exposes it as `anchoredRequest`. The existing tests could
not have caught this: their factories returned one shared mock.
Replace the two regex-based drift tests with TypeScript AST walks. The regex
keyed off the parameter being spelled `opts` and case labels being
identifier-safe, so `async sendV2(options)` or `case "send-v2":` slipped
through; both are now caught. Scoping the walk to the class also showed two
exemptions were bogus — `prove` and `waitForConfirmation` belong to other
classes in the file, while `submitProven`, `list`, `waitFor` and
`captureAnchor` are the real ones.
Add the anchored-summary integration test the co-signing path never had.
It advances the tip after capture and asserts the summary's own
blockCommitment equals the anchor's, so swapping in the unanchored
executeForSummary fails.
* test: assert the observed expiration delta rather than a guess
Running the new anchored-summary spec against a real build showed
expirationDelta is 0 for a consume request that sets no expiration, not
positive as assumed. Pin the observed value so the accessor is still
exercised.
* docs: correct what a signed summary actually binds
The summary's commitment preimage is six things: the account delta, the
input- and output-note commitments, the reference block, the expiration
delta and the user params. Three claims repeated across four documents did
not match that.
"It fails closed" was the worst of them. The summary binds the delta, not
the state it applies to, so divergence that leaves the delta and note sets
unchanged verifies clean — an unrelated nonce bump, arriving assets, or a
multisig signer-set or threshold change. Signatures gathered under one
threshold survive it being lowered. signature.masm binds the final nonce as
user param 0; the multisig component zeroes those, so it binds nothing.
"A differing request is rejected" was also wrong: the script root, advice
map, note args and foreign inputs are all outside the preimage, so two
requests with identical effects share a commitment.
Anchor invariants are computable over an invented chain, so promote the
block-header check from optional to a standard step, with the RpcClient
call it needs.
Also document that expirationDelta() returns 0 for "no expiration set"
rather than "already expired", so the natural deadline computation does not
reject every non-expiring transaction; that preview and captureAnchor run
on the main thread, matching the unanchored executeForSummary; and who owns
a captured anchor, since the hook cannot free a handle it hands out.
Add the nullish-request guard captureAnchor was missing, and note that
BlockHeader.proofCommitment aliases commitment() because the protocol field
it was named for no longer exists.
* chore(deps): bump the miden-vm/crypto stack to 0.29.4 * release: 0.16.0-rc.4
…xMiden#326) * Initial commit * Update miden sdk versions * create all-in-one package * update readme * update readme again * Update miden-wallet-adapter-react to React 19 * Update miden-wallet-adapter-reactui to React 19 * correct commit hash in reactui docs * Revert "correct commit hash in reactui docs" This reverts commit f9e81c0. * Update miden-wallet-adapter to React 19 * Fix mcisnomer publicKey -> accountId * version bumps, update readme, add changelog, docs * getPrivateNotes changes across all pacakges (0xMiden#10) * switch to yarn workspace structure - uses yarn@4.9.3 - consolidated dev deps - add dry-run option to publish script - upgrade typedoc * Base: Fix CustomTransaction constructor * Switch over to use PrivateDataPermission to control access to private accounts * Bump miden-sdk to 0.11.1 - minor update to AllowedPrivateData enum * update docs * update changelog * fix PrivateDataPermission * Add Ability to Sign Word via Wallet * Revert accidental commit to main * Add Ability to Sign Word in Wallet (actual) (0xMiden#30) * Expose PublicKey in useWallet React Hook (0xMiden#31) * Add ability to request assets from wallet * Bump version to 0.6.0 * Stop using Buffer in transaction constructors * Add typings for private notes * remove webpack from miden-wallet-adapter-miden * update example code in readme * update repository links for all packages * Bump miden-wallet-adapter to 0.6.3 - new patch versions for each individual packages * Add recipientAccountId to CustomTransaction * Create endpoint to import private note into account using Miden Wallet * Modify Sign Message to Sign Bytes and Handle Sign Data Flow * Allow users to specify note filters for requestPrivateNotes endpoint * update docs * Version 0.8.2: getConsumableNotes endpoint * Verion 0.9.0: bump miden-sdk to 0.12.3 * Update React UI components to match Miden color theme * update truncation of Miden address in WalletMultiButton * Version 0.10.0: replace accountId with address in all components * Initial commit * feat: initial integration (0xMiden#1) * chore: Adjust initial setup (0xMiden#2) * Add locks * Prettify * Eslint config * Change tsconfig location * Add format-check to makefile * feat: fix some bugs and add integration docs (0xMiden#3) * feat: fix some bugs and add integration docs * chore: remove console.log and change prettier to use single qoutes * Add react example (0xMiden#4) * fix: do client state sync and change the AccountStorage input, allow for accountSeed to be specified (0xMiden#5) * feat: Add useParaMiden react hook (0xMiden#6) * useMidenPara react hook * Adapt react example to the new hook package * Update readme for react example * Fix build * Remove docs directory and update README (0xMiden#7) * Update README instructions and bump react hook version (0xMiden#8) * Update README * Bump miden para react * feat: Add modal signing confirmation to signing API (0xMiden#9) * Add modal and tests * Wire up the new modal to the signing flow * feat: Move Para and Miden Client to peer/dev dependencies (0xMiden#10) * Move miden and para sdks to peer and dev deps * Add info about peer deps into readme * Adapt hooks README to match * Remove the linking instructions from README * Bump versions * feat: Allow picking storage mode when creating para miden client (0xMiden#11) * chore: Add badges to README for license and CI status (0xMiden#13) Added badges for license, test, and build status to README. * feat: Add account selector UI component to the base integration (0xMiden#15) * Bump versions and yarn locks * fixup! Bump versions and yarn locks * Pass multiple wallets to createParaMidenClient * fixup! Bump versions and yarn locks * Account selector modal * Adapt types * Add account selection modal test * Add create-miden-para-react vite template (0xMiden#16) * Pin version * Fix example not loading * Fix vite config * New agents md * Add create-miden-para-react template * v0.0.2 * Enrich the create template with basic scaffolding code and ensure it runs (0xMiden#17) * Pin version * Fix example not loading * Fix vite config * New agents md * Add create-miden-para-react template * v0.0.2 * Add agents md to the create package * Explain that App.tsx is now bundled * v0.0.3 * Update agents and readme * Ensure no install vite step which overrides file changes * v0.0.4 * Avoid interactive prompts * v0.0.5 * Change the default template * chore: Update integration instructions (0xMiden#18) * v0.0.6 * Bump versions * fix: Make account selector auto resolve with only 1 account (0xMiden#19) * Bump versions * fix: Make account selector auto resolve with only 1 account * Yarn locks * chore: Add tests and bump version (0xMiden#20) * Gracefully handle undefined for the seed string * Add tests * chore: Add tests and bump version * chore: Thin the default App template (0xMiden#21) * chore: Thin the default App template * Bump versions * Yarn lock * chore: Improve create new react example with disconnect para (0xMiden#22) * fix: Ensure public account is hydrated before calling newAccount() (0xMiden#23) * Refactor nodeTransportUrl to noteTransportUrl * Pass options down to client initializer * Throw if storageMode is private but accountSeed is not passed * chore: Bump versions to 0.10.5 (0xMiden#24) * Bump versions to 0.10.5 * Yarn lock * chore: Add docstrings (0xMiden#25) * chore: Simplify core library code in utils.ts (0xMiden#26) * Simplify core library code in utils.ts * Bump versions to 0.10.6 * Yarn lock * fix: Transaction Summary in singing modal and make signing modal optional (0xMiden#27) * feat: tx summary in singing modal * chore: update tests * chore: wrap para signing for times and bump package version * feat: Allow showing custom signing popup through exposing a cb, bump version (0xMiden#28) * feat: Allow showing custom signing popup through exposing a cb * Bump version * fixup! Bump version * Yarn lock * fix: Expose show signing modal flag in use hook (0xMiden#29) * fix: Expose the showSelectionModal in use hook * Bump version * Yarn lock * chore: Add react example (0xMiden#30) * feat: add example for react * chore: format and remove use miden hook * chore: Rewrite AGENTS.md and README.md for the react example (0xMiden#31) * New AGENTS.md * Fix README * Switch to yarn on the react example * chore: Fill missing deps for react example (0xMiden#33) * Fill missing deps for react example * Bump version * Initial commit * feat: initial integration * feat: add react example * feat: restructure as monorepo with @miden-sdk scoped packages (0xMiden#3) * feat: restructure as monorepo with @miden-sdk scoped packages - Migrate from pnpm to yarn 1.22.22 - Add dual CJS/ESM build with esbuild - Create @miden-sdk/miden-turnkey-react package with useTurnkeyMiden hook - Create @miden-sdk/create-miden-turnkey-react CLI scaffolding tool - Update examples/react to use the new packages - Add publishConfig for npm scoped package publishing * docs: use yarn instead of npm in READMEs * refactor: move Miden config from env vars to code defaults * fix: react example and create-miden-turnkey example (0xMiden#4) * Use para's lite react sdk instead of the full react sdk (0xMiden#36) * feat: use para's lite react sdk instead of the full react sdk * feat: update create-miden-para-react and move deps to peerDeps * remove changes * remove @types/node from devDep * fix: add miden-sdk to dev dep * chore: add tests, bump versions (0xMiden#37) * chore: scope packages and refresh docs * fix: align local para sdk types for tsup * test: add react hook and cli scaffolding coverage * test: make create template buildable in e2e * test: add example app e2e and local pack install * chore: drop local miden-para dev dep in react package * ci: run create and example app e2e * ci: handle libasound2t64 on ubuntu * fix: stub optional Para connectors for Vite * fix: remove invalid esbuild external config * test: use yarn for example app e2e install * Add timeouts to example app e2e * Fix example e2e wait for older Puppeteer * Ensure example app e2e shuts down cleanly * feat: Add functionality for waiting of the transaction and bump up sdk versions (0xMiden#62) * feat: add wait for transaction * chore: add changelog * chore: remove unnecessary changes and add .prettierrc * chore: bump up miden-sdk, add miden-sdk to peer deps * fix: correct typos and parameter naming inconsistencies - Fix typo: WalletTransactionSuccessOuput → WalletTransactionSuccessOutput - Fix parameter name: interval → timeout in waitForTransaction - Fix changelog heading: ## Features → ### Features --------- Co-authored-by: Wiktor Starczewski <poszerny@gmail.com> * chore: migrate to 0.13 (0xMiden#39) * chore: migrate from @demox-labs/miden-sdk to @miden-sdk/miden-sdk@^0.13.0 * chore: bump all packages to 0.11.0 * chore: bump all packages to 0.13.0 * chore: regenerate use-miden-para-react lockfile * feat: TurnkeySignerProvider - Unified Signer Interface Integration (0xMiden#5) * feat: add TurnkeySignerProvider with comprehensive tests Implement TurnkeySignerProvider to bridge Turnkey SDK with the unified signer interface from @miden-sdk/react. Implementation: - TurnkeySignerProvider: Wraps children with SignerContext - useTurnkeySigner: Hook for Turnkey-specific extras (client, org, account) - setAccount: Allows apps to set account after Turnkey auth flow - signWithTurnkey: Routes signing to Turnkey's signRawPayload API Test Coverage (12 tests): - Provider renders children - Provides SignerContext to descendants - useTurnkeySigner throws outside provider - Returns client, organizationId, and account - isConnected false initially - setAccount() updates connection state - disconnect() resets state - SignerContext includes correct name ('Turnkey') - signCb routes to Turnkey signing - storeName includes account address - accountConfig has correct publicKeyCommitment - Handles account without public key gracefully Package updates: - Added test script: `yarn test` - Added react-test-renderer devDependency Exports: - TurnkeySignerProvider, TurnkeySignerProviderProps - useTurnkeySigner, TurnkeySignerExtras * refactor: move TurnkeySignerProvider to use-miden-turnkey-react package * refactor: make TurnkeySignerProvider self-sufficient * feat: implement connect() flow and add react-signer example * fix: alias @miden-sdk/react to local miden-client checkout * chore: yarn lock * fix: fix Turnkey passkey login and set up yarn workspaces - Fix SessionType enum: use SessionType.READ_WRITE instead of raw string - Use Turnkey SDK class with passkeyClient + indexedDbClient for proper READ_WRITE session flow (init keypair, get public key, login) - Check for existing session before re-authenticating - Set up yarn workspaces to eliminate recursive file: link duplication (~11GB -> ~1.5GB node_modules) - Remove sub-package yarn.lock files (consolidated into root) - Add publish script to handle private: true workspace root - Migrate @demox-labs/miden-sdk imports to @miden-sdk/miden-sdk in src/ - Fix Vite config: resolve aliases and fs.allow for hoisted node_modules * chore: bump package versions to 1.0.1 * feat: default apiBaseUrl and defaultOrganizationId in TurnkeySignerProvider * feat: migrate @demox-labs/miden-sdk to @miden-sdk/miden-sdk@^0.13.0, fix builds and tests * feat(react): MidenFiSignerProvider - Unified Signer Interface Integration (0xMiden#65) * feat(react): add MidenFiSignerProvider with comprehensive tests Implement MidenFiSignerProvider to bridge MidenFi wallet adapter with the unified signer interface from @miden-sdk/react. Implementation: - MidenFiSignerProvider: Bridges useWallet to SignerContext - Must be used inside WalletProvider - Builds SignerContext from wallet connection state - Routes signing to wallet.signBytes Test Coverage (16 tests): - Provider renders children - SignerContext when connected/disconnected - signCb throws when not connected - isConnected matches wallet.connected - connect() delegates with correct params - disconnect() delegates to wallet - SignerContext includes correct name ('MidenFi') - signCb routes to wallet.signBytes - accountConfig uses wallet publicKey as commitment - accountType is RegularAccountImmutableCode - storeName includes address for isolation - Context updates on wallet state change Package updates: - Added test script: `yarn test` - Added vitest, @testing-library/react, jsdom, react-dom - Added vitest.config.ts Exports: - MidenFiSignerProvider, MidenFiSignerProviderProps * feat: unify MidenFiSignerProvider with WalletProvider - Merge WalletProvider functionality into MidenFiSignerProvider - Default wallets to [MidenWalletAdapter()] when not provided - Default autoConnect to true (reconnect on page reload) - Add appName prop for customizing the default adapter - Export useMidenFiWallet hook for wallet operations - Keep backward compatibility exports for WalletProvider Usage is now simplified to: <MidenFiSignerProvider> <MidenProvider config={{ rpcUrl: 'testnet' }}> <App /> </MidenProvider> </MidenFiSignerProvider> * chore: Migrate to 0.13 client * chore: Migrate peerDependencies to @miden-sdk/miden-sdk@0.13.0 * chore: Rename package to @miden-sdk/miden-wallet-adapter@0.13.0 * docs: update changelog for wiktor-signer branch * chore: Migrate to neww ork, add react signer example * chore: Bump package versions to 0.13.0 * fix: point @miden-sdk/react alias to npm package and exclude tests from tsc build * feat: ParaSignerProvider - Unified Signer Interface Integration (0xMiden#38) * feat: add ParaSignerProvider with comprehensive tests Implement ParaSignerProvider to bridge Para wallet SDK with the unified signer interface from @miden-sdk/react. Implementation: - ParaSignerProvider: Wraps children with SignerContext - useParaSigner: Hook for Para-specific extras (para client, wallet) - Auto-detects EVM wallets and builds signer context on connection - Creates signCb from Para's signing flow with optional modal Test Coverage (8 tests): - Provider renders children - Provides SignerContext to descendants - useParaSigner throws outside provider - Returns para client and wallet - Connection state management (connected/disconnected) - EVM wallet filtering (ignores non-EVM wallets) Exports: - ParaSignerProvider, ParaSignerProviderProps - useParaSigner, ParaSignerExtras * chore: add react/tsx config * feat: add signer example * refactor: make ParaSignerProvider self-sufficient ParaSignerProvider now internally manages QueryClientProvider, ParaProvider, and SignerContext so consumers only need a single provider wrapping MidenProvider. - Wrap ParaProvider and QueryClientProvider internally - Import SignerContext directly from @miden-sdk/react (now required peer dep) - Remove signerContext prop — no longer needed - Add appName, queryClient, and paraProviderConfig props - connect() opens Para modal, disconnect() calls logoutAsync - Re-export useModal/useLogout for convenience - Migrate create-miden-para-react template to signer pattern - Remove use-miden-para-react, wagmi, viem deps from create template - Update all tests and examples * refactor: move ParaSignerProvider from lean package to React package Move ParaSignerProvider, useParaSigner, useModal, useLogout and their types into @miden-sdk/use-miden-para-react so the core @miden-sdk/miden-para stays framework-agnostic and does not pull in react, @getpara/react-sdk-lite, @tanstack/react-query or @miden-sdk/react as peer dependencies. * fix: unmount test renderers to prevent CI hang from leaked setInterval Tests created React renderers with a 2s polling interval but never unmounted them, keeping the Node.js process alive indefinitely. Also added timeout-minutes: 15 to CI workflows as a safety net. * fix: use @miden-sdk/miden-sdk instead of @demox-labs/miden-sdk * refactor: use useClient() instead of duplicate ParaWeb instance * fix: dedupe @tanstack/react-query in Vite config * fix: update test mocks for useClient and @miden-sdk/miden-sdk * fix: use SignerContext from @miden-sdk/react and dedupe shared packages - Import SignerContext from @miden-sdk/react instead of local signer-types.ts so ParaSignerProvider and useSigner() share the same React context - Remove now-unused signer-types.ts - Bump @miden-sdk/react peer/dev dep to ^0.13.1 - Use published @miden-sdk/miden-sdk@0.13.0 and @miden-sdk/react@0.13.1 in the example instead of local file: refs - Dedupe @miden-sdk/miden-sdk and @miden-sdk/react in Vite config to prevent duplicate module instances across file: dependency boundaries * fix: prevent WASM concurrency crashes and stabilize signer context - Use refs for Para client, showSigningModal, and customSignConfirmStep to prevent unnecessary effect re-runs in ParaSignerProvider - Initialize signerContext with a stable disconnected placeholder instead of null to prevent MidenProvider from creating a local-keystore client that races with buildContext's WASM operations - Stabilize wallet state with functional update to avoid identity changes - Use openModal() directly for connect instead of signer.connect() - Add resolve.alias and postinstall to deduplicate @miden-sdk/miden-sdk WASM instances across file: dependency tree - Switch example to devnet and use local file: refs for miden-client * fix: bump dependency versions in create-miden-para-react CLI - @miden-sdk/use-miden-para-react: ^0.10.10 → ^0.13.0 (^0.10.10 only resolves to 0.10.x which lacks ParaSignerProvider) - @miden-sdk/react: ^0.13.0 → ^0.13.1 (0.13.0 lacks SignerContext export needed by use-miden-para-react) * fix: use local use-miden-para-react in CI and dev via MIDEN_PARA_LOCAL_DEPS * fix: add @tanstack/react-query as explicit dependency in scaffolded app * fix: externalize wagmi in template vite config * fix: externalize entire wagmi ecosystem in template vite config * fix: externalize optional packages via resolve plugin to handle subpath imports * fix: add @miden-sdk/react dep to example app and clean up template vite config * bump package versions to 0.13.1 * fix: replace local file: aliases with @miden-sdk/miden-sdk@^0.13.0 and @miden-sdk/react@^0.13.2 * Use npm publish for browser auth flow and regenerate docs (0xMiden#66) * chore: Update READMEs to @miden-sdk org and bump versions to 0.13.1 (0xMiden#67) * test: Add comprehensive test suite and CI workflow (0xMiden#68) Add vitest tests for packages/core/base (helpers, transactions, errors, adapter) and packages/wallets/miden (MidenWalletAdapter). Wire up CI with GitHub Actions to run tests on every push to main and on PRs. * Fix: Update args for `createClientWithExternalKeystore` for v13 (0xMiden#41) * fix: update args to for `createClientWithExternalKeystore` for v13 * chore: update deps * chore: update lockfile * chore: bump version to 0.13.2 * fix: clean up stale comment and version inconsistencies - Remove outdated comment about missing SDK typings - Use caret range for @getpara/web-sdk in use-miden-para-react - Bump @miden-sdk/miden-para peer dep to ^0.13.2 * chore: update use-miden-para-react lockfile --------- Co-authored-by: Wiktor Starczewski <poszerny@gmail.com> * feat: support customComponents prop in ParaSignerProvider (0xMiden#40) * feat: support customComponents prop in ParaSignerProvider * chore: bump all packages to 0.13.3 and require @miden-sdk/react ^0.13.3 * feat: support customComponents prop in TurnkeySignerProvider (0xMiden#6) * feat: support customComponents prop in TurnkeySignerProvider * chore: bump packages to 1.0.2, update miden-sdk to ^0.13.1 and react to ^0.13.3 * feat(react): Support custom account components in MidenFiSignerProvider (0xMiden#69) * feat(react): add accountType, storageMode, and customComponents props to MidenFiSignerProvider Allow dApps to configure account type, storage mode, and pass custom AccountComponents (e.g. from compiled .masp packages) when creating signer accounts. Previously these were hardcoded to RegularAccountImmutableCode and public storage mode. Companion PR needed in miden-client to handle customComponents in initializeSignerAccount. * feat: add createAccount method to wallet adapter Thread createAccount through the adapter stack so dApps can request custom account creation via the wallet extension: - base/types: CreateAccountParams, CreateAccountType, CreateAccountStorageMode - base/signer: interface + abstract class - wallets/miden/adapter: MidenWallet interface + MidenWalletAdapter impl - react/MidenFiSignerProvider: wired into WalletContextState + useMemo - tests: adapter (not-connected, connected, customComponents) + react * chore: bump versions to 0.13.2 and update miden-sdk to ^0.13.1, react-sdk to ^0.13.3 * docs: add MidenFiSignerProvider usage guide with customComponents, accountType, and storageMode * chore: update yarn.lock * chore: bump all package versions to 0.13.3 (0xMiden#71) * chore: bump all package versions to 0.13.3 (0xMiden#70) * fix: resolve workspace:^ deps before npm publish, bump to 0.13.4 (0xMiden#72) The publish script was publishing packages with "workspace:^" in their dependency fields, which breaks npm install for consumers. The script now resolves workspace:^ to real version ranges (e.g. ^0.13.4) before publishing and restores the original package.json after. Also reverts accidental workspace:^ on @miden-sdk/react (external dep). * fix: Do not autoconnect as default, bump to 0.13.5 (0xMiden#73) * fix: Do not autoconnect as default * chore: bump all package versions to 0.13.5 * fix(react): update autoConnect tests to match new default (false) * Improve DX: bundling CSS, adding a Vite plugin, fixing login state management (0xMiden#42) * fix: auto-import Para modal styles in ParaSignerProvider Import @getpara/react-sdk-lite/styles.css directly in ParaSignerProvider so consumers don't need a manual CSS import. Mark CSS files as sideEffects so bundlers preserve the import. * chore: bump versions to 0.13.4 * fix: use reactive useAccount() hook instead of polling isFullyLoggedIn() Replace manual polling with useAccount() from @getpara/react-sdk-lite which subscribes to the Para SDK's internal state machine. Also close the modal automatically when login is detected. * feat: add paraVitePlugin for Vite config automation Exported at @miden-sdk/use-miden-para-react/vite. Handles node polyfills and Solana/Cosmos connector stubs so apps don't need manual Vite configuration for Para integration. * fix: use esbuild plugin for connector stubs, resolve polyfills from project root data:text/javascript aliases don't work for dynamic imports in pre-bundled deps. Use esbuild onResolve/onLoad + Vite resolveId/load hooks instead. Also use createRequire from project cwd so vite-plugin-node-polyfills resolves from the consuming project. * fix: use configResolved for esbuild stub plugin injection Same issue as midenVitePlugin - config() esbuild plugins get overwritten by vite-plugin-node-polyfills. Move to configResolved. * fix: address security review findings and CI failures - Use full regex escaping in paraVitePlugin connector stub filter - Add permissions block to build workflow (contents: read) - Commit updated yarn.lock to fix --frozen-lockfile CI failure - Add package-lock.json to .gitignore (repo uses yarn) * fix: add useAccount and closeModal to test mocks The reactive useAccount() hook and closeModal were added to ParaSignerProvider but the test mock was missing them. * chore: bump package versions and fix bundled dependency (0xMiden#7) * fix: Dont prebundle miden-sdk/react * chore: bump all package versions by 0.0.1 * fix: remove as any cast and add storeName param to createClientWithExternalKeystore * chore: bump package versions (0xMiden#8) * fix: Add miden sdk to deps * chore: bump package versions * fix(ui): Chrome Web Store install badge and modal polish (0xMiden#74) * fix(ui): fix Discover Miden modal styling and broken Chrome badge - Add border:none/background:none to close button (removed browser default border) - Reduce box-shadow opacity from 0.6 to 0.15 (was too harsh) - Add overflow:hidden to modal wrapper (fix gray clipping artifact at bottom) - Adjust padding/margins for better badge spacing - Replace broken Chrome Web Store badge URL (old Google Storage URL returns 403) - Update wallet adapter URL to Chrome Web Store listing - Add preview-modal.html for standalone testing - Add UI package aliases to react-signer vite config * chore: bump all package versions to 0.13.6 * feat: add discover-miden example app for testing wallet modal * chore: remove preview-modal.html * chore: migrate to 0.14.0 (0xMiden#75) * chore: migrate to 0.14.0 * fix(deps): update yarn.lock for 0.14.0 migration * chore: migrate to 0.14.0 (0xMiden#43) * chore: migrate to 0.14.0 - Bump all @miden-sdk/* dependencies to ^0.14.0 - Replace WebClient with MidenClient throughout - createParaMidenClient() now returns MidenClient (breaking) - Migrate examples to MidenClient resource API - Add importAccountId prop to ParaSignerProvider - Remove NoteType.Encrypted (removed upstream) * refactor: replace inline type imports with top-level import type * chore: bump create-miden-para-react deps to 0.14.0 * refactor: use object-based params for accounts.insert * fix(ci): sync yarn.lock with @miden-sdk/miden-sdk@0.14.0 bump * fix(ci): sync use-miden-para-react yarn.lock with 0.14.0 deps * fix(create): pin rollup and esbuild so vite-plugin-top-level-await loads under vite 8 * fix(create): externalize @getpara/aa-* packages in scaffold vite config * chore: add publish orchestrator for miden-para, use-miden-para-react, create-miden-para-react * fix: cast SignerContext to host React typings in ParaSignerProvider (0xMiden#44) Avoids TS errors during dts build when @miden-sdk/react ships with a different @types/react version than the consumer (e.g. yarn link setups where the linked package has its own node_modules). The cast is runtime-safe — same Context object, just reconciled against the host app's React typings. * chore: migrate to 0.14.0 (0xMiden#9) * chore: migrate to 0.14.0 * chore: bump turnkey package versions to 1.14.0 * refactor: use object-based params for accounts.insert * chore: add publish orchestrator for miden-turnkey, miden-turnkey-react, create-miden-turnkey-react * chore: migrate to yarn 4 workspace (0xMiden#45) * chore: migrate to yarn 4 workspace Yarn 1 was injecting npm_config_registry=https://registry.yarnpkg.com into child processes, which broke `yarn publish` because ~/.npmrc only has an auth token for registry.npmjs.org. Yarn 4 does not override the registry env var, so publish now authenticates correctly against npmjs.org. - Vendor yarn-4.9.3 binary under .yarn/releases and point .yarnrc.yml at it - Convert root package.json into a workspace (packages/*) and bump packageManager to yarn@4.9.3 - Collapse the three per-package yarn 1 lockfiles into a single yarn 4 lockfile at the repo root - scripts/publish.js: run `yarn install --immutable` once at the repo root instead of `yarn install --frozen-lockfile` per package - .gitignore: standard yarn 4 rules (keep .yarn/releases, drop the rest) - packages/create-miden-para-react/package.json: bin field was normalized from an object to its string shorthand by `yarn install` — semantically identical, kept as-is to avoid churn on the next install * ci: update workflows for yarn 4 workspace - Add `corepack enable` before yarn install so the packageManager field is honored on the runner - Replace `yarn install --frozen-lockfile` with `yarn install --immutable` (yarn 4 syntax) - Drop the per-package install step for packages/use-miden-para-react; the root workspace install covers it - Replace `yarn --cwd packages/use-miden-para-react build` with the workspace-aware `yarn workspace @miden-sdk/use-miden-para-react build` * chore: migrate to yarn 4 workspace layout mirroring miden-para (0xMiden#10) - Vendored yarn 4.9.3 via .yarn/releases + .yarnrc.yml - Root @miden-sdk/miden-turnkey is no longer private; workspaces trimmed to packages/* so the examples don't block publish installs - Dropped link:../../ self-dep from use-miden-turnkey-react; tsconfig paths now resolves @miden-sdk/miden-turnkey to ../../src/index.ts for tsup's dts build - examples/react-signer pinned to file:../.. + the react package (matches miden-para/examples/react-signer); examples/react keeps semver refs - scripts/publish.js rewritten: yarn install --immutable once at the root, simpler stripOrchestratorScripts (no link/private/workspaces rewriting), drop --skip-install - scripts/build.mjs and scripts/postpack.mjs replaced with the miden-para versions (dual ESM/CJS browser build with sideEffects, deterministic tarball naming) - Fresh yarn 4 lockfile * feat: add Devnet to WalletAdapterNetwork, bump to 0.14.1 (0xMiden#79) Closes 0xMiden#78. Adds the missing Devnet variant to WalletAdapterNetwork so dApps targeting devnet no longer need to pass Testnet while configuring MidenProvider with rpcUrl: 'devnet'. Bumps all packages 0.14.0 -> 0.14.1. * chore: bump miden-sdk to 0.14.4, version to 1.14.1 (0xMiden#11) * chore: bump miden-sdk and react sdk to 0.14.4, version to 1.14.1 * ci: add build workflow and refresh yarn.lock * chore: bump miden-sdk to 0.14.4, version to 0.14.1 (0xMiden#46) * chore: bump miden-sdk and react sdk to 0.14.4, version to 0.14.1 * chore: refresh yarn.lock for 0.14.4 * chore: bump miden-sdk to 0.14.4, version to 0.14.2 (0xMiden#80) * chore: bump miden-sdk and react sdk to 0.14.4, version to 0.14.2 * chore: refresh yarn.lock for 0.14.4 * fix(react): always import the wallet's existing account, bump to 0.14.3 (0xMiden#81) MidenFiSignerProvider was handing an optional `importAccountId` to the signer context's accountConfig — undefined by default. When unset, @miden-sdk/react's initializeSignerAccount takes the "rebuild account from scratch" branch, which (a) assumes the dApp knows every creation parameter the wallet used and (b) hits a broken `AuthScheme.AuthEcdsaK256Keccak` lookup on the public AuthScheme const in @miden-sdk/react <= 0.14.4, throwing "invalid enum value passed". Always pass the wallet's bech32 address as importAccountId so the React SDK takes the import-by-id branch, which doesn't touch AuthScheme at all. This is also the semantically correct behavior — the wallet owns the account, and its address is the on-chain account ID. Callers can still override via the `importAccountId` prop. Bumps all workspace packages from 0.14.2 to 0.14.3. * fix(miden): dispatch requestTransaction by type so Consume/Send work (0xMiden#91) `MidenWalletAdapter.requestTransaction` forwarded the `{type, payload}` wrapper straight to the wallet's generalized `requestTransaction` endpoint, which only accepts a custom-transaction payload. Consuming a note via the typed API — `requestTransaction(new Transaction(TransactionType.Consume, new ConsumeTransaction(...)))` — therefore failed with `WalletTransactionError: INVALID_PARAMS: Invalid CustomTransaction payload`, even though the dedicated `requestConsume` path works. Dispatch by `transaction.type` instead: route `Send` to the wallet's `requestSend` and `Consume` to `requestConsume` (both proven endpoints), while `Custom` and bare/legacy payloads keep using the generalized `requestTransaction` endpoint. This makes the typed `Transaction` / `TransactionType` API (and `Transaction.createConsumeTransaction` / `createSendTransaction`) work end-to-end. Tests cover all four routes (consume, send, custom, bare payload). Fixes 0xMiden#88. * chore: upgrade to miden SDK 0.15 (0xMiden#47) * chore: upgrade to miden SDK 0.15 (0xMiden#12) * chore: upgrade to miden SDK 0.15 (0xMiden#92) * chore: bump miden SDK to 0.15.1 (0xMiden#48) * chore: bump miden SDK to 0.15.1 (0xMiden#13) * chore: bump miden SDK to 0.15.1 (0xMiden#93) * feat(guardian): expose requestGuardianInfo on the wallet adapter (0xMiden#96) * feat(guardian): GuardianInfo type + requestGuardianInfo on base signer props * feat(guardian): concrete adapter requestGuardianInfo * feat(guardian): expose requestGuardianInfo on useWallet + useMidenFiWallet * docs(guardian): changelog + clearer test name for requestGuardianInfo * chore(release): bump all packages to 0.15.2 (0xMiden#97) * feat: rebrand to Bread Wallet and bump packages to 0.15.3 (0xMiden#98) Rename the adapter's display name from "Miden Wallet" to "Bread Wallet", update the download link to the new Chrome Web Store listing, and swap in the Bread brand icon. WalletModal now derives its recommended wallet from the exported MidenWalletName constant instead of a hardcoded string, so the name and the modal lookup can no longer drift. Bump all packages 0.15.2 -> 0.15.3. * fix(#470): re-drive connect() when already connected so dApp reconnect repopulates the account (0xMiden#108) The in-app wallet browser preserves a dApp's JS context across a park/restore, so the MidenWalletAdapter (and the React providers) survive with connected===true. connect() early-returned when already connected, so a second "Connect Wallet" was a silent no-op and the account was never repopulated — "Connect Wallet works only once" (0xMiden/wallet#470). Drop the `connected` check from the connect guards at all consumer layers so a repeat connect() re-drives the handshake (cheap: the wallet returns the current account for an existing permission without re-prompting) and re-emits `connect`. Concurrency stays guarded by the adapter's in-flight check and the providers' isConnecting ref. - packages/wallets/miden/adapter.ts: guard on `connecting` only. - MidenFiSignerProvider/WalletProvider: same; a failed re-drive keeps a still-live session (setName(null) only when actually disconnected); WalletProvider.handleConnect made idempotent to avoid redundant re-renders. - Tests: adapter re-drive + concurrent-guard; provider + hook re-drive. * fix: unify WalletContext so useWallet works under MidenFiSignerProvider (0xMiden/wallet#177) (0xMiden#99) * fix(0xMiden#314): reject requestSend/Consume/Transaction when the wallet returns no transaction id (0xMiden#109) The wallet reports "success" as soon as a transaction is ACCEPTED into its queue, not when it lands on chain, so a resolved call with no `transactionId` means it was never submitted. The adapter's `return result.transactionId!` turned that into a silently-resolved `undefined` the dApp reads as success — a custom tx with an output note "succeeds" yet never reaches the chain. Guard all three request paths with `requireTransactionId`: reject with WalletTransactionError (emitting `error`) when the wallet returns no id, so the dApp can surface the failure and poll `waitForTransaction(txId)` for on-chain confirmation. Restructured so the guard runs outside the wallet-call try/catch (no double-wrap). * fix: commit ECDSA public keys with Poseidon2, not Rpo256 (0xMiden#22) * fix: commit ECDSA public keys with Poseidon2, not Rpo256 * test: pin the ECDSA commitment preimage and hash family * fix: make yarn test actually discover the test files * chore: correct package metadata for the web-sdk move (0xMiden#23) * chore: correct package metadata and declare the miden-sdk peer dependency (0xMiden#113) * fix: unbreak the scaffolded template build (0xMiden#53) * fix: commit ECDSA public keys with Poseidon2, not Rpo256 (0xMiden#51) * fix: commit ECDSA public keys with Poseidon2, not Rpo256 * test: pin the ECDSA commitment preimage and hash family * fix: make yarn test actually discover the test files * chore: correct package metadata for the web-sdk move (0xMiden#52) * feat: adopt the adapter, para and turnkey packages on the 0.16 line --------- Co-authored-by: Chris Womack <womackchrisw@gmail.com> Co-authored-by: Kevin Wang <krwang4094@gmail.com> Co-authored-by: Dennis Garcia <dennis.garcia97@gmail.com> Co-authored-by: Dennis Garcia <dennis@demoxlabs.xyz> Co-authored-by: Utkarsh Sharma <114555115+0xnullifier@users.noreply.github.com> Co-authored-by: 0xnullifier <utkarsh382004@gmail.com>
…unction (0xMiden#333) * fix(web-client): let newLocalProver track the client's default hash function `newLocalProver()` built `LocalTransactionProver::new(ProvingOptions::default())`. That names the prover crate's default, Blake3 — not the client's, which miden-tx 0.16 set to Poseidon2 so transaction proofs are recursion-ready. The two agreed on 0.15, where LocalTransactionProver derived Default, and stopped agreeing here. On the same SDK: proveTransaction(result, TransactionProver.newLocalProver()) -> Blake3 proveTransaction(result, undefined) -> Poseidon2 and nothing in the JS surface exposes HashFunction, so a caller could neither see nor override which one they got. Nothing rejects a Blake3 proof today — the verifier reads the hash tag out of the proof and dispatches, and the 0.16 batch kernel is still a skeleton. The cost is forward: the recursive verifier accepts only Poseidon2, so a Blake3 proof becomes unbatchable as soon as the batch kernel verifies proofs in-VM, and a client shipping one breaks on a network upgrade it did not participate in. Delegating to LocalTransactionProver::default() leaves one source of truth, so the two cannot drift apart again. mobile-prover had the same pin and moves with it, so the native prover keeps producing the same kind of proof as the WASM one. Verified on the built MT dist by reading the tag out of a real ProvenTransaction: before 0x01 Blake3_256, after 0x04 Poseidon2. Local proving gets slower — an arithmetization-friendly hash costs roughly 1.6-2.2x on published Miden VM figures, and more with fewer cores. * docs(web-client): state the real reason the client default moved to Poseidon2 The previous commit said miden-tx 0.16 chose Poseidon2 "so transaction proofs are recursion-ready, because the recursive verifier accepts only Poseidon2". The second half is true and the first half is not the reason. miden-base b5a5ea25f (PR #3152, "Use Poseidon2 hash function instead of BLAKE3 during the benchmark proving phase") changed the default as review feedback on a BENCHMARK change: "instead of a Poseidon2-specific convenience constructor, change the prover default itself". Recursion is not mentioned in the commit, the PR, or issue #3129. The stated motive is that benchmarks should prove with the protocol's native hash. The recursion constraint is real — miden-verifier's recursive path rejects anything but Poseidon2 — but it is a downstream consequence, not the motive, and presenting it as the motive made the change sound like a coordinated migration it was not. The batch prover is evidence of that: it still takes ProvingOptions::default() = Blake3, with no setter, so on 0.16 transaction proofs are Poseidon2 while batch proofs are not. No behaviour change; comments and the changelog entry only. Also corrects the cost figure, which PR #3152's own Criterion run puts at +157% on single-p2id-note rather than the 1.6-2.2x quoted from the VM README. --------- Co-authored-by: igamigo <ignacio.amigo@lambdaclass.com>
* ci: report WASM proving performance vs the PR base branch (#328)
* ci: report WASM proving performance vs the PR base branch
* fix(bench-bot): round 1 — the artifact must not author the verdict
The reporting half of the bot renders a fork-controlled artifact while holding
a write token, so anything in that JSON which changes what the comment claims
is an integrity hole, not data.
F-010 P1 thresholdPct/thresholdProvisional/lowerIsBetter/calibration were read
from the artifact; all four are constants on the producing side, and
a fork could silence a regression, invert it to "faster", or bolt the
"this is all noise" banner onto a 4x slowdown. Pinned on the trusted
side instead.
F-011 P1 GitHub autolinks, emoji shortcodes and character references survived
sanitization, so a name could inject links, images, authority glyphs,
and issue cross-references that notify a target issue's subscribers.
Names now render inside a code span, where none of it applies.
F-012 P1 the control-character strip missed the C1 block and every format
character, letting bidi overrides reverse a rendered row.
F-013 P1 the raw JSON parse error reached stderr, and V8 quotes the offending
bytes verbatim, so a crafted artifact could emit a workflow command
in the privileged job's log.
F-014 P1 the size hard-cut sliced the joined body, landing inside a table row
or an open <details> — which collapses the rest of the comment.
F-015 P1 name truncation split surrogate pairs, returning ill-formed UTF-16.
F-002 P1 a head-only run (base build failed) emits base:null, which the
renderer refused, so the documented fallback never rendered.
F-003 P1 a calibration run wrote prNumber:0, which the renderer rejected, so
the job summary — a calibration run's only output — always failed.
F-016 P1 "stop if no report" tested artifact presence, but the artifact is
uploaded unconditionally, so a bench job that died early turned the
informational bot red.
F-017 P1 the verdict headline named the biggest mover while the emoji tracked
any regression, so a regression could headline as an improvement.
F-018 P1 the concurrency group collapsed every workflow_dispatch onto one key,
so each calibration dispatch cancelled the previous one.
F-019 P1 the header claimed setup runs once per rep rather than once per side;
base and head draw independent grind lotteries, so averaging shrinks
that term rather than cancelling it.
F-020 P1 calibration defaulted to 8 reps and PRs to 5, so the noise floor was
measured at one sample size and applied at another.
F-021 P1 5% was documented as 3 sigma of 1.79%, which is 5.37%.
Also: validate the dispatch run id and the artifact baseSha without grep's
per-line matching, apply the run cross-check on the dispatch path too, use the
job summary's real size budget, and correct the claim that a fork's job summary
lives on the fork's own run page.
Tests go from 12 to 27, covering each of the above.
* fix(rev): round 2 — artifact extraction escaped the trusted checkout
F-035 P0 the fork-controlled zip was extracted into $GITHUB_WORKSPACE, and
download-artifact bundles an unzip-stream whose path guard is
^-anchored and non-global: a mid-path `..` escaped `report/` and could
overwrite the renderer the next step runs with a write token. Extract
to RUNNER_TEMP, refuse unexpected entry names, and assert the checkout
did not move.
F-036 P1 the verdict was still artifact-authored by arithmetic — pinning the
threshold and direction left `value`/`min`/`median`/`max` trusted, so
samples showing a 10% regression could be reported as 8% faster. Every
statistic is now recomputed from `samples`, which becomes mandatory
and shape-checked; schemaVersion 2 on both sides.
F-037 P1 bound artifact size and total sample count; a 200 KB zip inflated to
200 MB of JSON and OOMed the privileged reporter.
F-038 P1 a head-sha mismatch called setFailed, so an ordinary push during a
bench run turned the default-branch reporter red. Annotate and decline.
F-039 P1 a malformed results.json failed the reporter; render failures now post
nothing instead.
F-040 P1 schemaVersion did not move when the field meanings did.
F-041 P2 strip surrogates and default-ignorable code points; a JSON lone
surrogate made the body ill-formed UTF-16.
F-042 P2 update the check run in place instead of stacking duplicates.
F-043 P2 key the base-dist cache on the toolchain and build definition, not
just the base sha.
F-044 P2 the bench job's summary step could fail after a successful benchmark,
and appended a half-written body to the step summary.
F-045 P3 reject dot-segment slugs; drop the unjustifiable bias figure from the
ABBA warning; stop claiming interleaved sides on a head-only run.
* fix(rev): round 3 — confine the artifact instead of inspecting it
F-046 P0 round 2's extraction fix did not confine anything. $RUNNER_TEMP is
/home/runner/work/_temp, so a crafted entry still reached
$GITHUB_EVENT_PATH and the staged code under work/_actions that the
write-token steps execute, and both new guards passed because the
escaped file is not in the directory they inspect. Worse, `git status
--porcelain` executes core.fsmonitor, making the detection step itself
the sink. Replaced download-artifact with an API fetch that refuses by
size before downloading, plus `unzip -o -j` against an explicit member
list: no entry name reaches a path decision, so there is nothing left
to detect. Both guards deleted.
F-047 P1 the verdict rested on the threshold alone, discarding the per-rep
spread that produced the aggregate. A movement is now significant only
when it also clears a paired sign test across repetitions; one that
clears the floor while its repetitions disagree is reported unresolved.
F-048 P1 two PRs from one fork branch cancelled each other's benchmark.
F-049 P1 the round-2 find guard let any fork redden the trusted workflow by
adding a fourth file to the artifact.
F-050 P1 the two API steps still threw on a race the earlier steps were made to
tolerate — a PR locked or a head commit GC'd during a 90-minute bench.
F-051 P1 the base worktree, a second checkout with its own node_modules and
release target/, was never reclaimed before the head build.
F-052 P1 the size cap ran after extraction, so it bounded node's heap and not
the disk.
F-053 P2 bound the artifact-authored run counts, and stop asserting a σ measured
at 6x3 next to a run that reported one sample.
F-054 P2 calibration.md claimed a known-σ tail for an estimated σ, and called a
geometric tail heavy.
F-055 P2 skip the dual-build job for changes that cannot reach the WASM path.
F-056 P2 the extraction logic is now a tested script; the traversal that
defeated round 2 is a test case.
F-057 P3 cap stacked combining marks; strip private-use and unassigned.
F-058 P3 a stray separator in the cache key, and an assertion that could not fail.
* fix(rev): round 3 (bugbot) — the extractor still restored symlinks
F-059 P1 `unzip -j` confines a traversal but restores symlinks, so an entry
named results.json carrying the symlink mode bit became a link to any
absolute path — followed by the size check, the `[ -f ]` test and the
renderer, whose output is a public PR comment. Switched to `unzip -p`
into a redirect this script owns: nothing in the archive chooses a
path, a mode or a link target. Duplicate member names are refused
rather than concatenated.
F-060 P1 the byte cap ran after inflation, so a 2 MiB zip could write gigabytes
before being measured. `head -c` now bounds the stream itself.
* fix(bench): round 4 — non-finite and negative measurements, silent failure paths
F-061 P1 recomputed mean overflowed to Infinity and rendered "∞" in head-only runs
F-062 P1 negative sample timings inverted the verdict sign via a negative baseline
F-063 P1 renderer bugs were reported as hostile artifacts and silenced
F-064 P1 a failed member write could leave truncated JSON marked usable
F-065 P1 createClient retried with Worker restored, measuring one side under
rayon contention the other did not have
F-066 P1 teardown rejections let a run exit 0 with resources still open, and let
later repetitions run against a page that failed to close
F-067 P1 every refusal was PR-invisible; a neutral check run now says which
F-068 P1 documented that workflow_run always judges a PR with the default
branch's renderer, whatever the PR targets
F-069 P2 exact-match schemaVersion had no safe rollout order
F-070 P2 reporter concurrency cancelled across two PRs sharing a fork branch
F-071 P2 every non-finite diagnostic printed "got null"
F-072 P2 unvalidated byte cap aborted with a shell trace; ::error went to stdout
F-073 P2 methodology called the spread unknown while gating on that same spread
F-074 P2 comments still described unzip -j and implied a cross-PR dist cache
F-075 P3 usage omitted --calibrate
* fix(rev): round 5 — tests that could not fail, and advice that backfired
F-077 P1 the unresolved note advised raising --reps, which makes the unresolved
verdict strictly more likely: unanimity is Φ(δ/s)^reps
F-078 P1 the stream-cap test measured the disk after the script deleted the
file, so removing the cap kept it green at a 449 MB peak
F-079 P1 ctx.json is trusted, but its rejections exited 1 and were blamed on
the fork; any context failure is now exit 3
F-080 P1 the exit-code contract was untested for 0 and 3, so swapping refusal
and internal error in either direction kept the suite green
F-081 P1 sanitization was tested on `name` only, and the name-truncation
fixture was exactly at the cap so it never truncated
F-082 P2 zero-delta repetitions could manufacture a consistent verdict
F-083 P2 a 24-rep run inherited a σ measured at 6 reps; single-prove runs
claimed a minimum that does not exist
F-084 P2 three files disagreed on whether averaging cancels or shrinks the
grind, and the spread was attributed to interference alone
F-085 P2 the threshold compared unrounded magnitudes against a rounded display
F-086 P2 the discard policy was asserted in prose but never checked against the
executed counts already in the artifact
F-087 P2 a failed comment left a green check; bails now record why
F-088 P2 three extractor branches had no coverage; missing members unnamed
F-089 P2 two degradation rungs were never executed by any fixture
F-090 P2 five tests passed for the wrong reason
F-091 P2 comments described mechanisms the code no longer uses, including two
written in round 4
F-092 P2 the calibration doc quantified false positives only; the unanimity
leg's power cost and the ABBA parity are now documented
F-093 P2 the reporter's trigger names bench.yml by display name, untested
F-094 P2 a long message consumed the whole log budget, leaving no stack
F-095 P3 three test names promised assertions the tests did not make
* fix(rev): round 6 — red team on failure modes and the estimator's blind spot
F-096 P1 a failed comment overwrote the check run holding the report
F-097 P1 branch-keyed reporter concurrency silently dropped queued reports
F-098 P1 re-run failed jobs 409'd on upload, serving attempt 1's numbers
F-099 P1 a slowdown missing each rep's fastest prove reported +0.00%
F-100 P1 an overrunning base build took the head build down with it
F-101 P2 dist cache key ignored the wasm-opt version that rewrites the bytes
F-102 P2 the one provably first-party failure had no specific reason
F-103 P2 head-only warning claimed a failed build where none was attempted
F-104 P2 methodology claimed both sides were built in the job; cache says else
F-105 P2 ABBA imbalance warning named no side, so it was unactionable
F-106 P2 thread-pool check asserted dispatch; it only asserts pool size
F-107 P2 the estimator's discarded regressions were undocumented
F-108 P3 unquoted $rc (SC2086)
* fix(rev): round 7 — regression re-read of every prior fix, plus trust-boundary and power corrections
F-124 P1 pr.json of `null` crashed the trusted workflow instead of refusing
F-125 P1 a zero base figure claimed "no base measurements" over a table of them
F-126 P1 the sign test passed unconditionally below four artifact-chosen reps
F-127 P1 round 6's mean cross-check borrowed a sigma from another statistic
F-128 P1 page.evaluate had no timeout, so a wedged prove burned the job
F-129 P1 the refusal exit code collided with node's own, hiding our bugs
F-130 P1 a slow reporter could overwrite a newer push's report
F-131 P1 the base commit link was fork-chosen with only a syntax check
F-132 P2 the last rep's teardown check discarded a complete measurement set
F-133 P2 the static server leaked a descriptor on every aborted request
F-134 P2 setup order was fixed base-first, the one leg that was not ABBA'd
F-135 P2 producer accepted counts the renderer refuses, wasting a whole job
F-136 P2 step timeouts did not compose inside the job budget
F-137 P2 an incomplete base dist destroyed the run instead of degrading
F-138 P2 three assertions could not fail
F-139 P2 the last truncation rung was reachable, not unreachable as claimed
F-140 P2 the documented power figures were wrong; replaced with measured ones
F-141 P2 the mean note explained an improvement as a slowdown
F-142 P2 both copies of the verdict rule described the pre-majority version
F-143 P2 a dying rayon worker was invisible while the pool shrank
F-144 P2 CI duplicated the test target; knip entries were no-ops
F-145 P3 the checkout comment claimed a property false on the dispatch path
F-146 P3 assorted stale wording
* fix(rev): round 8 — trust-boundary crashes, contradictory verdicts, unusable power model
F-108 P1 ToPrimitive on artifact fields reddened the trusted job on demand: a
fork's pr.json can carry an object whose own toString is a string, so
Number()/String() threw outside every try/catch. Type-check first.
F-109 P1 worker-close guard fired on healthy teardowns (4 false alarms per run,
confirmed against real Chromium); latch the intent, not page.isClosed()
F-110 P1 a report mixing head-only rows with zero-base rows claimed "every
benchmark's base figure is zero", which the table below contradicted
F-111 P1 pulls.get in the pre-write head re-check was unguarded: an API outage
failed the default-branch job. Decline and report instead of posting.
F-112 P1 --reps 1000 --proves 1000 cleared both flag caps and then emitted ~2M
samples against a 200k renderer cap — a full job for a refused report
F-113 P1 verdict-power.mjs drew 4.8M numbers from an LCG with a measured period
of 10,466, so every published rate was a cycle artifact. mulberry32,
plus a tail check against the analytic normal so it cannot recur.
F-114 P1 the power model scaled per-rep sigma up as sqrt(reps), holding the
aggregate's spread constant — i.e. modelling averaging as useless. It
published "a true 8% regression is called significant 0.8% of the time
at 24 reps"; the real figure is 43%, and silence falls 7.3% -> 0.2%.
F-116 P1 heading read "No significant change (largest -0.4%)" directly above a
note reporting a +8% mean slowdown — opposite sign, 20x the magnitude
F-115 P2 legend glossed the ❔ emoji as one of the four states it marks, so it
contradicted the heading above it on the other three
* bench: keep a rendered report from vanishing, and make the timeout caps fit
Round 9 of review. The reporter had three ways to lose a complete measurement
with the job green and nothing on the pull request:
- The stale-head stand-down set no reason on the theory that announcing it would
overwrite the newer reporter's check. It cannot: the fallback writes to
workflow_run.head_sha and the newer reporter writes to the new head, so they
land on different commits. The premise that a newer reporter is always coming
is also false when the push that moved the head matched bench.yml's
paths-ignore, or when the newer bench run fails.
- The fallback downgraded an existing rendered report to "no comment posted".
Re-running a bench job replaces the artifact via overwrite: true, so a second
attempt that died before measuring found the first attempt's report and
overwrote it while the sticky comment still showed the numbers. Real reports
now carry an external_id the fallback refuses to touch.
- When a rate limit took out both the comment and the check run, both swallowed
the failure and the fallback's condition could not see either. The check-run
step now records whether it published, and the fallback fires on that.
The step timeouts did not compose: 30 + 30 + 20 against a 90-minute job left ten
minutes for a preamble that cannot fit in it, so the designed worst case died
inside the benchmark and produced no report. Budget raised to 120 with the
accounting written down, and the Chromium download capped.
The producer's in-page deadlines could not fire late in a run — a setup wedging
at minute twelve would trip its ten-minute deadline eight minutes after the
runner killed the step. They are now clamped to the remaining budget, which the
workflow passes in, so the diagnostic always precedes the kill.
Also: teardown failures reach the artifact and the comment banner instead of
leaving a red job beside a normal-looking report; a worker death recorded after a
repetition's last prove is read before the samples are retained; the odd-reps
setup-order imbalance warns (--reps 5 --proves 3 was silent); a stale base dist
is cleared before the mv that would otherwise nest inside it, and the gate checks
the wasm payload as well as the entry point; the power simulator's generator
check now covers serial independence, which a duplicate-draw stream defeated; and
the doc's tables come from one memoised draw per quantity instead of publishing
one number three ways.
* bench: name the real cause when a PR closes mid-run, and honour two stated guards
Trust-boundary audit of round 9 found no way for PR-authored code or data to
reach the privileged reporter; these are the three smaller gaps it did find.
A pull request closed inside the bench window returned from the identity step
without recording why, so the fallback inferred the cause from an unset output
and published "the pull request the report claimed could not be confirmed" — a
forgery-flavoured message for a PR whose identity had verified fine. It now
records the reason, read ahead of the inference.
Two guards did not do what their comments claimed. RUN_URL_RE was documented as
tightened against `..` while still accepting
https://github.com/../../actions/runs/N, which GitHub normalizes into a link to
a different repo; it now shares its segment pattern with SLUG_RE so the two
cannot drift again. And getWorkflowRun was the one unguarded await in a step
whose contract is to decline rather than fail, so a dispatch against a retired
run id reddened a default-branch run.
The traversal test fails against the old regex and passes against the new one.
* bench: the ABBA interleave cancelled itself in the calibrated configuration
The prove-level flip was keyed on `(i + rep) % 2` while the open order was
already keyed on `rep % 2`. Both alternations turned on the same bit, so they
cancelled exactly and the effective prove order was a function of the prove index
alone: base went first in one retained prove of three in EVERY repetition at the
default `--reps 6 --proves 4`, and at `--proves 2` never went first at all. That
is a fixed positional asymmetry in the configuration the noise floor was
calibrated at — the one class of error repetitions cannot average out, and the
thing the interleave exists to remove.
The guard did not catch it because the guard restated the parity rule the design
was supposed to satisfy rather than measuring what the code did, and the code had
stopped satisfying it. Both the comment on the loop and the table in
calibration.md asserted an alternating per-repetition lean (2, 1, 2, 1) that the
code never produced.
So the ordering moves into `crates/web-client/scripts/bench-order.mjs`, the
warnings now COUNT the order that module produces, and `bench-order.test.mjs`
pins the invariants. Reintroducing the original `(i + rep)` coupling fails four
of those ten tests. Verified equivalence over all 264 (reps, proves) pairs up to
24 x 12 before switching the driver over.
Consequence for the floor: 1.79% sigma was measured under the unbalanced
interleave, so the pending recalibration is a remeasurement of a changed
quantity, not just a transfer to CI hardware. calibration.md says so now.
Also from this round's statistics review:
- calibration.md said a verdict needs four repetitions; the code has required six
since MIN_REPS_FOR_SIGN_TEST was pinned to CALIBRATED_REPS.
- verdict-power.mjs claimed to apply the verdict rule verbatim. It cannot model
the mean cross-check at all, since it is fed per-repetition scalars and has no
per-prove distribution to average, so its `silent` column is an upper bound on
true silence. Both the script and the tables now scope themselves to the
headline minimum-estimator verdict.
- The provisional-floor note told readers CI "is quieter, so the real floor is
likely tighter". The direction is unknown: a laptop has interactive load a
runner does not, a runner has virtualisation and neighbours a laptop does not.
Saying it is likely tighter invited trusting a movement just under threshold.
- The sign test's `1/2^(reps-1)` was labelled one-sided in four places. It is the
any-direction rate; per direction is half that.
And knip has been failing on this branch since verdict-power.mjs was added, since
nothing imports a standalone reproduction script. Declared.
* fix(bench): escape teardown strings, bound teardown, cap every timed step
Round 10 of review. The teardown-failure banner added in the previous round
was the injection this pipeline had otherwise been hardened against.
Renderer:
- Teardown entries were sanitized and then interpolated as raw GFM, so a fork
could put a live "[Security review passed](...)" link, a remote image and a
`#1` cross-reference (which backlinks from the target issue under this repo's
own bot identity) into the privileged PR comment. Wrapped in a code span, like
every other fork-controlled string here. The test covering this field passed
against the bug; it now carries the benchmark-name test's outside-span check.
- A benchmark name was the one fork-controlled string reaching column 0 of
stdout, so `::error::...` rendered as a workflow-command line. Indented.
Producer:
- Teardown was unbounded and runs before results.json is written, so a wedged
browser close did not merely leak a process: it hung until the runner killed
the step and a complete set of measurements was never written. Each close now
has a deadline and is recorded as a teardown failure if it misses it.
- Removed the post-teardown rewrite of results.json. Teardown completes in the
`finally` upstream of the results object, so the second write could not fire.
- A late worker death could be delivered after the post-prove check and then be
swallowed by the `closing` latch. Added a protocol round-trip barrier plus a
live worker count before anything is retained.
- `--budget-minutes` accepted values that disabled the clamp in both directions:
0.001 put every deadline past the budget at issue time, 1e308 overflowed to
Infinity. Both refused with the reason.
- The sample cap charged head-only runs for a base they never measure.
Workflow:
- The 120-minute budget omitted the worktree reclaim, the cache save and the
client-PR injection, and summed to exactly the cap. All three capped, budget
at 150 with slack.
- The benchmark step's cap and the producer's budget were two hand-synced
literals; drift silently restores the "killed with no diagnostic" failure.
Both now read one job-level value.
- `retries: 3` never covered secondary rate limits, which report 403 and are
exempt by default, while four comments claimed it did.
- The fallback's header comment described the opposite of what it now does, its
rate-limit message claimed no comment was posted on runs where one was, and
three refusals caused by GitHub being unreachable produced a message reading
as a forgery accusation.
* fix(bench): restore the ABBA composition, refuse rather than floor at budget end
The previous commit's ABBA change was wrong, and wrong in the direction it
claimed to fix. Its premise was that the prove-level flip `(i + rep) % 2`
cancelled against the open order's `rep % 2`. It did not: the driver
canonicalised `sides` to [base, head] before the prove loop, so the open order
never reached the flip and both parities were carried by the one expression.
Removing `rep` from the flip while leaving that canonicalisation in place is
what actually broke it. The composition lost a bit, and base then went first iff
the prove index was even — the same way in every repetition. At the calibrated
`--reps 6 --proves 4` that is 6 of 18 retained proves instead of 9; at
`--proves 2` base never went first at all. Both are the fixed positional
asymmetry the interleave exists to remove, and the printed balance note reported
the run as alternating throughout, because it asks bench-order.mjs while the
loop did something else.
`sides` now stays in open order, which is what `proveOrder` documents as its
input and what `orderBalance` simulates, and the ordering is identical to what
the code produced two commits ago. The driver asserts the open-order invariant
every repetition, so reintroducing a sort fails loudly instead of quietly
biasing the numbers. Two tests pin the composition: base-first must differ
between repetitions, and canonicalising the input must still collapse it.
Also reverted the claim that the recorded 1.79% noise floor was measured under a
broken interleave. It was not, and the recalibration is an ordinary transfer to
CI hardware.
Separately, `deadlineFor` floored every deadline at 10s once the budget was
nearly spent, which is the magnitude of a normal prove: it aborted proves that
would have finished inside the runner's cap, reported them as `did not finish
within 10000 ms — treating it as wedged`, and threw upstream of the results
write, discarding a complete set of measurements over a deadline the budget had
manufactured. It now refuses below a 60s floor with an error naming the budget.
* fix(bench): gate the script tests in CI, bound the per-rep close, drop the 403 retry
Round 11. Three findings here correct earlier rounds of this same branch.
The benchmark script tests were reachable only from a step inside bench.yml — an
informational job with its own paths-ignore that never blocks a merge. So the
124 tests covering the trusted renderer, the interleave and the artifact
extractor gated nothing, and a break in the sanitizer or the verdict logic could
merge freely. They now run as their own job in test.yml, ungated.
The per-repetition `context.close()` was the one close still unbounded. It sits
in the driver's per-repetition `finally`, upstream of the results write, and
`Promise.allSettled` waits for settlement — the `.catch` only converts a
rejection. A close that never settled held the run until the runner killed the
step, discarding every repetition that had already succeeded. Same deadline as
the browser and server closes now.
Reverted the `retry-exempt-status-codes` override added last round. Dropping 403
to reach secondary rate limits is worse than not reaching them:
@octokit/plugin-retry backs off `Math.pow(retryCount + 1, 2)` seconds — a fixed
1, 4, 9 — and ignores `Retry-After`, so the retries fire inside the wait GitHub
requires, likely escalate the block, and still exhaust, while every permanent
permissions 403 burns 14s first. The comments now state what the retries do and
do not cover; the paths that must not fail over a rate limit already decline
rather than fail.
Also, keyed the reporter's concurrency group on the head sha and head repository
instead of the bench run id. Two reporters can exist for one sha without sharing
a run id, and the sticky-comment action creates when it finds no header, so two
that interleave before either posts leave a permanent duplicate comment.
Documentation corrections, all cases of a doc describing an earlier revision:
the repetition floor read four in one paragraph and six in another; the
`meanDeltaPct` comment cited 5.39% as the spread of a mean when it is the
median's; `--calibrate`'s usage text claimed it drives the comment's calibration
banner, which the renderer deliberately takes from the workflow event instead;
and the 1.07%-at-four-repetitions figure is only reachable with the floor lifted.
* fix(bench): exit when a close is abandoned instead of waiting out the step cap
Bounding the teardown closes let the results reach disk, but not the process
reach an exit. `reportTeardownFailures` sets `process.exitCode`, which only
applies once the event loop drains, and abandoning a wedged `browser.close()`
leaves Playwright's transport ref'd — so the run sat there with its measurements
already written and its summary already printed until the runner killed the step
at `timeout-minutes`. That surfaces as a timeout rather than a failed run, which
is the same "killed with no diagnostic" outcome the budget clamp exists to
prevent, and it spends up to the whole step cap doing nothing.
An abandoned close now forces the exit, flushing stdout and stderr first because
`process.exit` discards buffered writes and stdout is an async pipe under CI —
otherwise the exit meant to preserve the diagnostic is what loses it.
The comment justifying the non-unref'd timer was wrong in both halves: a wedged
browser does not leave "nothing else pending", and the unref'd variant does not
exit early on that path either. The timer stays non-unref'd, but for the case
that actually depends on it — a wedged listening socket, where nothing else
holds the loop open. Verified both directions: with a ref'd handle held open, the
run now exits 1 with all output flushed, and without this block it hangs
indefinitely.
Also corrected two descriptions that claimed more than the code does. The worker
count in `settle()` was documented as an independent backstop for a lost worker
event; Playwright removes the worker from `page.workers()` and emits the `close`
in the same synchronous callback, so a lost event takes the count with it. It is
a consistency assertion on Playwright's bookkeeping, and now says so. And a
`settle()` barrier failure was reported as a page error "during its proves",
pointing the reader at the prover for something the proves did not cause.
* fix(bench): deadline the setup-failure close, the last unbounded one
`openSide`'s catch block closed the context with its own bare
`await context.close().catch(...)`, which the two previous rounds of close-
bounding walked past. The `.catch` handles a rejection; it does nothing for a
close that never settles.
This is the worst-placed of the four. A wedged context is most likely exactly
here — the failure classes that break `page.goto` or the setup evaluate are the
ones that also break teardown — and this close runs before any results exist, so
a hang costs the entire step with no artifact at all, rather than costing a clean
exit after the numbers are already on disk. Also dropped its `console.error`,
which double-printed against the teardown summary the same way the per-repetition
close did before it was fixed.
All four close sites are now bounded: both contexts directly, the browser and
servers in the teardown, and the per-repetition close transitively. Verified
separately that the failure path does not need the forced exit added last commit
— an uncaught throw terminates in 0.04s with a ref'd handle held open, so only
the success path could hang.
Condensed the retry rationale, which was duplicated verbatim across five API
steps; the full reasoning stays on the fetch step and the rest point at it. And
gave the runner calibration an owner and a trigger in calibration.md, since a
provisional floor that nobody ever replaces is the outcome to avoid rather than a
safe default.
* fix(bench): keep the repetitions already measured when the budget runs out
Last round's `deadlineFor` change traded a bad diagnostic for a worse outcome. It
refused all work once fewer than 60s of budget remained, but the 60s floor was
compared against a budget that had already had the 90s teardown reserve taken
off, so a 30-second settle barrier was declined with 149 seconds of wall clock
still on the step. The refusal then threw, and the throw is upstream of
`emitResults()`, so a run that had completed six of seven repetitions wrote no
artifact at all and the reporter posted "no usable report". The comment claiming
it "costs the same run either way" was false by exactly those 150 seconds.
Two changes. The refusal now scales to the ceiling being requested rather than a
flat floor, so 30s of work is declined when 30s remain, not when 60 do. And
running out of clock is no longer treated as a failed benchmark: it is a distinct
error type, the driver stops the loop on it instead of propagating, and the
repetitions already collected are written and reported. Every repetition in
`samples` is whole — one is pushed only after both sides finish — so there is
nothing partial to keep.
That required the artifact to declare what it MEASURED rather than what was
requested, since the renderer refuses any artifact whose `samples` group count
disagrees with `reps`; emitting the requested count would have thrown the kept
repetitions away on the reporting side instead. `repsRequested` and
`stoppedEarly` record the difference. Verified through the real renderer: a run
truncated to four repetitions now renders "Unresolved +10.00% ... but 4
repetitions", which is the honest report, where before it produced nothing. A
budget stop with zero completed repetitions is still a failure, because there is
nothing to report and the renderer requires at least one.
The accepted minimum budget was also the mathematical boundary — satisfied at
t=0 and refused a millisecond later — so the smallest valid budget could not
launch a browser. It now requires working headroom above the two reserves, and
the diagnostic no longer rounds 2.5 minutes to "3" and tells the user to raise a
budget they never passed.
* fix(bench): run the renderer when it is invoked through a symlink
Found while checking that the previous commit's artifact changes stayed readable
by the renderer already on the reporting side: the copy under test produced no
output and exited 0. The cause is the main-module guard, which compared
`import.meta.url` against `pathToFileURL(process.argv[1])`. Node derives the
former from the resolved path and leaves the latter as the caller typed it, so any
symlink on the way in made the two differ and the script declined to run — exiting
0 having rendered nothing, which the reporting workflow reads as "ran fine,
nothing to post" rather than as a failure.
CI invokes it by its real path, so this never fired there. It is fixed anyway
because a silent success is the worst available shape for the mistake: the one
outcome that neither posts a report nor tells anyone why. Both sides now go
through realpath, with the literal comparison kept as a fallback so an unreadable
argv[1] cannot cause the script to skip itself.
Two tests: one asserts a symlinked invocation still reaches its usage error and
behaves identically to the direct one, the other that importing the module does
not execute it, which is the property the first must not break. The first fails
against the old comparison.
With the guard fixed, the compatibility check that started this ran in both
directions. The renderer at the previous revision reads the new producer's
artifacts correctly, rendering a truncated six-repetition run as "Unresolved,
4 repetitions", and the current renderer reads an artifact without the new
fields. That is the evidence behind the note added to `ACCEPTED_SCHEMA_VERSIONS`
explaining why `reps` becoming the measured count does not require a bump.
* fix(bench): keep an even number of repetitions when a run stops short
The previous commit let a truncated run report what it had measured, and in doing
so reintroduced the asymmetry the interleave exists to remove. The setup order
alternates on repetition parity, so an even count sets up base first exactly as
often as head first and an odd count cannot: stopping at three leaves one side
having set up twice on an idle machine and the other once. That is a fixed
positional bias, the one class of error more repetitions do not average away — the
reason the order alternates at all — so an odd tail would have bought one extra
sample by tilting every sample kept.
A short run now gives up its odd tail. The rule lives in bench-order.mjs next to
the alternation it protects, rather than inline in the producer, because the two
have to change together; the test asserts the property against `opensBaseFirst`
itself rather than restating the parity arithmetic, so it still holds if the
alternation is rewritten.
Also fixes an ordering mistake introduced along with it: the guard that decides
whether anything is reportable ran before the balancing, so a run truncated to
exactly one repetition passed the guard, balanced down to zero, and emitted an
artifact the renderer refuses for having no repetitions. The guard now judges the
count that will actually be emitted, and says which of the two cases it hit.
Documented in calibration.md, since what a truncated run may claim is a
statistical decision rather than an implementation detail: the retained count is
stated in the report, stays below the sign test's floor and so comes back
unresolved rather than confident, and routine truncation means the budget is
wrong rather than the run.
* fix(bench): disclose a truncated run in the comment, and report what it ran
A run that stopped on its budget rendered identically to a complete one. The
producer emitted `stoppedEarly` and `repsRequested` and the renderer read neither,
so the comment quoted a repetition count, printed the methodology text describing
a full protocol, and gave the reader no way to tell the run had been cut off. The
calibration doc claimed the gap was "visible rather than something to infer",
which was false as shipped — nothing surfaced it.
The comment now discloses it, above every note that quotes a repetition count,
stating what was retained against what was configured. The prose is composed here
from validated integers: `stoppedEarly` is read for its PRESENCE only and its
message never rendered, because a fork controls that string and comment prose is
pinned on this side for the same reason the verdict is. Nine injection payloads
through that field — autolinks, images, cross-references, workflow commands, a
team mention — reach nothing.
`repsExecuted` was also wrong on a truncated run, reporting the retained count
plus the warm-up when a repetition had additionally been dropped for parity. It
now reports what the process ran, and the renderer bounds it rather than dropping
the equality check: a complete run must still execute exactly one more than it
retains, and a stopped run at most two, so the protocol claim stays verifiable on
both paths and an artifact asserting a truncation it did not have is refused.
On whether a truncated run should be allowed a verdict at all: a review argued it
should not, on the grounds that stopping on elapsed time selects the reported
prefix by its own runtime. The censoring is real but it does not reach this
statistic, because the reported figure is a paired difference and the budget is
consumed by both builds together, so the selection is on their sum and cancels in
the ratio. `docs/benchmarks/truncation-bias.mjs` measures it against complete
runs: identical mean delta to within 0.1% and identical directional-verdict rate,
under the null and a true 8% effect, including under thermal drift — the
mechanism that should have broken it, and does not, drift being common mode. The
one row where a false-positive rate rises does so for the COMPLETE run and less
for the truncated one, so tails cause it rather than truncation. The verdict
already prices the real cost, which is lost power, by refusing to resolve below
the repetition floor.
calibration.md carries that table and the reasoning, replacing the earlier
hand-wave that called partial reporting "honest" without saying why it is sound.
* fix(bench): explain a declined report instead of implying it was forged
Three paths refused a report without recording why, and the fallback check run
infers a cause from what is unset. With PR_NUMBER unset it lands on "the pull
request the report claimed could not be confirmed against the fields GitHub
populated" — which accuses the author of forging a report when all they did was
push a commit or retarget the PR.
The head-moved path is the common one, and it is worse than it reads: a docs-only
follow-up push matches bench.yml's paths-ignore, so it starts no replacement bench
and cancels nothing. A finished multi-hour measurement disappeared with nothing
anywhere saying so. The pre-write recheck further down had already learned this
and sets a reason; the earlier identity gate, which catches the same case sooner,
did not. It now says the head moved and names both commits. The retarget path says
the base is no longer in the PR's history and to re-run. The head-repo mismatch
stays a hard refusal — it is the gate a forged report would have to pass — but
says what was checked rather than what was suspected, since the benign way to
reach it is one branch backing two open PRs.
Retargeting also never re-ran the bench at all: the default pull_request types
omit `edited`, which is the only event a base change fires, and this repo moves
PRs between main and next routinely. The workflow now takes `edited` with a job
condition that drops the title and body edits sharing that event.
That last change needed a guard on the reporter, and finding it is why the commit
is worth reading twice. A `paths-ignore` match never creates a workflow run, but a
job skipped by an `if` does — the run completes and still fires `workflow_run`.
Verified against this repo's own history: runs whose jobs are all skipped report
conclusion "skipped" (`gh api .../actions/runs`). So without the guard every PR
title edit would have published "the benchmark run produced no usable report". A
failed or cancelled bench still reports, since that is the point; only a skipped
one is silent.
Also: the unresolved note told a truncated run's author to "re-run with the
default --reps", which they had already used, and where more repetitions would
miss the budget by more. It now points at the budget. The producer's stdout said
`reps=6` while the artifact and comment said four. And two comments described the
concurrency key as per-run when it is per-head-sha, contradicting a third comment
eight lines away — the drift that gets the wrong thing "fixed" next round.
* docs(bench): add a runbook for the half of the bot nobody can see
The reporting workflow runs on the default branch, so it does not appear in the
pull request's checks and is not the run anyone thinks to open. Every path that
declines to post now records a cause, but knowing to go looking for it on a
neutral check run — and what each message means — was knowledge that existed
nowhere.
troubleshooting.md maps the messages the bot can publish to what to do about
them, in the order they actually occur, and covers the cases where the answer is
counter-intuitive: a docs-only push that moves the head starts no replacement
bench because of paths-ignore, so no report is coming and the run has to be
dispatched by hand; a title edit skips the job on purpose; the reporter always
executes the default branch's renderer, which is why a renderer change cannot be
tested from the pull request that makes it. Every quoted message was checked
against the string the code actually emits.
Also adds `make lint-bench-workflows`. The bot's behaviour lives largely in
workflow expressions, where a typo in an `if:` disables a job silently rather
than failing loudly — the change two commits ago turned on the `edited` event and
needed a new job condition and a new reporter guard to avoid publishing "no usable
report" on every title edit, which is exactly the class of mistake a linter
catches. Scoped to the two benchmark workflows because the other six carry 17
pre-existing findings; not wired into CI because actionlint is absent from the
pinned installer this repo uses for every other CI binary, and adding an unpinned
download or a third-party action is a separate decision.
* fix(bench): stop a budget-squeezed deadline from discarding the run
The round-12 fix scaled deadlineFor's REFUSAL to the ceiling being asked
for but went on returning min(ceiling, left), and nothing compared the
two. So only the band below the 60s floor was fixed. Above it, work was
started under a deadline the budget had squeezed by up to 10x — a setup
whose ceiling is ten minutes, granted seventy seconds — and when that
blew, evaluateWithDeadline raised the "treating it as wedged" error. That
is a plain Error, so the driver set benchError and threw upstream of the
results write: every repetition already measured was destroyed, and the
comment blamed a prover hang that had not happened. A setup takes ~80s
and a prove under 2s, so the live band was the one a real setup lands in.
Same failure eleven seconds of budget apart gave opposite outcomes.
A deadline the budget chose is not evidence about the process. deadlineFor
now reports whether it clamped, and a timeout under a clamped deadline
raises the budget error instead, so the run stops cleanly and keeps its
numbers. A wedge is only a wedge when the full ceiling was available.
The arithmetic moves to bench-budget.mjs, tested with a synthetic clock
against the property it has to satisfy: every grant is refused, full, or
clamped-and-flagged, never a fourth thing. Restoring the old expression
fails that sweep. This is the same technique that made the interleave
rule the one clean piece of the last three rounds — assert the property,
not a restatement of the expression — and it is here because this is the
third consecutive round to find a defect in these few lines, each earlier
fix having been checked by reasoning about one configuration while the
adjacent one stayed broken.
Also, from the same review:
- An odd --reps is refused rather than warned about. The setup order
alternates by repetition, so an odd count opens base first once more
than head: the fixed positional asymmetry a truncated run now drops a
repetition to avoid. A clean --reps 7 kept it and, being above the
repetition floor, published a confident verdict carrying it, with only
a stderr line to say so. Both balance checks become assertions, since
reaching them now means bench-order.mjs and the even-reps rule have
diverged rather than that the operator asked for a lopsided run.
- Truncation only ever shortens a side. On a head-only run samples.base
is empty and assigning a length grew it into holes serializing as
nulls, harmless purely because summarize filters empty groups first.
- The claim that truncation is self-limiting is scoped to the calibrated
request, in both the renderer docblock and calibration.md. It holds
because truncating six cannot leave more than four; a dispatch at a
higher --reps can truncate to a count that still clears the floor.
- fileURLToPath moves inside the try whose purpose is to stop the
main-module guard throwing at module scope, that call being the one way
it could.
- The deadline is validated where it is used. setTimeout coerces a
non-number delay to zero, so a malformed value does not throw, it times
out every step instantly and calls each one wedged — which is what the
unbudgeted path did here mid-change, returning a bare number to call
sites that had started spreading the result.
* fix(bench): withhold the verdict when the calibration does not cover the run
The claim that a clock-truncated run's paired comparison "stays sound"
rested on an assumption nothing had checked. Selection acts on the run's
total duration while the reported quantity is the difference, and
Cov(head + base, head - base) = Var(head) - Var(base)
is zero exactly when the two sides have equal run-level variance. The
simulation backing the old claim applied its noise to both sides equally,
so it measured the assumption rather than testing it. Giving one side a
12% run-level factor and nothing to the other shifts the truncated
estimate by many points while a complete run of the same model does not
move at all; symmetric noise cancels even with drift and heavy tails.
Whether the two binaries have equal run-to-run variance on this workload
is unmeasured, and a change to allocation or memory layout is a plausible
way to break it -- which is the kind of change this bot exists to catch.
So a truncated run now reports its numbers and withholds the ruling. The
table, the samples and the disclosure all still render, and the note says
why. This costs nothing in the common case, because a truncated run means
the budget was already too tight, and it retires the whole argument: no
one has to accept an unmeasured symmetry assumption to trust a published
number.
Two more shapes get the same treatment, since the reason generalises --
the threshold is 3σ of this estimator AT the calibrated configuration and
does not transfer downward on either axis:
- An odd retained count, which sets one side up first once more than the
other. The producer refuses an odd --reps, but reps is
artifact-authored, so the renderer no longer relies on that.
- Fewer than three retained proves. The threshold is 3σ of a mean of
per-repetition minima over three proves, and the minimum of fewer is
noisier, so the same cutoff is under 3σ there. Repetitions were floored
for exactly this while proves were only footnoted, which was
inconsistent: both legs weaken identically, and a
one-repetition-one-prove artifact was the cheapest route to a confident
verdict.
All three are one-directional. More repetitions or more proves tighten the
estimator, making the fixed threshold more than 3σ rather than less, so
only the downward direction is blocked and the ordinary run is untouched.
The mean-of-all-proves cross-check is gated too. It is a second
directional claim, and exempting it would have let a comment assert a mean
movement in a note directly below a headline that had just declined to
rule.
truncation-bias.mjs is replaced by truncation-selection.mjs, which
measures the identity above rather than one case of it. Its predecessor
also did not implement the renderer's actual paired sign test, modelled
three proves where the producer runs four, and charged no setup time to
the budget, so its agreement with the calibrated rate was not evidence of
what it was cited for.
Also: the no-schema-bump argument was resting on a false premise. It
claimed old and new values coincide on every artifact the previous
producer could emit; they do not, since a parity drop emits
repsExecuted = reps + 2, which a renderer built before that change
refuses outright. The real reason no bump is needed is narrower and now
stated: no renderer has ever shipped, so the branch's intermediate commits
are not a compatibility surface -- and that reason expires on merge.
* fix(bench): classify a timeout by whether the deadline was short, not by who chose it
`clamped` says the budget picked the deadline rather than the ceiling. That is a
fact about bookkeeping, and using it to decide what a timeout MEANS was wrong in a
way the numbers make plain: a prove's ceiling is five minutes for work that takes
under two seconds, so across the clamped band 100% of prove grants and 96% of setup
grants are larger than the work needs. Branching on it reclassified a genuinely
deadlocked prover — timing out under a 294-second deadline — as the budget running
out, told the reader in as many words that it was "not a hang", and kept the run.
That is the previous defect with its sign flipped: it stopped losing good numbers
and started keeping bad ones.
`grantDeadline` now takes the work's realistic duration and returns `starved`, and
the classification branches on that. `clamped` stays, for the log line. Over a real
45-minute timeline, 22 of the 26 clamped grants are now correctly read as hangs and
only the 4 genuinely short ones stop the run.
The structural problem was that none of this was tested. Four consecutive rounds
found a defect in the deadline logic and every one lived in the wiring, not the
arithmetic — the pure function was checked each time and the use of its answer was
not. `deadlineFor` and `evaluateWithDeadline` move into bench-budget.mjs behind an
injected clock, and twelve tests drive them with a fake page. Reintroducing each of
the four historical defects now fails the suite; before this commit all four could
be live together with every test green.
Also, sized from the run rather than from a guess: a repetition sets up BOTH sides,
so a default run pays 14 setups, not the 7 the budget arithmetic counted. At the
estimated 80s setup that was 21 minutes against a 20-minute step, which would have
retained 4 repetitions and put every report below the 6-repetition verdict floor.
The step is now 45 minutes, and the producer measures and reports the real setup
cost so the budget can be resized from a measurement.
- upload the artifact before the job summary, which runs fork-authored code over
fork-authored data and had no timeout, so a hang there discarded a benchmark that
had already succeeded
- validate an odd `reps` at dispatch instead of after two 30-minute builds
- re-throw a TypeError from the settle barrier instead of filing it as a page error
- document that `workflow_run` only fires the default branch's reporter, so the bot
is silent between landing on `next` and reaching `main`
- five comments still described the balance checks as warnings after they became
assertions
* fix(bench): stop an API outage reddening main, and skip runs that cannot pay off
The artifact fetch was the one step whose failure had nothing to do with the pull
request and still failed the job — on the trusted side, so a GitHub 404, a
permissions 403, or an exhausted 5xx retry showed up as a broken `main`. The
fallback check run already carried a message for exactly that state, which was
unreachable while the step could abort the job.
Two triggers that spend a multi-hour 8-core run for no signal:
- Draft pull requests, which is where pushes are most frequent and a proving
number least useful. `ready_for_review` runs the bench that was skipped.
- Changes under `.github/scripts/**`. The renderer cannot move a proving number,
and a change to it cannot be exercised by the PR's own bench run in any case:
`workflow_run` always executes the default branch's copy. Their tests are a
merge gate in test.yml, so nothing is lost.
* fix(bench): stop the heading claiming significance the provisional floor cannot support
`THRESHOLD_PROVISIONAL` only ever changed prose, so a movement still got a
`⚠️ +20.00% slower` heading — which reads as "this pull request regressed proving"
— directly above a note saying the floor here is unknown in both magnitude and
direction. Whichever of the two a reader believed, the comment had already denied
it. The note then asked the reader to "treat movements near the threshold as
unresolved", delegating the judgement that every other precondition exists to make
on their behalf.
Resolved by weakening the heading, not by withholding the row. The gates in
`verdictPreconditions` all cover cases where the ESTIMATE is compromised —
truncation selects the sample, an odd repetition count adds a fixed positional
bias — and a direction measured under those is not worth reporting. A provisional
floor is not that: the estimate is as sound as it will ever be and only the cutoff
is a guess carried from a laptop, so refusing to name the direction of a large,
repetition-consistent movement would discard the signal the bot exists to surface.
The number and the paired agreement stay; the claim of significance goes.
Corrected two claims that were wrong rather than merely strong:
- The truncation figures understated the demonstrated risk by 4-5x. Symmetric
run-level noise shifts the estimate -0.19pp, not -0.08pp, and a one-sided 12%
factor shifts it -18.90pp or +22.66pp, not 4.4pp — several times the 5.40%
floor rather than a fraction of it. The argument for withholding a truncated
run's verdict is much stronger than the numbers next to it said.
- "The repetition's fastest prove is its clean compute cost" claims more than a
minimum earns. It is a lower-tail order statistic, equal to the uncontended
cost only if the residual variation is all non-negative interference AND a
near-zero-interference prove was actually sampled. Restated as the best
observed warm prove, with the blind spot it implies stated next to it.
Also, the 45-minute budget is a hypothesis and now says so. Completion is a
distribution over fourteen setups, not a threshold, and it falls off a cliff:
comfortable below 120s per setup at any plausible spread, effectively silent by
180s. `docs/benchmarks/budget-reachability.mjs` reproduces the table. Nothing has
measured where on that curve the real runner sits.
* fix(bench): say why a report was declined, and keep the runbook level with the workflow
A malformed `pr.json` left `skip_reason` unset, so the fallback inferred one and
landed on "the pull request the report claimed could not be confirmed against the
fields GitHub populated" — a forgery accusation, for a producer bug or a truncated
upload. The identity-race paths were given accurate causes in an earlier round and
the artifact-shape paths were missed. `refuseAsMalformed` now names what actually
happened, and pointing the reporter at a run that is not a Proving Benchmark run
says that instead of implying a forged identity.
The runbook had gone stale against changes made in this same branch: it did not
mention that drafts are skipped, and its `paths-ignore` list predated the reporter
and the scripts being added to it. A runbook that omits the two most likely causes
of silence is worse than none, because it reads as authoritative.
- `bench-comment.yml` joins `paths-ignore`. The rationale already written for
`.github/scripts/**` applies to it verbatim: it cannot move a proving number
and cannot be exercised by the PR's own bench run, because `workflow_run`
always executes the default branch's copy. `bench.yml` stays out — it owns the
build and is part of the dist-cache key.
- A vanished zip no longer reddens the default branch. The extractor exits 0 for
every hostile or unusable archive by contract and reserves non-zero for the zip
being gone, which is a runner fault the fallback already reports.
- The comment sizing the comment-write race said 90 minutes; the job cap is 180.
* fix(bench): classify a timeout against the setup cost this machine actually has
`starved` asked the right question and answered it from a constant. Comparing the
grant against a hardcoded 90s was the same category error as the `clamped` version
it replaced, one level out: wherever a real setup exceeds `90s × STARVATION_FACTOR`,
a grant between the two reports `starved: false`, so a healthy run with a
legitimately slow setup has its timeout called a hang — which throws upstream of
the results write and destroys every repetition already measured, then blames the
prover for it. Across a plausible 60-600s range that is 420 of 541 durations, and
raising the budget relocates the band instead of closing it. It fails in the unsafe
direction precisely when the estimate errs low, which is the one direction nothing
has excluded.
The producer already measures what the classification needs. It now takes the
slowest setup it has actually timed, falling back to the estimate only for the
first setup of a run, when nothing has been measured yet — harmless, because early
grants are the full ceiling anyway. False discards go from 420/541 to 0 while a
genuine deadlock under the full ceiling is still discarded.
Two changes so this cannot come back the same way:
- `expected * STARVATION_FACTOR > ceiling` is now refused. Past that ratio the
FULL CEILING counts as starved and nothing can ever be called a hang, which
resurrects the defect the flag exists to prevent — and it was reachable by
following this repo's own instructions to resize `expected` from a real run.
- The work profiles moved into `bench-budget.mjs` and the test imports them. It
had been declaring its own copies, so retuning `SETUP_EXPECTED_MS` to a value
that breaks the scheme shipped with the suite green. All five defects this area
has produced now fail the suite when reintroduced; that retune was the fifth.
Also: deleted a comment block, orphaned when `evaluateWithDeadline` moved out,
that still told the reader `clamped` decides what a timeout means. That rule was
the previous defect, asserted as current behaviour, in the file where all five
have lived — which is how this area keeps regressing.
* docs(bench): state the constraints in the present tense, not the history
CLAUDE.md asks that code comments describe current state rather than history, and
these files had drifted a long way from that: several blocks narrated which earlier
version of themselves was wrong, one announced that its own rationale "expires the
moment this merges", and the schema note spent eleven lines distinguishing a wrong
reason from the actual one.
Each is now the rule a future editor needs, in the tense they need it. The schema
block says plainly that the version-1 latitude ends at merge and any widening after
that needs a bump — which is the actionable half and was buried under the
explanation. The concurrency block says do not group by branch and why the obvious
settings both drop reports. The budget accounting says every timed step must be
listed and summed, rather than recounting two compositions that did not hold.
No behaviour change; the invariants are unchanged and the tests are untouched. Test
comments keep their rationale, since explaining what a test pins is a present-tense
statement about the test.
* feat(bench): make PR runs opt-in until the runner's noise floor is measured
The threshold this bot compares against was measured on a laptop, so the renderer
withholds every ruling and reports movements as observations. Meanwhile a
qualifying push spends about three runner-hours on an 8-core machine. Paying that
by default for a report that declines to conclude is not a trade worth making, and
it is the strongest argument against shipping this at all.
So a pull-request run now needs the `bench` label. Calibration is unaffected and
was always the intended first step: `workflow_dispatch` with `calibrate: true`,
per docs/benchmarks/calibration.md. Once the floor comes from this runner class and
THRESHOLD_PROVISIONAL is false, two clauses come out of the job's `if:` and the bot
runs on every qualifying push — the `if:` block says which two.
The label clause is scoped to `pull_request` events. A dispatch has no pull
request, and an object filter over null yields an EMPTY ARRAY rather than null, so
an unscoped `contains` reads false and would have skipped the calibration dispatch
— the one run that has to work before any of this is worth enabling. Nine event
shapes are checked against the intended outcome.
Also wires `make lint-bench-workflows` into CI as a merge gate. It had been
deliberately left out because actionlint is absent from taiki-e/install-action's
tool list, which is how this repo pins CI binaries; pinning the version and its
sha256 is reproducible without that mechanism, and the exclusion mattered more
than the inconvenience — much of this bot's correctness lives in workflow
expressions, where a typo in an `if:` disables a job silently rather than failing
loudly, and one of those jobs holds a write token over fork-controlled input. The
bug in the paragraph above is exactly that failure mode.
* docs(bench): name the unlabelled skip in the reporter stand-down
An unlabelled PR is now the common reason bench.yml skips its job, and…
…xMiden#343) 0xMiden#341 made the calibration configuration injectable on `main`. Without this the two lines diverge on exactly the file that change was meant to unify, and the next re-calibration here pays the assertion-swapping tax again. The renderer and its tests come across unmodified from `main` — that is the point, and the evidence for it: the same 136 tests pass here against a 1.9% Poseidon2 profile and there against a 5.4% Blake3 one, with no assertion edited. Only .github/scripts/bench-profile.mjs differs between the lines, and only in its data. Also brings `derivationSentence()` into the refactored shape and takes it from the profile. It was added on this line and does not exist on `main`, so a straight copy of main's renderer would have regressed it — and regressed it into precisely the bug it was written for: main's hardcoded "sits above that observed maximum rather than at 3σ" is false here, where the largest observed movement (1.17%) sits below 3σ (1.85%). It now branches on the calibration record, so it is correct on both lines and after any future re-calibration. `main` should take `derivationSentence()` too. It is inert there today — main's maximum really did exceed 3σ, so it renders the same sentence either way — but leaving it out means the next calibration on main can silently make the prose false again, which is the failure it exists to prevent.
…at failed rc.5 (0xMiden#345) * fix(release): build the adopted packages in CI and repair the five that failed rc.5 * fix: drop the now-unused copyfiles dep and correct a typo
…tw blocking (0xMiden#346) * fix(types): correct the adopted packages' type resolution and make attw blocking * docs: changelog entries for the type-resolution fixes
* chore: bump the pinned nightly to 2026-05-23 for rustc 1.98 * chore(deps): bump miden-client to 0.16.0-rc.4 * docs(changelog): note the client 0.16.0-rc.4 adoption * chore(idxdb-store): update comments * refactor(idxdb-store): namespace settings by scope * fix(web-client): export __heap_base for the nightly-2026-05-23 MT build (0xMiden#359) * fix(web-client): export __heap_base for the nightly-2026-05-23 MT build * chore: refresh the lockfile onto the published rc.4 --------- Co-authored-by: Wiktor Starczewski <poszerny@gmail.com>
…0xMiden#352) * cover cursor paging against the SQLite store's ordering contract * drop expiredBefore from the public JS surface * record the expiredBefore query throwing
…0xMiden#348) * feat(web-client): commit fee conversion info to transaction auth args * fix(web-client): repair the async consume break and the fee-info gaps F-002 P0 useConsume/useSessionAccount passed a Promise to submitNewTransaction F-003 P0 ~35 test, helper and bench call sites did not await the now-async newConsumeTransactionRequest F-001 P0 the new test asserted through AdviceMap.get, which did not exist; added the accessor and made the assertions capable of failing F-008 P1 the five fee-reading constructors held the client borrow across an IndexedDB await while still bound raw by the proxy, bypassing _serializeWasmCall; reclassified onto the serialized path F-004 P1 fee_conversion_info.test.ts was in no CI shard, so it never ran F-009 P1 newB2AggTransactionRequest still built a fee-less request; the helper's claim that no constructor could forget was false F-005 P1 no CHANGELOG entry, including for the breaking async signature F-006 P2 FeeConversionInfo missing from the typedoc entry point F-010 P2 withAuthArg and withFeeConversionInfo silently clobber each other F-011 P3 verificationBaseFee is a rate, not a per-transaction amount * fix(web-client): finish threading the consuming account through consume F-015 P0 newConsumeTransactionRequest requires the consuming account, but 28 call sites across the integration suite, the bench script and the react-sdk mockchain tests still passed only the note list, so every one of them threw before reaching the chain F-016 P0 the fee-aware builder was missing from the transactions unit mock, leaving 18 tests red F-017 P1 the CHANGELOG described the new argument as optional and claimed the account id crosses by value; it is required, and both it and submitNewTransaction borrow it F-018 P2 feeAwareTransactionRequestBuilder is public and was undocumented * fix(web-client): pay fees on every request-building path, not just the Rust constructors The fee-aware builder reached only the six Rust convenience constructors, so every path that assembles a request in hand-written JS or React still aborted with ERR_FEE_CONVERSION_INFO_MISSING on a fee-charging chain. Widen it to those paths, make the two PSWAP constructors fee-aware, and close the gaps that let the change ship undetected: MidenClient never implemented the method its own docs called, the React mocks swallowed dropped awaits, and the test guarding the concurrency invariant this change widens ran in no CI shard. F-019 P1 useSend / useMultiSend / useCreateNetworkNote built fee-less requests F-020 P1 raw-bound methods could re-borrow the client mid-await F-021 P1 PSWAP consume/cancel built fee-less requests with no usable workaround F-022 P1 AdviceMap.get returned null under napi, undefined under wasm-bindgen F-032 P1 twelve PSWAP test sites handed a Promise to submitNewTransaction F-033 P1 MidenClient.feeAwareTransactionRequestBuilder was documented, never implemented F-034 P1 narrative docs promised a builder guard that does not exist F-047 P1 React mocks accepted thenables, hiding dropped awaits F-051 P2 executeProgram sat in two classification sets at once F-057 P0 restore the js/client.js coverage exclusion dropped while editing its comment * fix: reject unclassifiable fee conversion info; harden test guards Round 6 of the review loop. The substantive fix is a pre-execution guard for `withFeeConversionInfo`, the branch's only JS path that can set upstream's `declares_fee_conversion_info` flag. On an account carrying a custom auth procedure, upstream's validation classifies it as `Custom`, counts zero standard auth components, and hits an `assert_eq!` — a wasm trap taken while the client borrow is held, leaving the client unusable for every later call. Since `TransactionRequest::declares_fee_conversion_info()` is public upstream, the four execute entry points can now classify with `from_procedures`, which cannot panic, and return `FEE_CONVERSION_INFO_UNCLASSIFIABLE` instead. The rest closes holes in the guards this branch added, each confirmed by re-running the mutation it was blind to: - The shard guard and the classification script read their configs through the TypeScript parser, so a commented-out entry no longer counts as present. Textual comment-stripping is not an option here: `"test/*.node.test.ts"` contains `/*`. - The declared-surface guard is rebuilt on the AST, which brings `[Symbol.dispose]` and `[Symbol.asyncDispose]` under test for the first time, fails on any name shape it cannot resolve, and now also checks that nothing public is implemented without being declared. - The dropped-`await` guards are re-applied after mock overrides merge, and the constructor mocks always resolve — without that second half, a synchronous override meant a dropped `await` never produced a Promise to catch. Documentation: the custom-auth constraint on the builder JSDoc, transactions.md and the README; the fee salt as a second source of per-call divergence in `useChainAnchor`; why the network-account example's bare builder is correct; the miden-standards pin as a constraint on auth-component classification; the true scope of the `seed` parameter. * docs: transport the request in every co-signing example A multisig request now carries fee conversion info whose salt is drawn fresh on each build, and the multisig auth procedure uses that auth argument as the transaction summary's replay-guard salt. Two independently built requests describing the identical transfer therefore commit to different summaries. Every co-signing example shipped the anchor and the summary but left the request to be rebuilt locally, which under that change makes the co-signer's comparison fail and report the proposal as tampered with when nothing is wrong. The examples now serialize and transport the request, and the surrounding prose says why. The salt rationale also claimed the value "cannot be a value the builder invents", which the convenience constructors have always contradicted. * fix: reject a fee commitment whose auth argument was overwritten `withAuthArg` and `withFeeConversionInfo` write the same slot, so calling the former second replaces the commitment while leaving the conversion-info preimage in the advice map keyed by the discarded one. That aborted in the VM with ERR_FEE_CONVERSION_INFO_MISSING, a message naming neither the collision nor the two methods involved, and every doc surface described the combination as something nothing rejects. It is exactly detectable: upstream attaches the preimage keyed by the auth argument it just set, so a request declaring conversion info whose advice map holds nothing under its current auth argument has had that argument overwritten. Checking at execution rather than in the builder leaves room for the advice map to be extended after the request is built, which the multisig signing flow does. * fix(rev): round 3 — chain-anchor docs, surface checks and transaction resources * expose withAuthArg so a serialized request can carry fee conversion info * feat(web-client): expose feeNote and userOutputNotes on ExecutedTransaction Since 0.16 the kernel emits its TX_FEE note as an output note, so outputNotes() returns one more note on a fee-charging chain than it did at fee 0. Every consumer had to infer which one that was from the 0xfee note TAG - a plain u32 that any caller-supplied output note can carry, so a tag-based split can both mislabel an ordinary note as the fee and erase it from the transaction's totals. These accessors split by NOTE SCRIPT ROOT against TxFeeNote::script_root(), the kernel's own designation, which a note the transaction was merely asked to create cannot forge. rust-client already does exactly this when deciding what to track (transaction/mod.rs); this makes the same answer available to JS callers instead of leaving each one to re-derive a weaker version of it. Motivated by four independent positional-pick bugs in the wallet, all invisible at fee 0 because the kernel skips the fee branch entirely. * apply review findings from the fee-conversion-info loop * spell preempted without the hyphen so typos-check passes * teach the react-sdk request mock about withAuthArg * keep one changelog entry per change after the restack * refuse a replaced fee commitment on the execute paths too * keep the react peer range on the client version the tree ships * changelog: announce the fee conversion info request API * build the adopted packages against the tree's client, not a published one * give generated faucets a wallet interface so they can be funded * resolve a linked protocol PR when building the sdk * port fee conversion onto the salt the client now takes * changelog: drop the entries that moved to 0xMiden#352 and the restack duplicates
…0xMiden#362) * feat(web-client): expose the standard guarded-multisig auth component * style: rustfmt the guarded-multisig component
kutluhaneth46
force-pushed
the
fix/auth-scheme-guarded-multisig-365
branch
from
September 6, 2026 14:17
5091970 to
c5fc539
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
8 participants
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.
Summary
AuthGuardedMultisigConfignow accepts the public stringAuthScheme(AuthScheme.ECDSA/AuthScheme.Falcon) on browser and Node entry points.resolveAuthSchemebefore calling the native constructor, so the shadowed WASM/napi enum is no longer required at the package root.resolveAuthSchemealso passes through already-resolved numeric discriminants.Fixes #365
Test plan
pnpm --filter @miden-sdk/miden-sdk run test:unit— utils tests including newwrapAuthGuardedMultisigConfigcoveragenew AuthGuardedMultisigConfig(approvers, 1, guardian, AuthScheme.ECDSA)from@miden-sdk/miden-sdk(browser + Node)AuthScheme.FalconAccountType/ faucet exports unchanged