Repository navigation
v0.10.5: Ready/Braavos stack (#546, #548, #550, #552, #553) with review fixes, plus audit fix - #559
Merged
Merged
Conversation
A watch-only account (any Braavos today, an Argent account with a guardian, a rotated or unsupported owner, or an unauthorizable live release) rendered a fully normal Home - live Send, Swap and Shield, a complete send flow, even a fee quote - and the refusal only landed at submission. A new privileged wallet.accountSignability message answers for the active account: Braavos family-wide until Braavos signing lands, deployed Argent through the live signability probe, everything else signable; advisory by construction, so a failed probe answers signable and the transaction path keeps its authoritative checks. A session cache mirrors the deployment cache (both verdicts age out, since restrictions appear and lift), Home hydration loads it beside the deployment status, and the hero renders "Read only" in the stale-note style under the total while Send, Swap and Shield grey out; Receive needs no signature and stays live. The verdict vocabulary is the recovery module's restriction set plus "braavos", shared end to end so the picker, the flag and the refusal copy can never disagree. Fixes #545. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 45f5284)
Review of #546: the Shield tile only switched which flow it opened for a read-only account, so it stayed an enabled "Activate private balances" button into a signing flow the account cannot complete — the last-click failure #545 removes for Send and Swap. shieldTile now renders a true disabled control for a read-only account ("Shield unavailable — this account is read only"), and the Home test asserts it is disabled rather than only checking Send and Swap. Also from review: classify the wallet.accountSignability hydration poll in the panel-poll unlock-window suite (it already uses the non-renewing session; the suite now proves it), and add a signability-cache test for the freshness window, the forced refresh, the account-switch drop, and the off-Home/locked/no-account guards. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> (cherry picked from commit 9ffeb3d)
Braavos was refused family-wide at the signing boundary, though an account whose stark key is its only signer verifies the same plain (r, s) pair this wallet already produces - confirmed in braavos-account-cairo (SIG_LEN_STARK accepts the bare pair) and live on mainnet from a dataset fixture. The blanket requireAuthorizableFamily gate is gone. In its place, the deployed-compatibility check (now living beside its Argent sibling in account-compatibility) verifies on chain state what the family rule guessed: the live class must be a known Braavos release, the wallet's stark key must be a registered signer, and no strong signer (2FA/3FA) may be active - unless a deferred-removal request has expired by the pinned block's own timestamp, after which the account itself treats the strong signers as gone. Typed data and the privacy relay run the same check through requireLiveAuthorizableClass. Deployment stays refused where every deploy payload is built, since the extended Braavos deployment signature is not produced here yet. An advisory braavosSigningRestriction probe mirrors the Argent one for the recovery scan and the Home flag: a new "strong-signer" restriction labels rows "2FA active - view only", a stark-only deployed Braavos row reads as a plain deployed account, and an undeployed one stays view-only. All five Braavos refusals read properly through friendly-error-copy. Fixes #547. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit dd45efa)
A funded-undeployed Braavos account on the modern deployment class could only be watched: every Braavos deployment was refused because the family's extended deploy authorization was not produced here. The signer now assembles it exactly as the vendor app does, the layout braavos-account-cairo pins and the vendor dataset proved on mainnet: plain (r, s) over the deploy hash, the AdditionalDeploymentParams naming the implementation to install (no extra signer, no limits, the chain id stashed from the deploy details), and a second stark pair over poseidon(params) from the same key. The parameters sign before the transaction hash on purpose - the submission safety check treats the key's last signed hash as the one broadcastable transaction, and only the deploy hash is broadcastable; the first mainnet proof run caught exactly that ordering and the test now pins it. Deployment inputs accept only the modern deployment class (single-sourced as BRAAVOS_DEPLOYMENT_CLASS_HASH beside the implementation it installs); older base-class counterfactuals keep the existing refusal, since their expected layout has not been established. Estimation signs for real with validation on - Braavos constructors validate the deploy authorization even on query versions, and a query-version hash can never be submitted - while Argent keeps its mock query signer and OpenZeppelin keeps skipping validation. The recovery picker offers "Funded and ready to deploy" for modern-class counterfactuals, and the Home verdict treats them as deployable like any other account. Proven on mainnet through this exact platform path: braavos-matrix/5 and /6 deployed (tx 0x3fcf7128...), both now running the current Braavos implementation. Fixes #549. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 1506b25)
A Braavos account with a strong signer (2FA/3FA) was view-only with no way forward from the phrase. Braavos exposes a phrase-only escape hatch, and this wires it into the read-only Home flag. A new platform module submits the one transaction such an account accepts from the stark key alone: a single self-call to deferred_remove_signers, empty calldata, no paymaster, and a fee bounded by the account's on-chain cap (MAX_ETD_FEE_V3). It deliberately bypasses the strong-signer refusal that guards every other submission, mirroring the account contract's own carve-out, and it refuses anything that is not a deployed strong-signer Braavos account whose stark key is registered. After the account's execution time delay (default four days) the account treats the strong signers as removed, so the existing signing checks start passing with no further transaction; cancelling during the window needs the 2FA device, which the copy says because the carve-out does not extend to the cancel. The background exposes privileged review and submit handlers; the signability verdict carries the recovery state (pending expiry, or the delay a fresh request would take) so the Home flag renders all three states: read-only with a Recover account button, read-only mid-removal with the delay and the device-cancel note, and ordinary once the account becomes signable. The verdict and flag follow #548's checks, which already flip a strong-signer account to signable the moment its removal delay expires. Proven on mainnet through the wallet's own review/submit path: braavos-matrix/4 (simulated 2FA) accepted the removal request (tx 0x1706142958afa1a978e291cfa159aa22e12e5c72e13834644830fc0aff73564) and reads pending, usable after the four-day delay. Fixes #551. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> (cherry picked from commit 8842a9c)
The pending read-only flag stated the delay in days; show the exact moment the account becomes usable instead — "Usable on 09/10/26 at 13h28 UTC". Formatted with pure civil-from-epoch integer arithmetic so the render layer reads no wall clock. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> (cherry picked from commit 886002a)
A Ready (Argent) account with a guardian was view-only with no way forward from the phrase. Argent's escape is the symmetric feature to Braavos' deferred removal: the owner key alone may remove the guardian, in the two steps the account's escape carve-out accepts from a single owner signature. A new platform module runs that flow. trigger_escape_guardian(None), a single self-call with the owner signature, starts the security period (seven days by default); after it, escape_guardian() removes the guardian and the account signs normally again — #508's checks already pass once get_guardian is zero. It deliberately bypasses the guardian refusal that guards every other submission, mirroring the account's carve-out, and refuses anything but a deployed guardian-restricted account whose owner is the wallet key. The escape fee is bounded by the account at five STRK, so that protocol cap is the ceiling the submission honours; the reviewed fee is the quote the user saw. Estimates run with validation on, because the escape call does real work in __validate__ (the anti-DoS rate-limit bookkeeping) that a skipped-validation estimate under-quotes — the first mainnet attempt reverted out of gas on exactly that and the module now estimates correctly. The escape state is read through the account's own get_escape_and_status, whose status reflects the chain's clock. The background exposes privileged review and submit handlers and the signability verdict carries the escape stage, so the read-only Home flag renders the Ready states alongside Braavos: a guardian account offers "Recover account"; a running escape shows when it unlocks and that the guardian can still cancel; once the delay passes it offers "Complete recovery"; and it clears when the guardian is gone. Cancelling needs the owner and guardian together, so the phrase cannot stop it alone. Proven on mainnet through the wallet's own review/submit path: ready-matrix/20 (guardian set) accepted the escape trigger and reads pending, completable after the seven-day security period. Fixes #544. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> (cherry picked from commit 75dd885)
start.ts had grown past the 500-line file cap (check:files failed on the bundle). The paint loop's choice between onboarding, lock screen, and the dashboard — and the 36-argument dashboard call — is a pure projection of the render cache plus navigation state, so it moves to app/screens/scene-content.ts. Behaviour is unchanged; the class-hash verification lookup moves with it since it only fed that call. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
#548 accepted any class the table files under Braavos, and any stark signer anywhere in get_signers. Neither matches the account contract: - A live account never runs a base/deployment class (their constructors replace themselves with an implementation), so the live-class gates now accept only the implementation releases that expose get_signers and verify a bare (r, s): BRAAVOS_SIGNING_CLASS_HASHES (current + legacy). The deployment classes 0x13bfe... and 0x03d16c... are refused. - Braavos verifies a two-felt authorization against stark[0] only. The wallet key must be the first stark signer; an empty list, or the key at stark[1], refuses as "owner does not match". The rules move to braavos-compatibility.ts and are shared by the authoritative gates (isCompatibleAccountDeployed, requireLiveAuthorizable Class) and the advisory recovery-scan probe, so they cannot drift again. The signer builder checks the STORED class, which fixes the address and so must be a Braavos deployment class; the implementation is checked live. Tests pin both gates and the probe on: base/deployment class refused, stark[1]-only refused, empty stark list refused, webauthn-only 2FA refused, the expiry boundary (expire_at == block time refuses, one second earlier signs), one pinned block for every read, the typed-data and relay gates refusing a base class, the builder refusing an implementation class as stored class, and the recovery scan labelling Braavos rows through the Braavos probe. Fixtures that used a base class as the LIVE class move to the current implementation. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The Argent typed-data branch already re-pins when a load-balanced sibling has not seen the accepted block (JSON-RPC 24); the Braavos branch added in #548 pinned once, so roughly one Braavos typed-data or relay request in eight would fail on the wallet's own endpoint. The submit-path check (isCompatibleAccountDeployed) pinned without any retry for Argent and Braavos, and #548 had also started pinning the Argent multisig class read. One helper now runs every read behind a decision at a freshly pinned block and re-pins the whole decision on block-not-found, up to three times, then fails closed. Both gates use it for Argent and Braavos. The multisig check is a single class read with nothing to keep consistent, so it reads at latest again, as before #548. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
After #548 Braavos is no longer view-only, but chain-ops and the multisig deploy view still said so, and the getBraavosSigningRestriction doc block sat under a stray copy of the Argent probe's comment. isCompatibleAccountDeployed threw "RPC returned an invalid Argent account class hash" for a malformed class answer on any family, Braavos included. It now says "RPC returned an invalid account class hash" and translates to "Couldn't read this account. Try again." (it had no friendly copy and was shown verbatim). The multisig deploy view keeps Braavos out of the funding list; only its comment changes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Review nits from #546: isAccountSignabilityResponse accepted any string as accountId, though the id keys the signability cache and is compared with the active account id; it now uses the shared isAccountId grammar, like isDeploymentStatusResponse beside it. A duplicated JSDoc block above RecoveryState is dropped, and isRecoveryState's comment now says what it validates. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
check:coverage already failed on the bundle base (functions 94.86% against the 95% floor); the recovery stack's new side-panel client (runtime-recovery.ts) sat at 20% function coverage, and the scene selection split out of start.ts was only reached through start.ts. sceneContent is pinned branch by branch, including the dashboard's positional argument mapping and the rule that class-hash verification is shown only for the account it belongs to (a mutation that drops that rule fails). The recovery client is pinned on its four message types, the fallback reason for a malformed reply, and passing an error reply's own reason through. Coverage is now functions 95.07%, lines 95.45%. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Review of #550: an undeployed modern-class Braavos account reached the automatic deploy-then-invoke (and deploy-then-declare) paths, which cannot work for it. The bulk estimate gives every non-Argent family skipValidate, so the DEPLOY_ACCOUNT half carried an empty signature the Braavos constructor rejects; and ensureAccountDeployed was handed the first signed hash as the expected deploy hash, which for Braavos is the deploy signature's parameters hash, so a deployment that did go out would have been reported as status unknown. Invoke estimate/submit and declare estimate/submit now refuse an undeployed Braavos account right after the deployment read, before any signing, estimation, or broadcast: "Deploy this account first (Settings → Deploy account)." for the modern class, and the existing "does not deploy Braavos accounts" for an older base class. ensureAccountDeployed refuses Braavos too, so the aux hash can never be taken for the broadcast one. dApp requests reach the same platform functions, so they are covered by the same gate. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Review of #550: the Home verdict treated a modern-class Braavos counterfactual as signable, so Send and Swap stayed live into a deploy-then-act submission that cannot work for Braavos (and that the transaction path now refuses). The verdict now answers readOnly with a new "undeployed" reason for it, added to ReadOnlyReason and its strict validator. Home greys Send and Swap and says "Not deployed — Deploy this account first (Settings → Deploy account)." instead of "Read only", which would be untrue: the account signs once its own reviewed deployment lands. Shield keeps its existing deploy-first route rather than becoming a dead control. An older base-class counterfactual keeps the "braavos" read-only verdict. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Review of #550: the Braavos deployment path had no platform test through estimateAccountDeploymentFee or submitAccountDeployment, so three mutants survived — Braavos-only skipValidate in the estimate or the submission, the submission estimating with the Argent mock query signer, and dropping the public-key check on the parameters signature. The new tests run both through the real signer against a Sepolia stub: validation stays on, the 15-felt signature carries the implementation at sig[2] and the provider's chain id at sig[12], each stark pair covers the hash it should, and the accepted hash is the last one signed (a node answering with the parameters hash is an unknown outcome). A parameters signature from another key is refused before any estimate or broadcast. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Review of #550: the module header still said OpenZeppelin accounts only, and estimateAccountDeploymentFee claimed it never signs privately, which is false for Braavos - its constructor validates the deploy authorization even on query versions, so the estimate signs a query-version hash (never submittable) with the account key. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Follow-up to the #550 review: an undeployed Braavos account's Send (and NFT send) review degraded to "Couldn't estimate ... retry", offering a retry that can never succeed - the account deploys only through its own reviewed step. The refusal is now single-sourced and typed. UNDEPLOYED_BRAAVOS_REFUSAL and isDeployableBraavos live in wallet-model beside the deployment class (the platform gate, the Home verdict and notice all read them), and the platform gate throws UndeployedBraavosError, split into its own module by the size budget and re-exported from deployment. reviewWalletFundedFees throws it up front for an undeployed modern-class Braavos account, before any estimate (so the deployment's query signature is never produced), and passes the platform's typed error through if the two deployment reads disagree. The handler error reaches the panel as "Deploy this account first (Settings → Deploy account)."; every other estimate failure stays a retryable review, matched by type, not text. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Follow-up to the #550 review: a dapp invoke from an undeployed Braavos account was reviewed like any undeployed account ("The network fee is estimated once this account is deployed."), which suggests the request would deploy it - it cannot; the account deploys only through its own reviewed step. estimateInvokeApprovalFee now answers a deploy-first review for it, by account state before any estimate or signer is built, or on the platform's typed UndeployedBraavosError; every other failure keeps the retryable state. The new `deployFirst` flag travels on PendingApproval (queue, snapshot, fee updates - a fresh review replaces it rather than inheriting it) and the strict validator accepts it only in its one consistent shape on an invoke. The card shows "Deploy this account first (Settings → Deploy account)." with no fee and no retry, Approve stays disabled, and a forged approval is refused with the same text. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Follow-up to the #550 review: the review said neither estimate signs, which stopped being true in general when Braavos deployment estimates began signing a query-version deploy with the account key. It holds here only because an undeployed Braavos account is now refused before either estimate; the comment says that, and why the order matters. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Follow-up to the #550 review: the new "undeployed" Home verdict had no UI Lab checkpoint. walletDashboard now takes the signability verdict (absent renders a normal account, as before), the portfolio scenario gains "undeployed-braavos", and its story asserts the "Not deployed" notice and its deploy-first line, disabled Send and Swap, and no "Read only". README checkpoint total recounted from walletScenarios: 208. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Integration fix: three comments added by the #550 fixes used the word the secret-material boundary check reserves for key/vault/signing modules (scripts/file-standards-policy.ts). Reworded the comments; no code change, and the approved-file list is not widened. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The pending Ready flag said "Your guardian can still stop this". The account's cancel_escape needs the owner and the guardian signatures together, so the guardian alone cannot stop it and neither can the phrase. State that instead. The Ready confirm label also claimed the account would be "usable in 7 days" when completing an escape is immediate; drop the delay from the label. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The removal request was estimated with skipValidate, so the quoted fee skipped the account's __validate__ work. Estimate with validation on, as a Braavos deployment does (the phrase key signs only the query-version hash). The review now states a maximum fee: the validated spend plus the wallet's stated 50% headroom, never above the account's 100 STRK cap. The submission is held to that figure exactly (and to the cap, and to STRK), so the number a confirmation shows is a true ceiling. The review also returns the moment the removal takes effect if sent now (latest block time plus the account's delay), refuses a request already pending, and refuses Ledger accounts. At submit the signer re-checks the unlock session after signing and reports the hash before broadcast, the accepted hash must match the signed one (matchingAcceptedRpcWriteHash, so a mismatch is post-broadcast ambiguity), and a failure after acceptance is indeterminate rather than a determinate error. The shared guards live in recovery-shared.ts for both recovery flows. Platform tests now pin the exact call, the validated estimate, the ceiling and cap, the lock and hash checks, the account refusals (including an expired request and a foreign stark key), and the submit path. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…nfirmation The guardian escape hardcoded a seven-day security period, read the escape through a second provider, reported a pending owner escape as "none", and held the submit only to the unit and the 5 STRK cap. - The security period comes from get_escape_security_period; the state now carries the completion window (ready_at until ready_at + period, after which the escape expires) and the review states it: estimated from the latest block for a start, the account's own for a completion. - A live owner escape (a guardian-approved key replacement, which trigger_escape_guardian silently cancels) is reported. Starting then needs an explicit replaceOwnerEscape acknowledgement, re-checked at submit. - The submit must name the step the user confirmed; a changed step (an escape that became ready, expired or was cancelled meanwhile) is refused. - Fees: the review states the validated spend plus the stated 50% headroom, capped at 5 STRK; the submit is held to that ceiling exactly, to the cap, and to STRK, with a zero tip (inside the 1 STRK tip cap). - Estimates run validated through the Argent query signer, which the account accepts for query-version transactions, so the review never asks a key to sign — no Ledger prompt, no phrase key. Ledger accounts are refused outright: the device could only blind-sign a guardian removal. - Lock re-check, signed-vs-accepted hash matching and post-acceptance indeterminacy come from recovery-shared.ts, as for Braavos. The background now passes the period through; it does not yet send the confirmed step, so a Ready submit fails closed until the confirmation lands in the following commits. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The recovery review/submit handlers move to account-recovery.ts (re-exported
from account-signability so the wallet-ops table is untouched) and change in
four ways:
- A submit names the account its confirmation was reviewed for; a switch in
between is refused. Ready submits must name the confirmed step, and may
carry replaceOwnerEscape: true only (recovery-inputs.ts, exact key sets).
- Ledger accounts are refused before any estimate or signer is built.
- Every accepted submission is recorded to activity ("Start 2FA removal",
"Start guardian removal", "Complete guardian removal"). A lost or
mismatched reply is recorded under the signed hash and returned as the
typed submission-unknown reply, never as a determinate failure.
- Both reviews return one shape for the confirmation: step, maximum fee,
readyAt (Braavos: takes effect after; Ready: completion opens), expiresAt
(Ready's completion deadline) and replacesOwnerEscape.
The signability verdict now carries Ready's completion window and stage for
pending, ready and expired escapes, and reports a pending owner escape as
stage owner-escape instead of nothing. Wire types move to recovery-types.ts
(runtime-types was at the size cap). Key-boundary catalog entries carry live
payloads; the two new refusals have reviewed panel copy.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The recover-account button relabelled itself on the first tap and submitted on the second, showing neither the fee nor what the removal does, and the armed state lived on the button indefinitely. The first press now reviews and replaces the read-only flag's body with a confirmation built from DOM nodes (no HTML sink): what is removed, when it takes effect or can be completed (absolute UTC, from the chain), who can still cancel, the maximum network fee in STRK, and explicit Cancel and confirm buttons. The panel resets on Cancel, after 60 seconds, and with any re-render (it never attaches to a flag replaced while the review was in flight). A submit sends back exactly what was shown: account, fee, step, and the owner-escape acknowledgement only when the panel warned about it. After any submit the panel is spent and Home force-refreshes the verdict. The flag states Ready's completion window and deadline, says when an escape expired (restarting only through the confirmation), warns when the guardian is replacing the key instead of showing nothing, and tells a Ledger account recovery is unavailable instead of offering it. Recovery copy and the notice move to recovery-view.ts and the binding to recovery-ui.ts (size budget); the reply validators to responses-recovery.ts. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
trigger_escape_guardian(None) is rejected in __validate__ while a backup guardian is set (argent/backup-should-be-null), so such an account reached the review only to fail with a raw RPC revert. The platform now reads get_guardian_backup before any estimate or signer: a backup guardian is refused with a stated reason, and a failed or empty read refuses too (fail closed) — transport errors keep their own classification and endpoint fallback. The recovery state carries guardianBackup (absent / present / unknown); the signability verdict maps present and unknown to the stages backup-guardian / backup-unknown, ahead of any other Ready stage, so Home shows no Recover button and says why. Both platform refusals have reviewed panel copy. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Integration fix: - The undeployed-Braavos signability test (from the #550 fixes) used the placeholder account id "a", which the #546 fix now correctly rejects with isAccountId. Use a valid id. - Remove the client test written for the pre-rewrite recovery runtime client; the #552/#553 fixes rewrote that client and runtime-recovery.test.ts covers it (100% functions). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The Ready start's acknowledgement was a bare boolean, so an owner escape cancelled and re-triggered between review and submit was cancelled under consent given for a different one. The recovery state now reports the pending escape with its ready_at, the review returns it as `replaces`, and the submit carries `replaceEscapeReadyAt`: the platform refuses unless it names exactly the escape found at submit. The background parser accepts only a positive safe integer for it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
readEscapeState ignored the escape's new_signer. The owner key alone can trigger_escape_guardian(Some(new)) — from the Ready app, say — and escape_guardian() then installs that guardian, but the wallet showed it as "Removing the guardian", offered "Complete recovery", and its confirmation said the phrase alone would control the account. The Option<SignerStorageValue> serde is now parsed (None = [1], Some = [0, stored_value, signer_type]; anything else fails closed). A guardian escape to a new guardian is a pending change, not a removal: its status reads none, so review and submit never take the complete step; a live one surfaces as stage guardian-change with a neutral notice, and starting a removal (which trigger_escape_guardian resets it for) needs the same ready_at-bound acknowledgement as an owner escape, with its own warning. An expired one is inert. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The Braavos and Ready pre-sign gates read the live class, signers, owner, guardian, backup guardian, pending request and escape at `latest` over several calls, so a decision could combine states that never coexisted. They now run under account-compatibility's atAcceptedBlock (exported for reuse, with its JSON-RPC 24 re-pin), and the two advisory restriction probes take the pinned block identifier (latest stays their default for the scan and the Home flag). For Ready, the escape state the step is decided from is read at the same block as the gate. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The recovery gates reported "owner does not match the wallet signer" for
every restriction other than the one they lift, including an unrecognised
release and (Ready) an unsupported owner type. Each restriction now names
its own cause, reusing the strings the transaction path already throws:
an unrecognised Braavos release, an unsupported Argent owner type, a
changed owner, and for an Argent release "not one this wallet can
recover" (true both of an unknown class and of a known one whose
authorization this wallet cannot produce).
The 2FA/guardian flow also stops borrowing the account-discovery fallback
("Couldn't recover accounts…"): its runtime and background fallbacks are
now its own diagnostics with their own panel copy, as are the escape
decoding refusals.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
handleAccountSignability turned a failed deployment-status read into "undeployed", so a deployed Braavos account flashed "Not deployed" on a transient RPC failure. A failed read is now no verdict: the flag answers signable, as the handler's doc says a failed probe does, and the transaction path keeps its own fail-closed checks. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The re-render test only looked in the live document, where a panel built into the discarded flag is invisible anyway, so removing the isConnected guard survived. It now checks the discarded flag itself and that no expiry timer was armed for it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
styles-base.ts sat at 499 lines. The flow-screen header, grouped-list, option-row and action-row rules move verbatim to core/styles-lists.ts, composed immediately after BASE_STYLES, so SIDEPANEL_STYLES is byte-identical (checked by diffing the composed stylesheet) and styles-base.ts drops to 373 lines. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Within 12 hours of a previous escape attempt the account's assert_valid_escape_parameters rejects the step with argent/last-escape-too-recent, which reached the user as a raw RPC error. At the same pinned accepted block as the gate and the escape state, the step's own clock is now read — get_last_owner_trigger_escape_attempt for a start, get_last_owner_escape_attempt for a completion — and the step is refused unless the block time is strictly after it plus 12 hours, exactly the account's condition. An unreadable clock refuses. As a backstop for a race past that check, an estimate that reverts with exactly that reason (as text or as its felt short-string) maps to the same refusal; any other failure passes through unchanged. Both refusals have reviewed panel copy. The escape-state reads move to argent-escape-state.ts (size budget). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Two advisories published after the last audit made check:audit fail on release/v0.10.5 (the promotion PR and nightly run it): - GHSA-pqg4-j6r4-53mv (critical): shell-quote quote() command injection, affected >=1.8.4 <1.11.0. Reached through web-ext > fx-runner, which pins 1.10.0. Pin 1.11.0. - GHSA-wq5f-xc86-pv6w (high): sharp via its librsvg dependency, affected <0.35.5. Reached through the website's wrangler > miniflare. Bump the existing pin 0.35.4 -> 0.35.5 (libvips 1.3.3 -> 1.3.4). Both are build/dev tooling and not in the extension bundle. The lockfile delta is exactly those packages; check-config-standards pins the new values. Same pattern as the source-map-js pin (#555). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The escape-limit clock read swallowed every failure into "Couldn't check this account's escape limit", so a transport failure never reached withRpcFallback's next endpoint and a JSON-RPC 24 miss on the pinned block never reached atAcceptedBlock's re-pin. The backup-guardian gate read rethrew transport but not the block miss, and the Home state's backup read swallowed both. isRetryableReadError (transport or block-not-found) now propagates from all three; only an answer from the account (or an empty one) maps to its fail-closed result. The other pinned-gate reads (class, signers, owner, guardian, escape, period, Braavos pending request) never caught at all. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…eady_at
An acknowledgement named only the pending escape's ready_at, so a guardian
change and an owner escape at the same ready_at were interchangeable.
The submit now carries replaceEscape: { kind, readyAt } end to end — UI
client, strict background parser (exact keys, known kind, positive whole
ready_at), platform input — and the platform refuses unless both match the
escape found at submit.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
recoveryContext refused an account without publicKeyX with "second factor removal unavailable", whose panel copy blames the connection. Every AccountRef shape carries publicKeyX (AccountRefCore), so the branch could never run; it is removed and the reason documented, and a non-vendor contract type stays refused by the platform gate. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
derek-flossman
requested review from
Ryun1,
adrienlacombe,
l-henri,
omarespejel,
robertkodra and
teddyjfpender
as code owners
October 6, 2026 16:25
This was referenced Oct 6, 2026
l-henri
approved these changes
Oct 6, 2026
l-henri
left a comment
l-henri
left a comment
Contributor
There was a problem hiding this comment.
Approving. Note: I co-wrote the underlying stack (#546/#548/#550/#552/#553), so I focused my scrutiny on @derek-flossman's review-fix commits — the content I did not write — and read the highest-risk changes directly rather than trusting the description.
Verified by reading the code:
- My seven base commits came through intact — the only patch-id drift is a benign
handleSetAccountHiddenimport-list merge (the #540 integration) across three commits plus onestart.tscomment; no logic change. - Braavos class gating (
bab18225) — signing is correctly refused for base/deployment classes and strong signers before the key is touched; only implementation releases on the first stark signer sign. - New-guardian escape safety (
41350e9b) — a guardian escape that installs a new guardian is parsed from itsOptionserde (variant 1/len 4 = removal, variant 0/len 6 = change) and is never presented or completed as a removal. Real safety fix. - Deps (
63e84e77) —sharp0.35.5 andshell-quote1.11.0 lockfile integrity hashes both match the npm registry exactly; versions match the overrides; config guard tightened. Build/dev tooling only. - CI green on this head.
Trusted (not individually read): the remaining ~20 derek hardening commits (fee/hash/lock binding, backup-guardian/Ledger refusals, RPC re-pin), via green CI + the four senior and two bundle reviews with clean mutation runs, and because each is a refusal or a binding (tightening, not loosening).
Non-blocking: the body's "unchanged apart from one import line" slightly undercounts the rebase reconciliation (benign). The on-chain release QA (Sepolia escape/deploy/2FA runs) is a real pre-ship gate, and the revert-after-accept recovery message + fee-ceiling background test remain as follow-ups.
This was referenced Oct 6, 2026
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.
Brings @l-henri's Ready/Braavos stack (#546, #548, #550, #552, #553) into v0.10.5, together with every fix from four independent senior reviews, as separate labelled commits. It also includes a release-blocking dependency audit fix.
How to read this branch
What the stack adds (Henri)
Home says an account is read only…,Disable Shield for a read-only account…Sign for stark-only Braavos accounts(r, s)pair, verified live againstget_signersand the deferred-removal state.Deploy funded Braavos counterfactuals from the walletRecover a Braavos 2FA account…,Show the recovery unlock as an absolute UTC datetimedeferred_remove_signers, behind Braavos's 4-day delay.Recover a Ready account with a guardian…trigger_escape_guardian(None)thenescape_guardian(), behind the account's security period.Review fixes
#546 / #548: signing hardening and line caps
start.tsis back under the cap (scene selection moved toscene-content.ts).0x3957f9…current,0x816dd0…legacy). Base and deployment classes are refused in every gate.get_signers().stark[0], the signer the contract verifies.atAcceptedBlockre-pins live checks on a JSON-RPC 24 miss (3 attempts, then fails closed).isAccountIdused for the account id.#550: deployment
UndeployedBraavosErrorwith the message "Deploy this account first (Settings → Deploy account)."#552 / #553: recovery
ready_at;Integration
styles-base.tssplit, with byte-identical stylesheet output.Dependencies (release blocker)
fix(deps): pin patched sharp and shell-quote. Two advisories published since the last audit madecheck:auditfail onrelease/v0.10.5:shell-quote, viaweb-ext);sharp/libvips, viawrangler).Consent copy (reviewed and approved by the release owner)
Verification (Bun 1.3.3, whole branch at 76cd1f2)
bun run check: passes (2,513 tests, 0 failures)check:coverage: passes (functions 95.17%, lines 95.68%, floor 95%)check:audit: passes (fails on the current release branch without the deps commit)bun run --filter @bramble/ui-lab check: passes (232 tests)build: passestest:e2e: 21/21Reviews:
braavos-account-cairo.Release QA (required before shipping; on-chain)
DEPLOY_ACCOUNTwith class0x3d16c7a…, salt = public key, a 15-felt signature withsig[2] = 0x3957f9…andsig[12] = SN_SEPOLIA, and the address matching Bramble;set_escape_security_period(600): start (calldata[0x1]), wait, complete (escape_guardian, thenget_guardianreads 0);Follow-ups (not blocking)
Supersedes #546, #548, #550, #552 and #553, which will be closed with a pointer here.
🤖 Generated with Claude Code