Skip to content

fix(background): cap concurrently open requests - #1521

Merged
Comp0te merged 3 commits into
developfrom
WALLET-1419-cw-open-request-cap
Aug 26, 2026
Merged

fix(background): cap concurrently open requests#1521
Comp0te merged 3 commits into
developfrom
WALLET-1419-cw-open-request-cap

Conversation

@ost-ptk

@ost-ptk ost-ptk commented Aug 25, 2026

Copy link
Copy Markdown
Member

Description

Hardening on top of the sweep. sign and signTypedData already refuse at MAX_STORED_PAYLOADS = 10 before registering a request; connect, switchAccount, signMessage and decryptMessage had no cap at all — supersede was the only brake, and CANCEL_GRACE_MS = 250 lets a burst register everything first. With the mirror persisting open rows, an unbounded burst would also be a session-storage write-amplification vector.

The sweep (previous PR) is the prerequisite that makes a cap safe: without it, a pinned-open row would consume a cap slot for the whole browser session, turning frozen states into a slow denial-of-approvals.

What changed

  • MAX_OPEN_REQUESTS beside MAX_RESPONDED_TOMBSTONES in windowManagement/reducer.ts. Steady-state concurrency is ~1–2 (one shared approval window plus the Ledger second window) — the cap only bounds the 250 ms burst.

  • windowRequestOpened refuses the write at the cap and returns state unchanged (reference-identical). Open requests are never evicted, and a refused write consumes no ordinal.

  • A silent reducer refusal produces no response by itself, so each of the four capless handlers gets an explicit post-dispatch re-read — the analogue of the existing getPayload(...) == null check. On refusal the dapp receives that method's existing negative shape and openWindow is never called.

  • Cap only — no errorCode. Four of the six response shapes cannot carry one (connectResponse/switchAccountResponse are bare booleans; signMessageResponse/decryptMessageResponse are {cancelled} | {…}), and their *Error creators lack error: true, so the SDK would resolve the dapp promise instead of rejecting (the WALLET-1346 masking defect). Widening the wire shapes is a separate ticket.

  • The mirror's write cap is tightened from an undocumented headroom to the enforced relation MAX_SESSION_ROWS = MAX_RESPONDED_TOMBSTONES + MAX_OPEN_REQUESTS, with a test asserting it.

  • sign/signTypedData (second commit): the same post-dispatch re-read applies to them too — at the cap they refuse with their own MAX_STORED_PAYLOADS-style response (errorCode: tooManyPendingRequests, a real error channel) and never open a window. Their payload actions are dispatched before windowRequestOpened, so on refusal the branch also dispatches windowRequestResponded({requestId}) — the WALLET-1418 orphan mechanism the vault reducer already keys off — reclaiming the stored payload immediately (the windowManagement case no-ops for an id with no open row). Without this, ~20 capless opens plus 10 refused signs would pin every payload slot and kill signing for all origins until the next unlock (review finding). The MAX_STORED_PAYLOADS refusal itself fires earlier and is byte-unchanged.

  • The four capless methods still answer a cap refusal with their negative shapes only — an explicit errorCode for them is WALLET-1436 (wire-shape change + the WALLET-1346 masking fix).

    On the number itself: MAX_OPEN_REQUESTS = 20 is not arbitrary — the vault already admits 2 × MAX_STORED_PAYLOADS (deploys + EIP-712 maps, 10 each) concurrently open sign-family requests, so any smaller cap would fire before the payload caps in the all-sign-family case. That floor is necessary, not sufficient — the four capless methods consume open slots outside its quantifier (review finding; the comment and test are scoped accordingly).

Verification

  • npx jest src/background/ — 936 tests pass; npx tsc --noEmit clean; eslint/prettier clean.
  • windowManagement/reducer.ts stays at 100/100/100/100; handlers/sagas floors hold.
  • Pinned by tests: at the cap the write is refused and state returns reference-identical; each refusal produces the method's negative response and no openWindow call; a completed request frees a slot; sign's existing refusal still fires first; the MAX_SESSION_ROWS relation.

Linked tickets

WALLET-1419

Checklist

  • Make sure this PR title follows semantic release conventions: https://semantic-release.gitbook.io/semantic-release/#commit-message-format

  • If the PR adds any new text to the UI, make sure they are localized — no UI text added

  • Include a screenshot or recording if implementing significant UI or user flow change — background-only, no UI change

  • When this PR affects architecture changes wait for review from Dmytro before merging

@ost-ptk
ost-ptk force-pushed the WALLET-1419-cw-open-request-cap branch from 349f4fb to ca546c5 Compare August 25, 2026 11:26
@ost-ptk
ost-ptk marked this pull request as ready for review August 25, 2026 11:37
@ost-ptk
ost-ptk requested a review from Comp0te August 25, 2026 11:37

@Comp0te Comp0te left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Read the cap end to end — the reducer's refusal path, the six handler read-backs, the session mirror, and the tests — against the merge base. The cap itself is the right shape and the ordinal/no-evict invariants hold. The comments below run along one seam: the refusal is a silent state transition, and the code around it hasn't caught up. One path strands an already-accepted payload; two texts assert the strand can't happen; two tests restate definitions rather than exercise the boundary; and the count that drives the refusal is blind to both origin and provenance.

Comment thread src/background/handlers/sdk-methods.ts
Comment thread src/background/redux/windowManagement/reducer.ts
Comment thread src/background/redux/windowManagement/reducer.ts
Comment thread src/background/handlers/sdk-methods.ts
Comment thread src/background/redux/windowManagement/session-store.test.ts Outdated
Comment thread src/background/redux/windowManagement/reducer.ts
@ost-ptk
ost-ptk force-pushed the WALLET-1419-cw-open-request-cap branch from ca546c5 to ea8f4cf Compare August 26, 2026 06:03
@ost-ptk
ost-ptk force-pushed the WALLET-1419-cw-open-request-cap branch from ea8f4cf to a52509e Compare August 26, 2026 06:32
@ost-ptk
ost-ptk requested a review from Comp0te August 26, 2026 06:45
@ost-ptk
ost-ptk force-pushed the WALLET-1419-cw-open-request-cap branch from a52509e to c9b65ad Compare August 26, 2026 09:56
@Comp0te
Comp0te force-pushed the WALLET-1419-cw-open-request-cap branch from c9b65ad to 13769b9 Compare August 26, 2026 10:15
@Comp0te
Comp0te force-pushed the WALLET-1419-cw-open-request-cap branch from 13769b9 to 6b60ac9 Compare August 26, 2026 10:44
@Comp0te
Comp0te force-pushed the WALLET-1419-cw-open-request-cap branch from 6b60ac9 to efabb5a Compare August 26, 2026 10:55
@ost-ptk
ost-ptk force-pushed the WALLET-1419-cw-open-request-cap branch from efabb5a to 0572598 Compare August 26, 2026 11:21
@Comp0te
Comp0te force-pushed the WALLET-1419-cw-open-request-cap branch from 0572598 to 155942f Compare August 26, 2026 14:52
Base automatically changed from WALLET-1419-cw-reset-cancel-then-clear to develop August 26, 2026 15:01
@Comp0te
Comp0te force-pushed the WALLET-1419-cw-open-request-cap branch from 155942f to 7c0efa9 Compare August 26, 2026 15:01
@Comp0te
Comp0te merged commit 8b645a9 into develop Aug 26, 2026
3 of 5 checks passed
@Comp0te
Comp0te deleted the WALLET-1419-cw-open-request-cap branch August 26, 2026 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants