Some basic problem about Smart Contract

Question

I just started learning how to write smart contracts. I have some questions about crowdfunding.I follow the instructions of my textbook.

My operating system is Windows 10. It compiles OK, but when I check the state, it always show "Funding", and it doesn't end. First,I use Mist Wallet to deploy the contract.Set the duration and goalAmount,I set 1800 sec and 10 ether.Then I press the deploy button. Second,I use geth to Transaction =>(cf.fund.sendTransation({from:eth.accounts[1],gas:5000000,value:web3.toWei(10,"ether")}) Up to now is no problem.All 10 ether have sent to the account. Last,I check the state =>(cf.status() => "Funding"/cf.ended() => "false") It used to be "Campaign Succeeded"/true

Is there any problem in my program?

pragma solidity ^0.4.18;

contract CrowdFunding {

    struct Investor {

        address addr;

        uint amount;

    }

    address public owner;

    uint public numInvestors;

    uint public deadline;

    string public status;

    bool public ended;

    uint public goalAmount;

    uint public totalAmount;

    mapping (uint => Investor) public Investors;

    modifier onlyOwner () {

        require(msg.sender == owner);

        _;

    }

    constructor(uint _duration, uint _goalAmount) public {

        owner = msg.sender;

        deadline = now + _duration;

        goalAmount = _goalAmount;

        status = "Funding";

        ended = false;

        numInvestors = 0;

        totalAmount = 0;

    }

    function fund() public payable {

        require(!ended);

        Investor storage inv = Investors[numInvestors++];

        inv.addr = msg.sender;

        inv.amount = msg.value;

        totalAmount += inv.amount;

    }


function checkGoalReached () public onlyOwner {

        require(!ended);

        require(now >= deadline);

        if(totalAmount >= goalAmount) {

        status = "Campaign Succeeded";

        ended = true;

        require(!owner.send(address(this).balance));

        uint i = 0;

        status = "Campaign Failed";

        ended = true;

        while(i <= numInvestors) {

            require(!Investors[i].addr.send(Investors[i].amount));

            i++;

          } 

  }

    }

    function kill() public onlyOwner {

        selfdestruct(owner);

    }

}

Answer 1

I think your MAIN problem is the wrong use of "send" or "require".

I added for you some debug variables and some comments in the require, changed the overall style and something here and there, MORE an inverted logic of "send" checking (Send is the low-level counterpart of transfer. If the execution fails, the current contract will not stop with an exception, but send will return false. Apparently you expected a "true" on fail)

May be it now works as you want to.

Try debugging starting from this (and simplify it after you verified it works as you expect to!):

pragma solidity ^0.4.25;

contract CrowdFunding {

    struct Investor {

        address addr;

        uint amount;
 }

address public owner;

uint public numInvestors;

uint public _starting;
uint public deadline;
uint public _current;

string public status;

bool public ended;
uint public goalAmount;
uint public totalAmount;

mapping (uint => Investor) public Investors;

modifier onlyOwner () {     
    require(msg.sender == owner, "Not owner");
    _;
}

modifier notEnded () {
    require (!ended, "Crowfunding ended.");
    _;
}

modifier deadlineReached () {
    require (now >= deadline, "Crowfunding still running.");
    _;
}

constructor(uint _duration, uint _goalAmount) public {

    owner = msg.sender;

    _starting = now;
    deadline = _starting + _duration;

    goalAmount = _goalAmount;

    status = "Funding";

    ended = false;

    numInvestors = 0;

    totalAmount = 0;
}

function fund() public payable notEnded {

    Investor storage inv = Investors[numInvestors++];

    inv.addr = msg.sender;

    inv.amount = msg.value;

    totalAmount += inv.amount;

}


function checkGoalReached () public onlyOwner notEnded deadlineReached {

    if(totalAmount >= goalAmount) {

        status = "Campaign Succeeded";
        ended = true;

        // send funds to contract owner
        require(owner.send(address(this).balance),"Not able to send funds to owner."); //if not succeeds in sending funds, revert

        return;

    }   

    status = "Campaign Failed";
    ended = true;

    for(uint i = 0; i <= numInvestors; i++) {

        require(Investors[i].addr.send(Investors[i].amount));  //this is critical: if you succeed in sending money back to all but one, nobody shall have its money back

      } 
}

    function kill() public onlyOwner {

        selfdestruct(owner);

    }

}

I suggest you to read Security considerations and Common pattern as well.

Hope this helps.