search

sablier-labs / v2-core

⏳ Core smart contracts of the Sablier V2 token distribution protocol
https://sablier.com
Other
311 stars 48 forks source link

Refactor asset terminology to tokens #991

Open PaulRBerg opened 3 months ago

PaulRBerg commented 3 months ago

To align the NatSpec with the terminology used in our PR and documentation site, we should refactor all references to 'assets' to say 'tokens' instead.

smol-ninja commented 3 months ago

I also like calling them tokens over assets because the asset is a broad category whereas token has a specific meaning.

But, I remember having a conversation during one of the discussions / PR review / on slack with @andreivladbrg. IIRC, the reason that the "assets" terminology was chosen was because the "token" was reserved to represent the NFT (as ERC721 "token"). Has that opinion be changed?

PaulRBerg commented 3 months ago

the reason that the "assets" terminology was chosen was because the "token" was reserved to represent the NFT

Correct, that was the original rationale in ~2022. But then, with time, we realized that the 'token' terminology is ubiquitous for ERC-20s. So much so that the slight confusion that may occur due to the NFT also being referred to as a 'token' is negligible compared to the inaccuracy of referring to ERC-20s as assets.

Has that opinion be changed?

Yes. I can't find it now, but we discussed it on Slack.

PaulRBerg commented 1 week ago

Are we still on track to perform this rename in v1.3.0?

smol-ninja commented 1 week ago

Yes.

razgraf commented 3 days ago

I still think there's a need to separate the ERC721 "token" from the ERC20 "token" it is entitled to.

we realized that the 'token' terminology is ubiquitous for ERC-20s

I agree with this, but how do we quantify the "inaccuracy of referring to ERC-20s as assets" here? I vaguely remember our conversation during the Sablier Mainnet sprints re. MNA/MNT, but I still find using two separate terms clearer in contexts where both standards are working closely together.

Meaning I'd still vote for sticking with asset: ERC20 if we can (since we cannot redefine ERC721's tokenId because that's part of the standard). Not saying asset is the best (might still pick token if it were available) but in this context separation > conflict while adhering to popular terminology.

smol-ninja commented 3 days ago

Good point. My take is that we use streamID to refer to ERC721's token ID instead of tokenId in our telegram chats as well as in the docs. Sablier streams are represented by NFTs. I understand ERC721 standard uses tokenID but we don't use that name anywhere or do we?

Referring to EIPs, EIP20 does not use the word asset as well. So if we are referring to EIPs, crypto users seem to be familiar with token while referring to ERC20 and tokenID while referring to ERC721.

TL;DR we can use token for ERC20 and streamID for stream ID.

PaulRBerg commented 3 days ago

I am massively in favor of using the term token for referring to ERC-20s. We also discussed this on Slack, and not just about MNTs (maybe @maxdesalle can dig up the conversation, I can't find it now).

tokenId is a property that only some users see. Whereas all users see asset. I wouldn't name the latter property based on the former.

Also, nobody refers to NFTs as tokens, despite the eponymous reference in the acronym.

Plus, what Shub said.

razgraf commented 2 days ago

Fair points from both! Let's go with token then.