Auditing the Melon Protocol
“Only half of programming is coding. The other 90% is debugging.”
— Anonymous wisdom
In the run-up to our release on the main Ethereum network, we have arranged a series of audits of the Melon Protocol code. Recently, we’ve had the pleasure of working through the first such audit with Nick Johnson and Martin Swende.
We were quite impressed with the speed and depth of their work and, naturally, the output of the procedure was a set of bugs and inconsistencies they identified in our Solidity contracts, plus some helpful tips for dealing with them. With this feedback, we created a series of tickets in our issue tracker, and tackled them one at a time.
The end-result of these changes was an upgraded Protocol version, complete with, well, let’s get into that!
Bug squashing
As you might guess, the goal of the audit was to find lapses between what we thought the program did, and what it actually did. Finding bugs like this is not necessarily easy, especially when considering the universe of inputs a method can have.
Thankfully, several of these errors were identified over the course of the audit, and patches applied for each of them.
Some of the issues that were solved in the new release were focused on logic errors that were not previously caught. One example was mistakenly marking a function as constant when it actually modifies the state of the contract, which was fixed by simply removing the constant label.
A second example was an invariant breach, where the total number of shares could change within a function, when it should have remained constant. This was addressed by re-designing the function, and assuring the order of operations within it did not allow the breach.
Trimming the file tree
Like some plants, code must be pruned once in a while to get rid of the excess. We removed four full contracts from the project that were either unused, or were easily replaceable by another contract we were using.
Similarly, some code inside contracts we kept was able to be removed, as it was either unreachable or unnecessarily abstract.
We also took this time to re-organize our code at the directory level, and structure the tree in a more logical way. Take the two examples below; with this restructuring it’s easier to find what we want, and to know where to put new files when we create them. Simple yet important.
Getting with the times
Some of the changes requested by the auditors were as simple as using the latest Solidity syntax. This would be things like using require(someCondition) everywhere, instead of manually using throw, which is deprecated.
Also in this category would be switching to the more specific pure and view, rather than just declaring functions as constant.
Enforcing a style
Several of the audit suggestions were more stylistic, requesting that we have a more consistent style across the entire codebase.
Thus, we are currently in the process of enforcing a certain style guide at build time. We are leveraging the ESLint (JS) and Solium (Solidity) linters, such that they throw errors in the CI environment when our code is not up to spec. This ensures that our contracts and programs adhere to a coherent style, improving readability, and therein improving maintainability (and reducing the potential for human error).
Leveraging modularity
Over the past couple releases, have been using Dappsys packages to some extent. These are modular building blocks that can be extended and composed into more sophisticated contract suites.
Some of the auditors’ feedback regarding inconsistencies in our usage of a SafeMath contract led us to switch direction and use the Dapp safe math equivalent everywhere: DS-Math.
Upgrading our tests
Finally, we also took the opportunity of the audit to change our approach to testing a little. We added some tests written in Solidity, using the DS-Test library, and ported our existing JS tests to the more modern Ava test runner. Combined with the improved directory structuring mentioned above, running our tests with Ava can be seen as a general upgrade to the independence of the different tests (i.e. making them more strictly unit tests).
Next up
The next step, after making these changes, is to have a second audit of the Protocol, especially the pieces we have changed.
Working, clean, and secure code are our ultimate goals here. Having several sets of experts go through the codebase, and iterating on each of their suggestions, will go a long way in helping us achieve this.
This blog post is subject to change as the research & development phase is ongoing. Melonport will aim to update blog-posts regularly to represent our latest thinking on a best-efforts basis but there may occasionally be time-lags between latest thinking and updated documentation. With this in mind, the author of this blog assumes no responsibility or liability for any errors or omissions in the content of this blog.
