Idea to get rid of "BatchLockup" by using the "batch" function or reimplementing the create functions

[PaulRBerg]

Idea

Delete the BatchLockup contract and either (i) use the general-purpose batch function or (ii) re-implement the logic in Lockup.

Rationale

Users interacting with Sablier programmatically have to ERC-20 approve and whitelist our contracts (e.g., in token contracts in which only some addresses are allowed to transfer tokens).

Using one contract (Lockup) instead of two (Lockup plus BatchLockup) would lower the cognitive load for these users.

Here's a recent example:

General-Purpose Batch

The only downside I see with the general-purpose batch is that understanding what happened in the transaction through Etherscan will become slightly more complicated.

Reimplementing the Logic

This would be neater, but the contract size may prevent us from taking this approach.

Feedback

cc @sablier-labs/solidity and @sablier-labs/frontend for feedback

[razgraf]

There are a few other advantages to Batch Lockup

• I've explained this many times but much better UX for creating streams with Etherscan and other UIs that rely on function signature to ask for parameters (since batch gets bytes, there's no way to decode this)
• Allowance, since we can use the same Batch for multiple underlying Lockup contracts as long as the signature does not change

[andreivladbrg]

I've explained this many times but much better UX for creating streams with Etherscan and other UIs that rely on function signature to ask for parameters (since batch gets bytes, there's no way to decode this)

I feel like this argument no longer holds. We keep coming back to this discussion, and the only thing "stopping us" is the Etherscan UX. Which IMO is a small argument since it’s very unlikely for someone to create a batch of streams via Etherscan (why do we have a much friendlier UI for that?). Instead, this functionality is more useful for integrations, whether via contracts or interfaces, and it wouldn’t create any distinction between them.

I’ve made a comparison between Lockup and Flow (dedicated contract vs batch function). Since the token flow is User → Streaming Contract, and NFTs are minted, I believe Flow is more straightforward when using the batch function.

Flow: https://sepolia.etherscan.io/tx/0x3c8f1bb29ca53db699ef4ee7133ff774cb5708a6aee4ddd54d0ea5b85f419acf
Lockup: https://sepolia.etherscan.io/tx/0x6fa4f83697d110663f9c0ca20e59f7bf0222a8bc2bebf0c916549a5626e0bbca

Allowance, since we can use the same Batch for multiple underlying Lockup contracts as long as the signature does not change

Is this really the case? In practice, this has never happened—we’ve always deployed a new version of the Batch contract with each deployment. (and it would have been more helpful if we had three separate contracts for each model, but that is no longer the case)

[PaulRBerg]

@razgraf's point is valid, but @andreivladbrg also made strong points. My thoughts:

• I agree that the 'avoiding redeployment' argument should be discarded on the basis that it's very likely for BatchLockup to change as a result of Lockup changing
• I agree that there should be very few users creating streams via Etherscan, so we should never mind this aspect. However:
  • The UX for analyzing txs on Etherscan will get worse. This is relevant for accountants and data analytsts.
  • There are DAOs creating streams via governance proposals for which the bespoke create functions would be very helpful. A case in point is the integration with Shutter DAO and Decent.
• On the other hand, general-purpose batch functions are sort of becoming an industry standard. AFAIK, many trades on Uniswap are routed through a batch function. If DAOs can encode calldata for Uniswap, they can do it for Sablier, too. Plus, this applies only to batch creating streams. Single streams would still have the standard UX.

Synthesis:

• Reimplementing the create functions in Lockup would be best, but the contract size prevents that. Right?
• Using batch has the issues mentioned above.
• Idea: what if we refactor all create functions to take arrays as inputs? So we only provide create multiple functions.

Conclusion: not an easy call. There are subtle pros and cons regardless of what we do (including doing nothing).

[razgraf]

and the only thing "stopping us" is the Etherscan UX.

