Why are there no NFT-s?

[razgraf]

I've noticed that Flow does not have any NFTs. For reference (and possibly debate), would you mind describing the problem and the reasoning behind this decision @andreivladbrg?

[andreivladbrg]

Yes, this is correct, we do not have the NFTs in open-ended contracts (at least not anymore, initially they were implemented).

The rational behind is:

• the product is oriented towards the sender, and the NFT would be minted for the recipient.
• there is no "non-cancelable" optionality (all streams are cancelable).
• no deposit is required, thus there is no exact value of the NFT (it could actually entail "debt").
• the sender is able to retrieve funds from an ongoing stream (but not the amount that has already been streamed).
• there is no fixed duration for the stream.
• it would increase the gas fees and complexity. I started building the open-ended contracts with the goal of being as minimalist as possible, since the main use-case for them is straightforward (salaries with top-ups).
• we would need a new design for the NFT

Having said these points, I consider that there is no value in adding NFTs to the open-ended product, other than to say, "Look, we have NFTs for the payroll contracts, but LlamaPay doesn't."

Also, this NFTs discussion was initially raised by PRB in the first presentation of the contracts here (Spec Questions) and I've answered it this discussion and no counter-argument was presented.

[andreivladbrg]

Also, one comment from a private discussion with @smol-ninja, which I agree with:

"
I think its better if only sender is allowed to change recipient. The most anticipated use case seems to be payroll and from the legal point of view, it might not be a good idea to allow changing stream ownership by the recipient. By introducing ERC-721 also means we will have to introduce a new field similar to isTransferable
"

Later edit: we did not take any decision on this, we were just brainstorming the idea

[smol-ninja]

To correct the quoted comment, we did make a decision on this. We decided to open a discussion on whether open-ended streams should have NFT or not since in the current design, there is no concept of NFT.

There was one point we also discussed which is that Llamapay is a simple contract which makes it extremely cheap to use and adding NFT increases gas, however, I don't see that as a strong reason because gas is not the only reason why users use protocols (otherwise Llamapay would have been no. 1). Sablier has a great design, has never been hacked i.e. safest protocol to use, and a dedicated team constantly working on this niche.

I do agree that adding NFT means we will also have to design specs over the transferability of the stream similar to lockups. Because due to compliance reasons, some companies may not want their payee to be able to change the recipient address.

[razgraf]

NFTs are actually a pretty good growth hack, as well as a means to transfer value from one wallet to another.

Having NFTs would make the stream a money-lego, with users being able to add it to lending markets, borrow against it etc. One of the biggest use-cases with payroll is payday loans, so having the freedom to move the stream is required.

The problem of quantifying or classifying a stream is a layer that 3rd parties can asses, just like in the case of cancelable/non-cancelable streams. Open-ended streams can still accrue real value inside (e.g. by not withdrawing), represent a promise of future income etc. - by using the transferability aspect of the NFT, we'd open it to the same ecosystem of opportunities the lockup one have.

On the "Look, we have NFTs for the payroll contracts, but LlamaPay doesn't.", that's somewhat true, but as I said: it's a growth hack tool too. I'm worried open-ended would end up looking like a rip-off of Llama Pay's product if we don't engineer new functionalities in it or innovate in any way - the gas costs are negligible, as we're not in the market of offering the no. 1 most efficient product, but in the market of offering the no. 1 most complete and beautiful product (from design to code, from concept to functionality).

I've answered it https://github.com/sablier-labs/company-discussions/discussions/6#discussioncomment-7304440 and no counter-argument was presented.

Sorry about that, I haven't had a lot of time to dig into the existing discussions for OE yet, so some of these points I'm seeing for the first time. Feel free to ping me in them if I can give any sort of input - will also make an effort to read them one by one soon and ask questions.

[andreivladbrg]

NFTs are actually a pretty good growth hack

I can agree with this

Having NFTs would make the stream a money-lego, with users being able to add it to lending markets, borrow against it etc. One of the biggest use-cases with payroll is payday loans, so having the freedom to move the stream is required.

