Class FungibleToken is not checking if onNEP17Payment method exists before calling it. Should it?

[lock9]

The NEP-17 standard states that if a contract wants to refuse an incoming transfer, it MUST call Abort.

The existing FungibleToken implementation does not check if the method exists. This causes transfers to fail if the recipient contract didn't implement the onNEP17Payment instead of failing only unless the receiver aborts it. This may be fine, but this isn't what the standard was proposing.
Contracts should only call the onNEP17Payment callback after checking that it exists, likely using ContractManagement.hasMethod.

Changing the FungibleToken native contract is a huge breaking change. It's easier if we just change the standard to say that smart contract SHOULD check for the onNEP17Payment before calling it.

[roman-khimov]

The existing FungibleToken implementation does not check if the method exists.

That's exactly the intended behavior and it's compliant with NEP-17.

[lock9]

So if a contract wishes to be able to receive tokens, it must implement the callback. This is not stated in the document.

What about the part about "the receiver must call abort"? In my use case, I didn't call abort, and it was rejected.

In practice, there is no need to fail the transfer. Or is there?

[cschuchardt88]

I agree. thats why i created neo-project/proposals#164 Its to loose currently and miss leading... and contradicts itself.

[roman-khimov]

So if a contract wishes to be able to receive tokens, it must implement the callback. This is not stated in the document.

NEP-17: "If the receiver is a deployed contract, the function MUST call onNEP17Payment method on receiver contract". There are no conditions, just a method call. No method --- failure, because it MUST be called.

What about the part about "the receiver must call abort"?

1. It's the receiver choice, whether to accept the transfer or not.
2. It's important to ABORT and not THROW in this case.

In practice, there is no need to fail the transfer. Or is there?

https://github.com/AxLabs/grantshares-contracts/blob/aec526734132f762c99df01bb7642d2b5209c529/src/main/java/com/axlabs/neo/grantshares/GrantSharesTreasury.java#L175
https://github.com/nspcc-dev/neofs-contract/blob/f083b3a4f24dd93b4ac5f1c41a23ef522304e0a6/contracts/neofs/contract.go#L237

[lock9]

So in this case, this code is incorrect:

    if (ContractManagement.get_contract(to_address) is not None):
        if (ContractManagement.has_method(to_address, 'onNEP17Payment', 3)):
            contract.call_contract(to_address, 'onNEP17Payment',
                                   args=[from_address, amount, data])

We should use this instead:

if (ContractManagement.get_contract(to_address) is not None):
        contract.call_contract(to_address, 'onNEP17Payment',
                                   args=[from_address, amount, data])

?