search

status-im / status-desktop

Status Desktop client made in Nim & QML
https://status.app
Mozilla Public License 2.0
303 stars 79 forks source link

[Wallet Connect] Connection goes to an inconsistent state after personal sign is rejected #16113

Open alexjba opened 3 months ago

alexjba commented 3 months ago

Bug Report

Flow

Used dapp: OpenSea, Rarible, YearnFi

After pasting the connection string, and canceling the Sign request from the dapp, connection gets added to the list of connected dapps, but there is no interactions between dapp and the Status app.

I had to delete cookies on the dapp side and repeat the process, this time with succesfull Sign and all was good.

Recovery by disconnect fails

Used dapp: OpenSea

After disconnecting the dapp from the Status app, I didn't get notification on the dapp side that my account is disconnected, but after refreshing the page the status was "Connecting...", but this could be not our issue if we send notification about disconnection

Shared by @saledjenic in https://docs.google.com/spreadsheets/d/1QkV08jm0ECV01x3yWTLnVGCjTluKQ-IBDJXsHjNeAQg/edit?gid=0#gid=0

alexjba commented 3 months ago

To be checked after adding SIWE support https://github.com/status-im/status-desktop/issues/14996

alexjba commented 19 hours ago

Why we can't fully fix it:

This depends on the dApp implementation. Some dapps don't implement SIWE and will use the personal sign for Sign in. Then the dApp first needs to establish a session and then to ask for the signature. But if the signature is rejected the dApp drops the session without informing WalletConnect. As a result the dApp session remains active in Status, but not on the dApp side.

This can be fixed if the dApp implements SIWE as designed by WC. Then SIWE flow won't establish the session if the user isn't signing the message to prove the account ownership. Another option is for the dApp to disconnect the session if the signing fails and not silently dropping it.

New behavior

The connections management has been updated in the last refactoring and there's a new behavior for this edge case:

  1. The dApp session is established in Status, not established on the dApp side. But now, disconnecting from Status works as expected.
  2. The session can be re-esablished from the dApp side without deleting the cookies or refreshing the page.