Oracle Smart Contract Review 3

This article follows Oracle Smart Contract Review 2 where we dived into FixedPoint and SafeMaths to do computations in smart contracts. We've reviews many of the oracle contract dependencies so far. We just have one left.

Epoch

// File: contracts/utils/Epoch.sol

pragma solidity ^0.6.0;

contract Epoch is Operator {
    using SafeMath for uint256;

    uint256 private period;
    uint256 private startTime;
    uint256 private lastEpochTime;
    uint256 private epoch;

    /* ========== CONSTRUCTOR ========== */

    constructor(
        uint256 _period,
        uint256 _startTime,
        uint256 _startEpoch
    ) public {
        period = _period;
        startTime = _startTime;
        epoch = _startEpoch;
        lastEpochTime = startTime.sub(period);
    }

    /* ========== Modifier ========== */

    modifier checkStartTime {
        require(now >= startTime, 'Epoch: not started yet');

        _;
    }

    modifier checkEpoch {
        uint256 _nextEpochPoint = nextEpochPoint();
        if (now < _nextEpochPoint) {
            require(msg.sender == operator(), 'Epoch: only operator allowed for pre-epoch');
            _;
        } else {
            _;

            for (;;) {
                lastEpochTime = _nextEpochPoint;
                ++epoch;
                _nextEpochPoint = nextEpochPoint();
                if (now < _nextEpochPoint) break;
            }
        }
    }

    /* ========== VIEW FUNCTIONS ========== */

    function getCurrentEpoch() public view returns (uint256) {
        return epoch;
    }

    function getPeriod() public view returns (uint256) {
        return period;
    }

    function getStartTime() public view returns (uint256) {
        return startTime;
    }

    function getLastEpochTime() public view returns (uint256) {
        return lastEpochTime;
    }

    function nextEpochPoint() public view returns (uint256) {
        return lastEpochTime.add(period);
    }

    /* ========== GOVERNANCE ========== */

    function setPeriod(uint256 _period) external onlyOperator {
        require(_period >= 1 hours && _period <= 48 hours, '_period: out of range');
        period = _period;
    }

    function setEpoch(uint256 _epoch) external onlyOperator {
        epoch = _epoch;
    }
}

hmmm ... that contract is necessary. All Tomb forks work with epochs. The clock of Tomb forks tick once every fixed period, known as epoch. These periods can be any fixed time span, but 6 hours as become the de facto standard. It would be a bad idea to launch a Tomb fork whose epochs would be different.

On the first line of the smart contract, we see that Epoch is Operator. Ok, looking at the end, we see that the Operator can change the period, and the current epoch.

Bad idea: you don't change the rules once the game has started. setPeriod() and setEpoch() are just suspicious: remove them. Once we've done that, we can get rid of the is Operator and that's another dependency we can say good bye to.

When you look at the contract state variables, you see:

    uint256 private period;
    uint256 private startTime;
    uint256 private lastEpochTime;
    uint256 private epoch;

First: make period a constant. It will show everybody what the period is and that it won't change.

By making period a constant, you also save a storage read (sload call) each time you need that value. This saves 2 100 gas on first use of period, then 100 gas for each subsequent use.

startTime and lastEpochTime are also useless. Stop pretending the time itself has started when you launched your application. We can make time start with the usual Unix Epoch: 1970-01-01 00:00:00 UTC.

The real reason for the Epoch is to divide time in 6 hour long periods. It will work just as well if you start your timeline with the Unix Epoch.

Finally, we have just one state variable left: epoch. We need it to store when the mighty clock of our Tomb fork last ticked. So, this one, we keep it. But uint256 is a bit overkill. uint32 is big enough, and it will save us some space for our other state variables.

Not convinced? uint32 goes up to 4 billions, which is around 1 billion days (4 epochs per day), which takes us around 2 940 000 years from now. Unless you are convinced your application will still be running in almost 3 million years, uint32 is quite enough.

Oracle

// File: contracts/Oracle.sol

pragma solidity 0.6.12;

