1

I making a contract to sell stuff in a decentralized way and want to use following method to transfer founds.

function transferToken(address token, address from, uint256 amount) internal {
    IERC20 token_ = IERC20(token);

    uint256 pBalance = token_.balanceOf(address(this));

    bool transferResult = token_.transferFrom(from, address(this), amount);
    require(transferResult == true, "Unable to transfer");

    uint256 nBalance = token_.balanceOf(address(this));
    require(pBalance.add(amount) == nBalance, "Invalid transfered amount");
}

The thing is that the contract verify if new balance is the previous one plus the last transfer. This works fine, but I have a doubt.

What happen when two calls to this method usign same token are made in the same block? Each transaction is executed following transaction index? At time that second one is executed pBalance will be updated with first one?

I hope I was clear

Thanks!

2
  • 1
    so what i understand is that 2 people call that function at the same time and you are worry that the second transaction is gonna overwrite the first one? am i right?
    – haxerl
    Commented Mar 21, 2019 at 14:43
  • 1
    Yes, everything will be executed in a synchronous manner, so nothing to worry about in this aspect. Commented Mar 21, 2019 at 15:05

2 Answers 2

1

There is no concurrency in the EVM. It is single threaded.

Even though two or more transactions could call this method, each using the same ERC20 token, within the same block... There is an order established within that block. An order of execution. No two transactions occur at the same exact time. One will complete, or fail, before the next one starts.

Given this, you have nothing to worry about.

0

You don't need to worry about concurrency, so you should be safely able to reduce this to:

function transferToken(address token, address from, uint256 amount) internal {
    IERC20 token_ = IERC20(token);
    require(transferResult = token_.transferFrom(from, address(this), amount), "Unable to transfer");
}

In case it helps, there is a known issue with the ERC20 interface itself and allowances.

In the case that the user approves a transfer and does not follow through, the client should reset the allowance to zero. What's less intuitive is the case where this is an existing allowance and the user wishes to increase it. The user should first set it to zero and confirm before setting to the new higher value, otherwise there is a security hole that could be exploited by a nasty contract.

Hope it helps;

4
  • I dont think that attack is feasible because you have be very prepare for it and you have to know the exact time to do it so the chace it happen it's pretty slim and no one have ever exploited it successful yet
    – haxerl
    Commented Mar 22, 2019 at 1:05
  • 1
    I would argue that the check afterward should still be made given that you do not trust the ERC20 token for various factors. Source code could not be verified and malicious actors modified the implementation of the ERC20 intereface. Given that the ERC20 token is trusted, then the check is not needed. But this check would have to be required beforehand or enforce at the smart contract level. Commented Mar 22, 2019 at 16:05
  • 1
    Also, OpenZeppelin has updated the approve() function and added increaseAllowance and decreaseAllowance. github.com/OpenZeppelin/openzeppelin-solidity/blob/master/… Commented Mar 22, 2019 at 16:10
  • That does the trick when used correctly. Thanks for adding that. Commented Mar 22, 2019 at 16:22

Your Answer

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

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