Allow deposit from non-token owner

[bogdan]

Fixes #101

CC @itzmeanjan

[itzmeanjan]

Thanks @bogdan , we'll review it.

[zorancuc]

depositor is not checked how it's related with token to deposit but used just for event. I think we need to check how depositor is related with msg.sender or tokenId

[bogdan]

depositor is not checked how it's related with token to deposit but used just for event. I think we need to check how
depositor is related with msg.sender or tokenId

Do you think it makes sense to do? Once a predicate is approved for token, anyone can call depositFor and pay the gas. What problems do you see with that?

[nitinmittal23]

@bogdan @zorancuc According to this PR, once the approval to mintableERC20Predicate is given then anyone can come and deposit on behalf of user. This is good for your use case but not in general terms.

Let say i just gave approval to mintableERC20Predicate but afterwards i decided not to bridge it. but now anyone can come and deposit on my behalf. You see, i have ERC721 today.. but tomorrow someone deposited it for me on polygon. Although it will be in my account, but i will have to spend ETH to bring it back on ethereum (root) chain

[bogdan]

@nitinmittal23 I think this scenario is quite surreal for the following reasons:

• If you approved someone to do the operation, it is already a privilege that you can withdraw the approval which is not available with a lot of custody contracts that are used as approvals
• Approve operation already cost you something so thinking in advance if you want to finish the deposit after an approval is quite bad idea
• There is very little reasoning to finish someones deposit to make damage because you get zero value yourself
• The amount of damage caused is quite low: you can't make someone lose his token this way.
• From the end user UI perspective, I think there should be no way to stop the deposit once a predicate is approved: it's all just unnecessary details. In the ideal world, deposit should happen with one transaction, but not two and have a regular way to cancel using a front running with higher gas price from the same account with the same nonce.

[alexletu]

Wanted to follow up and see if there are any plans on merging this functionality into production. We're in a scenario where we would like for a user to be able to deposit to polygon by interacting with our contract first (to send some metadata via state sync). However, it looks as though that would require the user to approve our contract, our contract then can transfer tokens to itself, from which it can then approve the predicate then call the deposit function - resulting in excess gas use compared to the scenario where we can specify to the bridge a different source for tokens other than msg.sender.

I see the argument that can be made that this opens up the possibility of griefing by a malicious third party; however, I think it's reasonable to assume that it is a highly problematic scenario since the attacker would need to pay the entire fee (which is comparable to the amount the user would need to pay to bring the tokens back to the root chain - i.e. the attacker would be paying fees on the same order of magnitude as to the victim).

Additionally, if a user has already approved the predicate transfer, wouldn't the best practice be to operate under the assumption that those tokens are no longer under your ownership anyways, since 1. we could not guarantee that the the permitted address is not invoked in some other manner, and 2. it seems unnatural to provide functionality for the case in which a user 'changes their mind' after an approval and before transferFrom can take place. It feels more like an accommodation for buyer's remorse.

[bogdan]

100% agree with @alexletu. We are having the same scenario and concerns.