How to transfer ETH from contract to external address after crowsale?

Question

I am trying to learn creating smart contracts. For practice, I created token and crowdsale contracts by following the examples on ethereum.org and deployed it. Token distribution is working as expected, but when the contract deadline is met(Funding goal is also achieved), the funds from contract account are not transferred to Beneficiary account.

Here is the code:

pragma solidity ^0.4.11;
contract token { function transfer(address, uint){  } }

contract Crowdsale {
    address public beneficiary;
    uint public fundingGoal; uint public amountRaised; uint public deadline; uint public price;
    token public tokenReward;
    mapping(address => uint256) public balanceOf;
    bool fundingGoalReached = false;
    event GoalReached(address beneficiary, uint amountRaised);
    event FundTransfer(address backer, uint amount, bool isContribution);
    bool crowdsaleClosed = false;

    /* data structure to hold information about campaign contributors */

    /*  at initialization, setup the owner */
    function Crowdsale(
        address ifSuccessfulSendTo,
        uint fundingGoalInEthers,
        uint durationInMinutes,
        uint etherCostOfEachToken,
        token addressOfTokenUsedAsReward
    ) {
        beneficiary = ifSuccessfulSendTo;
        fundingGoal = fundingGoalInEthers * 1 ether;
        deadline = now + durationInMinutes * 1 minutes;
        price = etherCostOfEachToken * 1 ether;
        tokenReward = token(addressOfTokenUsedAsReward);
    }

    /* The function without name is the default function that is called whenever anyone sends funds to a contract */
    function () payable {
        if (crowdsaleClosed) revert();
        uint amount = msg.value;
        balanceOf[msg.sender] = amount;
        amountRaised += amount;
        tokenReward.transfer(msg.sender, amount / price);
        FundTransfer(msg.sender, amount, true);
    }

    modifier afterDeadline() { if (now >= deadline) _; }

    /* checks if the goal or time limit has been reached and ends the campaign */
    function checkGoalReached() afterDeadline {
        if (amountRaised >= fundingGoal){
            fundingGoalReached = true;
            GoalReached(beneficiary, amountRaised);
        }
        crowdsaleClosed = true;
    }


    function safeWithdrawal() afterDeadline {
        if (!fundingGoalReached) {
            uint amount = balanceOf[msg.sender];
            balanceOf[msg.sender] = 0;
            if (amount > 0) {
                if (msg.sender.send(amount)) {
                    FundTransfer(msg.sender, amount, false);
                } else {
                    balanceOf[msg.sender] = amount;
                }
            }
        }

        if (fundingGoalReached && beneficiary == msg.sender) {
            if (beneficiary.send(amountRaised)) {
                FundTransfer(beneficiary, amountRaised, false);
            } else {
                //If we fail to send the funds to beneficiary, unlock funders balance
                fundingGoalReached = false;
            }
        }
    }
}

Am I missing something?

Answer 1

You've possibly provided too little information to really be helpful.

Can you post the contract's address and the parameters used to create it?

address ifSuccessfulSendTo       =
uint fundingGoalInEthers         =
uint durationInMinutes           =
uint etherCostOfEachToken        =
token addressOfTokenUsedAsReward =

Regardless, couple things you can try:

1. Make sure you are calling safeWithdrawal() from the same address you used to create the contract. If this transaction fails, then fundingGoalReached goes back to false, which means you have to call checkGoalReached() to set it back to true before calling safeWithdrawal() again.

2. When calling the contract's methods try sending a large amount of gas, eg 200,000, to be safe (unused gas will always be sent back to you).

---

balanceOf[msg.sender] = amount;

Also, this seems problematic, as it sets the balance value of the depositor each time and doesn't increment it, eg. when making multiple deposits.

amountRaised += amount;

Finally, this is typically not advised as it opens up overflow problems. Probably better to use something like SafeMath.sol by zeppelin-solidity.

Answer 2

Have you executed the safeWithdrawal() function on the contract from the address you specified as the beneficiary when you deployed the contract?

The funds from contract account are not transferred to Beneficiary account automatically.