0

Currently, I am planning to implement an ERC20 compliant transfer function in my ERC20 compliant contract as follows:

function transfer(address _to, uint256 _amount) public returns (bool success) 
    {
        if (balances[msg.sender] >= _amount 
            && _amount > 0
            && balances[_to] + _amount > balances[_to]) 
        {
            balances[msg.sender] -= _amount;
            balances[_to] += _amount;
            Transfer(msg.sender, _to, _amount);
            return true;
        } else {
            return false;
        }
    }

I would prefer to use "require" for the if block and not use the else, but then if it's false, I don't see how I'd then return a false. It would return an error or exception - correct?

Perhaps that would be the preferred behavior, but is it OK that a "false" does not get returned by the function when the require logic is not met?

I've also asked two instructors on Udemy, and I do believe that either of their answers would work, but their answers were different. Thus, it seems an important question to have discussed on record for our community.

I also plan to add openzepplin's safe math library functions as needed.

Improve this question
6
  • 1
    There's no need to make the transfer fail if _amount == 0. In fact, it is counterproductive. Consider the case where a contract wants to transfer a varying amount of tokens. Its calculation might end up with 0. There's nothing inherently wrong with that, so there's no reason to make a calling contract think the transfer failed. Throwing on 0 is a common anti-pattern in Solidity development which hurts the equal treatment of contracts and humans.
    – Jesbus
    Jan 1, 2018 at 18:55
  • 1
    @JesseBusman could you elaborate on "throwing on 0 anti-pattern in Solidity"?
    – medvedev1088
    Jan 1, 2018 at 18:58
  • 1
    @medvedev1088 From Wikipedia: "An anti-pattern is a common response to a recurring problem that is usually ineffective and risks being highly counterproductive." I've reviewed many Solidity contracts and found a large amount of cases where contracts make a transaction fail if a user tries to call a function with an argument equal to 0, even though it is completely harmless. For example: Minting 0 tokens, burning 0 tokens, withdrawing 0 ether, transferring 0 tokens, etc. It's a very common mistake I have seen.
    – Jesbus
    Jan 1, 2018 at 19:02
  • 1
    @JesseBusman do you have a general guideline on which cases should be handled with require and which cases should be handled with returning a status code e.g. true/false (or simply ignoring the illegal arguments in case the function returns nothing)? One could argue that any require could be converted to if-else and it will be harmless in this case.
    – medvedev1088
    Jan 1, 2018 at 19:15
  • 1
    @JesseBusman something like: if the method call with such an argument will result in a consistent and meaningful outcome even without any additional checks for this specific value of this argument, then the method shouldn't throw in this case. Does it make sense?
    – medvedev1088
    Jan 1, 2018 at 19:26

1 Answer 1

Reset to default
2

Per https://github.com/ethereum/EIPs/blob/master/EIPS/eip-20-token-standard.md#transfer:

The function SHOULD throw if the _from account balance does not have enough tokens to spend.

So I'd say you should indeed use require, but don't include the _amount > 0 clause. From the same documentation:

Note Transfers of 0 values MUST be treated as normal transfers and fire the Transfer event.

In general, I think it's fine if this function never returns false, and I believe that most implementations I've seen do behave that way.

Improve this answer
4
  • 1
    While I agree with your answer the guidelines in the EIP seem confusing to me. Why do they have to mix the status code (success) with exceptions? If insufficient balance should throw then what should return false for success?
    – medvedev1088
    Jan 1, 2018 at 19:40
  • I'd say from the answers I received from my Udemy instructors, your answer is very sound. Thanks!
    – Thom Ives
    Jan 1, 2018 at 19:50
  • 1
    @medvedev1088 I agree with everything you said. :-) The guidance seems quite unclear to me. I can imagine reasons for wanting a boolean return value versus aborting the transaction, but because of the somewhat conflicting guidance and the variety of contracts out there, I don't think we're truly able to gain any benefit from the return value in practice.
    – user19510
    Jan 1, 2018 at 19:52
  • 1
    @medvedev1088 Your comments reflect the same struggles that I have had. It seems we are still struggling from the newness of all this. I too wish this whole Boolean return issue with ERC20 compliance was more clear. I like the idea that using require forgoes gas expenditures when something does not meet required conditions.
    – Thom Ives
    Jan 1, 2018 at 19:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.