Gas issue in stacked splitters

[Evert0x]

The problem with balanceOf()

To see the balance of the tree structure you can call balanceOf() of the root and this will trickle down the balanceOf() calls and sum up the returned values.
A significant gas overhead occurs when doing a deposit or withdraw

     m
     |
     1
     |
    / \
  2    x
 /\
y  z

Numbers are spitters, letters are strategies.

For example on a deposit (or withdraw), 1 will call balanceOf() of both underlying nodes to make a decision where to deposit money. For 2 to get it's balance it will call balanceOf() of y and z, which are more expensive calls as it's touching these yield bearing protocols. So when the runtime is still executing in 1 it will touch the balanceOf() of all x, y and z. If the decision is to go right and deposit into 2, the logic in 2 will again call the expensive balanceOf() of both y and z. As you can imagine this will get more expensive the more splitters are stacked.

Solution

Cache balanceOf() values during runtime of deposits and withdraws in all splitters.

How?

The deposit(), withdrawAllByAdmin(), withdrawAll(), withdrawByAdmin(), withdraw() in the masterStrategy are wrapped by the following modifier

  modifier balanceCache() {
    childOne._prepareBalanceCache();
    _;
    childOne._expireBalanceCache();
  }

The implemenation of the functions in splitters look as follows

  function _prepareBalanceCache() external returns(uint256) {
    cachedChildOneBalance = childOne._prepareBalanceCache();  
    cachedChildTwoBalance = childTwo._prepareBalanceCache();  

    return cachedChildOneBalance + cachedChildTwoBalance
  }

  function _expireBalanceCache() external {
    delete cachedChildOneBalance;
    delete cachedChildTwoBalance;
  }

Splitters will only use these cached values in their _deposit() and _withdraw() function and not call balanceOf() of their childs.

The implementation for nodes is as follows

  function _prepareBalanceCache() external returns(uint256) {
    return balanceOf();
  }

  function _expireBalanceCache() external {  }

Now for every deposit (or withdraw) it will call _prepareBalanceCache() first and store the balances of the childs in the contract.

During runtime in the splitters it will not call balanceOf() anymore, it will use the cachedChildOneBalance. This mitigates the gas issue of calling the expensive balanceOf() function on strategies multiple times during execution.

Implementation

To use a cached variable in the deposit and withdraw functions we need to make sure the cache is always filled during runtime.

In the current implementation the entry point for an external account (e.g. owner) is on the root, every splitter and every strategy. Because all these contract expose admin withdraw functions.

To simplify the cache implementation we want to remove the external account entry points in the splitters, only make them available in the root and strategies. The strategies don't need a cache, as they are always at the bottom of the tree. The root is the only entry point that prepares and expires the cache.

Gas costs

https://github.com/wolflo/evm-opcodes/blob/main/gas.md#a7-sstore

The default value of the cached variables will stay the same outside of runtime. There are some implemenation that use non-zero default values to make transaction cheaper, but using the Istanbul hardfork logic this isn't the case.

The storage writes always go like this (default --> cache --> default)

Using default = 0

0 -> cache
20k gas costs

cache -> 0
100 gas costs
19,9k gas refund

--+
200 gas costs

Using default = !0

!0 -> cache
2900 gas costs

cache -> !0
100 gas costs
2800 gas refund

--+
200 gas costs