Dec 20, 2018 Technology

Safely Using ERC20s

A technical look at the safe and secure use of ERC20 tokens during the Decentraland Auction

Ignacio Mazzara

Share

At Decentraland we want to distribute the remaining 9,300 unowned LAND parcels that were leftover from the first auction in 2017 which gave birth to Genesis City. We decided that the best way to distribute all of this LAND was through a Dutch auction, which is more suitable for on-chain transactions.

A Dutch auction can be thought of as one in which all parcels are set at the same high price, which gradually decreases over time. Whoever the first “bidder” is on any given parcel, wins the parcel at the current price.

An additional goal was to expand our community by inviting other ERC20 based projects to participate, like BNB, ZIL, DAI, MAKER, to name a few. This way we could guarantee that most of the LAND will be sold during the Auction, by opening up the Auction to a wider community.

As part of these partnerships, we decided to burn all tokens exchanged for LAND 🔥with the exception of DAI which we sent directly to a charity foundation called NeedsList.

As an outcome of the first audit, we found out that most ERC20 token core methods were not as standardized as we initially thought. After taking a look at different implementations we noticed transfer, transferFrom and approve had core differences between the different deployed contracts. Let’s take a look…

transfer

Standard

From the Zeppelin ERC20 implementation:

function transfer(address to, uint256 value) public returns (bool) {
 _transfer(msg.sender, to, value);
 return true;
}

No reverts

Tokens as RCN return false in case the pre-conditions are false:

function transfer(address _to, uint256 _value) returns (bool success) {
 if (balances[msg.sender] >= _value) {
   balances[msg.sender] = balances[msg.sender].sub(_value);
   balances[_to] = balances[_to].add(_value);
   Transfer(msg.sender, _to, _value);
   return true;
 } else {
   return false;
 }
}

Without returning value

Tokens as BNB hasn’t a return value when performing a transfer:

function transfer(address _to, uint256 _value) {
 if (_to == 0x0) throw; // Prevent transfer to 0x0 address. Use burn() instead
 if (_value <= 0) throw;
 if (balanceOf[msg.sender] < _value) throw; // Check if the sender has enough
 if (balanceOf[_to] + _value < balanceOf[_to]) throw; // Check for overflows
 balanceOf[msg.sender] = SafeMath.safeSub(balanceOf[msg.sender], _value); // Subtract from the sender
 balanceOf[_to] = SafeMath.safeAdd(balanceOf[_to], _value); // Add the same to the recipient
 Transfer(msg.sender, _to, _value); // Notify anyone listening that this transfer took place
}

transferFrom

Standard

From Zeppelin ERC20 implementation:

function transferFrom(
   address from,
   address to,
   uint256 value
 )
   public
   returns (bool)
 {
   require(value <= _allowed[from][msg.sender]);

   _allowed[from][msg.sender] = _allowed[from][msg.sender].sub(value);
   _transfer(from, to, value);
   return true;
}

No reverts

Tokens as RCN return false in case the pre-conditions are false:

function transferFrom(address _from, address _to, uint256 _value) returns (bool success) {
 if (balances[_from] >= _value && allowed[_from][msg.sender] >= _value) {
   balances[_to] = balances[_to].add(_value);
   balances[_from] = balances[_from].sub(_value);
   allowed[_from][msg.sender] = allowed[_from][msg.sender].sub(_value);
   Transfer(_from, _to, _value);
   return true;
 } else {
   return false;
 }
}

approve

Standard

From Zeppelin ERC20 implementation:

function approve(address spender, uint256 value) public returns (bool) {
 require(spender != address(0));

 _allowed[msg.sender][spender] = value;
 emit Approval(msg.sender, spender, value);
 return true;
}

With clean-first approach

Tokens as MANA check if the allowed balance is 0 or will be set to 0 before setting it:

function approve(address _spender, uint256 _value) returns (bool) {
   // To change the approve amount you first have to reduce the addresses`
   //  allowance to zero by calling `approve(_spender, 0)` if it is not
   //  already 0 to mitigate the race condition described here:
   //  https://github.com/ethereum/EIPs/issues/20#issuecomment-263524729
   require((_value == 0) || (allowed[msg.sender][_spender] == 0));

   allowed[msg.sender][_spender] = _value;
   Approval(msg.sender, _spender, _value);
   return true;
 }

Without clear approval

