batch function to replace separate periphery contract

[smol-ninja]

Originally shared here and discussed here in details.

The idea is to include a batch function which can be used to construct an array of calls to multiple public functions from the core contract.

function batch(bytes[] calldata calls, bool revertOnFail) external payable {
    for (uint256 i = 0; i < calls.length; i++) {
        (bool success, bytes memory result) = address(this).delegatecall(calls[i]);
        if (!success && revertOnFail) {
            _getRevertMsg(result);
        }
    }
}

• The above function can be seen as a replacement of the separate periphery contract and it can be used to expose any API through the user interface.

• Since it's a delegate call to itself and batch can only be applied to public functions, this no additional risks are introduced by it.

• No need to remove the noDelegateCall modifier. The delegate call is made from the contract itself and not from an outside proxy contract.

function _preventDelegateCall() private view {
    if (address(this) != ORIGINAL) {
        revert Errors.DelegateCall();
    }
}

In the above function, address (this) == ORIGINAL always.

Example

bytes[] memory calls = new bytes[](2);
calls[0] = abi.encodeWithSelector(ISablierOpenEnded.create.selector, sender, recipient, ratePerSecond, asset);
calls[1] = abi.encodeWithSelector(ISablierOpenEnded.deposit.selector, OpenEnded.nextStreamId(), amount);
openEnded.batch(calls, true);

FWIW Uniswap also has a similar function.

@sablier-labs/everybody.

[PaulRBerg]

Man I so wish that specifying multiple contract entrypoints were a feature supported by the EVM.

This is an interesting proposal.

My only concerns are is (i) security and (ii) developer UX.

• On the first point, since Uniswap uses a similar model, we should be fine, too.
• On the second point, bespoke functions like createAndDeposit would be easier to use compared to this low-level batch approach. But that seems a reasonable cost to pay for fewer functions and fewer tests.

My take:

• Let's proceed with implementing this in a PR and see how it actually looks like in practice

[smol-ninja]

Alright sounds good.

Should we also remove all batch functions from the contract (unless they require special modifier) such as pauseAndRefund, createMultiple etc.? cc @andreivladbrg.

I am in favour of removing them and bring them back in later if we find batch to be very complicated in practice.

[PaulRBerg]

Yeah, let's remove them

[smol-ninja]

Linked issues:

• Implement batch to execute an array of public calls #90
• Remove composite functions #92