this is the problem - I don't see this working in an "open" environment given the points I've mentioned (too many "variables" that someone should take account for - on the other hand for lockup the NFT makes total sense)

I'm worried open-ended would end up looking like a rip-off of Llama Pay's product if we don't engineer new functionalities in it or innovate in any way -

It is a real concern that we should have. In terms of contracts, the way it is implemented is quite different (@smol-ninja, please correct me if the implementation is not different). We focus on the Stream entity, whereas LlamaPay focuses on the payer details.

And, for LlamaPay, you can't deposit for specific recipients, meaning you can't prioritize. I plan to open a discussion outlining all the differences.

the gas costs are negligible, as we're not in the market of offering the no. 1 most efficient product, but in the market of offering the no. 1 most complete and beautiful product (from design to code, from concept to functionality).

Agree with this

Sorry about that, I haven't had a lot of time to dig into the existing discussions for OE yet

No prob

I've just restarted working for OE, so I need to open multiple discussions about the directions and what extra functionalities we should have. As a start, the README should be a good reference.

[smol-ninja]

this is the problem - I don't see this working in an "open" environment given the points I've mentioned

Can you please explain it again? I didn't get what you meant here?

please correct me if the implementation is not different

Yes its different in the context of that each stream on Llamapay is a separate contract while in OE, its just one contract that handles all the streams. This opens up for new functionalities such as making deposits to multiple streams at once (this would actually be cheaper in our case).

I agree with @razgraf comment and I think we should include NFT in OE too.l

[andreivladbrg]

Can you please explain it again? I didn't get what you meant here?

When you want to borrow against something, in general, there are some criteria that should be respected. In the context of OE streams, they would be considered too "volatile"

there are no non-cancelable streams - sender can cancel the stream without even depositing to it
the withdraw function is public - anyone can trigger it
not a fixed duration with a fixed rate - you can't say that by the end of X date I will have this amount

Also, in the context of lockup with a non-cancelable streams, there isn't a stable integration made atm (This is why I am pessimistic about this working for open-ended or having real value). We still need to build our own NFT lending platform.

Yes its different in the context of that each stream on Llamapay is a separate contract while in OE, its just one contract that handles all the streams. This opens up for new functionalities such as making deposits to multiple streams at once (this would actually be cheaper in our case).

Yeah, besides the factory pattern design, they don't have data structure for a "Stream"

[smol-ninja]

I got it now. I understand your point about the risks involved in lending with OE NFTs and I agree with it. However, there are two other features that Sablier NFTs offer (and are true in the present):

1. Visualization: The NFT provides a nice visualization of the stream, displaying all the relevant information. Even without lending (which is anyways not an active utility at the moment), the NFT itself represents ownership of the stream. Users can see it in their crypto wallets and have the assurance that the stream belongs to them. Without the NFT, the payment becomes invisible and can only be verified on the Sablier user interface.

2. Transferability: While, to me, it makes sense to only allow the Sender to change the recipient. From the perspective of the recipients, it's helpful to have the option to transfer the stream. For example, if their wallet gets compromised, recipients can take quick action (as opposed to requesting Sender to change it) as hackers can only stream ERC20 tokens. Additionally, keeping the specs similar to lockup contracts ensures easy onboarding for our current users.

[smol-ninja]

Do we all agree to have NFT for OE? @andreivladbrg tagging you for your vote.

[andreivladbrg]

On the last all-hands call, we have decided to implement the NFT in OE as well. However, I want to mention that doing so would considerably increase the complexity and the gas costs for the create adjustRatePerSecond and withdraw functions if we choose to keep the adjustRatePerSecond function.

Why

Based on Rusty's findings from the first audit, if the NFT is transferred to a blacklisted address, the cancel function would revert. To address this, we would need to implement a new function, withdrawWhenCanceled, which withdraws the balance at that moment in time. We can't apply the methodology from the lockup contract here because we do not have a fixed deposit amount.

In the case of adjustRatePerSecond, we need to perform the ERC20 transfer to the recipient because of the same reason explained in the README on why we need to update the lastTimeUpdate variable:

