2

I'm trying to implement a Crowdsale contract using OpenZeppelin 4.x since this version has deleted the Crowdsale.sol from its repo.

My goal is to send ETH to the contract, receive "Gold" in exchange and redirect the received ETH to the pot wallet.

Gold is a simple ERC20 default Token.

Here is the code:

// SPDX-License-Identifier: BSD-Protection
pragma solidity ^0.8.0;


import "https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v4.1.0/contracts/token/ERC20/ERC20.sol";
import "https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v4.1.0/contracts/utils/math/SafeMath.sol";

import "./gold.sol";

contract Crowdsale {
    using SafeMath for uint256;
    ERC20 private token;
    address payable private wallet;
    uint256 private rate;
    uint256 public weiRaised;

    constructor( uint256 _rate, address payable _wallet, address _token )  {
        require(_rate > 0);
        require(_wallet != address(0));

        rate = _rate;
        wallet = _wallet;
        token = Gold(_token);
    }


    receive() external payable {
        require( msg.value > 0 );
    
        uint256 tokens = msg.value.mul( rate );
        weiRaised = weiRaised.add( msg.value );
    
        token.transfer( msg.sender, tokens );
        wallet.transfer( msg.value );
    }


}

I'm always running out of gas....

4
  • Seems the answer is here ethereum.stackexchange.com/questions/33026/… but this tells only "why" and didn't solve the problem (I think)
    – Magno C
    May 21, 2021 at 17:50
  • The code transfers token from the crowdsale to msg.sender. Did the crowdsale have enough tokens to sell? You can comment out token.transfer line to see if it fails again or not.
    – Ismael
    May 21, 2021 at 18:21
  • @ismael where in the code did you saw that? token.transfer is not the crowdsale itself but the object of the change. I'm telling it to transfer funds to the buyer.
    – Magno C
    May 21, 2021 at 18:22
  • It is one of the most common mistakes that developers make. It usually means it is not clear how ERC20 transfer and transferFrom work.
    – Ismael
    May 21, 2021 at 23:07

2 Answers 2

1

Gotcha!

Many problems here:

  1. The Crowdsale contract was without a single coin in the Gold token balance. Solved sending some Gold to the Crowdsale address using Metamask.

  2. I'm calculating uint256 tokens = msg.value.mul( rate ); but msg.value is in wei. so sending 0.001 ETH is still too much money. ( 100000000000 wei multiplied by rate will be too much gold to send ). This I can't see how to solve because my intent is to use the BSC and I don't know how the BNB works in this case.

  3. at least I'm spending too much gas as Richard Horrocks pointed in his answer.

4
  • Good investigating :-) +1 May 21, 2021 at 20:30
  • Any clue to item 2 ?
    – Magno C
    May 21, 2021 at 20:31
  • 1
    You mean the units used in BSC/BNB? They use the same units (wei/Gwei/ether/etc.) May 21, 2021 at 20:35
  • Wonderful!! Thanks.
    – Magno C
    May 21, 2021 at 20:41
0

You may have done this deliberately, but this function isn't declared as a function (no function keyword):

    receive() external payable {
        require( msg.value > 0 );
    
        uint256 tokens = msg.value.mul( rate );
        weiRaised = weiRaised.add( msg.value );
    
        token.transfer( msg.sender, tokens );
        wallet.transfer( msg.value );
    }

As such, it is a Receive Ether Function, which is a type of fallback function.

Importantly, these functions only have 2,300 gas available to them. This means you can't make any changes to storage variables as well as make transfers, so the below line will never work because it pushes you over this limit:

    weiRaised = weiRaised.add( msg.value );

Edit:

Okay - I didn't read your comment noting the other answer.

To solve the problem, you'll need to either explicitly declare the receiving function as a named function, or remove the line that is writing to storage.

9
  • I understood all but the "receive()" was made just for receive ETH sent directly to the contract via wallet normal "SEND". I've done this deliberately indeed to make the things easy to the buyer and to me (no dapps). So ... if I can't do any "write" opperation inside "receive()" or "fallback()" what is the goal of it? I mean .. "Ok. Received... so what?'
    – Magno C
    May 21, 2021 at 18:15
  • I can't (or I don't want) to make a named function because I will need to call it via dApp (WEB3 or other code stuff) and it will be more complicated.
    – Magno C
    May 21, 2021 at 18:17
  • Ok.. ok.... insite! So you're tell me the only problem is the weiRaised = weiRaised.add( msg.value ); because it is a "write" opperation? Will the "transfer"s be ok without it? Because I'll need gas to write the transfers too ...
    – Magno C
    May 21, 2021 at 18:19
  • 1
    Hmm. Thinking about it, you have two transfer() calls... Are you testing in Remix? If so, you should be able to see how much gas the transaction is using in the console logs. May 21, 2021 at 19:28
  • 1
    I think I've found the problem! The Gold contract gives no token to the Crowdsale contract... I have only this in its constructor: _mint( address(this), 15000000 * (10 ** uint256( decimals() ) ) ); as you can see, the Gold contract takes all money to itself. Any idea to solve this? How can I give money to the Crowdsale?
    – Magno C
    May 21, 2021 at 19:38

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.