Reducing noise to improve Signal quality

In this post we will discuss how software quality is maintained and guaranteed in Signal. We’ll go over the use of Continuous Integration, analyse test coverage and explore changes that would improve Signal’s code quality. We conclude by estimating, based on the above, the technical debt in Signal Android.

Since we are unaware of any interesting changes happening in the near future, we will not go further into Signal’s roadmap.

Ensuring software quality in Signal

The software quality of Signal is a collective effort of both the contributors and the Signal team. Contributors are asked to read and follow the contributor guidelines. They describe a number of do’s and dont’s about creating issues and new features. Furthermore, they recommend those who want to contribute to make sure that their contributions are in line with the code style guidelines. Those guidelines can be summarized in three rules. Firstly, methods should be short and self-explanatory, thus not needing comments within them. Secondly, imports should be fully quantified and in a predefined order. Lastly, the naming of variables and fields should follow the conventions as laid out in the guidelines.

To further increase the quality of the code that composes Signal, the built-in Android linter is being used, with a number of rules of which the severity has been modified.

Lastly, Signal deploys both unit tests and UI tests to verify the functionality of the app. Besides the app, a separate set of unit tests make sure that the signal library works as intended.

Ensuring quality, continuously

All the components of the software quality process as discussed before come together in the Continuous Integration (CI) process of Signal. To understand the CI process, we first have to understand how Signal is built.

The build process of Signal is using the Gradle build tool and allows to create different variants of the app. The Gradle configuration file for Signal decomposes the build variant into two components: the build type and the build flavor. A build variant is the combination of a build type and a build flavor. A short summary of the different build types and flavors is given below.

Build types

• Debug Shrinks the size of the produced Android package (APK) using Proguard. This type also configures which Proguard rules for optimization are to be used.
• Staging Extends the Debug type, but additionally configures values for using the remote staging environment such as the URL of the staging content delivery network.
• Flipper Extends the Debug type, but disables the shrinking by Proguard in order to allow the usage of the Flipper debugging tool.
• Release Does not extend the Debug type, but does use the Proguard rules that have been configured in it and explicitly enables shrinking.

Build flavors

• Play This flavor builds a package that can be offered through the Google Playstore.
• Website This flavor builds a package that can be offered through a website. To achieve this, it configures a URL from which the app can fetch its own updates. Although this functionality has been added in 2017¹, there is no sign that this configuration is used at the time of writing.

The CI configuration for Signal is triggered on pushes and pull-requests and consists of two checks. The first check makes sure that all contributors have signed the Contributor License Agreement.

The second check first performs a minor setup of the environment and then executes a single Gradle task to perform quality assurance, which has been visualized below. All used icons are obtained from Pixabay². The quality assurance consists of four separate checks that all have to pass in order for the Gradle task to succeed. The four checks have been summarized below.

Steps in the CI process

• Signal-Android:testPlayReleaseUnitTest This step runs the unit tests of the Android app on the play flavor of the release build type.
• Signal-Android:lintPlayRelease Using the same build variant as the previous step, this step runs the built-in Android linter to enforce code consistency.
• libsignal-service:test Executes the tests of the Signal service library used for secure communication. This step does not involve a specific build variant.
• Signal-Android:assemblePlayDebug Assembles the APK for the play flavor of the debug build type.

Uncovering the coverage

The Signal-Android project only uses JUnit tests to verify functionality, no integration or device tests are used. As seen in the figure below, the overall coverage of the project is rather low.

The Signal library that implements the Signal protocol also has a total of 6% Class coverage and 5% Method coverage, but important to note is that the crypto package, which arguably is the most important, has the highest coverage (46% Class, 48% Method).

Interesting here is that although the Signal protocol is proven to be cryptographically sound³, the given implementation has no such guarantees. However, since many institutions have performed independent audits of the implementation⁴⁵ and all recommended using Signal, the lack of coverage does not imply the software is not secure.

The development cycle that makes use of initial beta releases in order to test new features allows for catching mostly UI bugs and improving usability. Being reluctant with changes to the workings of the protocol, whilst focusing more on new features and bug fixing and relying on the open source verification model keeps the application secure. However the lack of testing in general can be considered to be one of Signal’s main sources of technical debt.

Hot-spots in the codebase

