Description
Three defects in CantonConnectProvider, found by an adversarial review of PR #47. Grouped because they are all lifecycle handling in the same component and one fix pass covers them.
1. Two overlapping connect() calls leak event listeners permanently. Each call tears down the current wiring, connects, then stores a fresh teardown closure. The second call overwrites the first's closure, so the first wiring's three listeners can never be removed. A later disconnect() unhooks only the last set, breaking the "no wallet events after disconnect" invariant the suite asserts elsewhere.
2. Startup failures pass silently. The mount sequence has no rejection handler, so a failing init() leaves the provider looking idle with no surfaced error and an unhandled rejection. Separately, session restore sets status to connected before reading accounts — so a failing account read leaves connected + not-locked + no party, indistinguishable from a healthy intermediate state. In the failed-connect path, the recovery probe can also throw and replace the original error, so a caller sees a different error than the one recorded in state.
3. disconnect() never clears connectError. A failed attempt followed by a disconnect leaves the stale error on screen.
Steps to reproduce
Leaked listeners: mount a provider with an auto-picker → call connect() twice without awaiting the first (or double-click a connect button that is not disabled) → let both settle → disconnect() → push an accountsChanged from the wallet. The first wiring's handler still fires.
Silent init: make init() reject (bad adapter config) → mount. Status stays idle, nothing is surfaced, console shows an unhandled rejection.
Bad restore state: restore a session where listAccounts() rejects → status is connected, isLocked false, party undefined.
Stale error: cancel a picker so connectError is set → disconnect() → the error is still there.
Expected vs actual behavior
Expected: a second connect() while one is in flight does not start a second attempt; every wiring that is created can be removed; a failed init or restore surfaces an error and does not claim connected; disconnect() returns the provider to a clean slate.
Actual: listeners outlive disconnect, failures are invisible, and a disconnected provider can still show a connect error.
Environment
canton-connect: unpublished, branch feat/1-canton-connect (PR #47)
@canton-network/dapp-sdk: 1.4.0
React: 19, Node: 24
Additional context
To be closed by the #51 PR, which rewrites these same paths — the re-entrancy guard is already in #51's design, and the account read that needs reordering is the one #51 replaces. Filed so the diff carries nothing the board cannot explain.
Description
Three defects in
CantonConnectProvider, found by an adversarial review of PR #47. Grouped because they are all lifecycle handling in the same component and one fix pass covers them.1. Two overlapping
connect()calls leak event listeners permanently. Each call tears down the current wiring, connects, then stores a fresh teardown closure. The second call overwrites the first's closure, so the first wiring's three listeners can never be removed. A laterdisconnect()unhooks only the last set, breaking the "no wallet events after disconnect" invariant the suite asserts elsewhere.2. Startup failures pass silently. The mount sequence has no rejection handler, so a failing
init()leaves the provider looking idle with no surfaced error and an unhandled rejection. Separately, session restore sets status toconnectedbefore reading accounts — so a failing account read leavesconnected+ not-locked + no party, indistinguishable from a healthy intermediate state. In the failed-connect path, the recovery probe can also throw and replace the original error, so a caller sees a different error than the one recorded in state.3.
disconnect()never clearsconnectError. A failed attempt followed by a disconnect leaves the stale error on screen.Steps to reproduce
Leaked listeners: mount a provider with an auto-picker → call
connect()twice without awaiting the first (or double-click a connect button that is not disabled) → let both settle →disconnect()→ push anaccountsChangedfrom the wallet. The first wiring's handler still fires.Silent init: make
init()reject (bad adapter config) → mount. Status staysidle, nothing is surfaced, console shows an unhandled rejection.Bad restore state: restore a session where
listAccounts()rejects → status isconnected,isLockedfalse,partyundefined.Stale error: cancel a picker so
connectErroris set →disconnect()→ the error is still there.Expected vs actual behavior
Expected: a second
connect()while one is in flight does not start a second attempt; every wiring that is created can be removed; a failed init or restore surfaces an error and does not claimconnected;disconnect()returns the provider to a clean slate.Actual: listeners outlive disconnect, failures are invisible, and a disconnected provider can still show a connect error.
Environment
canton-connect: unpublished, branch feat/1-canton-connect (PR #47) @canton-network/dapp-sdk: 1.4.0 React: 19, Node: 24Additional context
To be closed by the #51 PR, which rewrites these same paths — the re-entrancy guard is already in #51's design, and the account read that needs reordering is the one #51 replaces. Filed so the diff carries nothing the board cannot explain.