client: fix connection persistence bug

[TalDerei]

references penumbra-zone/dex-explorer#154

this is a subtle wallet connection bug. Initially, I thought the culprit was related to mobx's observer HOC, where the connection state wasn't properly being observed. Upon digging deeper, I narrowed in on the client connection implementation, specifically this no-op comment. If the connection is set, indicating an active connection, the nullish coalescing operator prevents initializing a new connection. Debugging the connection state revealed that this no-op behavior becomes problematic in a specific (yet common) case: the connection is set, but the underlying connection is broken. This could be from either the connection timing out, provider detaching, etc. and the no-op behavior prevents the frontend from re-establishing connection, even when the extension is logged in.

Problematically, the same error is encountered when the user manually denies the connection. If this error isn't explicitly caught and propagated back to the caller, then a a denied connection would immediately trigger an attempt to re-establish the connection. For other types of errors, the connect method was modified to clean up the existing connection resources and attempts to establish a new connection to the attached provider.

We should sanity check the security implications of this remediation given that the last audit caught some connection related vulnerabilities.

---

Screen.Recording.2024-12-09.at.10.51.50.PM.mov

[changeset-bot]

🦋 Changeset detected

Latest commit: d558919

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@penumbra-zone/client	Minor
minifront	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[TalDerei]

Is this sufficient for ensuring no dangling connection resources persist?

[VanishMax]

This looks good to me. Should we create a new issue for the disconnect from wallet persisting the connection after page reload, which you showed on the video?

[TalDerei]

This looks good to me. Should we create a new issue for the disconnect from wallet persisting the connection after page reload, which you showed on the video?

sure, it's a separate bug on the dex explorer side. If you directly disconnect from the dex explorer, it behaves properly and prompts a wallet reconnection. But if you first lock the wallet, disconnect from the dex explorer, and refresh, the connection persists as you pointed out.