/*
  ______                __       _______
 /_  __/___  ____ ___  / /_     / ____(_)___  ____ _____  ________
  / / / __ \/ __ `__ \/ __ \   / /_  / / __ \/ __ `/ __ \/ ___/ _ \
 / / / /_/ / / / / / / /_/ /  / __/ / / / / / /_/ / / / / /__/  __/
/_/  \____/_/ /_/ /_/_.___/  /_/   /_/_/ /_/\__,_/_/ /_/\___/\___/

    http://tomb.finance
*/
// fixed window oracle that recomputes the average price for the entire period once every period
// note that the price average is only guaranteed to be over at least 1 period, but may be over a longer period
contract Oracle is Epoch {
    using FixedPoint for *;
    using SafeMath for uint256;

    /* ========== STATE VARIABLES ========== */

    // uniswap
    address public token0;
    address public token1;
    IUniswapV2Pair public pair;

    // oracle
    uint32 public blockTimestampLast;
    uint256 public price0CumulativeLast;
    uint256 public price1CumulativeLast;
    FixedPoint.uq112x112 public price0Average;
    FixedPoint.uq112x112 public price1Average;

    /* ========== CONSTRUCTOR ========== */

    constructor(
        IUniswapV2Pair _pair,
        uint256 _period,
        uint256 _startTime
    ) public Epoch(_period, _startTime, 0) {
        pair = _pair;
        token0 = pair.token0();
        token1 = pair.token1();
        price0CumulativeLast = pair.price0CumulativeLast(); // fetch the current accumulated price value (1 / 0)
        price1CumulativeLast = pair.price1CumulativeLast(); // fetch the current accumulated price value (0 / 1)
        uint112 reserve0;
        uint112 reserve1;
        (reserve0, reserve1, blockTimestampLast) = pair.getReserves();
        require(reserve0 != 0 && reserve1 != 0, "Oracle: NO_RESERVES"); // ensure that there's liquidity in the pair
    }

    /* ========== MUTABLE FUNCTIONS ========== */

    /** @dev Updates 1-day EMA price from Uniswap.  */
    function update() external checkEpoch {
        (uint256 price0Cumulative, uint256 price1Cumulative, uint32 blockTimestamp) = UniswapV2OracleLibrary.currentCumulativePrices(address(pair));
        uint32 timeElapsed = blockTimestamp - blockTimestampLast; // overflow is desired

        if (timeElapsed == 0) {
            // prevent divided by zero
            return;
        }

        // overflow is desired, casting never truncates
        // cumulative price is in (uq112x112 price * seconds) units so we simply wrap it after division by time elapsed
        price0Average = FixedPoint.uq112x112(uint224((price0Cumulative - price0CumulativeLast) / timeElapsed));
        price1Average = FixedPoint.uq112x112(uint224((price1Cumulative - price1CumulativeLast) / timeElapsed));

        price0CumulativeLast = price0Cumulative;
        price1CumulativeLast = price1Cumulative;
        blockTimestampLast = blockTimestamp;

        emit Updated(price0Cumulative, price1Cumulative);
    }

    // note this will always return 0 before update has been called successfully for the first time.
    function consult(address _token, uint256 _amountIn) external view returns (uint144 amountOut) {
        if (_token == token0) {
            amountOut = price0Average.mul(_amountIn).decode144();
        } else {
            require(_token == token1, "Oracle: INVALID_TOKEN");
            amountOut = price1Average.mul(_amountIn).decode144();
        }
    }

    function twap(address _token, uint256 _amountIn) external view returns (uint144 _amountOut) {
        (uint256 price0Cumulative, uint256 price1Cumulative, uint32 blockTimestamp) = UniswapV2OracleLibrary.currentCumulativePrices(address(pair));
        uint32 timeElapsed = blockTimestamp - blockTimestampLast; // overflow is desired
        if (_token == token0) {
            _amountOut = FixedPoint.uq112x112(uint224((price0Cumulative - price0CumulativeLast) / timeElapsed)).mul(_amountIn).decode144();
        } else if (_token == token1) {
            _amountOut = FixedPoint.uq112x112(uint224((price1Cumulative - price1CumulativeLast) / timeElapsed)).mul(_amountIn).decode144();
        }
    }

    event Updated(uint256 price0CumulativeLast, uint256 price1CumulativeLast);
}

At last, here is the long awaited oracle smart contract.

It does not do that much, which is very good, but there are still quite a few things we can shred from it.

First, the start time stuff (see Epoch paragraph).

Then, the oracle does twice as much work as we need. In the context of a Tomb fork, we only care about the Base token price in FTM. We'll never use the inverse, and just in case we are going to need it, it will be easy to compute anyway, so why bother computing it, and storing the result on the blockchain each time we call update()?

Then, if we spend some time thinking, we can get rid of most of the dependencies: FixedPoint, SafeMath.

. . . . .

I've finished with my review of this Oracle contract, that was more or less duplicated at least 400 times on several blockchains.

Unnecessary dependencies, useless state variables, ... typical junior code, like a lot of what we find on the blockchains.

It's ok. The tech itself is junior, and nobody has clearly figured out what web3 really is yet. We'll improve.

And still, there are some brilliant lines of code in that smart contract. The cumulative price hack, used to compute the time average is excellent!!

In the following article, I'll present you my revised, clearer, more understandable, lighter, self contained and gas efficient version of the oracle contract.