clearApproval should not fire event

[mg6maciej]

Because this function is called only from _burn and _transfer in which cases also Transfer is emitted, clearApproval should only set state to 0 (might be more gas efficient to do it conditionally).

According to spec (https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md)

    ///  When a Transfer event emits, this also indicates that the approved
    ///  address for that NFT (if any) is reset to none.
    event Approval(address indexed _owner, address indexed _approved, uint256 indexed _tokenId);

OZ also does it incorrectly, but "slightly" better.
https://github.com/OpenZeppelin/openzeppelin-solidity/blob/07020e954475a4fdd36e0252e88717b60f790b71/contracts/token/ERC721/ERC721BasicToken.sol#L300-L303

[lknix]

Hm, yes, I think this is a valid reasoning based on the EIP comment. @fulldecent can you comment on it?

[MoMannn]

You are correct.

Here is a similar discussion for reference:
ethereum/EIPs#841 (review)

[xpepermint]

Alright, to the PR ...

[fulldecent]

I can’t see the context here. Is there a PR?

[xpepermint]

@fulldecent the PR for this is in progress and will be ready by tomorrow. I will ask you to check that just to be sure it's aligned with the standard.

[fulldecent]

Yes, got it! clearApproval should not emit. We are currently emitting a superfluous event.

[MoMannn]

#101