To find out which components are most commonly updated, we used the git effort script, which is part of the git-extras set of tools. It iterates over commits and counts how many changes there have been in each file of the repository. After filtering away build files, configuration files, resources and the Signal library, which is a different repository, we end up with the following list.

From this list, it is clear that a few components within the application can be regarded as so-called hotspots: The ConversationActivity.java and ConversationFragment.java have been altered relatively frequently. This makes sense, as adding new functionality to conversations in Signal might require some refactoring in how the conversation is structured. Secondly, files related to the registration and PIN lock functionality have also been subject to some changes. RegistrationLockDialog.java, RegistrationPinV2MigrationJob.java and CodeVerificationRequest.java are the main files affected. Since Signal has recently improved its registration lock functionality⁶, it is logical for these files to have been altered more frequently. At the very top of the list is the FeatureFlags.java file. These flags are used to disable functionality that is not entirely complete yet. Overall, it is clear that most code is written once, and not frequently edited thereafter.

What’s wrong, and how to fix it

The Software Improvement Group (SIG) has analysed Signal’s codebase, distinguishing code quality in 9 different categories: volume, duplication, unit size, unit complexity, unit interfacing, module coupling, component balance, component independence, and component entanglement. Out of those aspects, we will neither discuss volume, as it is hard to give refactoring recommendations, nor component balance as this is the area in which Signal achieves the highest score. We will address each of the other categories from the category with the highest score down to the lowest. We will describe what each measure means, along with examples of “bad practices” in Signal and a recommendation on how to fix these where applicable.

• Duplication Code duplication means that the same exact code is used at 2 or more places in the codebase which means the code becomes harder to maintain. The best refactoring candidate in Signal is an entire java file Hex.java which is present in both libsignal/service and the util folder of Signal. To refactor this, Signal should remove one of the instances of Hex.java and redirect all usages to the other instance.
• Unit Size and Unit Complexity Unit size describes the size of methods, whenever this grows too large the methods will most likely have more than one responsibility. Unit complexity is when the McCabe complexity⁷ of a method becomes too large which makes it hard to understand and test. Although there are several methods which have 100+ lines, or a McCabe complexity of 50+ in Signal, 2 methods jump out, namely Classic​Open​Helper.​onUpgrade (667 lines, McCabe of 162) and SQLCipher​Open​Helper.onUpgrade (448 lines, McCabe of 83). As the names would suggest, these methods share similar functionality and implementation. Both methods contain large if blocks which would be good candidate places to use helper functions. This would reduce both unit size and unit complexity.
• Unit Interfacing Unit Interfacing represents the number of parameters of a method. One of the methods which is indicated to have bad unit interfacing is Push​Service​Socket.uploadToCdn. We could try to reduce the number of inputs for this method (currently 14!), by for example using a data object to encapsulate some of the used fields. However, if we look at the class as a whole, a bigger problem might be the root cause. The class is almost 1500 lines long and has almost 50 fields. The best approach here would likely be to identify different functionality within the class and split up the class accordingly.
• Module Coupling Module coupling relates to how classes interact with each other. A high coupling often indicates that a class has several responsibilities. Even though a lot of module coupling is present in the Signal Android application, upon closer inspection, the majority of it is only logical, by the way the application is and should be structured. For example, the Recipient.java file contains a lot of module coupling, but as the recipient is directly related to a contact, a job and the corresponding notification, these couplings are not redundant.
• Component Independence and Entanglement A high component independence means that a single component affects many other components which leads to the code being more difficult to reason about. Component entanglement is somewhat similar in that it concerns how components affect each other. For both of these aspects it is important to keep in mind that SIG used the componentization we created and thus not the components defined by Signal itself. The score of both is very low due to the same categories of classes, namely communication and database classes. These classes are both highly entangled and have many dependencies. This is however to be expected as they are key components which are necessary for many operations.

In the end, we can conclude that while Signal has a quite extensive continuous integration system, the tests present leave a large chunk of the code base uncovered. While adding a feature may be relatively straightforward, one may not be sure all existing functionality still works properly. There are a few hot-spots in the code base, where features had to be reworked entirely: This hints at the presence of some technical debt in the ‘accidental prudent’ quadrant ⁸. Although there are still improvements to be made, Signal’s code quality as a whole is adequate.