Tokens as BNB doesn’t have a way to clear approvals:

function approve(address _spender, uint256 _value) returns (bool success) {
 if (_value <= 0) throw;
 allowance[msg.sender][_spender] = _value;
 return true;
}

We saw an opportunity to create SafeERC20.sol: a library that brings an abstract layer above the ERC20 standard interface to provide a way to call its methods safely by checking pre and post-conditions.

Also, every method returns a bool that could be wrapped with a revert call to prevent the loss of all the Ether sent as gas when a transaction fails. This is really useful for tokens developed before Solidity included the revert call and still use throw and/or assert.

SafeERC20 interface

safeTransfer

Perform the transfer method from ERC20.

• Check if the value to be transferred is lower or equal to the account balance.
• Check if account balance after the transfer is equal to previous balance minus the value transferred.

function safeTransfer(IERC20 _token, address _to, uint256 _value) internal returns (bool) {
 uint256 prevBalance = _token.balanceOf(address(this));

 if (prevBalance < _value) {
     // Insufficient funds
     return false;
 }

  address(_token).call(
     abi.encodeWithSignature("transfer(address,uint256)", _to, _value)
 );

 if (prevBalance - _value != _token.balanceOf(address(this))) {
     // Transfer failed
     return false;
 }

 return true;
}

safeTransferFrom

Perform the transferFrom method from ERC20.

• Check if the value to be transferred is lower or equal to the account balance.
• Check if the value to be transferred is lower or equal to the allowance of the account which is going to perform the transfer.
• Check if account balance after the transfer is equal to previous balance minus the value transferred

function safeTransferFrom(
       IERC20 _token,
       address _from,
       address _to,
       uint256 _value
   ) internal returns (bool)
{
 uint256 prevBalance = _token.balanceOf(_from);

 if (prevBalance < _value) {
     // Insufficient funds
     return false;
 }

 if (_token.allowance(_from, address(this)) < _value) {
     // Insufficient allowance
     return false;
 }

 address(_token).call(
     abi.encodeWithSignature("transferFrom(address,address,uint256)", _from, _to, _value)
 );

 if (prevBalance - _value != _token.balanceOf(_from)) {
     // Transfer failed
     return false;
 }

 return true;
}

safeApprove

Perform the approve method from ERC20.

• Check if the allowance set is equal to the required value to approve.

function safeApprove(IERC20 _token, address _spender, uint256 _value) internal returns (bool) {
 address(_token).call(
     abi.encodeWithSignature("approve(address,uint256)",_spender, _value)
 );

 if (_token.allowance(address(this), _spender) != _value) {
     // Approve failed
     return false;
 }

 return true;
}

clearApprove

Method to clear approval.

Tokens as BNB don’t accept 0 as a valid value for approve. So if calling safeApprove with 0 fails, the library will try with 1 WEI.

function clearApprove(IERC20 _token, address _spender) internal returns (bool) {
 bool success = safeApprove(_token, _spender, 0);

 if (!success) {
     return safeApprove(_token, _spender, 1);
 }

 return true;
}

Full code

Caveats

• Using interface methods like transfer will fail for tokens that don’t return a value, such as BNB. This is because since the Byzantium hard fork, the EVM has a new opcode called RETURNDATASIZE. This opcode stores the size of the returned data of an external call. The code then checks the size of the return value after an external call and reverts the transaction in case the return data is shorter than expected. You can find more information about this issue here, along with a list of tokens with this problem (and how to deal with it using assembly).
• Some tokens check if you are going to transfer a value <= 0, and will throw an error if that’s the case. We decided to handle it in the library by not checking the success of the transfer call, but by checking the balance after calling the method. So if you want to transfer 0, the transfer call will fail but the post-condition of checking the balance will succeed.
• We avoided the use of assembly even though it consumes less gas because it is a black box for the standard Solidity developer and also error prone. This is way easier to read and understand.

Finally, with Solidity 0.5.x the methods .call(), .delegatecall() and .staticcall() all return (bool, bytes memory) to provide access to the return data. We are working on an updated version of our library that’ll support for these changes.

We hope that this will help further the standardization of most of ERC20, ERC721 and other widely used tokens used within the community.

Special thanks to Agustin Aguilar who discovered the first differences between ERC20 tokens in the audit for the LANDAuction contract and Patricio Palladino for shedding some light about the Solidity compiler.