Token-URI interfaces

[andreivladbrg]

As we discussed on the call we should add an address argument to both the linear and pro contracts that will be passed in the constructor. This contract will implement a tokenURI function to produce the NFT-SVG from the stream values.

The primary reason behind this proposal is to close the v2-core repository and pass it to auditors. Furthermore, the logic for the SVG is quite voluminous, and the core repository is already a large codebase.

How should we name those interfaces? My current suggestion is to use something like:

• ISablierV2LinearSvg and ISablierV2ProSvg
• ISablierV2LinearNftBuilder and ISablierV2ProNftBuilder
• ISablierV2LinearNftDescriptor and ISablierV2ProNftDescriptor

Uniswap has used the "descriptor" keyword. Do you have any other suggestions?

[razgraf]

Why not just call it "art"? As in ISablierV2Art.

Also, do we need two separate ones? I don't think we're gonna showcase segments in the PRO svg just because it might be hard to generate them in the art. Even so, we could just leave segments[] empty and use a common interface.

[andreivladbrg]

Why not just call it "art"? As in ISablierV2Art.

I personally think that the name is vague. I believe that using a more specific name would be more appropriate in this case.

We can use a common interface, but we'll still need to implement two separate functions. The reason being that we need to call the getStream function, which returns different structs for linear and pro streams, in order to obtain the values required to build the SVG. Therefore, it's necessary to have distinct functions for linear and pro streams.

[PaulRBerg]

I personally think that the name is vague

Fair enough. As I said in my comment below, start with whatever name you like; we can easily refactor the contract once it is implemented.

We can use a common interface, but we'll still need to implement two separate functions.

Not necessarily.

The reason being that we need to call the getStream function

We could just call and pass the values returned by getStartTime, getEndTime, getDepositAmount, and streamedAmountOf`, which are all shared functions.

[andreivladbrg]

We could just call and pass the values returned by getStartTime, getEndTime, getDepositAmount, and streamedAmountOf`, which are all shared functions.

We could, but we have be sure that we'll use only the common variables.

[PaulRBerg]

This decision will have to be informed by the 3D artwork itself .. nevertheless, as I have argued below, there's so much complexity we can bake in the artwork. Let's go with the vanilla data and not use the bespoke structs. This will help us reduce the complexity of the contracts, which will lower the audit costs.

[PaulRBerg]

I agree with @razgraf that we should not have two token URI generator contracts. It sounds overkill because:

1. The SVG artwork won't be so advanced to handle the linear and the pro contracts differently.
2. It would add differential logic in the linear and the pro contracts, making them more difficult to audit. With a single artwork generator contract, we can implement it in the abstract contract SablierV2Lockup.

Naming-wise, my vote goes to the following options:

1. ISablierV2NftDescriptor
2. ISablierV2Art
3. ISablierV2Artwork
4. ISablierV2NonFungibleTokenDescriptor

I don't like ISablierV2Svg, because SVG is just a format. You typically don't name products/ deliverables after the file format used, e.g. you don't say SablierV2LockupSol because the contract is written in Solidity. Builder is okay but I like the names above better.

@andreivladbrg - feel free to start with whatever option you like best. We can easily refactor the contracts once they are implemented (pro tip: learn these VSCode shortcuts to perk up your refactoring game): CMD+SHIFT+L and CMD+D).

[andreivladbrg]

e.g. you don't say SablierV2LockupSol because the contract is written in Solidity.

Good point, ISablierV2Svg is not a good name.

[PaulRBerg]

Locking, as we have agreed upon the name of this contract, and its API.