0

Ive just published a contract to etherscan, but notice something that i miss. One of my payable function (deposit()) is not set to be called only from owner. Is it going to make me troubles and should I redeploy?

I was trying to do a transaction when the constructor is called. It was planned to be only once.

here is how it looks:

 event TokenPurchase(address indexed purchaser, address indexed beneficiary, uint256 value, uint256 amount);

function SimpleCrowdsale(uint256 _startTime, uint256 _endTime, uint256 _rate, address _wallet, address _wallet2) public {
    require(_startTime >= now);
    require(_endTime >= _startTime);
    require(_rate > 0);
    require(_wallet != address(0));
    require(_wallet2!= address(0));

    startTime = _startTime;
    endTime = _endTime;
    rate = _rate;
    wallet = _wallet;
    token = new SimpleToken(TOKEN_SUPPLY);
    wallet= _wallet2;

    token.mint(_wallet2, DEPOSIT_TOKENS);
    deposit();
  }

  function deposit() public payable {
    TokenPurchase(msg.sender, wallet, msg.value, DEPOSIT_TOKENS);
  }
  • because currently can be called from the contract probably by everyone – Ivan Apr 6 '18 at 9:46
  • It shouldn't be any problem, what you are doing is to emit an event when you receive ether. If you limit this function only to the owner of the contract then no one will be able to send you ether through deposit(). But is not easy to know what is going on here, is this all the code of the contract? – Jaime Apr 6 '18 at 9:54
0

Your deposit function only emits an event and accepts Ether. Otherwise it doesn't really do anything, so it cannot be used maliciously.

Of course if you wish only owners to be able to deposit Ether into the contract then you need to redeploy. But remember to cover also the fallback function (and other possible workarounds) if you really don't want anyone else to be able to send Ether to the contract.

  • Actually, I want to use deposit() only when the contract is init(). But you are right nothing happen when is called after the init :) so probably will leave it as it now – Ivan Apr 6 '18 at 10:02
  • Be aware that the constructor isn't public payable, so the msg.value will always be 0. From your description sounds like you want the constructor to be public payable and the deposit function to be internal. – norganna Apr 6 '18 at 13:26
  • I want the constructor to call the payable function, but this function to not be public (if it's possible). Currently, this function is public and when I call it form the first time in the constructor it's transferring some tokens to another account. But the problem is that this function is public and i have the option to call it unlimited times. So when I try to call it from outside is trying to transfer 0eth. And i am fine with it. That's why i asked does my contract is weak because of that. – Ivan Apr 6 '18 at 15:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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