Crowdsale contract and safe withdrawals

Question

According to crowdsale contract at https://www.ethereum.org/crowdsale tokens are transferred to a user account when "function () payable" is called by using external function tokenReward.transfer(msg.sender, amount / price). Nevertheless when user claim its ethers at safeWithdrawal() because fundingGoal hasn't been reached, there is no call to tokenReward.transfer() external function, so it seems to me that the bought tokens are not withdrawed properly, although ether is sent to the user.

Has anybody used this code to crowsale tokens and perform a withdrawal as a user? Do the token balance at token contract decrease properly when safeWithdrawal is called? How is that possible?

Refs.:

Pre ico and ico crowdsale contracts

Buy ICO tokens?

Buy and sell of my tokens

Answer 1

Has anybody used this code to crowdsale tokens and perform a withdrawal as a user?

IMO that code is more a sketch of an actual ICO and not meant as a secure and proper ICO reference implementation. It has at least one issue described below. Furthermore, I am not aware of anyone using exactly this code, most of the time the upper limit (cap) seems to be more relevant than the lower limit that is implemented here.

Do the token balance at token contract decrease properly when safeWithdrawal is called? How is that possible?

This contract tracks the paid contributions (in Wei) in addition to the separate token contract (see below for some issues regarding this implementation). Inside the safeWithdrawal function you have the following line that takes care of resetting the paid contribution when the funding limit has not been reached:

balanceOf[msg.sender] = 0;

This token sale contract aims at working with any token. Hence "destroying" the reimbursed tokens would have to be handled on a case-by-case basis in that token contract. Specifically, you as a user would have to first call the approve function of a standard token and now the contract here could attempt to burn the tokens by sending them to an unspendable address (e.g. 0x000...) or send it to itself.

Your point that upon reimbursing Ether, tokens are not destroyed/reimbursed is correct. However, you could argue that these tokens are simply meaningless if all Ether have been reimbursed. You could go even further and say that you actually save gas for every single investor that wants their Ether reimbursed - the token is meaningless anyway if the project did not get any Ether, so why reallocate token balances?

Now for

The issue:

This crowd sale contract overwrites contributed Ether per address instead of increasing the balance:

uint amount = msg.value;
balanceOf[msg.sender] = amount;

This is an issue for investors that send multiple transactions and would then lose funds in the reimbursement period. In fact all Ether except for the ones send in the last transaction would be unspendably locked in the smart contract forever.

I would therefore suggest

A fix:

uint amount = msg.value;
balanceOf[msg.sender] += amount;

Of course it would be good practice to a library like SafeMath instead of doing a simple increment.

Now you do not have to bother about reimbursing or destroying Ether anymore, any investor can always simply get their contributed amount back if the funding limit has not been reached. Of course every token holder should be aware that in that case they are simply involved in yet another totally Useless Ethereum Token.