NEP5 call onPayment after transfer

[shargon]

• Add onPayment method

Close #108

[erikzhang]

onPayment method should be in the target contract. And the from parameter is not needed.

[ixje]

onPayment method should be in the target contract. And the from parameter is not needed.

I think from can actually be useful. The target contract can have different execution based on the from address

[roman-khimov]

I think first of all we need a new standard here. NEP-5 is NEP-5 and it shouldn't be changed.

[ixje]

yes that also

[roman-khimov]

And also, onPayment definition can't really be a part of NEP-5 because it should also be invoked for non-NEP-5 receiver contracts. NeoFS mainnet contract is not NEP-5, but it'd be nice to get an onPayment invocation for it too.

[shargon]

I think first of all we need a new standard here. NEP-5 is NEP-5 and it shouldn't be changed.

We use different standards for neo2 and neo 3. Before mainnet it's only an amend.

[ixje]

I think first of all we need a new standard here. NEP-5 is NEP-5 and it shouldn't be changed.

We use different standards for neo2 and neo 3. Before mainnet it's only an amend.

This just breaks more existing tools unnecessary

[erikzhang]

I think first of all we need a new standard here. NEP-5 is NEP-5 and it shouldn't be changed.

I think it is compatible with NEP-5. Because there is no onPayment method in any deployed contract on Neo 2. So nothing actually changed.

[roman-khimov]

I think it is compatible with NEP-5

It's not enough IMO. A standard only works if it's the standard, it doesn't change in any way and any reference to it has exactly the same meaning. Otherwise "NEP-5 compliance" doesn't really mean anything, it could NEP-5 of this or that or some other revision and no one would be able to tell what exactly is meant by this.

[vncoelho]

I also believe it is still compatible with NEP-5.
In principle, it is basically an improvement that is connected with NEO 3 capabilities. The onpayment brings a subtle change in the way contracts can optionally react.

[ixje]

I also believe it is still compatible with NEP-5.

compatibility != compliancy
Just because the added behaviour doesn't necessary burn the system down, doesn't mean it is compliant. As @roman-khimov said, what's the point of a standard (without some form of revision identifier) if you can't identify the EXACT requirements it has. That's like saying you don't need to tag any software builds because what's the point?

[EdgeDLT]

onPayment has use for any kind of contract, not just NEP-5 ones, right? Then it should be its own standard regardless of how compatible it is with existing NEP-5.

I'd rather you amend Neo3's definition of NEP-5 to declare that it must follow a new onPayment NEP, rather than put onPayment straight into NEP-5. Ideally we can avoid amendment entirely and just leave NEP-5 alone. I think a NEP-5 token upgraded to take advantage of new functionality is not a NEP-5 token, it is something new, NEP-20 etc.

[vncoelho]

onPayment has use for any kind of contract, not just NEP-5 ones, right? Then it should be its own standard regardless of how compatible it is with existing NEP-5.

This is a good point, @EdgeDLT.

[vncoelho]

@EdgeDLT, in fact, this onPayment has other uses as you mentioned, for example, a deployed multi-sig that does not want to receive tokens, airdrops, etc...
Or even a wallet that want to keep it maximum amount limited to X.

[EdgeDLT]

Automatically refunding accidental transfers seems like a nice use case.

[vncoelho]

I think it would abort execution, @EdgeDLT, not even a need to refund.

[erikzhang]

onPayment has use for any kind of contract, not just NEP-5 ones, right?

I'm afraid not. The parameter list is NEP-5 specific. For example, if it is NFT transfer, then the parameter list of onPayment should contain tokenid.

[erikzhang]

If a contract is deployed but the onPayment method doesn't exist, should the transfer be processed?

[vncoelho]

Maybe yes, @erikzhang, because any valid address should be able to receive assets.
onPayment could be seen as a suggested standard and possible protection.

[roman-khimov]

If a contract is deployed but the onPayment method doesn't exist, should the transfer be processed?

We can make onPayment mandatory for payable contracts.

[erikzhang]

We can make onPayment mandatory for payable contracts.

Or if there is no onPayment method, we think it is not payable. We don't need payable field anymore.

[igormcoelho]

I think @EdgeDLT has a good point.. are we able to create some new standard, NEP-XX which is named like "NEP-5 on Neo3", just including this amend?
This way, amend would have a new number, but tokens could keep popularly calling NEP-5 (on both Neo2 and Neo3). Naturally, implementations of NEP-5 over Neo3 would include this extra checking.

[erikzhang]

I created a new PR with a new NEP number: #126

[shargon]

Closed in favor of #126