ElasticSwap contest - 0x0x0x's results

Handle

0x0x0x

Vulnerability details

It costs less gas to shift than division. So rather than dividing by a power of 2, right shift can be used to save gas.

Occurrences

./libraries/MathLib.sol:43:        return ((a * WAD) + (b / 2)) / b;
./libraries/MathLib.sol:55:        return ((a + (n / 2)) / n) * n;
./libraries/MathLib.sol:67:        return ((a * b) + (WAD / 2)) / WAD;
./libraries/MathLib.sol:85:            uint256 x = y / 2 + 1;
./libraries/MathLib.sol:88:                x = (y / x + x) / 2;

Handle

0x0x0x

Vulnerability details

removeLiquidity.sol#baseTokenQtyToRemoveFromInternalAccounting is used only once and caching it does cost extra gas. So

        uint256 baseTokenQtyToRemoveFromInternalAccounting =
            (_liquidityTokenQty * internalBalances.baseTokenReserveQty) /
                totalSupplyOfLiquidityTokens;

        internalBalances
            .baseTokenReserveQty -= baseTokenQtyToRemoveFromInternalAccounting;

can be replaced with

        internalBalances.baseTokenReserveQty -= (_liquidityTokenQty * internalBalances.baseTokenReserveQty) / totalSupplyOfLiquidityTokens;

Handle

0x0x0x

Vulnerability details

In Exchange.sol, all functions start with calling isNotExpired to check whether the function call expired or not. Which is implemented as follows:

    function isNotExpired(uint256 _expirationTimeStamp) internal view {
        require(_expirationTimeStamp >= block.timestamp, "Exchange: EXPIRED");
    }

But comments of all functions describe _expirationTimeStamp as * @param _expirationTimestamp timestamp that this transaction must occur before (or transaction will revert)

This description is slightly wrong, since when _expirationTimeStamp == block.timestamp transaction can still be executed. So a better description would be: * @param _expirationTimestamp timestamp that this transaction will revert after

Handle

0x0x0x

Vulnerability details

Impact

!= 0 is a cheaper operation compared to > 0, when dealing with uint.

Occurrences

./contracts/Exchange.sol:113:        if (tokenQtys.liquidityTokenFeeQty > 0) {
./contracts/Exchange.sol:176:        require(this.totalSupply() > 0, "Exchange: INSUFFICIENT_LIQUIDITY");
./contracts/Exchange.sol:178:            _baseTokenQtyMin > 0 && _quoteTokenQtyMin > 0,
./contracts/Exchange.sol:235:        if (liquidityTokenFeeQty > 0) {
./contracts/Exchange.sol:267:            _baseTokenQty > 0 && _minQuoteTokenQty > 0,
./contracts/Exchange.sol:304:            _quoteTokenQty > 0 && _minBaseTokenQty > 0,
./libraries/MathLib.sol:125:        require(_tokenAQty > 0, "MathLib: INSUFFICIENT_QTY");
./libraries/MathLib.sol:127:            _tokenAReserveQty > 0 && _tokenBReserveQty > 0,
./libraries/MathLib.sol:266:            baseTokenQtyDecayChange > 0,
./libraries/MathLib.sol:336:            quoteTokenQtyDecayChange > 0,
./libraries/MathLib.sol:347:        require(quoteTokenDecay > 0, "MathLib: NO_QUOTE_DECAY");
./libraries/MathLib.sol:388:        if (_totalSupplyOfLiquidityTokens > 0) {
./libraries/MathLib.sol:496:                _baseTokenQtyDesired > 0,
./libraries/MathLib.sol:500:                _quoteTokenQtyDesired > 0,
./libraries/MathLib.sol:606:            _baseTokenReserveQty > 0 &&
./libraries/MathLib.sol:607:                _internalBalances.baseTokenReserveQty > 0,
./libraries/MathLib.sol:664:            _baseTokenQty > 0 && _quoteTokenQtyMin > 0,