Obol fees will be applied retroactively to all non-distributed funds in the Splitter

[zobront]

When Obol decides to turn on fees, a call will be made to ImmutableSplitController::updateSplit(), which will take the predefined split parameters (the original user specified split with Obol's fees added in) and call updateSplit() to implement the change.

function updateSplit() external payable {
    if (msg.sender != owner()) revert Unauthorized();

    (address[] memory accounts, uint32[] memory percentAllocations) = getNewSplitConfiguration();

    ISplitMain(splitMain()).updateSplit(split, accounts, percentAllocations, uint32(distributorFee()));
}

If we look at the code on SplitsMain, we can see that this updateSplit() function is applied retroactively to all funds that are already in the split, because it updates the parameters without performing a distribution first:

function updateSplit(
    address split,
    address[] calldata accounts,
    uint32[] calldata percentAllocations,
    uint32 distributorFee
)
    external
    override
    onlySplitController(split)
    validSplit(accounts, percentAllocations, distributorFee)
{
    _updateSplit(split, accounts, percentAllocations, distributorFee);
}

This means that any funds that have been sent to the split but have not yet be distributed will be subject to the Obol fee. Since these splitters will be accumulating all execution layer fees, it is possible that some of them may have received large MEV bribes, where this after-the-fact fee could be quite expensive.

Recommendation

The most strict solution would be for the ImmutableSplitController to store both the old split parameters and the new parameters. The old parameters could first be used to call distributeETH() on the split, and then updateSplit() could be called with the new parameters.

If storing both sets of values seems too complex, the alternative would be to require that getETHBalance(split) <= 1 to update the split. Then the Obol team could simply store the old parameters off chain to call distributeETH() on each split to "unlock" it to update the fees.

(Note that for the second solution, the ETH balance should be less than or equal to 1, not 0, because 0xSplits stores empty balances as 1 for gas savings.)

[boulder225]

Hey team! Please add your planning poker estimate with Zenhub @OisinKyne @samparsky