Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NEP-17/NEP-11 needs update #164

Open
cschuchardt88 opened this issue Oct 8, 2023 · 9 comments
Open

NEP-17/NEP-11 needs update #164

cschuchardt88 opened this issue Oct 8, 2023 · 9 comments

Comments

@cschuchardt88
Copy link
Member

cschuchardt88 commented Oct 8, 2023

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:
flamingo-finance/flamingo-contract-swap#37
NeoNEXT/neoline#131

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

@superboyiii
Copy link
Member

superboyiii commented Oct 8, 2023

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

@cschuchardt88 cschuchardt88 changed the title NEP-17 needs update NEP-17/NEP-11 needs update Oct 8, 2023
@cschuchardt88
Copy link
Member Author

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
Copy link
Contributor

ixje commented Oct 8, 2023

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
Copy link
Contributor

roman-khimov commented Oct 8, 2023

  1. SHOULD/MUST/RECOMMEND are used as per RFC 2119. This SHOULD be a part of NEP-1.
  2. Refs. Replace NEP-5 with NEP-17 #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
Copy link
Member Author

cschuchardt88 commented Oct 9, 2023

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
Copy link
Member

shargon commented Oct 9, 2023

, 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
Copy link
Member Author

cschuchardt88 commented Nov 18, 2023

@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
Copy link
Contributor

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
Copy link
Member Author

cschuchardt88 commented Nov 22, 2023

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants