Modularize NFTs and streaming logic

[PaulRBerg]

Idea

I've been thinking about the prospect of rearchitecting V2 Core this:

• Create a global "Vat" registry similar to MakerDAO's Vat.
• Issue multiple stream NFT contracts with different parameters (e.g. cancelable vs non-cancelable, locked vs non-locked)
• The Vat would have an admin with permission to list new NFT contracts. Previously listed NFT contracts cannot be delisted.
• The Vat would store the ERC-20 tokens and implement the streaming functions (e.g. streamedAmountOf)
• The NFTs would redirect most calls to the Vat, except for those functions that are strictly NFT-related

Benefits

• Globalize the stream ids across all Lockup stream contracts (see the problems discussed in https://github.com/sablier-labs/v2-interfaces/discussions/143)
• Solve issues like Issues with using stream NFTs as collateral for lending #684 by separating the NFT logic from the streaming logic.

Flow

Create

1. User calls create function on the NFT contract
2. NFT contract performs the necessary checks.
3. NFT queries nextStreamId
4. NFT is minted with correspondent steram id
5. ERC-20 tokens are transferred to the VAT

Streamed Amount Of

1. User calls streamedAmountOf directly on the Vat, providing a global streamId
2. (Optionally) The NFT contract could mirror streamedAmountOf and redirect the call to the Vat, for convenience

Withdraw

1. User calls withdraw on the NFT contract
2. The call is redirected to the Vat.
3. The Vat checks if the msg.sender is a known NFT contract
4. Assets are withdrawn to the provided to

Burn

1. User calls burn on the NFT contract
2. The NFT contract does not call the Vat
3. The NFT is burned

Diagram

flowchart LR;
  V[("Vat")];
  N0["LockupLinear-Cancelable"];
  N1["LockupLinear-NonCancelable"];
  N2["LockupLinear-Locked"];
  N3["LockupLinear-NonLocked"];
  U(User);

  U-->N0;
  U-->N1;
  U-->N2;
  U-->N3;
  N0-->V;
  N1-->V;
  N2-->V;
  N3-->V;

Loading

[andreivladbrg]

Starting of by saying that its an Interesting idea.

Expanding our architecture is inevitable if we aim to improve/multiply our products.

From what you've said, I'm not sure whether you are suggesting:

1. The VAT contract would store both linear and dynamic streams mappings.
2. The NFT contracts would store the stream mappings (not sure how the vat would be able to calculatestreamedAmountOf).

In my answer, I'll assume you intended for option 1.

My take on this is that while we might be able to make it work, merging the logic for linear and dynamic streams into a single vat doesn't seem ideal. Even if the vat doesn't inherit the ERC721, its complexity and size would likely be high. We would need two mappings for the streams:

mapping(uint256 id => LockupDynamic.Stream stream) private _dynamicStreams;
mapping(uint256 id => LockupLinear.Stream stream) private _linearStreams;

The Vat would store the ERC-20 tokens and implement the streaming functions (e.g. streamedAmountOf)

Vat would most likely need to have 2 different implementations for all functions.

Globalize the stream ids across all Lockup stream contracts (see the problems discussed in https://github.com/sablier-labs/v2-interfaces/discussions/143)

Vat would indeed globalize the stream ids.

Solve issues like #684 by separating the NFT logic from the streaming logic.

This issue isn't resolved directly because of the VAT (changing the architecture isn't required), but it's addressed because we introduce new different contracts: LockupLinear-Cancelable, LockupLinear-NonCancelable, etc.

User calls burn on the NFT contract
The NFT contract does not call the Vat
The NFT is burned

If the NFT contract does not call the Vat how does it know if the stream is depleted?

From my POV, we would benefit the most from this architecture if we can find a way to generalize the struct and the calculations for linear streaming that can be used for both vesting and payroll use cases, one NFT contract would force you to "lockup" your funds, while the other would allow you to deposit funds continuously. Or an even one more interesting idea would be to make the VAT a vault(which inherits ERC4626) to also implement a yielding streaming functionality, this way there would not be collisions between ERC721 and ERC4626.

Also, extending the architecture to something like "VAT" is the direction we should aim for, tho more research needs to be done.

Suggestion: we should keep this open, but not include it in the 2.1 version.

[PaulRBerg]

Thanks for sharing your thoughts. I agree with your conclusion that we should postpone this refactor until we crystalize the implementation details.

1. The VAT contract would store both linear and dynamic streams mappings.

This

I'll assume you intended for option 1.

Yes

Vat would most likely need to have 2 different implementations for all functions.

Yes. What's wrong about that? It doesn't sound difficult to implement.

This issue isn't resolved directly because of the VAT (changing the architecture isn't required), but it's addressed because we introduce new different contracts:

Correct. As discussed in person, the Vat proposal is somewhat orthogonal to the NFT issue, but it would still be a helpful refactor in and of itself. For example, by globalizing the streaming logic in one central contract, we would reduce the deployment costs for each contract.

If the NFT contract does not call the Vat how does it know if the stream is depleted?

Good point. We need to do a whiteboard session someday to fully sketch out the details.

we would benefit the most from this architecture if we can find a way to generalize the struct and the calculations for linear streaming that can be used for both vesting and payroll use case

Agree

one more interesting idea would be to make the VAT a vault(which inherits ERC4626) to also implement a yielding streaming functionality

Not feasible. The Vat would be meant to hold all assets. You can't have on single supply of ERC-20 tokens (the vault tokens) that represent multiple underlying ERC-tokens.