fix(dapps) restoring internet connection for the Wallet Connect SDK #15816
Conversation
Jenkins BuildsClick to see older builds (21)
|
stefandunca
force-pushed
the
fix_net_dropping-15598
branch
2 times, most recently
from
3a30386
to
9b93c82
Compare
stefandunca
force-pushed
the
fix_net_dropping-15598
branch
from
9b93c82
to
3cca3a0
Compare
|
I have 2 issues I'm looking on right now on this branch. First would be that I can't disconnect a dapp. The connection was done while being offline. 
|
I'm not sure how would this be possible. Please have a look at the WC logs in debug.
I'm looking into this right now. It is intermitent. Sometime works sometime doesn't. We report undefined for the account address when it doesn't work. |
I think I know why; saw this in storybook with this popup: |
@Seitseman The issue here is that we're trying to select a watch only account. So when in the left we have a watch only account selected and we open the dApp the preselected account is not working. I think this will be fixed by your current work, right? |
hmm, maybe we should fix that! Adding a bug |
Can't reproduce anymore and managed to disconnect one the dapp was properly connected..Was trying out different scenarios with the internet connection on and off |
ui/app/AppLayouts/Wallet/services/dapps/sdk/generated/bundle.js
Outdated
Show resolved
Hide resolved
| name: "Status", | ||
| description: "Status Wallet", | ||
| url: "http://localhost", | ||
| icons: ['https://avatars.githubusercontent.com/u/11767950'], |
There was a problem hiding this comment.
There was a problem hiding this comment.
That is what I found. However, haven't seen in production our icon displayed even though the API to retrieve it is available in the SDK.
ui/StatusQ/src/networkchecker.cpp
Outdated
|
|
||
| void NetworkChecker::checkNetwork() | ||
| { | ||
| QNetworkRequest request(QUrl("http://example.com")); |
There was a problem hiding this comment.
Quoting from the example.com...
We provide a web service on the example domain hosts to provide basic information on the purpose of the domain. These web services are provided as best effort, but are not designed to support production applications. While incidental traffic for incorrectly configured applications is expected, please do not design applications that require the example domains to have operating HTTP service.
There was a problem hiding this comment.
There is already this isOnline flag that's based on the number of peers. https://github.com/status-im/status-desktop/blob/master/src/app/modules/main/view.nim#L270
Wouldn't it do the trick?
There was a problem hiding this comment.
or another idea.. The WC feature depends on the WC services, right? In this case WDYT of pinging their service instead?
There was a problem hiding this comment.
E.g. Windows OS API has some funcs to check if OS is online. Raymond Chen . Could we use their webservices http://www.msftconnecttest.com/connecttest.txt . If not, then in the comments to that post there are links to identical Fedora services
@alexjba , yes, you're right, changes to #15647 should cover discussed case. |
|
Thanks for the review. See answers inline:
Using connected peers would need some testing given the complexity of waku and previous reports with
See answer above regarding using waku network for internet connection availability.
That is a good idea and could also work, however I don't know which is a stable endpoint to check (something that won't reject the connection in some extreme cases). Following on your concerns I found that there are other endpoints used for this purpose that I will try
That could be an improvement for later and will check with the design but for now when we need a quick fix the timeout error that appears on missing connection should be enough. |
Add a new NetworkChecker QObject to StatusQ to be used in checking internet connection status. This is used by the WebEngineLoader to only allow loading of web pages when there is an active internet to cover for a corner case on MacOS where the internet connection is not reestablished if the WebEngineView was loaded without an active internet connection. Closes: #15598, #15806
stefandunca
force-pushed
the
fix_net_dropping-15598
branch
from
3cca3a0
to
fce5f41
Compare
Closes #15598, Closes #15806
HEADfix(dapps) Wallet Connect internet connection reestablishing issueNetworkCheckerQObjecttoStatusQto be used in checking internet connection status.WebEngineLoaderto only allow loading of web pages when there is an active internet to cover for a corner case on MacOS where the internet connection is not reestablished if theWebEngineViewwas loaded without an active internet connection.HEAD~1chore(dapps) remove unused Wallet Connect authentication from the SDKHEAD~2chore(dapps) update Wallet Connect SDK to latest version 1.13.01.11.2to1.13.0HEAD~3chore(dapps) remove the POC wallet connectAffected areas
Wallet Connect dapps
StatusQ checklist
Added
NetworkCheckerused in theWebEngineLoadercomponentImpact on end user
Dapps availability is restored on internet connection restore without user intervention