astralship / eos

Auction on Ethereum - review - request for comments - current version deployed to Ropsten
https://ropsten.etherscan.io/address/0xafc7d50c3befceb590e7dcd86aa315f372d0f725#code
4 stars 2 forks source link

It is not safe to use loop to transfer funds. It may never work #11

Open fosgate29 opened 6 years ago

fosgate29 commented 6 years ago

https://github.com/astralship/eos/blob/4a4929d7e434ff8f2aec148fd038f30fa3cf26e4/contracts/Auction.sol#L88

I checked the code again and realized there is glitch.

When you do a transfer(), execution flow is transfered to an external code. For example, If the address that you are refunding is a smart contract and it hangs or revert when receiving funds, you won´t be able to refund your users using this function.

You are safe because you have refund() and each user can go and get its funds back.

I am writing this issue just to let you know what can happen when using a loop and transfer.

stefek99 commented 6 years ago

I see!

Their fallback function might not be payable...

And throw / revert / failure...

Pretty epic!