Tenderly too. @gavriliumircea and I debug using Tenderly on a regular basis (every week a user has a custom token they can't stream, so we need to simulate what exactly fails) and having an actual form allows us to also do it in a clean way, even on our phones.

For integrations agree there is not much of a difference, at least for the current use-cases (not involving decoding tx data, as we didn't see such a use-case yet).

general-purpose batch functions are sort of becoming an industry standard

They're a smart choice, would love to see them used more! But in practice, outside of us and Uniswap, are there any other protocols using batch? 🤔 Again, not that they shouldn't, just curious.

what if we refactor all create functions to take arrays as inputs?

Cool idea! This would indeed provide a solution for both problems (maintenance and accessibility). I think... Hopefully they parse arrays well.

[PaulRBerg]

Tenderly too. @gavriliumircea and I debug using Tenderly

As discussed here, if push comes to shove, we should prioritize UX over our comfort as developers.

But yes, this is a relevant argument in favor of a clear form (whether BatchLockup or reimplementing create functions in Lockup).

outside of us and Uniswap, are there any other protocols using batch?

Balancer used a similar model, IIRC.

But then, all Safe txs are abstract (not identical with batch, but similar-ish).

Cool idea!

Thanks. I am optimistic that we can make it happen.

[andreivladbrg]

I agree with this proposal.

The best argument for keeping it was this: avoid requiring the user to ERC20.approve all three contracts (LD, LL, LT). Now since we now use a singleton architecture, we no longer have this problem. On the contrary, there are two contracts to approve if we keep it. (as you mentioned)

IMO, maintaining the Batch Lockup contract is not worth it anymore.

Another argument for removing it is to have consistency across the Lockup and Flow versions, as one has a dedicated Batch contract while the other does not. (I think this can also be abstracted at the app level)

[PaulRBerg]

Thanks for confirming.

I actually haven't made that argument about ERC-20 approving; I was referring to the issue faced by the developers quoted in the OP. However, this is another good argument in favor of removing BatchLockup.

And good point about consistency with Flow.

[smol-ninja]

Good points raised from all sides. Personally, I am not in favour of removing BatchLockup. Users who have implemented whitelisting in their ERC20 contracts are likely prepared to handle the additional complexity this introduces. They are already whitelisting several other addresses (admins, treasuries etc), and one more won't offer them a bad experience.

To address the concern raised in the OP, we could add a dedicated section in the docs specifically for these users, informing them on which two contracts need to be whitelisted. The section can have guidelines for different kinds of tokens requirements that we have seen so far.

Beyond the debugging issues mentioned with Etherscan, another reason to retain BatchLockup is the potential requirement to move some functions from the Lockup contract into BatchLockup if contract size become an issue in the future. Functions are withdrawMaxAndTransfer, withdrawMultiple specially those not relying on msg.sender. Keeping BatchLockup allows us room to adapt if needed. Or remove them entirely.

Reimplementing the Logic in Lockup

I would strongly disagree with this approach. We should focus on maintaining only the core logic in the contract to avoid splitting up again due to size limitations.

On the contrary, there are two contracts to approve if we keep it

If I’m not mistaken, senders only need to approve BatchLockup. Please correct me if I’m wrong.

Another argument for removing it is to have consistency across the Lockup and Flow versions

I don’t think consistency between Lockup and Flow is necessary. The two protocols are quite different and can have different designs systems.

My suggestion would be to keep BatchLockup for at least a year. This would give us time to evaluate how much SablierLockup evolves and whether moving functions to BatchLockup becomes necessary. Based on that, we can revisit the decision to remove it or keep it next year. In few months, we will likely be dealing with problems like what to do with existing composite functions, which will be very important decision for us.

[PaulRBerg]

Thanks for your feedback. Good points.

However, I'm curious what you think about my proposal to refactor the create functions in Lockup to take arrays as inputs?

If I’m not mistaken, senders only need to approve BatchLockup. Please correct me if I’m wrong.

This is wrong because permissioned token contracts using whitelists have to whitelist both BatchLockup and the Lockup contracts so that the tokens can be transferred between contracts and white-listed users.

I don’t think consistency between Lockup and Flow is necessary

Agree.

My suggestion would be to keep BatchLockup for at least a year.

Tentatively in favor. But I want to sleep on this proposal some more.

[smol-ninja]

However, I'm curious what you think about my proposal to refactor the create functions in Lockup to take arrays as inputs?

I don't have anything against it.

• If its just an array of the same inputs (Lockup.CreateWithDurations calldata params, LockupDynamic.SegmentWithDuration[] calldata segmentsWithDuration), it will have a simple loop logic. But we will still have to introduce a new struct and pass that as input parameter. So, this would change the API.

struct CreateWithDurationsLD {
  Lockup.CreateWithDurations params;
  LockupDynamic.SegmentWithDuration[] segmentsWithDuration;
}

or go back to previous version where we had separate structs for each model and no shared Lockup.CreateWithDurations, this would make the above struct look cleaner.

• If the logic is similar to BatchLockup, where it only allows one token for each tx and calculates the total amount inside the loop, then we wont have to introduce a new struct but it will have more logic in the loop. This would also increase contract size by a bigger margin compared to the above approach.

In any case, we are looking at breaking changes.

[PaulRBerg]

IMO changing the API is worth it in this case, especially since updating create functions doesn't require changes to the subgraph or maintaining a multiplicity of function signatures in the UI code (whereas for cancel or withdraw, this is necessary).

we are looking at breaking changes

Breaking, yes, but as I explained above, it's a softer kind of break.

---

The logic should be similar to BatchLockup. I will let you decide if we should introduce separate structs or keep the shared model.

[smol-ninja]

How about just using batch in the UI instead of re-implementing the create function, and not using BatchLockup temporarily to see how it goes?

• Tenderly would still work and be able to decode the trace correctly.
• Etherscan input data would be a mess, but it's always a mess when structs are involved.

The UX for analyzing txs on Etherscan will get worse. This is relevant for accountants and data analytsts

It's not true from what I am seeing from Andrei's examples:

1. Using batch: https://sepolia.etherscan.io/tx/0x3c8f1bb29ca53db699ef4ee7133ff774cb5708a6aee4ddd54d0ea5b85f419acf
2. Using batchLockup: https://sepolia.etherscan.io/tx/0x6fa4f83697d110663f9c0ca20e59f7bf0222a8bc2bebf0c916549a5626e0bbca

The Etherscan UX remains the same. The only bad experience is decoding "Input Data," but as I said, it's bad in both cases.

What do you think? What are your arguments against this?

[PaulRBerg]

Interesting idea to use batch in the UI, but the frontend team won't have time for this refactor any time soon.

The Etherscan UX doesn't stay the same.

[PaulRBerg]

I'm still on the fence about this decision.