safe_mint and safe_transfer should require _invokeNFTReceiver to return true

[marc0olo]

I guess you would want this entrypoint to throw and error and fail if the recipient / receiver doesn't implement this interface or returns false.

see:

• ae-nft/contracts/NFT.aes

  Line 165 in 05e4ef9

  None => false

• ae-nft/contracts/NFT.aes

  Line 168 in 05e4ef9

  false

[marc0olo]

basically I mean the require is missing somewhere here. either in this method or where the method is invoked, e.g.:

• ae-nft/contracts/NFT.aes

  Line 115 in 05e4ef9

  _invokeNFTReceiver(Contract.address, _to, _token_id, _data)

[arjanvaneersel]

What I tried to say on Discord is that ERC-721 doesn't throw in case the receiver doesn't implement the method. So therefore I use the bool to indicate whether the call was successful or not. However what I do notice is that with the changes today I removed prolongating the bool result up. So instead of require and throw, it should at least return the bool, so that the developer of a contract can decide whether to throw or not.

[marc0olo]

let me rephrase the issue to be more clear. IMO an entrypoint called safe_mint or safe_transfer should require that check to be true. do you agree? this is what I tried to explain

[arjanvaneersel]

I agree with you, but it would mean that we break ERC-721 compatibility.

[marc0olo]

I agree with you, but it would mean that we break ERC-721 compatibility.

can you elaborate on that one? I don't understand 😅 let's get first the discussion about the interface getting started in the AEX proposal and then digging deeper into implementation details

[arjanvaneersel]

You are right. It should throw if the call fails. I included this in the AEX proposal