Allow deposit by someone who is not a token owner

[bogdan]

Motivation

In our app we want to provide people a better user experience when they deposit their tokens to polygon.
That is why we want RootChainManager#depositFor to be callable by an address that is not a token owner (like approved or approved for all).

Problem

That is not currently possible with MintableERC721Predicate (I didn't check others, but it is probably the same issue there).

Here is why:
RootChainManager always uses _msgSender as a depositor:

pos-portal/contracts/root/RootChainManager/RootChainManager.sol

Line 313 in 88dbf0a

_msgSender(),

depositor is always passed as a first argument to ERC721#safeTransferFrom which should always be a token owner:

pos-portal/contracts/root/TokenPredicates/MintableERC721Predicate.sol

Line 94 in 88dbf0a

IMintableERC721(rootToken).safeTransferFrom(depositor, address(this), tokenId);

Solution

Use ERC721#ownerOf instead of depositor when calling ERC721#safeTransferFrom.

If that is not acceptable, can you explain a motivation of this limitation?

[itzmeanjan]

Hello @bogdan , given that https://github.com/OpenZeppelin/openzeppelin-contracts/blob/4961a51cc736c7d4aa9bd2e11e4cbbaff73efee9/contracts/token/ERC721/ERC721.sol#L181 kind check is implemented in root contract and depositor is non-owner address but allowed to transfer on owner's behalf, does it not solve your problem ?

[bogdan]

@itzmeanjan nope, here the problem is in the from argument of safeTransferFrom that in the end should be exactly equal to token owner: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/4961a51cc736c7d4aa9bd2e11e4cbbaff73efee9/contracts/token/ERC721/ERC721.sol#L332 all the time.

[itzmeanjan]

Yes, I get it now. For ERC721 we can probably implement it.

[itzmeanjan]

Would you be able to send one PR @bogdan ?