search

sablier-labs / v2-core

⏳ Core smart contracts of the Sablier V2 token distribution protocol
https://sablier.com
Other
310 stars 48 forks source link

Implement general-purpose "batch" function in Lockup #1053

Open PaulRBerg opened 5 days ago

PaulRBerg commented 5 days ago

Implement a general-purpose "batch" function in Lockup similar to the batch function in Flow.

Notes: this doesn't mean we should remove the BatchLockup contract. We should keep that for gas optimization purposes, backward-compatibility, and better UX/developer experience.

andreivladbrg commented 22 hours ago

Question: I started working on the batch function for Lockup. As I was about to write tests for it, I realized that with the new renounceMultiple function, we will end up with "multiple" functions for each independent Lockup action (cancel, create, renounce, and withdraw). This made me question: do we really need it? The unique "multiple" functionalities achieved via this function would be related to ERC721 actions (burn/transferFrom) and composite functions. Unlike Flow, here we only have two unique composite action (related to Lockup): withdraw + cancel (or vice versa) and withdraw + renounce

I believe we can implement it since the maintenance costs aren’t high, but I better to ask you as well first @sablier-labs/solidity

smol-ninja commented 21 hours ago

IIRC, the goal with batch in Lockup is to open room for experimentation for new composite functions through UI as well as through external integrators. Other than what you mentioned, another example could be cancel + create (useful if someone wants to change stream shape).

Regarding tests, given that its a simple batch function, what do you think about writing a simple test for it i.e. "if function exists, success, if function does not exist, revert"?

The batch tests in Flow seem like over engineered since we cannot test all the combinations anyways. Those tests are very specific to Flow whereas batch is supposed to be a generic function.

andreivladbrg commented 20 hours ago

example could be cancel + create (useful if someone wants to change stream shape).

good point, agree

looks like we are all on the same page

continuing with the tests without over engineering them 👍

PaulRBerg commented 18 hours ago

Good question @andreivladbrg.

Good points made by @smol-ninja.

On top of cancel + create, the batch function is about allowing for unknown potentially useful combinations in in Lockup.