WalletConnectSwiftV2 [Dispatching] Native socket support

Due Dilligence

Description and motivation can be found here: https://www.notion.so/walletconnect/Native-Socket-Client-bdf981fa68f24a5e8ea4f285040a8e2c

[ ] Breaking change
[ ] Requires a documentation update

Jun 15 '23 11:06 alexander-lsvk

@alexander-lsvk should I re-review it?

Jul 06 '23 15:07 llbartekll

Hi,

@llbartekll asked me to give some feedback here in response to https://github.com/WalletConnect/WalletConnectSwiftV2/issues/1205 . I was trying to use my own wrapper around URLSession as the websocket for this library and recently gave up due to some recurring issues I couldn't find the time to debug fully. My 2 cents on the issues and some of the comments in the notion document:

Relying on Third party libs: I've always been a "don't use a lib unless necessary" type person, but the current project i'm working on really hammered that home. I spent months dealing with dozens of issues making my project impossible. E.g. I needed changes to the Swift version of the sodium crypto library, the creator used a cocoapods template project to set it up, which for opinionated reasons adds "Quick" and "Nimble" libraries for unit tests. The versions used were incompatible with the latest Xcode, causing all PRs to fail with no meaningful error displayed in github actions. The maintainers weren't really around and would only look at each PR to see if the build status failed. If it did they said they would not merge it and then disappear again. The painful part is that Quick and Nimble weren't actually used apart from a single unit test that cocoapods adds as an example, so all of this pain was completely unnecessary. After dozens of similar issues, i'm very cautious of using third party libraries. I've only been using starscream for 2 weeks now and already have 1 crash report from 1 of our internal testers from limited usage. The protocol supplied in WC2 matches an older version of starscream, I wasn't sure how some of the logic translated to the new one, so i've left it on 3.1.2 for now, and it feels very much like i'm back into the types of issues I had with sodium, caught between multiple versions
Using URLSession: I found it quite easy to get it working in a basic setup, but it times out after 5 minutes of inactivity. I had a lot of trouble getting the reconnection flows to work in WC2 with this issue. Being unfamiliar with the business logic of the project adds some challenges to trying to figure this out. It would work well on simulator + device testing, but some combination of backgrounding / foregrounding / phone going into sleep etc, caused "something" to break a loop somewhere and the socket would just die until the app was killed. Wasn't easy to reproduce and haven't had the time to try investigate it in order to swap starscream back out

In my opinion the need for frequent reconnections is a design issue. I've previously done some work in non mobile spaces (limited web frontend, cloud, IoT, etc) that involved using sockets. Lots of devs love third party sockets like socket.io and starscream, as these custom implementations often don't adhere to the standard timeout rules in the websocket spec, and instead try to keep sockets open artificially longer or indefinitely in situations where it should have timed out. While from an initial developer experience point of view this is handy and easy to setup, it has very poor interoperability with other platforms + implementations. Every project i've worked on with a websocket has always implemented a custom keepalive on the application level. E.g. sending a "ping" every 60 seconds to keep the socket open. To me this is the first thing thats needed in WC2. Reconnections should be logic used for when the app is going to suspend / resume, not just a mechanism to keep the connection alive (outside of network issues obviously). Users will run into all sorts of issue with trying to send a payload to a device thats in the middle of a reconnection

While initially trying to debug this, I was trying to build a reconnect button + popups in my app to allow internal testers to try determine if the socket was down due to an issue connecting to WC server, or due to my handling of reconnections. I found the limitations around automatic versus manual network setup in WC2 to be very strange. I was confused why in automatic mode I was forbidden from calling a function to reset the socket and force it to connect again. I want the SDK to try manage all this for me, but don't see why, when an edge case arises, I can't tell it to manually disconnect + reconnect. I ended up switching to manual mode and handling the sleep / wake up process myself, so that I could add this button for testing. Managing it manually fixed some issues, as being unfamiliar with the logic of the SDK, it was easier and quicker to just clear everything and setup again. But it didn't fix everything (likely needing some kind of queue to manage multiple attempts to connect / reconnect coming in quickly)

I think custom keepalives and the ability to have a semi automatic network setup would go a long way to improving the usage of the library, and i'd be very keen to get rid of starscream and help test a custom implementation. Although my current app is not in production yet, we can run some experiments with beta testers after we get them on boarding in a couple of weeks