Why are we calling the ERC721 _isApprovedOrOwner function?

[PaulRBerg]

I was looking at our implementation of _isApprovedOrOwner:

https://github.com/sablierhq/v2-core/blob/2445865e3269481fb47fb320e71d2e1f42382cf6/src/SablierV2Linear.sol#L201

We need to define it in the abstract contract SablierV2 because it's needed by the common implementations in there. That is fine. What is not fine is the fact that we're piggybacking off the standard ERC-721 implementation written by OpenZeppelin:

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/49c0e4370d0cc50ea6090709e3835a3091e33ee2/contracts/token/ERC721/ERC721.sol#L239-L242

They check whether the owner is the zero address, but this is a redundant check in our case, because all of our functions are protected by the streamExists modifier.

It is a protocol invariant that if the stream exists, the NFT exists, too. It is not possible for the stream to exist without the NFT existing. However, once #169 is implemented, it will be possible for a stream to not exist while a "zombie" NFT still exists, though even in that case, the streamExists modifier will do its job and prevent any misuse of our functions.

Therefore, I suggest we rewrite a custom _isApprovedOrOwner implementation to remove the check for the owner address.

Update: I can confirm that after implementing _isApprovedOrOwner myself and calling _ownerOf instead of ownerOf (to avoid the zero address check), all tests are still passing. Therefore, I suggest we march on with my proposal.

[PaulRBerg]

Locking, since this has been fixed in #201.