sablier-labs / v2-periphery

🎛 Peripheral smart contracts for interacting with Sablier V2
https://docs.sablier.com
GNU General Public License v3.0
24 stars 7 forks source link

Add a "name" constructor parameter in the MerkleStreamer contract #233

Closed PaulRBerg closed 7 months ago

PaulRBerg commented 9 months ago

It would be helpful to allow users to specify an optional name when deploying a MerkleStreamer contract.

The use case is to enable users to personalize their campaigns - what happens if they make one accidental deployment, and then make another deployment? They will see two similar entries in the Airstreams dashboard:

Screenshot 2023-12-14 at 12 46 23 PM

Instead of "DAI by 0xf31..c634", it would be clearer to have a bespoke name.

The UI can default to passing the contract address as the name.

Cc @andreivladbrg, @razgraf.

smol-ninja commented 9 months ago

Setting a string name can increase deployment costs if users pick long names (that they might do in practice) and the user would blame us for the high cost.

Either we limit the number of letters people can use in the name, or we could handle this issue on the user interface instead. In the latter case, we can create a way for users to export their UI metadata in a JSON file in case their browser resets. Gnosis has a similar feature that lets you do this sort of thing for your SAFEs.

razgraf commented 9 months ago

@smol-ninja what you're proposing is a labeling system for airstreams. While fair and on our roadmap, just like the other labeling system we discussed a while back, @PaulRBerg is most likely taking about a global/public name that everyone can see (not saved locally).

The "off-chain" alternatives:

Another option would be to only emit this name as an event (that we later index with the subgraph) in the contract (without storing it anywhere afterwards) but this probably increases gas costs too.

It's also worth noting that if we allow people to add custom names/labels we have to introduce a form of validating a label is not (a) a link (b) obscene (c) fake/impersonating-projects and so on. XSS wouldn't be an issue, but showing ads or obscene words in the UI would.

PaulRBerg commented 9 months ago

Setting a string name can increase deployment costs if users pick long names

We can revert the construction of the contract if the length of the string is greater than e.g. 30 characters

We limit the number of letters people can use in the name

Yep

@PaulRBerg is most likely taking about a global/public name that everyone can see (not saved locally).

Yep.

This approach is 10x easier to implement for everyone - our UI, integrators, etc.

The "off-chain" alternatives:

We should discuss that in a separate discussion in V2 Interfaces.

Another option would be to only emit this name as an event ... but this probably increases gas costs to

The gas cost would be lower than with storing the string, but that wouldn't fully address the problem. Integrators should be able to query the contract directly and get a name for the campaign rather than jump through hoops via the subgraph.

It's also worth noting that if we allow people to add custom names/labels we have to introduce a form of validating a label is not (a) a link (b) obscene (c) fake/impersonating-projects and so on

Great point

PaulRBerg commented 8 months ago

It's also worth noting that if we allow people to add custom names/labels we have to introduce a form of validating a label is not (a) a link (b) obscene (c) fake/impersonating-projects and so on. XSS wouldn't be an issue, but showing ads or obscene words in the UI would

More feedback from you, @razgraf (on the call):

There is already this risk via ENS. People can put obscene words in the ENS label itself.

smol-ninja commented 8 months ago

I want to mention that I would mark it as work: complicated. Adding name wasn't so straightforward since it required parameterising the constructor arguments using a new struct, I called, CreateWithLockupLinear.

Both of these changes can be found in https://github.com/sablier-labs/v2-periphery/pull/262.

PaulRBerg commented 8 months ago

That's interesting! Thanks for the heads-up, @smol-ninja.

Btw are you not able to re-label issues yourself? I should look into your permissions.

smol-ninja commented 8 months ago

I can change the labels. But I wanted to have an agreement first.

PaulRBerg commented 3 weeks ago

It looks like the naming feature has been a success

SCR-20240826-rosr