neo-project / proposals

NEO Enhancement Proposals
Creative Commons Attribution 4.0 International
136 stars 113 forks source link

NEP-17/NEP-11 needs update #164

Open cschuchardt88 opened 11 months ago

cschuchardt88 commented 11 months ago

The community and I had a discussion on discord. How NEP-17 is VERY unclear for symbol method.

Now NEP-17 states:

Returns a short string representing symbol of the token managed in this contract. e.g. "MYT". This string MUST be valid ASCII, MUST NOT contain whitespace or control characters, SHOULD be limited to uppercase Latin alphabet (i.e. the 26 letters used in English) and SHOULD be short (3-8 characters is recommended). This method MUST always return the same value every time it is invoked.

Now most people will interpret MUST SHOULD and NOT as they are defined in the English dictionary. The word that is mostly confusing is the word SHOULD. Should (English definition) is defined as:

used to indicate obligation, duty, or correctness, typically when criticizing someone's actions. "he should have been careful"

We all talked about if one particular entity is in violation of what symbol says. We have a value of FLP-bNEO-GM. One would say that this token is in violation of this rule. Others will argue that SHOULD means the same as recommended and some will say NEP-17 follows RFC2119. RFC2119 defines SHOULD as:

This word, or the adjective "RECOMMENDED", mean that there may exist valid reasons in particular circumstances to ignore a particular item, but the full implications must be understood and carefully weighed before choosing a different course.

Nowhere does it say that NEP follows RFC spec. However NEP-1 only states that the RFC 822 is to be for a particular part of NEP . So developers are making the assumption that it implies that NEP is in compliance with RFC rules.

We need clarification on how NEP-17's symbol should be interpreted for SHOULD. I suggest new proposals one for NEP-17-1 to correct this issue. What SHOULD means.

What does this mean:

SHOULD be short (3-8 characters is recommended)

be short (3-8 characters is recommended) is VERY unclear. What is be short even mean when you add in recommended. Is it short? Can I have 1MB string? Who knows?

I suggested that this entity create new proposal for LP (liquidity pool) tokens.

But it comes down to this repo. Everyone, including me thinks that this repo goes nowhere when it comes to suggesting new proposals.....

References: https://github.com/flamingo-finance/flamingo-contract-swap/issues/37 https://github.com/NeoNEXT/neoline/issues/131

Full Conversation https://discord.com/channels/382937847893590016/382937847893590019/1160257458594381844

superboyiii commented 11 months ago

@neo-project/core @roman-khimov @Liaojinghui @shargon Need your opinion.

cschuchardt88 commented 11 months ago

Lets not forget NEP-11

symbol's definition:

Returns a short string symbol of the token managed in this contract. e.g. "MNFT". This symbol SHOULD be short (3-8 characters is recommended), with no whitespace characters or new-lines and SHOULD be limited to the uppercase latin alphabet (i.e. the 26 letters used in English).

ixje commented 11 months ago

I think these specs in general are way too loose and makes it hard to consume this data outside of being a node in a meaningful way. In my opinion standards should be strict and enforced. e.g.

Returns a short string symbol of the token managed in this contract. e.g. "MNFT". 
This symbol MUST be between 3-8 characters (inclusive), MUST not contain whitespace characters or new-lines,
 and MUST be limited to the uppercase latin alphabet (i.e. the 26 letters used in English).

Similarly for decimals(). There's only a limit imposed by the VM but imo there no real reason to have 30+ decimals, yet there are contracts that use something like that. We should have been strict and say "max decimals of 8".

roman-khimov commented 11 months ago
  1. SHOULD/MUST/RECOMMEND are used as per RFC 2119. This SHOULD be a part of NEP-1.
  2. Refs. #126, these things were discussed there. NEP-17 is much better than NEP-5 in that sense.
  3. FLP-bNEO-GM is still a good test case for the naming scheme, it fits into NEP-17 limitations, but it won't if you're to reword requirements as MUST.
  4. Some strengthening can still be helpful, like "SHOULD fit into 3-8, MUST fit into 20". But not a lot more than that I'd say.
cschuchardt88 commented 11 months ago

This is the way I interpret it. If i were to valid it.

if (Symbol == ASCII)
{
    if (Symbol == WHITE_SPACE || Symbol == CONTROL_CHAR)
    {
        // Invalid
    }
    if (Symbol == UPPER_CASE)
    {
        if (Symbol == LATIN_ALPHABT)
        {
            // Is Valid
        }
        if (Symbol == SHORT_STRING && Symbol.Length == 3-8)
        {
            // Unknown
        }
        else
        {
            // good
        }
    }
}
shargon commented 11 months ago

, SHOULD be limited to uppercase Latin alphabet (i.e. the 26 letters used in English) and SHOULD be short (3-8 characters is recommended).

I think that the first should be a MUST

cschuchardt88 commented 10 months ago

@shargon @roman-khimov

Can we please fix NEP-11 standard everything says SHOULD everywhere. It's so illogical.

The parameter owner SHOULD be a 20-byte address. If not, this method SHOULD throw an exception.

The parameter to SHOULD be a 20-byte address. If not, this method SHOULD throw an exception.

The parameter tokenId SHOULD be a valid NFT ID (64 or less bytes long). If not, this method SHOULD throw an exception.

The parameter amount SHOULD be greater than or equal to 0 and SHOULD be less than or equal to pow(10, decimals()). If not, this method SHOULD throw an exception.

You get the point?

roman-khimov commented 10 months ago

We can't fix NEP-17/NEP-11, we can only replace them. Even though we have some things mentioned here and #149/#150, I'm not sure they justify new standards. But I'm open to suggestions/PRs at the same time.

cschuchardt88 commented 10 months ago

I think we are at the point now, specially with the new sidechain coming out. We should create proposals NEP-17-1 and NEP-11-1. With NEP-11 and NEP-17 it is to flexible, which is ok. Now we need a more restrictive proposal. So developers can ensure the interface is correct instead of guessing or doing what they think is best practices. I have had a hard time in the pass when I 1st started in neo; building an app that uses this logic. Come to find out, they can throw exceptions give you random return lengths, invalid addresses, etc. It was annoying building the app and than having to change it for a special case. Because someone wanted to be cute....It should be the same for all NEP-11 and NEP-17 contracts; on how to call or what to expect from a method or contract with that specification.

There is no reason to add SHOULD than, if it is recommended (for future proposals).