fix(dialog): only clear accounts on explicit wallet_disconnect #1040
Conversation
Previously, dialog accounts were cleared whenever a request came in without an explicit `account` field (e.g. wallet_sendCalls without `from`). This caused accounts to be incorrectly cleared on normal requests, requiring re-authentication. Now accounts are only cleared when the request method is explicitly `wallet_disconnect`, preserving account state through requests that simply don't specify a `from` address. Fixes regression from ithacaxyz#1009
|
|
Someone is attempting to deploy a commit to the Ithaca Team on Vercel. A member of the Team first needs to authorize it. |
commit: |
|
|
||
| // Clear accounts on dialog init. If the parent has a connected account, | ||
| // it will be synced via when the first request comes in (requireAccountSync). | ||
| porto._internal.store.setState((x) => ({ ...x, accounts: [] })) |
There was a problem hiding this comment.
This change regresses account persistence for per-action Porto initialization
Our use case
We don't initialize Porto on page load because the library is substantial and most users visiting a page won't need wallet functionality. Instead, we initialize Porto on-demand when the user clicks a button that requires wallet interaction (e.g., "Connect Wallet" or "Send Transaction").
This means:
- User clicks "Connect Wallet" → Porto initializes, dialog opens, user connects
- User clicks "Send Transaction" → Porto dialog re-initializes for this action
- The connected account from step 1 should persist and be available for step 2
The regression
Unconditionally clearing accounts in onInitialized breaks this flow:
- User connects → account stored ✓
- User clicks another action → dialog re-initializes
onInitializedfires → accounts cleared ✗wallet_sendCallsarrives withoutfromfield (relies on connected account)requireAccountSyncdoesn't trigger (becauseaccountis undefined in the request)- No account available → request fails
The architectural mismatch
This change assumes onInitialized = "app startup" where no prior state should exist. In our architecture, onInitialized = "dialog opened for an action" and prior state (the connected account) should persist across these initializations.
The parent context maintains the account state, and requireAccountSync exists to sync when needed. But clearing accounts on every init breaks this before the sync logic even runs.
Resolution
I guess I'm not sure what to suggest as a resolution. Just revering this change works for us, but you're clearly aiming for a behavior by clearing on init. If we have to preserve that behavior, perhaps we add an option, or change this to sync from the parent on init? Not sure.
2 participants
Previously, dialog accounts were cleared whenever a request came in without an explicit
accountfield (e.g. wallet_sendCalls withoutfrom). This caused accounts to be incorrectly cleared on normal requests, requiring re-authentication.Now accounts are only cleared when the request method is explicitly
wallet_disconnect, preserving account state through requests that simply don't specify afromaddress.Fixes regression from #1009
NOTE: The original condition
!account && connectedAccount?.addressused the account parameter fromonDialogRequest, which is derivedfromrequest fields likefrom. This was intended to detect "user no longer has an account," but it conflated "request doesn't specify an account" with "user disconnected." The fix makes this distinction explicit by checking the request method.However, there may be edge cases where disconnection occurs and we don't detect it: