Should we merge the SablierV2Linear and SablierV2Cliff contracts?

[PaulRBerg]

Description

It's becoming increasingly more difficult to maintain the SablierV2Linear and SablierV2Cliff contracts. They have a lot of overlapping behavior, but because Solidity doesn't have something equivalent to Rust traits, we have to maintain and test the two contracts separately. Each depends upon a slightly different Stream struct.

Really the only argument against this merge is the gas efficiency of not having to deal with the cliff time. But is it so significant to warrant the maintenance cost?

We should do some back-of-the-napkin calculations to get an idea about the magnitude at play here. If it's a few hundred gas (say), it would totally be worth it from a development POV to merge the two contracts.

Pros

1. Easier for devs to integrate Sablier V2
2. Easier to maintain the contracts and the tests
3. Smaller audit cost

Cons

1. Higher gas cost for cliff-less users (create costs ~11k more, withdraw costs ~2k more)

Conclusion

All in all, I think that merging the two contracts would be worth it. The slightly higher gas cost should be fine. The gas cost is also influenced by the underlying ERC-20 token used for streaming, so whatever small optimization we do here might not matter as much as we'd think it does.

[PaulRBerg]

@andreivladbrg when you finish the gas estimations, could you post them here as a Markdown table? Thanks.

[PaulRBerg]

I just thought about this: keeping the contracts separate would also imply a greater cost for auditing the protocol.

At this point I'm very confident that merging the contracts would be good.

Though it would still be good @andreivladbrg to run the gas calculations, just so we know in hindsight the difference in gas costs.

[andreivladbrg]

Gas cost

SablierV2Cliff	Avg	Median	Max		SablierV2Linear	Avg	Median	Max
cancel	36750	47503	55892		cancel	33862	47503	51781
create	173225	201850	209150		create	161702	179355	186655
createWithDuration	211075	211075	211075		createWithDuration	186619	186619	186619
withdraw	41835	59475	63608		withdraw	39661	57196	59441

I was expecting there to be a significant difference between the create functions because of the extra storage variable, but I am surprised by the difference between the withdraw functions even tho they have pretty much the same implementation.

Gas report generated from this commit: 40f561e

[razgraf]

I'm all for merging the two. My views:

• Complexity: both auditing and maintaining two contracts seems to be adding a lot on our plate, especially taking into account the other monstrosity which is Sablier Pro that will demand 5x more time. This feels like a logical cost-saving strategy on our side.
• Gas cost: at ~12000 average difference between the two, at a random price of 50 GWEI and an ETH price of $1200 we save the user around $1.6. Having in mind that mainnet will be mostly populated by DAOs or companies with vesting schedules I don't think this pricing will be a problem
• Adjacent gas cost: With two contracts, any user that wants to experiment with both will have to approve twice for the same asset.
• Similarity: At the end of the day a linear stream is a cliff stream with the cliff time = 0. We can manage the conceptual split in the middleware (subgraph) or app and label a stream as lineas vs cliff there instead of at the contract level.

[PaulRBerg]

Excellent feedback. I didn't even consider the adjacent gas cost!

Re subgraph labeling - there might not be a need for this, at all.

[PaulRBerg]

Locking this discussion since we ended up implementing this.