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

Draft NEP 26 and Draft NEP 27: New NEPs describing onNEPXXPayment callbacks #169

Merged
merged 13 commits into from
Jun 5, 2024

Conversation

roman-khimov
Copy link
Contributor

Please add more implementation references.

nep-Y.mediawiki Outdated Show resolved Hide resolved
nep-Y.mediawiki Outdated Show resolved Hide resolved
nep-Y.mediawiki Outdated Show resolved Hide resolved
nep-Y.mediawiki Outdated Show resolved Hide resolved
nep-Y.mediawiki Outdated Show resolved Hide resolved
nep-Y.mediawiki Outdated Show resolved Hide resolved
nep-Z.mediawiki Outdated Show resolved Hide resolved
nep-Z.mediawiki Outdated Show resolved Hide resolved
nep-Z.mediawiki Outdated Show resolved Hide resolved
nep-Z.mediawiki Outdated Show resolved Hide resolved
Fixes neo-project#168.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
@shargon
Copy link
Member

shargon commented Mar 12, 2024

Seems good to me, maybe we can use NEP-117 and NEP-111, something related to the parent? or we need to be consecutives?

@AnnaShaleva
Copy link
Member

or we need to be consecutives?

Huh, we don’t have to. Seems that’s exactly the case of special numbers:

Assign a NEP number (almost always just the next available number, but sometimes it's a special/joke number, like 666 or 3141) in the pull request comments.

@roman-khimov
Copy link
Contributor Author

I'd go consequently. #156 is 24, I'd leave 25 for #160 (it came way earlier), so it's 26/27 for these ones.

@shargon
Copy link
Member

shargon commented Mar 14, 2024

Please add more implementation references.

Burguer neo?

@roman-khimov
Copy link
Contributor Author

Link? We can add everything we have, I think it's pretty popular.

@shargon
Copy link
Member

shargon commented Mar 15, 2024

Link? We can add everything we have, I think it's pretty popular.

https://github.com/neoburger/code/blob/beb433288bd7b0da9d2245e2a0ae5af5309da7ed/BurgerNEO.cs#L42

Signed-off-by: Roman Khimov <roman@nspcc.ru>
bNEO uses it and we can't say it's wrong.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
Mediawiki is not Markdown.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
@roman-khimov
Copy link
Contributor Author

Added bNEO and also added a note on ASSERTs.

Copy link
Member

@shargon shargon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's good for me

Copy link
Member

@AnnaShaleva AnnaShaleva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also good to me, need to update status and add NEP numbers (I'm not allowed to resolve conversations in this repo).

shargon
shargon previously approved these changes Apr 2, 2024
@superboyiii
Copy link
Member

@roman-khimov Could you update status and add NEP numbers? No.25 and No.26 will be OK.

nep-Y.mediawiki Outdated Show resolved Hide resolved
nep-Y.mediawiki Outdated Show resolved Hide resolved
Co-authored-by: Jimmy <jinghui@wayne.edu>
Co-authored-by: Jimmy <jinghui@wayne.edu>
nep-Y.mediawiki Outdated Show resolved Hide resolved
nep-25.mediawiki Outdated Show resolved Hide resolved
Co-authored-by: Jimmy <jinghui@wayne.edu>
Jim8y
Jim8y previously approved these changes May 20, 2024
@Jim8y Jim8y added the accepted label May 23, 2024
@Jim8y Jim8y changed the title New NEPs describing onNEPXXPayment callbacks Draft NEP 26 and Draft NEP 27: New NEPs describing onNEPXXPayment callbacks May 23, 2024
@Jim8y Jim8y requested a review from a team May 24, 2024 11:23
shargon
shargon previously approved these changes May 24, 2024
@Jim8y Jim8y requested review from a team, AnnaShaleva and cschuchardt88 May 26, 2024 03:47
Copy link
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@roman-khimov, the details look good to me, apart for the first motivation paragraph that I will comment there.

On the other hand, I thought there were additional recommendation in the semantics.

In such case I think you can do an append to NEO-11 or NEP-27.
This does not necessary need a number. There is no sense to me.

@Jim8y
Copy link
Contributor

Jim8y commented May 26, 2024

@roman-khimov, the details look good to me, apart for the first motivation paragraph that I will comment there.

On the other hand, I thought there were additional recommendation in the semantics.

In such case I think you can do an append to NEO-11 or NEP-27. This does not necessary need a number. There is no sense to me.

@vncoelho YOU DO NOT CHANGE ANY EXISTING NEP. ITS NEP, A STANDARD, NOT SOME DOCUMENT YOU CAN UPDATE AND UPGRADE FROM TIME TO TIME.

@vncoelho
Copy link
Member

In my opinion it is a New Informational NEP or a documentation issue.

Do we have any place to classify the NEPs?

For example: https://eips.ethereum.org/informational

@roman-khimov
Copy link
Contributor Author

  1. NEP-11/NEP-17 can't be changed. They can only be replaced by some new standards.
  2. Motivational part covers it all, try explaining "I'm implementing the token receiver method for NEP-11/NEP-17 and not NEP-11/NEP-17" quickly. Now you can just say "my contract is NEP-26/NEP-27 compliant".
  3. See also Replace NEP-5 with NEP-17 #126 (comment)
  4. There are some details there were not taken into account by original standards, like ASSERT.
  5. There are new opcodes (ABORTMSG/ASSERTMSG) that we know about, but that people will be asking about if not for these standards.

@vncoelho
Copy link
Member

Motivational part covers it all, try explaining "I'm implementing the token receiver method for NEP-11/NEP-17 and not NEP-11/NEP-17" quickly. Now you can just say "my contract is NEP-26/NEP-27 compliant".

Sorry it was not motivational part. It was below it. I already left some comments

Add a bit more details, signify the need for script hash check.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
@Jim8y Jim8y requested a review from a team May 31, 2024 09:59
@superboyiii
Copy link
Member

superboyiii commented Jun 5, 2024

@roman-khimov Please update readme. We can merge this first incase approvals are reverted. @shargon

@Jim8y Jim8y dismissed vncoelho’s stale review June 5, 2024 03:48

will not apply

@Jim8y Jim8y merged commit c0e1fd5 into neo-project:master Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants