Status.im Q3 2018 Wallet Bugfixing Retrospective
Over the last couple of months we carried out a series of major bugfixing efforts directed at Status Wallet. The initial motivation behind that was the need to make the switch to Mainnet possible. An overview:
Basically, we wanted to use dapps, exchange SNT and keep kitties in our app. And in order to do that in a responsible way, we needed our Wallet to be in top shape. This lead us to a rather broadly defined idea 170 - wallet improvements that eventually evolved into dedicated product team for Wallet. Early on, we decided to address Wallet in two parallel tracks:
- The existing implementation was subjected to through testing, bugfixing, refactoring to flush out any bugs and security issues.
- Our UX team focused on Wallet redesign testing and validation, with the main goal to learn from the past experiences with Status Wallet and provide the most intuitive crypto experience for our users.
At this point it seems that we are finally starting to run out of critical Wallet bugs, so it makes sense to take the opportunity to look back and summarize what has been done so far and what were the main challenges.
General stats:
Issues closed 184
Bugs fixed 126
Releases 13
The workflow we adopted roughly involved:
- 1Bug detection or reproduction
- 2Aggressive triage and prioritization (more on that in a separate post)
- 3Bugfixing work to figure out the immediate cause and push a PR to patch it up
- 4Reflection time - Can we do this better? Is there a more general solution to prevent this problem and any similar kind of problems from reoccurring?
While actual bugfixing PRs offered immediate solutions, it was actually the last step that allowed us to break from the vicious cycle of never ending stream of bugs by allowing us to get rid of the causes that helped generate them. And it turned out that the vast majority of all bugs were in this or that way related to at least one of these hotspots:
Mainnet usage
Somewhat ironically considering that switching to Mainnet was our main motivation, but indeed not having used Mainnet much prior to this effort was the cause of a number of early high profile bugs. A year ago we were so lulled and comfy with Status only on Ropsten, where Ether is free, network fast and the actions more or less have no consequences. The moment we tried out Mainnet, some issues quickly emerged:
- The infamous token decimals bug that went undetected for quite a while simply because we didn't use a sufficiently diverse token set on test net. The issue became obvious once we switched to Mainnet and used a tad more diverse set of tokens (#4428)
- We see things like scary error messages in a completely different way when they are about real Ether with real value (#4930)
Parallel send transaction flows
There are three main ways to send a transaction from Status: directly from the Wallet tab, within a 1:1 chat and within a dapp. Initially, those three flows used completely separate code to run. This caused a number of inconsistency bugs, simply because of redundant code that wasn't always fully in sync.
We eventually fixed this by unifying all flows into a single robust Wallet Send flow. At first, this was achieved partially, by only unifying wallet and chat transactions into one flow. The main obstacle to achieve this was the old Bot API, so the first iteration involved makeshift "command shortcuts", i.e. a short term cludge that allowed us to bypass the most of Bot API and just redirect the chat commands to navigate to Wallet Send. This helped fix the inconsistencies, but as makeshift solutions usually do, it also carried over a number of its own problems while still inheriting all the Bot API baggage.
To add to the incidental complexity, chat commands were written as JavaScript code evaluated in a jail environment within Status app. If anything, the problem with this architecture was that developing pretty much any non-trivial work required major effort. This additional problem has since then been removed with Jan's awesome Bot API refactor.
Send transaction as a two-step operation
Send transaction initially involved calls to two separate operations on the status-go side: first operation to create a transaction and enqueue it on, and the second operation to sign and send it. Things were done like that in order to support a feature in Wallet that never caught on and was eventually removed anyway (anyone remembers ever using Sign later?)
On the other hand, this caused quite a bit of trouble with transaction flows, since what user experienced as a single operation where they, say, send some Ether to a friend, was actually two consecutive non-atomic operations. All of this lead to particularly awkward situations when user accidentally interrupts the flow between those two operations, e.g. by pressing the device back button on Android. A lot of duct tape was sacrificed to close those gaps until the underlying issue was eventually permanently fixed in a massive refactor by Andrey and Sebastian changed the Send Transaction endpoint to a single atomic operation.
And of course, there were some bugs caused by general code quality and test coverage issues, which we are constantly addressing with reviews and refactoring. It is safe to say that the overall quality of Wallet product has greatly increased over the last few months. Even so, as it is usually the case with any maintenance, our watch does not end here. Now that the existing Wallet baseline is starting to look reasonably stable, we're looking to apply the insights gained from all the work our UX team has done and continue building something we're proud of. Wallet FTW!
In the spirit of cryptokitties, I'll just end this document with a picture of a cute and cuddly animal:
