ℹ️ This library maintenance status

[nikitaeverywhere]

Hi @DmitryBespalov @MisterVants @llbartekll and others to whom this may concern.

We, I guess same as many companies around are looking at WalletConnect v1 integration.

We're wondering what's the maintenance status of this library, basically selecting between the two options available:

1. GitHub - trustwallet/wallet-connect-swift: WalletConnect Swift client SDK : archived, not official client
2. GitHub - WalletConnect/WalletConnectSwift: WalletConnect Swift SDK : not archived, official client

Could you please briefly tell which one should we go with since both are not getting updates? This one?

[DmitryBespalov]

Hi, please use this repo if you need v1 integration.

For the considerable time we (original authors) are swamped at the main job project and couldn't dedicate attention and time to this repo to properly resolve the issues and make the SDK user-friendly.

We do have it integrated in the safe-global/safe-ios. You can use our code as example or as is (or if you want and can extract our handling code in a lib or additional package for others to use too, that also works - in any case, contributions are very welcome! 😄).

I would recommend to use the v1.6.0 instead of 1.6.2 because unfortunately I did a bad job with updating dependencies and I think the latest version doesn't work in some use cases (with disconnecting).

[DmitryBespalov]

@ZitRos interesting what is your use case? wallet or a dapp (or both sides)?

[nikitaeverywhere]

Thanks a ton for getting back, makes it very clear. Some status info we're missing in the readme or docs for now, I swear it's valuable ^^

Our use case is Wallet. We're integrating v1 since all DApps are on it, and later we will maybe integrate v2 once it's stable enough or major DApps start implementing it.

[camilomv17]

@DmitryBespalov This kind of give me some insights of problems we started noticing with Walletconnect, particularly with trust wallet. I'm trying to use the 1.6.0 version but Swift Package Manager is not detecting it, probably something need to be changed on your side, Or probably I'm doing something wrong to add an older version to my project?

Thanks for your help and for all the effort you put on this library.

[camilomv17]

@DmitryBespalov forget about it, it was a conflict between versions of starscream used by other packages.
Thanks again!

[oscahie]

@DmitryBespalov I'm on @ZitRos' team and we've tried to follow your recommendation of using version 1.6.0 but unfortunately that causes a dependency clash with the Starscream ~> 3.1 requirement in that version, as our app currently uses Starscream 4.0.4. Therefore I guess that integrating version 1.6.2 is our only option to move forward.

On my quick tests I have noticed the disconnection issues too, and I see others have reported the same. Do you think that these are issues in WalletConnectSwift's code (e.g. where integrating with Starscream's new API) or rather bugs introduced by Starscream itself? In any case, I see you've mentioned in other comment that you will soon be working on fixing this, which is awesome! Because of project deadlines however, we might still try to troubleshoot those bugs on our own as well. If we happened to find a fix and submitted the changes in a PR, would you be happy to merge them?

[DmitryBespalov]

@oscahie If you fix it and it works, we'll merge after testing it

[oscahie]

Ok, after some playing around with the library here are my findings:

The issues around detecting disconnection actually arise from the switch to use Starscream's socket engine WSEngine with FoundationTransport in 1.6.2. Previously, in 1.6.0, the engine mode selection was automatic based on the OS version and happened within Starscream. That meant that for iOS 13+ Apple's URLSession-based websockets engine was used, which does not seem to suffer these problems. It seems to work quite nicely in fact, though I've only tested on the latest iOS 14. It could be a rougher experience on iOS 13 as sometimes happens when Apple introduces new APIs.

Therefore the obvious solution is to revert that particular change, either back to automatic engine selection, or perhaps to use WSEngine with TCPTransport (NWConnection-based) on iOS 12+ and FoundationTransport on earlier versions. On my brief tests here the TCPTransport option does not suffer of these disconnection issues either, but perhaps it has others that I'm not aware of? Probably there is a good reason why this change was made in 1.6.2 that escapes me.

Note however that Starscream's implementation for the native websockets engine could also do with a few improvements as well. For instance this simple one. Unfortunately the Starscream repo does not seem to be maintained anymore, judging by its last commit (some 10 months ago) and lots of issues and PRs open, so getting fixes from their side seems unlikely.

An alternative approach which I have explored is dropping the Starscream dependency altogether, and rewriting the WebSocketConnection class to use URLSessionWebSocketTask directly instead (Starscream is only a light wrapper for it). Of course this means also dropping support for anything older than iOS 13.0, so it's likely not something you might want to do, but anyway, I did it as a proof concept and could open a fork with it if you'd wanna take a look.

What do you think @DmitryBespalov, would at least a quick PR reverting the engine mode selection change be something you'd want to merge?

[DmitryBespalov]

First of all, thank you for research and work you're doing! 👏

I believe it is a safe bet to drop pre-iOS 13.0 support or even iOS 13. According to the numbers https://developer.apple.com/support/app-store/ 4% of active devices use older versions than iOS 14. In our own project's numbers (gnosis safe), even iOS 13 share was negligible and we support only iOS 14 and above.

Long-term, I would prefer to replace the starscream with the native API, but it is, obviously, a non-trivial change at the moment.

The change that you propose to revert was there because of a problem with the Starscream websockets:
#81 (comment)

@oscahie I'd be OK with reverting the change as long as the connection issues do not appear.

[oscahie]

Awesome then! Like I said, I have a working prototype that doesn't use Starscream, so we could potentially use that directly instead of reverting the change! But at any rate I'm going to try and verify first that the native engine works with Metamask. If so I'll open a PR and let's discuss further there.

In any case we'd like to maintain iOS 13+ support though, due to our own current requirements.

[oscahie]

@DmitryBespalov @zulkis I've opened a PR with my proposed changes to remove Starscream, see #124

[DmitryBespalov]

@oscahie I will take a look today

[DmitryBespalov]

Fixed in v1.7.0

[oscahie]

Thank you so much @DmitryBespalov!

BTW I think that issues #113, #114 and #117 may now be closed as well.

[DmitryBespalov]

Thank you, Oscar!

I want to test that it's fixed before closing it :)

[nikitaeverywhere]

Thank you a ton @DmitryBespalov! We hope not only us but many apps will now update to a fixed library version :)