because it would cause loss of funds for the recipient if the previous rate was higher or gain of funds if
the previous rate was lower

If we can no longer transfer funds (for the blacklist reason) we would need to add a new amount (beforeAdjustAmount - or any other name) in the Stream struct, which will be summed up in the withdrawableAmount function

---

Since, IMO the best argument for using an NFT is its visual aspect, what would you say if we do not inherit from ERC721 but instead only implement a tokenURI function ? In our case it would be streamURI😅 - might not be compatible with platforms like opensea
This approach would provide something visual without adhering to ERC721 (no transfers).

[andreivladbrg]

Why are you pessimistic about making it work?
if we don't have something stable for the lockup version - what do you mean by this?

sorry for not being clear, i was referring to the NFT marketplace use case: yes, we will offer this NFT feature, but i am just pessimistic that it will "work" (by work i mean that if it will be actually used) because it would be too complex to make the system work with an open-ended contract. since with the lockup, where it is less complex, we don't have an integration made already

[smol-ninja]

I've not reviewed the full comment yet but the gas comparison is not correct. Since during create, remainingAmount is the only field which will still be 0, we should put it in slot 3 and not in slot 1. Then, the gas consumption will be nearly same. I ran the tests with the following struct, and its 137,586 i.e. only 2.5k higher.

This is because create will write into slot 0, 1 and 2 but not 3.

struct StreamWithAmount {
    // slot 0
    uint128 balance;
    uint128 ratePerSecond;
    // slot 1
    address sender;
    uint40 lastTimeUpdate;
    bool isStream;
    bool isCanceled;
    // slot 2
    address asset;
    uint8 assetDecimals;
    // slot 3
    uint128 remainingAmount;
}

[andreivladbrg]

we should put it in slot 3 and not in slot 1. Then, the gas consumption will be nearly same

correct, didn't know we could do that, thanks

[smol-ninja]

so the new invariant will be: withdrawableAmount = min(balance, streamedAmount) + remainingAmount

I think there is a misunderstanding here. When I mentioned remainingAmount, I meant to use it in a different context.

In the current design, the calculation for the withdrawable amount depends on lastTimeUpdate. So when a stream is cancelled, we update its value and trigger the transfers. If we want to avoid withdrawing during cancel, we cannot update lastTimeUpdate. And as a result, the future calculation of _streamedAmountOf could be incorrect.

So my suggestion was that the remainingAmount would represent the available amount to withdraw, given there is sufficient balance. That is, it will include _streamedAmountOf + amounts from the previous checkpoints, allowing us to safely update lastTimeUpdate independently (for example, when the user restarts the stream).

An example

• When a user withdraws, remainingAmount += _streamedAmountOf. _streamedAmountOf also updated the lastTimeUpdate. Then it will take the min(remainingAmount, balance).
• When a user cancels a stream, it will update remainingAmount the same as above. In the future, when a user requests to withdraw on a canceled stream, we can just use min(balance, remainingAmount) without looking at / updating lastTimeUpdate.

Let me know if I have not made myself clearer or if this is less better than the one you had in your mind.

[andreivladbrg]

i see that we are not understanding each other :))

If we want to avoid withdrawing during cancel, we cannot update lastTimeUpdate. And as a result, the future calculation of _streamedAmountOf could be incorrect

if the stream is canceled, the streamed amount will always be zero (because we set rps = 0 - doesn't matter what value is lastTimeUpdate). the cancel function, instead of transferring the funds to the recipient, will simply sum the remainingAmount += min(remainingAmount, balance), and as said in my previous message:

---

i was not focusing on the canceled streams, as it is simply how to handle that, but on the withdraw function when remainingAmount is not zero.

the remainingAmount will only be updated with min(balance, remainingAmount), and the functions that will update this variable are: adjustRatePerSecond, cancel, and refundFromStream

one other aspect is that lastTimeUpdate will also be updated in these functions

An example

i will proceed with an implementation - it will be easier to see the code

[andreivladbrg]

I will tentatively close this discussion, as we have softly agreed, and I will reopen it in case of implementation issues.