Allow zero RPS in create function

[PaulRBerg]

This is an idea I first shared here — not allowing rps == 0 in the create function is not an objective necessity.

Users may want to set up the streams in advance, and fund them later.

Why not introduce a adjustRatePerSecondAndDeposit method to enable this behavior? Although, I think that the batch function can also do this, right?

CC @sablier-labs/solidity

[PaulRBerg]

To respond to @andreivladbrg's question here.

in that case why make 2 separate actions instead of one?

Because of what I said in the OP — users may want to set up the streams in advance so that they can share the URLs with the recipients.

This isn't an incredibly helpful feature, but its existence proves that rps == 0 is superfluous.

At a minimum, why not remove the check and save a little gas?

[smol-ninja]

I am in favor of this proposal. This can also be useful for DAOs where they want to create a stream using Governance, then fund it partially using EOAs and then begin it later (i.e. setting RPS non-zero). There may also be other use cases, but I cant think of them right now.

And why introduce adjustRatePerSecondAndDeposit when it can be achieved using the batch function? In the examples repo (cc @andreivladbrg), we can include examples to show how users can use batch to achieve such composability.

[andreivladbrg]

I agree with the proposal.

re adjustRatePerSecondAndDeposit, yes, we can achieve this with batch

[andreivladbrg]

#279

[andreivladbrg]

After seeing @smol-ninja's implementation of this removal, I realized that we actually don't need (as a necessity in the flow contract) the pause and restart functions, as the same result can be achieved with adjustRatePerSecond if we remove the rps = 0 check.

So, the question that comes to mind is: should we remove them and rely only on adjustRatePerSecond? The benefits are:

• Considerably less duplicated code.
• Improved DX, as we have only one function that changes the rps.
• Fewer entry points for modifying the state (assuming excessive functions that modify the state can be a security concern).
• Fewer lines of code, leading to a cheaper audit.

wdyt? @sablier-labs/solidity

[andreivladbrg]

If rps can be 0 in restart, then how its even a restart? It would do nothing. Correct?

correct, it would do nothing, but why would you restart with an rps = 0?
removing it saves gas by removing 2 checks, and makes our check consistent across all functions

Allowing rps to be 0 in adjustRatePerSecond atleast set it to 0.

which is exactly what pause function does

[smol-ninja]

which is exactly what the pause function does

That's true, but setting rps to 0 via adjustRatePerSecond also sets it to zero (doing something), whereas allowing rps to be 0 in restart is doing nothing.

why would you restart with an rps = 0

You wouldn't but lets say if something does that, it would emit a restart event but wouldn't restart the stream technically. Whereas, adjustRatePerSecond with 0 as the new rate per second would adjust it to 0 and emit a relevant event. Its the same argument as if stream is already paused, then pause should fail because even a paused stream can be paused again but it would create confusion by doing nothing. What do you think?

[andreivladbrg]

also sets it to zero (doing something), whereas allowing rps to be 0 in restart is doing nothing.

technically it updates the st to block.timestamp, which means it is doing something

it would emit a restart event but wouldn't restart the stream technically

the restart event would have rps = 0 emitted

Whereas, adjustRatePerSecond with 0 as the new rate per second would adjust it to 0 and emit a relevant event

yes, it would adjust it to zero, which means a pause and we are emitting an AdjustRatePerSecond event instead

then pause should fail because even a paused stream can be paused again

good point, that makes me think if we should remove notPaused completely

[andreivladbrg]

i have an idea, we can remove the internal restart function and use _adjustRatePerSecond and it would revert due to SablierFlow_RatePerSecondNotDifferent if you pass an amount equal to zero (since it is a paused stream which you restart)

[smol-ninja]

Sounds good to me.