Remove isStream boolean from stream struct

[andreivladbrg]

In this discussion (#271), @smol-ninja correctly pointed out that we can remove the isStream boolean and rely on a sender check to verify whether a stream is null. I agree with his rationale, and I believe we should make this change, as there is no objective necessity for this boolean, and it would save gas in the create function.

---

PS: I’m opening this discussion here because it’s relevant to flow as well, and we need to make a quicker decision since the audit starts next week. This is in contrast to lockup, where we don’t have anything scheduled yet

wdyt? @sablier-labs/solidity

[PaulRBerg]

I disagree based on the rationales I provided in that discussion.

What we should do is keep isStream but remove the sender == address(0) check, just like in Lockup.

Doing so would also create symmetry between the two contracts.

no objective necessity for this boolean

There is. As I said, a user can choose to intentionally set the sender to the zero address. From a logical point of view, it doesn't matter that we block it in the create function.

[andreivladbrg]

I have always wanted symmetry

agree, symetry is important

I had thought it was needed in Flow.Now that it turns out it's not needed in Flow, we should remove it in Lockup (because it's redundant).

there is some misunderstanding from my end because:

Shub said: "Anybody can accidentally set sender as zero address when creating the streams. Now because other functions such as cancel, renounce etc. are sender only, the stream creator would not be able to use them."

and you responded: "Agree with the general point"

do you still agree with this?

[PaulRBerg]

I don't remember the problem situation well. However, I don't think it's relevant anymore as I'm happy to keep the zero address check:

#280 (reply in thread)

[andreivladbrg]

great 🤝

your reply was posted after mines, so i didn't see it prior to it, i was talking about this comment

I don't think it's relevant anymore

you are correct, it is not, we agreed on keeping the zero check

[smol-ninja]

Great. So do we also agree to keep isStream field as well? (I am in favour as per #280 (comment)).

[andreivladbrg]

yep

[smol-ninja]

What we should do is keep isStream but remove the sender == address(0) check, just like in Lockup

We decided to implement a zero check for the sender address as per your comment here, and the relevant PR. Therefore, regarding your symmetry argument, we have it now.

a user can choose to intentionally set the sender to the zero address

Might be possible but I think there cannot be "any" incentive to do so. A stream with the sender as address(0) cannot pause it, cannot restart it, cannot refund from it. Wouldn't the same argument that applies to having a zero address check for the recipient apply here, which is, in my opinion, that accidentally setting them to the 0 address is a possible mistake?

However, logically, a stream can have 0 addresses for the sender and recipient (I am talking in theory; not talking specifically about our implementation), so defining whether a stream is valid or not based on the sender check does not seem logical to me, which, however, is not the case for rps == 0 and isPaused.

[PaulRBerg]

First part — see comment above.

Second part — good point. There can, however, be incentives for setting up a stream with the address zero. Permissionless payroll, i.e., a Flow stream that is guaranteed to be paid to the recipient.

Though I think it's an esoteric use case, so we might just want to keep the check in this case, both here and in Lockup.