Immediately transfer tokens when the vesting period has ended

[PaulRBerg]

When end time <= block timestamp, transfer the airdrop amount directly to the user, without creating a stream.

cc @sablier-labs/frontend, this may have implications for the informatinal tooltips in the UI

Discussed in #33

^{Originally posted by PaulRBerg December 23, 2024}

[razgraf]

It will have implications to indexers and app logic too. This makes campaign actions into a mixture of Instant and Airstreams actions, since the Claim event can now yield either a simple transaction (the transfer, post condition) or a streamId (pre condition). Won't be a problem, just something to have in mind.

[PaulRBerg]

Right. Would it help if we emitted different events?

[razgraf]

Event will be required for sure! But we could simply emit a Claim event with streamId set to 0 if we don't want to refactor too much. That'll help mark it as a direct transfer instead of a stream claim.

[PaulRBerg]

emit a Claim event with streamId set to 0 if

Yeah, that should work well. I wonder if a boolean flag would be helpful, though? That would be slightly clearer

[smol-ninja]

@razgraf since you suggested setting streamId to 0 for direct transfers. To recall, we have the following distinct Claim events:

1. Merkle Instant: event Claim(uint256 index, address indexed recipient, uint128 amount)
2. Merkle Lockups: event Claim(uint256 index, address indexed recipient, uint128 amount, uint256 indexed streamId)
3. Merkle VCA: event Claim(uint256 index, address indexed recipient, uint128 claimableAmount, uint128 totalAmount)

Do you think it would be better for UI integration if we just have a common Claim event:

event Claim(uint256 index, address indexed recipient, uint128 amount, uint256 indexed streamId) with the following conditions:

• amount will be claimedAmount in case of VCA.
• streamId will be 0 for Merkle Instant and Merkle VCA.

[razgraf]

It would have simplified things, yes. But since we already support two types of claim events, it doesn't really make a difference right now. So it's a question of simplicity at a contract level.

I do wish to stress on thing with this feature: users (like Polynomial) for Merkle Lockups relied on these contracts issuing a valid stream upon claim. So they may expect streams from Lockup-based campaigns and will (probably) not pay attention to the changelog since it's a bit of an internal logic change (e.g. will be hard to catch for subgraph integrators).

Are we 100% sure we want to still implement this feature, or is it something we can skip?

[smol-ninja]

Are we 100% sure we want to still implement this feature, or is it something we can skip?

@PaulRBerg, since you advocated for this idea. Wdyt?

Another approach to implement it is to trigger a withdrawal from the stream inside the claim if end time <= block timestamp. This would mean the claim function would first create the stream and then withdraw from it if the above condition is met.

A natural behaviour of a claimer could be to withdraw from the stream right after claim if it has been fully vested. So I see no problem in combining these two into a single transaction without causing any issues for integrators, as @razgraf mentioned above.

[razgraf]

Yes, creating the stream and claiming, as a bundle, would be great! AFAIK, the consideration around doing this single transfer instead of the atomic bundle was regarding gas. But that would come at the expense of backwards-compatibility and DX (both internal and external), which is why I'm asking again, before moving forward with this.

[PaulRBerg]

This ultimately boils down to how we signal the vesting/no vesting dichotomy. There appear to be three options:

1. Different event
2. Same event, but setting streamId = 0 and immediately transferring
3. Same event, but creating a stream and immediately withdrawing

I'm against Option 2 because it's an ad hoc solution. 'Sentinel' values are not self-explanatory; an integrator needs to carefully read the docs to understand what streamId = 0 means. On the flip side, two different events are more intuitive.

Option 3 is OK, but I have a slight preference for Option 1 because:

• It leads to the smallest amount of gas consumed, which is relevant on Mainnet.
• It simplifies our accounting. If we go with Option 3, @maxdesalle and our accountants would need to take stock of a new type of revenue-generating tx — one that distributes $2 to the airdrop contract, and $1 to Lockup.
• Generally, it is the cleanest and clearest solution.

users (like Polynomial) ... will (probably) not pay attention to the changelog since it's a bit of an internal logic change

We can tell them about it on Telegram.

I'll ask them what they think about Option 1.

[PaulRBerg]

How about introducing an enum and emitting an enum value as the first parameter of the Claim event?

enum ClaimType {
    VESTING,
    INSTANT
}

Note: naming is not final

[PaulRBerg]

Are we 100% sure we want to still implement this feature

Yes

1. First and foremost, to provide a good UX.
   • We received an explicit feature request for this — see Airdrops in which the tokens are immediately transferred if the vesting period has ended #33
   • It's convenient to not have to perform two txs to claim the underlying tokens when the vesting has ended already.
2. To avoid any potential appeals that we charge users twice (once for the airdrop claim, once for the stream withdrawal).

[smol-ninja]

How about introducing an enum and emitting an enum value as the first parameter of the Claim event

How does introducing an enum solve the problem? It does not standardise the Claim event because there will still be a streamId of value 0 in case of Instant. So, instead of introducing enum, I would vote for different events (option 1) or if having 0 value for streamId is acceptable, then we can go with Option 2.

Also, the argument that "an integrator needs to carefully read the docs to understand" also applies to enums as they are integers only.

I am inclined towards Option 2 but since the problem is relevant to integrators, I will let you and @razgraf make a decision on it.

[razgraf]

I'm in favor of Option 3 to be honest since that offers the least resistance (but good point re. accounting, will influence revenues calculation as well, although that still seems easier to handle than the new non-stream system).

[smol-ninja]

Any follow up comment on this @PaulRBerg?

[PaulRBerg]

@smol-ninja an enum with two possible values is much more intuitive than using streamId == 0 as a sentinel value. The latter cannot be inferred from implicit knowledge — it has to be explicitly understood through active effort.

It does not standardise the Claim event

I wasn't suggesting that. I was suggesting differentiation through enums.

---

I continue to be in favor of Option 1 because what's happening under the hood is different when the vesting period has ended VS when it has not. Two Claim events make sense because they signal different actions that have occurred in the protocol.

Yes, this requires a little bit of extra logic in the subgraphs/integrators' code, but it's not a lot.

Also, on Mainnet, we will be saving a lot of precious gas costs.

I've asked Polynomial for feedback on having multiple Claim events, and will follow up when we hear from them.

[razgraf]

Yes, this requires a little bit of extra logic in the subgraphs/integrators' code, but it's not a lot.

My 2c is it's not little extra logic either. They'd have users receiving two different entities from the same source: a stream vs. pure tokens, instead of a clear single entity like Instant, AVCA and the current version of Merkle Lockup. Therefore, I still think mixing the two systems under the same campaign type would cause misconceptions.

[PaulRBerg]

Polynomial will stop using Sablier because of the fees, so they are no longer a concern.

They'd have users receiving two different entities from the same source: a stream vs. pure tokens,

Exactly, but that's a good thing — the stream exists only for vesting, and if there's no vesting, there should be no stream.

would cause misconceptions

It may cause incomplete implementations where some user only look for streams or only for immediate transfers. But these are not misconceptions, just mistakes which can be easily fixed. The subgraph code will basically have two handlers, one for the first Claim event, and another for the other Claim.

But otherwise, I think that Option 1 will lead to exactly the opposite situation — fewer misconceptions — because we are not creating any superfluous entities.

Creating the stream and then immediately withdrawing from it is like touching your left ear with your right hand.

[PaulRBerg]

In any case, there have continued to be airdrops created on Ethereum Mainnet, and I think this argument alone is strong enough to go with Option 1. Creating a stream and withdrawing from it costs at least 200k gas.

This argument can be reframed like this — it's not fair for end users to pay extra fees (to Ethereum validators, not Sablier!) just for this slightly better developer UX

[smol-ninja]

Is it reasonable to assume that in 90% of lockup campaigns, the vesting period extends beyond the campaign's expiration? if so, in most cases, users will still need to pay the full gas cost (200k) for both creating a stream and withdrawing from it later.

Having said that, by implementing option (1) (different events), this feature would apply to only about 10% of cases (or potentially even fewer?). As a result, the expected savings would be significantly lower ($0.9 \times 200k + 0.1 \times 80k = 188k$) which is ~6%.

Now if we compare this 6% expected saving with the development effort and potential misconceptions (as @razgraf pointed out), is it justifiable?

With option (3), the expected savings would be zero, but there would also be no additional development effort for integrators.

With option (2), the expected saving is 6% with minimal development efforts.

What do you think looks better option from this perspective?

[PaulRBerg]

in 90% of lockup campaigns, the vesting period extends beyond the campaign's expiration?

That may be a reasonable assumption for duration-based vested airdrops, where the vesting period starts when the user claims. However, that doesn't sound reasonable for ranged airdrops (which have become the default option). What's the point of setting the expiration after the vesting end? Users who haven't claimed yet would be rug-pulled.

Speaking of this — I have created an issue in the Interfaces repo to add a disclaimer in case the expiration is earlier than the vesting period end.

the expected savings would be significantly lower ... ~6%

1. This figure doesn't accurately reflect the reality of future airdrops on Sablier — due to the reason provided above.
2. Even if the figure had been accurate, the gas savings would still be worth it. 6% of a large numbers of users is a large number.

if we compare this 6% expected saving with the development effort and potential misconceptions (as @razgraf pointed out), is it justifiable?

Absolutely! I suggest we adopt this heuristic: whenever we have to choose between minimal extra development and better UX (especially economic UX!), let's choose the latter.

be no additional development effort for integrators.

The extra effort is minimal and as I've explained above, my proposed design is clearer from an architectural POV since there are no sentinel values nor any faux-streams that are not-actually-streams.

potential misconceptions

Au contraire — fewer misconceptions with Option 1. See my previous comment.

[PaulRBerg]

Yet another benefit of Option 1 — having fewer faux streams means:

1. A cleaner dashboard for senders and recipients alike
2. An easier job building the metrics dashboard

[PaulRBerg]

Additional thoughts on this to try to persuade you @razgraf and @smol-ninja that Option 1 is the better approach:

• Let us forget about Sablier Lockup and imagine that we were a Solidity development company hired by someone to build their vested airdrop contract.
• Since there would be no Lockup contract, the vesting module may be implemented within the same airdrop contract.
• In that case, when the user claims the airdrop, why on Earth would we start a vesting process when the end time is in the past? Like why would we create a vesting entity in storage?
• Exactly — we wouldn't!
• Another analogy .. if you get a new job and the company offers you shares on the spot as a bonus, you do not sign a vesting legal contract with the end time in the past. There's no point in doing that. You just get the shares directly.

Also: this disagreement reminds me of the VCA discussion, where we ultimately decided to go with implementing the vesting logic within the VCA contract. Here the analogy is that the tokens are immediately disbursed to the user because that's what the contract should do.

[razgraf]

Let's then go with the different events approach. I can see why it might not be the right choice to implement superfluous logic, when analyzed in an independent context.

My own cost analysis still yields a larger effort of integration (separate events, extra state machine end states) so I cannot fully agree that it'd be an easier job / minimal extra effort, but I'm willing to give it a shot and implement it, for enhanced UX long term. It may prove beneficial long term, thus invalidating my cost analysis.

Note: Would be nice to have some diagrams here comparing (1) the different types of merkle airdrops and (2) the end states / lifecycles for each.

[PaulRBerg]

Thanks, guys!

I cannot fully agree that it'd be an easier job / minimal extra effort

I have never said it would be an easier job. Right from the get-go, I have conceded that it'd be more difficult, but minimally, and it is something worth doing per all the reasons provided above (mainly UX).

My own cost analysis still yields a larger effort of integration

Mine did, too.

Would be nice to have some diagrams

Indeed