Editing Synthetix (section)

= Other issues =

<span id="bad-engineering-practices"></span>
== Bad engineering practices ==

A cursory glance of the Synthetix solidity code shows poor coding practices. For instance: there are multiple places within the code that try to hide errors and anticipate what the user wants rather than crashing (with potentially disastrous consequences.)

One such example of this is the code used to check balances for exchanges. You would expect that if a user doesn’t have enough funds to cover the conversion that the software would crash with some kind of error. But Synthetix defaults to using all of their remaining balance. This is bad because the user might have made a mistake like accidentally getting the order of the pairs backwards or putting a decimal in the wrong place.

[hide_error]

<pre class="solidity">        // when there isn't enough supply (either due to reclamation settlement or because the number is too high)
        if (amountAfterSettlement &gt; balanceOfSourceAfterSettlement) {
            // then the amount to exchange is reduced to their remaining supply
            amountAfterSettlement = balanceOfSourceAfterSettlement;
        }</pre>
Another example is in the issuance code which seems to have some kind of weird logic to detect edge cases where users were ‘accidentally’ allocated more than they should have from an exchange (WTF?) Much of the code is a mind-fuck moment in general.

You can see that the price quotes are divided up into discrete time periods. It wasn’t my job to do a code audit for this project – I only glanced where there was ambiguity – but I have a feeling that the settlement logic around these price periods / edge-cases may be directly vulnerable.

[edge_case]

<pre class="solidity">// and deduct the fee from this amount using the exchangeFeeRate from storage
uint amountShouldHaveReceived = _getAmountReceivedForExchange(destinationAmount, exchangeEntry.exchangeFeeRate);

// SIP-65 settlements where the amount at end of waiting period is beyond the threshold, then
// settle with no reclaim or rebate
if (!_isDeviationAboveThreshold(exchangeEntry.amountReceived, amountShouldHaveReceived)) {
	if (exchangeEntry.amountReceived &gt; amountShouldHaveReceived) {
		// if they received more than they should have, add to the reclaim tally
		reclaim = exchangeEntry.amountReceived.sub(amountShouldHaveReceived);
		reclaimAmount = reclaimAmount.add(reclaim);
	} else if (amountShouldHaveReceived &gt; exchangeEntry.amountReceived) {
		// if less, add to the rebate tally
		rebate = amountShouldHaveReceived.sub(exchangeEntry.amountReceived);
		rebateAmount = rebateAmount.add(rebate);
	}
}</pre>
<span id="misleading-claims"></span>
== Misleading claims ==

The project claims that there is zero price slippage. Such a claim is provably false due to their poor implementation; Their inability to guarantee redemption of funds; Their inability to control external price feeds; And their inability to guarantee order execution time.

<span id="backdoors"></span>
== Backdoors ==

A key benefit of smart contracts is the ability to create protocols that operate deterministically with minimal trust. One can automate a broad number of contracts without ever having to depend on trusted third parties to play a key role in the system.

In Synthetix Exchange the benefits of smart contracts are lost because most aspects of the exchange can be controlled by the contract owners: They are free to suspend trading pairs. Liquidate positions. Set price feeds directly. Change SNX:sUSD ratios. Really, anything that you might want to set in as an investor can be changed by the admins [backdoor].

There are no client-side checks for adverse parameter selection within the contract state. You are trusting that the state in the exchange hasn’t been tampered with, and there is already at least one way to render the whole exchange userless. For example: the admin can set the settlement time to zero and it will make every trade fail forever. In this sense, no contract made on the exchange can ever be considered final or fully trustless.

For what it’s worth: the correct way to design contract systems that need to be upgraded in the future is to build in a signed update channel with clear change logs and allow users to decide whether or not they want to accept an upgrade. Given that Synthetix can change anything they want at any time it also puts them in a position where a lawmaker can easily argue that they are operating an unlicensed exchange. Whereas, if the system were designed securely that could not happen.

<span id="conclusion"></span>