fix(extension): unblock local proving via sdk re-entrancy + sync-mgr classification#247
Open
WiktorStarczewski wants to merge 1 commit into
Open
fix(extension): unblock local proving via sdk re-entrancy + sync-mgr classification#247WiktorStarczewski wants to merge 1 commit into
WiktorStarczewski wants to merge 1 commit into
Conversation
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
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.
Web SDK PR: #152
Summary
Three independent wallet-side fixes that, together with the linked web-sdk#152, unblock local proving on Chrome MV3 and stop the misleading "cannot reach the miden node" banner from firing during honest long proves.
@miden-sdk/miden-sdkto0.14.10via the linked-PR pipeline. The SDK fix is the load-bearing change:_withInnerWebClientwas deadlocking the moment its callback called back into a proxied wasm method on the same client (inner.getInputNote(...),inner.executeTransaction(...), etc.). All wallet usage ofproveLocallyViaOffscreenfor consumes and sends hung indefinitely. With the SDK fix, the offscreen-doc MT-WASM prove path actually completes.SyncManagerclassification (src/lib/miden/back/sync-manager.ts) — the 5 sSYNC_TIMEOUT_MSnow bounds the RPC call alone, not the lock acquisition. Lock-contention during an in-flight prove queues patiently behind it (the prove yields the wasm-client mutex during the offscreen step viayieldWasmClientLock); only an actual unresponsive RPC trips the timeout. Three consecutive timeouts still open the circuit breaker, but they only fire when the network is really sick — not when the wallet is just busy proving.playwright/e2e/helpers/miden-cli.ts: add--lockedtocargo install miden-client-cli. The published 0.14.8 crate ships aCargo.lockpinningmiden-assembly = 0.22.1, the same version the wallet's bundled SDK uses. Without--locked, cargo re-resolved tomiden-assembly@0.22.3which silently shiftsBasicFungibleFaucetMAST procedure roots (see miden-vm#3144). The wallet then fails to recognize CLI-deployed faucets and renders their tokens as "Unknown", which any test selector filtering by token symbol can't match.WalletPage.setDelegateProofEnabled(enabled)added to theWalletPageinterface and all three impls (Chrome, iOS, Android). Just flipslocalStorage['delegate_proof_setting_key']; the wallet reads that synchronously per render so the next form interaction picks it up.playwright/e2e/stress/stress.spec.ts: optionalSTRESS_LOCAL_PROVING=trueenv var that flips both stress wallets to local proving after creation. Lets you run the existing stress suite end-to-end against the local-prove pipeline.playwright/e2e/tests/send-public-local-prove.spec.ts: focused regression spec — single send/claim with local proving forced. Smaller scope than the full stress, fast to run for targeted CI gating.src/lib/miden/sdk/miden-client-interface.ts: keeps the per-stepprove-timingmarkers I added to localize the deadlock (betweenexecuteTransaction/proveViaOffscreen/submitProvenTransaction/applyTransaction). +20 LoC ofrecordProveTimingcalls; gated onMIDEN_E2E_TEST=trueso production users see nothing.How they fit
The SDK deadlock (#152) is what makes local proving impossible. Once unblocked, the next user-visible failure mode is the false "node unreachable" banner — that's #2 here. Together, local proving works AND looks healthy to the user.
The harness changes (#3) are independent of either fix's correctness — they just let CI exercise the path that was previously broken without anyone noticing.
Empirical verification
With this PR's harness changes + web-sdk#152's SDK fix linked in, ran the 5-tx stress against testnet with
STRESS_LOCAL_PROVING=true:Pre-fix: every consume hung at
inner.getInputNote; zero sends completed; 5-minute Playwright timeout.Post-fix:
[mt-offscreen-prove] tx_completed prove_ms=…fires for every opSyncManagerfalse-positives during prove — banner stays clear(Remaining stress harness failures trace to the conservation-check accounting drift and the
getByText('TST')selector vs "Unknown" tile rendering — pre-existing, downstream of miden-vm#3144, unrelated to anything here. The--lockedchange in this PR closes the harness-side variant by pinning the CLI's transitivemiden-assemblyto the same version the wallet SDK bundles.)Verification done locally
yarn lint— cleanyarn format— cleannpx tsc --noEmit— cleanyarn test— 1865/1865 passyarn build— succeedsTest plan
linked-web-sdk-pr-readystatus (web-sdk#152 merge + release)STRESS_LOCAL_PROVING=trueafter merge