search

anza-xyz / wallet-standard

Solana extensions to the Wallet Standard.
Apache License 2.0
82 stars 42 forks source link

Fix error issued when disconnecting wallet #14

Closed davide-scalzo closed 1 year ago

davide-scalzo commented 1 year ago

Fixing the error raised when a standard wallet is disconnected. Previously mentioned here https://github.com/solana-labs/wallet-adapter/issues/742 and here https://github.com/solana-labs/wallet-adapter/issues/185

changeset-bot[bot] commented 1 year ago

⚠️ No Changeset found

Latest commit: ac9ddcf5092effef3456e29eff83119027f0c1b8

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

jordaaash commented 1 year ago

This is intended behavior. When a wallet exogenously disconnects (meaning, the wallet in some way triggers the disconnection without the disconnect method being called on the adapter, whether by switching accounts or for any other reason) a WalletDisconnectError is thrown/emitted by the adapter. The code here is interpreting a change in the connected accounts. If previously there were connected accounts and now there are none, it interprets this as an exogenous disconnection. There is no other information about why the wallet was disconnected, and applications should always be prepared to handle errors thrown by the adapter.

jordaaash commented 1 year ago

I think I've found the source of the issue (thanks for your patience!) but we'll need a different change to fix it.

Taking Backpack's Standard Wallet implementation as an example, the expected call chain will look like:

1. standardWalletAdapter.disconnect()
  -> 2. backpackStandardWallet.features['standard:disconnect'].disconnect()
    -> 3. backpack.disconnect()

Here's 1 calling 2: https://github.com/solana-labs/wallet-standard/blob/8940eb4cb943d964a4eec6b47479a65fea4e2726/packages/wallet-adapter/base/src/adapter.ts#L151-L162

Here's 2 calling 3: https://github.com/coral-xyz/backpack/blob/b854f2bf2ab5cf269b09facb0f53c108e5f5b9f6/packages/wallet-standard/src/wallet.ts#L159-L161

The problem starts here: https://github.com/coral-xyz/backpack/blob/b854f2bf2ab5cf269b09facb0f53c108e5f5b9f6/packages/wallet-standard/src/wallet.ts#L134C5-L139

When 3 gets called, Backpack's Standard Wallet implementation is going to emit a change event. Before our call stack finishes, we end up here (the location of your PR): https://github.com/solana-labs/wallet-standard/blob/8940eb4cb943d964a4eec6b47479a65fea4e2726/packages/wallet-adapter/base/src/adapter.ts#L195-L206

What we want is to say "if we are endogenously disconnecting, don't interpret account changes as exogenous disconnections".

I think what we're going to need to do is add an internal disconnecting flag that we'll set inside this block: https://github.com/solana-labs/wallet-standard/blob/8940eb4cb943d964a4eec6b47479a65fea4e2726/packages/wallet-adapter/base/src/adapter.ts#L152-L158C10

And check here: https://github.com/solana-labs/wallet-standard/blob/8940eb4cb943d964a4eec6b47479a65fea4e2726/packages/wallet-adapter/base/src/adapter.ts#L197