MetaMask / metamask-sdk

The simplest yet most secure way to connect your blockchain-based applications to millions of MetaMask Wallet users.
https://metamask.io/sdk/
Other
188 stars 115 forks source link

feat: async termination of connection (using ack from server) #1022

Closed abretonc7s closed 1 month ago

abretonc7s commented 2 months ago

Explanation

A potential issue was discovered if a user terminates their connection without an internet connection.

The behavior to disconnect is to send a terminate message to the server, which will relay it to the dApp to inform that the connection has been destroyed.

However, if the wallet doesn't have an active internet connection, it would delete the connection without sending the event, potentially leaving the dApp uninformed that the connection was destroyed.

In this case, the dApp would need to provide a mechanism to disconnect and create a new connection.

To fix the issue, we have two approaches:

Only remove the connection after the terminate message has been acknowledged by the server. Remove the connection from the UI directly (set the state to removed) and wait for acknowledgment from the server before fully removing it from storage.

References

Checklist

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 67.25664% with 37 lines in your changes missing coverage. Please review.

Project coverage is 78.81%. Comparing base (1e41819) to head (9a56aaf). Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...ackages/sdk-communication-layer/src/KeyExchange.ts 0.00% 15 Missing :warning:
...unication/ConnectionManager/handleAuthorization.ts 50.00% 6 Missing :warning:
...rvice/ConnectionManager/handleJoinChannelResult.ts 0.00% 4 Missing :warning:
...ices/SocketService/EventListeners/handleMessage.ts 62.50% 3 Missing :warning:
...sdk-communication-layer/src/RemoteCommunication.ts 71.42% 2 Missing :warning:
...emoteCommunication/ConnectionManager/disconnect.ts 93.54% 2 Missing :warning:
...ion/MessageHandlers/onCommunicationLayerMessage.ts 0.00% 2 Missing :warning:
...services/SocketService/ConnectionManager/resume.ts 50.00% 1 Missing :warning:
...ketService/EventListeners/handleChannelRejected.ts 0.00% 1 Missing :warning:
...SocketService/MessageHandlers/handleSendMessage.ts 88.88% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1022 +/- ## ========================================== - Coverage 79.16% 78.81% -0.36% ========================================== Files 179 179 Lines 4084 4120 +36 Branches 1013 1016 +3 ========================================== + Hits 3233 3247 +14 - Misses 851 873 +22 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

sonarcloud[bot] commented 1 month ago

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud