Instant unlock + more flexible cliff in LockupLinear

[PaulRBerg]

Discussed in #903

^{Originally posted by smol-ninja April 16, 2024}

[!IMPORTANT]
The below proposal is for future release of Lockup and not v2.2.

Context

In the current design of Lockup linear, the amount unlocked at cliff is a function of the time elapsed. That is, if the cliff timestamp is 30% ahead of the start time, this would unlock 30% of the total amount at the cliff. This makes it less flexible for creating a cliff linear vesting.

[!NOTE]
In a cliff linear vesting, token distributions include a preset amount unlocked at the cliff followed by a linear distribution. The amount unlocks at the cliff can be any and does not need to follow the curve slope.

Existing solution

A Cliff linear vesting can be created using Lockup Dynamic (LD). It has a few problems:

1. It is not intuitive to use LD for creating Linear streams with cliffs.
2. Creating a cliff linear vesting stream is less gas efficient (same argument as why we chose to implement lockup tranched).
3. Despite having a cliff feature in Lockup Linear (LL), it does not support Cliff linear vesting.

Proposed solution

Modify the Lockup Linear design to allow the creation of cliff linear vesting streams. So that,

1. LL can be used to create streams with unlocks instantly followed by a linear curve (as requested by one of our users).
2. LL can be used to create a stream with a fixed amount of unlock at a cliff time followed by a linear curve.
3. it becomes more gas-efficient than doing the same with LD.
4. Existing curves such as Unlock-cliff and Unlock-linear on Sablier UI will become gas efficient. Because they are created using LD at present.

We might also add two percentages representing: (i) instant unlocked amount (ii) cliff amount.

if block.timestamp < cliffTime:
    streamedAmount = deposited * instantUnlockPct # instant unlocked amount
else:
    # elapsed time is defined from cliff time
    elapsedTime = (block.timestamp - cliffTime) / (endTime - cliffTime)
    streamedAmount = deposited * instantUnlockPct # instant unlocked amount
                   + deposited * cliffUnlockPct  # cliff unlocked amount
                    # stream linearly from cliff time
                   + deposited * (1 - instantUnlockPct - cliffUnlockPct) * elapsedTime

[andreivladbrg]

just some notes as i started to think about an implementation and the benefits of increasing the complexity of the LL model for the shapes, which ultimately has benefit of gas efficiency:

Implementation (no broker set)	Gas Usage	Shape
createWithTimestampsLL (cliff set)	132809	-
createWithTimestampsLD (2 segments)	179997	unlock-linear
createWithTimestampsLD (4 segments)	230095	unlock-cliff

---

since we will need 2 new storage variables (UD60x18 instantUnlockPct and UD60x18 cliffUnlockPct), this means 2 more storage slots, which would cause a 21,000 increase for each one, having the new

Implementation (no broker set)	Gas Usage
createWithTimestampsLL (cliff set)	132809
createWithTimestampsLL (cliff set ) instantUnlockPct set	132809 + 21000 = 153809
createWithTimestampsLL (cliff set ) instantUnlockPct + cliffUnlockPct set	132809 + 42000 = 174809
createWithTimestampsLD (2 segments)	179997
createWithTimestampsLD (4 segments)	230095

the gas improvement is still there, but personally not as much as i thought, which makes me wonder if the extra complexity (in create and _calculateStreamedAmount) is going to be worth it if most of the time the LL model is going to be used for classic linear and cliff linear shapes (argument with reference to this comment) wdyt @sablier-labs/solidity ?

---

also, note on the pseudocode in the OP, i believe this version would be better:

function _calculateStreamedAmount(uint256 streamId) internal view override returns (uint128) {
   // --snip --
   if (block.timestamp < cliffTime) {
       streamedAmount = depositedAmount * instantUnlockPct;
   } else {
       initialUnlockedAmount = depositedAmount * instantUnlockPct;
       cliffAmountUnlocked = depositedAmount * cliffUnlockPct;
       streamableAmount = depositedAmount - (initialUnlockedAmount + cliffAmountUnlocked);
       elapsedTimePercentage = (endTime - cliffTime) / (block.timestamp - cliffTime);
       streamedAmount = initialUnlockedAmount + cliffAmountUnlocked + (elapsedTimePercentage * streamableAmount);
   }
}

[smol-ninja]

Thanks for looking into it @andreivladbrg. Out of curiosity, why should we not use uint128 for both instantUnlockPct and cliffUnlockPct and fit them in the same slot? And then use UD21x18 to represent it? Do you see any limitation with that?

If we can do that, then it should be the following, right?

Implementation (no broker set)	LD	LL
unlock-linear	179997	132809 + 21000 = 153809
unlock-cliff	230095	132809 + 21000 = 153809

[smol-ninja]

In your pseudo code,

1. elapsedTimePercentage should have opposite ratio i.e. (block.timestamp - cliffTime) / (endTime - cliffTime).
2. Yes it looks much better.

[andreivladbrg]

why should we not use uint128 for both instantUnlockPct and cliffUnlockPct and fit them in the same slot?

indeed, you are right, it would be better

Do you see any limitation with that?

no, just more conversions, but it should be fine

should have opposite ratio i.e. (block.timestamp - cliffTime) / (endTime - cliffTime).

actually this is not correct

we are having this check (we know for sure that the block.timestamp is bounded between [cliff, end], i.e. endTime - cliffTime > block.timestamp - cliffTime):

v2-core/src/core/SablierLockupLinear.sol

Lines 199 to 202 in 9974448

	uint256 endTime = uint256(_streams[streamId].endTime);
	if (blockTimestamp >= endTime) {
	return _streams[streamId].amounts.deposited;
	}

thus, in your version elapsedTimePercentage would be 0

[smol-ninja]

You're right. I missed that its a percentage.

[andreivladbrg]

I have been working on this today and came across some issues:

1. It doesn’t matter if we use uint128 or uint256 as we need mappings (see benchmark below).
2. In the create function, we need to check if sum(instantUnlockPct, cliffUnlockPct) equals 100% (i.e., 1e18, as we represent these values with 18 decimals). Otherwise, there could be security issues

Function	Contract A Gas Usage	Contract B Gas Usage
num types (uint128 - A / uint256 - B) both vars set	20515	40060
num types (uint128 - A / uint256 - B) single vars set	20199	20046
mapping (uint128 - A / uint256 - B) both set	40213	40213
mapping (uint128 - A / uint256 - B) single set	20123	20123

Click to see solidity tests for gas

// SPDX-License-Identifier: GPL-3.0
pragma solidity >=0.8.20 <0.9.0;

contract A {
    uint128 internal u128A;
    uint128 internal u128B;
    mapping(uint256 => uint256) internal _u128As;
    mapping(uint256 => uint256) internal _u128Bs;

    function setToZero() internal {
        u128A = 0;
        u128B = 0;
        _u128As[1] = 0;
        _u128Bs[1] = 0;
    }

    function setU128NumsAll() internal {
        u128A = 123;
        u128B = 1234;
    }

    function setU128NumsSingle() internal {
        u128A = 123;
    }

    function setU128MapsAll() internal {
        _u128As[1] = 123;
        _u128Bs[1] = 1234;
    }

    function setU128MapsSingle() internal {
        _u128As[1] = 123;
    }

    // Gas test functions

    function testGasSetU128NumsAll() internal returns (uint256) {
        setToZero();
        uint256 gasBefore = gasleft();
        setU128NumsAll();
        uint256 gasAfter = gasleft();
        setToZero();
        return gasBefore - gasAfter;
    }

    function testGasSetU128NumsSingle() internal returns (uint256) {
        setToZero();
        uint256 gasBefore = gasleft();
        setU128NumsSingle();
        uint256 gasAfter = gasleft();
        setToZero();
        return gasBefore - gasAfter;
    }

    function testGasSetU128MapsAll() internal returns (uint256) {
        setToZero();
        uint256 gasBefore = gasleft();
        setU128MapsAll();
        uint256 gasAfter = gasleft();
        setToZero();
        return gasBefore - gasAfter;
    }

    function testGasSetU128MapsSingle() internal returns (uint256) {
        setToZero();
        uint256 gasBefore = gasleft();
        setU128MapsSingle();
        uint256 gasAfter = gasleft();
        setToZero();
        return gasBefore - gasAfter;
    }

    function testAllGasA() public returns (uint256 gasSetU128NumsAll, uint256 gasSetU128NumsSingle, uint256 gasSetU128MapsAll, uint256 gasSetU128MapsSingle) {
        gasSetU128NumsAll = testGasSetU128NumsAll();
        gasSetU128NumsSingle = testGasSetU128NumsSingle();
        gasSetU128MapsAll = testGasSetU128MapsAll();
        gasSetU128MapsSingle = testGasSetU128MapsSingle();
        return (gasSetU128NumsAll, gasSetU128NumsSingle, gasSetU128MapsAll, gasSetU128MapsSingle);
    }
}

contract B {
    uint256 internal u256A;
    uint256 internal u256B;
    mapping(uint256 => uint256) internal _u256As;
    mapping(uint256 => uint256) internal _u256Bs;

    function setToZero() internal {
        u256A = 0;
        u256B = 0;
        _u256As[1] = 0;
        _u256Bs[1] = 0;
    }

    function setU256NumsAll() internal {
        u256A = 123;
        u256B = 1234;
    }

    function setU256NumsSingle() internal {
        u256A = 123;
    }

    function setU256MapsAll() internal {
        _u256As[1] = 123;
        _u256Bs[1] = 1234;
    }

    function setU256MapsSingle() internal {
        _u256As[1] = 123;
    }

    // Gas test functions

    function testGasSetU256NumsAll() internal returns (uint256) {
        setToZero();
        uint256 gasBefore = gasleft();
        setU256NumsAll();
        uint256 gasAfter = gasleft();
        setToZero();
        return gasBefore - gasAfter;
    }

    function testGasSetU256NumsSingle() internal returns (uint256) {
        setToZero();
        uint256 gasBefore = gasleft();
        setU256NumsSingle();
        uint256 gasAfter = gasleft();
        setToZero();
        return gasBefore - gasAfter;
    }

    function testGasSetU256MapsAll() internal returns (uint256) {
        setToZero();
        uint256 gasBefore = gasleft();
        setU256MapsAll();
        uint256 gasAfter = gasleft();
        setToZero();
        return gasBefore - gasAfter;
    }

    function testGasSetU256MapsSingle() internal returns (uint256) {
        setToZero();
        uint256 gasBefore = gasleft();
        setU256MapsSingle();
        uint256 gasAfter = gasleft();
        setToZero();
        return gasBefore - gasAfter;
    }

    function testAllGasB() public returns (uint256 gasSetU256NumsAll, uint256 gasSetU256NumsSingle, uint256 gasSetU256MapsAll, uint256 gasSetU256MapsSingle) {
        gasSetU256NumsAll = testGasSetU256NumsAll();
        gasSetU256NumsSingle = testGasSetU256NumsSingle();
        gasSetU256MapsAll = testGasSetU256MapsAll();
        gasSetU256MapsSingle = testGasSetU256MapsSingle();
        return (gasSetU256NumsAll, gasSetU256NumsSingle, gasSetU256MapsAll, gasSetU256MapsSingle);
    }
}

[smol-ninja]

1. Can we map stream Id to the struct?

struct UnlockPercentage {
    uin128 instantUnlockPct;
    uint128 cliffUnlockPct;
}
mapping(uint256 streamId => UnlockPercentage) internal unlockPercentages;

This way, it will be mapped to a single storage. In your example, you are using two mappings therefore they are using separate slots.

2. I think the check should be instantUnlockPct + cliffUnlockPct <= 100%. These are the percentages of the total amount, and so the difference, 100% - instantUnlockPct - cliffUnlockPct, would be the percentage that is the streamable amount.

[andreivladbrg]

1. Can we map stream Id to the struct

it wouldn't make a change gas wise.

also there could be cases when an user wants:

1. instantUnlockPct = 0% && cliffUnlockPct > 0
2. instantUnlockPct > 0% && cliffUnlockPct = 0

think the check should be instantUnlockPct + cliffUnlockPct <= 100

correct, i was wrong (did a typo), sorry

[smol-ninja]

it wouldn't make a change gas wise

I did not understand why it should not make change gas wise. Can you please explain?

also there could be cases when an user wants

Yes thats true. So whether user wants 0 in one value or non-zero in both, the gas usage would be same because its one slot.

[andreivladbrg]

I did not understand why it should not make change gas wise. Can you please explain?

i was wrong, i tested again and indeed it would be more efficient for case when both variables are set, see this result:

Function	Gas Usage
both	20695
single var set (first)	20694
single var set (second)	20695