I actually do not like the concept of splitting the code into lots and lots of contracts.

I basically wrote one crowed sale contract that handles tiers. I do not need time limits.

Everything works fine. But If I send 100000000000000000 WEI (which is the max amount you can send in one transaction) after the second time it jumps into if(weiRaised.add(weiAmount) > cap[currentTier]) which makes no sense because twice 100000000000000000 WEI is exactly the cap[2]. Thus result in if(100000000000000000 + 100000000000000000 > 200000000000000000) which should be FALSE. I'm little confused.

In the end - it does not really matter because it just produces two 0 value transactions. Because overFunded = 0. But anyway. Seems awkward.

In short it looks like (I know it seems long but it's pretty straight forward and easy. I only put in the relevant function. Working code here:

contract Crowdsale{
using SafeMath for uint256;

// The token being sold
Token public token;

// address where funds are collected
address public wallet;

// how many token units a buyer gets per wei in each tier
mapping (uint256 => uint256) public rate;

// max cap in eacht tier
mapping (uint256 => uint256) public cap;

// the current tier we are in
uint256 public currentTier;

// amount of token raised in "wei"
uint256 public tokenRaised  = 0;

// amount of raised money in wei
uint256 public weiRaised    = 0;

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

function Crowdsale(address _wallet, IWToken _token) public {
require(_wallet != address(0));
require(_token  != address(0));

// set token address
token = _token;
owner = msg.sender;

// set rates
rate[1] = 23000;
rate[2] = 22000;
rate[3] = 21000;
rate[4] = 20000;

// set caps in Wei
cap[1] = 100000000000000000;
cap[2] = 200000000000000000;
cap[3] = 300000000000000000;
cap[4] = 1000000000000000000;

// the current tier
currentTier = 1;

wallet = _wallet;


// token purchase function
function buyTokens(address beneficiary) public payable {
require(beneficiary != address(0));
require(msg.value <= 100000000000000000);
require(msg.value > 0);

uint256 weiAmount = msg.value;

uint256 tokens;

// in case that the cap is exceeded
if(weiRaised.add(weiAmount) > cap[currentTier]){

  // split the fund
  uint256 overFunded     = weiRaised.add(weiAmount).sub(cap[currentTier]);
  weiAmount              = msg.value.sub(overFunded);

  // update state
  // calculate token amount to be created
  tokens        = weiAmount.mul(rate[currentTier]);
  weiRaised     = weiRaised.add(weiAmount);
  tokenRaised   = tokenRaised.add(tokens);

  // first fraction of token
  token.ICOmint(beneficiary, tokens);
  TokenPurchase(msg.sender, beneficiary, weiAmount, tokens);

  // go into next tier
  currentTier = currentTier+1;

  if(currentTier > 4){
    // Refund overspend wei

    // stop token sale
    stopped = true;
    } else {
     // fund the remaining
     tokens        = overFunded.mul(rate[currentTier]);
     weiRaised     = weiRaised.add(overFunded);
     tokenRaised   = tokenRaised.add(tokens);

     // second fraction of token
     token.ICOmint(beneficiary, tokens);
     TokenPurchase(msg.sender, beneficiary, overFunded, tokens);

   } else {
    // calculate token amount to be created
    tokens        = weiAmount.mul(rate[currentTier]);
    weiRaised     = weiRaised.add(weiAmount);
    tokenRaised   = tokenRaised.add(tokens);

    token.ICOmint(beneficiary, tokens);
    TokenPurchase(msg.sender, beneficiary, weiAmount, tokens);

function forwardFunds(uint256 _amount) internal {
  • Hi there. Can you post a set of code that compiles? (If the whole contract is too big, then a subset that compiles is probably as good.) Might be easier for others to play around with it that way. – Richard Horrocks Dec 17 '17 at 16:30
  • I’d recommend using the “ether” keyword instead of using numbers with so many zeroes, it will make it easier for you to spot a problem. Or doing 1 ** (10 ** 18) instead. – pabloruiz55 Dec 17 '17 at 17:32
  • @RichardHorrocks Compiling and working copy: gist.github.com/kn1g/2b82ad2678e75aca405483dc45a804bb I simplified everything a little bit. All imported contracts are the standard Zeppelin contacts. Does this help? – kn1g Dec 17 '17 at 17:45
  • @pabloruiz55 that would be more readable but should be the same in the end... – kn1g Dec 17 '17 at 17:47

When the contract is deployed we have

currentTier = 1
weiRaised = 0
cap[currentTier] = 100000000000000000

When the first payment of 100000000000000000 is sent we have

weiAmount = 100000000000000000
weiRaised.add(weiAmount) = 100000000000000000
cap[currentTier] = 100000000000000000

Then weiRaised.add(weiAmount) > cap[currentTier] is false. Only weiRaised is updated, currentTier remain unchanged

weiRaised = 100000000000000000

When the second payment of 100000000000000000 is sent we will have

weiAmount = 100000000000000000
weiRaised.add(weiAmount) = 200000000000000000
currentTier = 1
cap[currentTier] = 100000000000000000

And then weiRaised.add(weiAmount) > cap[currentTier] is true (remember currentTier was never modified). Inside the branch the variables will updated to

overFunded = 100000000000000000
weiAmount = 0
weiRaised = 100000000000000000
currentTier = 2

The overFunded is returned to the sender. Only currentTier is updated and events are triggered with 0 value. It is pretty much like this transaction never happened.

I strongly suggest to separate your logic into:

  • Tiers management
  • Payment processing

And to add unit test for edge case, ie when switching tiers.

  • Thanks! But what do you mean with separating the logic and unit test for edge case? – kn1g Dec 19 '17 at 22:44
  • Your function buyTokens is quite large, it is better to separate some functionality into smallers function, that are simpler to debug/test. For example getRemainingTierCap, updateCurrentTier , processPayment, etc. – Ismael Dec 20 '17 at 3:30

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.