search

cartesi / rollups-contracts

Smart Contracts for Cartesi Rollups
https://cartesi.github.io/rollups-contracts/
Apache License 2.0
21 stars 39 forks source link

Make ERC-20 portals reject failed transfers again #113

Closed guidanoli closed 11 months ago

guidanoli commented 1 year ago

📚 Context

The job of the ERC-20 portal is to manage ERC-20 token deposits to the DApp. It does so by calling the transferFrom function on the token contract and adding an input in the DApp's input box. This function returns a boolean value which signals if the transfer was successful or not.

Prior to 0.9.0, the ERC-20 portal required this boolean value to be true, which means that the DApp would only be notified of an ERC-20 if it were successful.

On 0.9.0, we decided to accept transfers in which the transferFrom function returned false, and added this information in the input itself. The rationale was that EIP-20 did not specify clearly the semantics of this value, and we would like the DApp back-end to handle this information however they want.

On October 25th, however, @gligneul pointed out that this behavior could open the possibility for DApp developers to mistakenly accept failed transfers, should they not check this boolean value within the input. @ZzzzHui, @diegonehab and I agreed.

✔️ Solution

Make the depositERC20Tokens function revert if transferFrom returns false. We may keep the input structure for backwards compatibility.

guidanoli commented 12 months ago

This has been fixed by #117

pedroargento commented 11 months ago

We should keep the true return value in the input to preserve backwards compatibility.

gligneul commented 11 months ago

We should keep the true return value in the input to preserve backwards compatibility.

@pedroargento, could you remove it in the next major release?

guidanoli commented 11 months ago

This issue has been addressed by #117 @gligneul We'll remove the success field on the next major release (tracked by #132)