Add function buyTokens() with no parameter to Crowdsale contract

Support Contracts
1

I want to use the simple Crowdsale.sol contract.
https://github.com/OpenZeppelin/openzeppelin-contracts/blob/release-v2.5.0/contracts/crowdsale/Crowdsale.sol

For added simplicity (= safety) I would like to add a function which send the tokens directly to msg.sender , so the buyer does not need to provide (his) address for the token to send to, thus eliminates a major possibility of (potentially very expensive) mistake.

  • Can I extend the contract like this?
  • I did not use nonReentrant within the function definition (actually it gives me a ReentrancyGuard - gas estimation error , hoping that underlying function buyTokens(msg.sender); will take care of it.
  • Is it safe , so no reentrancy risk this way? … or should I better copy the whole buyTokens(address); function from Crowdsale.sol and remove the parameter ?

Ideally I would not even need to call the function (as a token buyer) and would just send a certain amount of ETH to that contract, but as far as I understand that would not work as the transfer function does not provide enough gas to execute the Crowdsale contract.

pragma solidity ^0.5.17;

// https://docs.openzeppelin.com/contracts/2.x/crowdsales

import "https://github.com/OpenZeppelin/openzeppelin-contracts/blob/release-v2.5.0/contracts/crowdsale/Crowdsale.sol";

contract TokenCrowdsale is Crowdsale {
    constructor(
        uint256 rate,
        address payable wallet,
        IERC20 token
    )
        Crowdsale(rate, wallet, token)
        public
    {

    }
    
    function buyTokens() public payable {
        buyTokens(msg.sender);
    }
}
1 Like
3

Hi @SvenMeyer,

If you want this restriction then you could extend _preValidatePurchase in your crowdsale to check that the beneficiary is msg.sender.

As an aside, you linked to the 2.5.0 release branch of OpenZeppelin contracts. I recommend using tags so that you are using an official release of OpenZeppelin Contracts, e.g. OpenZeppelin Contracts v2.5.1: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v2.5.1/contracts/crowdsale/Crowdsale.sol

4

Thanks for the feedback. I will defintely first change to the official 2.5.1 release.

However, my question was a little bit different:

  • Crowdsale.sol already offeres a function to buy token and have it send to any address, and I do not mind to keep it that way
    function buyTokens(address beneficiary) public nonReentrant payable {...
  • However, in addition (!) I also wanted to have a function which does not need a recipient address as a parameter (because I think that in 99% percent of the cases the one who sends the ETH wants to receive the token and the need to provide an address is just a (big) possibility for error), so I just want to add another buyToken function with a different parameter signature, with actually zero parameter, which would send the purchased to to msg.sender.
    My main concern was just the nonReentrant part and that the way I implemented the function is still safe.
1 Like
5

@abcoathup Just discovered that there is also already a payable default function which allows to just send ETH to the contract (which looks like my function - so that should be ok ?) … as long as enough gas is provided.


    /**
     * @dev fallback function ***DO NOT OVERRIDE***
     * Note that other contracts will transfer funds with a base gas stipend
     * of 2300, which is not enough to call buyTokens. Consider calling
     * buyTokens directly when purchasing tokens from a contract.
     */
    function () external payable {
        buyTokens(_msgSender());
    }
1 Like
6

Hi @SvenMeyer,

If depends if your participants are using your app or whether they are interacting directly via the contract.

I assume your buyTokens() would be fine as you are then calling the non reentrant buyTokens(address) (as is done by the fallback), though I would appropriately test and audited as part of your solution.

You could use the fallback, as long as enough gas is provided.

If it is a concern that users will provide an invalid beneficiary, then you could enforce this restriction using _preValidatePurchase

As you are creating a crowdsale, I recommend looking at: Points to consider when creating a fungible token (ERC20, ERC777)

7

@abcoathup thanks again for yout valuable feedback !

1 Like