Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refundable post delivery crowdsale #1543

Merged
Merged
Changes from 2 commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -0,0 +1,20 @@
pragma solidity ^0.4.24;

import "./PostDeliveryCrowdsale.sol";
import "./RefundableCrowdsale.sol";


/**
* @title PostDeliveryRefundableCrowdsale
* @dev Extension of RefundableCrowdsale contract that only delivers the tokens
* once the crowdsale has closed and the goal met, preventing refunds to be issued
* to token holders.
*/
contract PostDeliveryRefundableCrowdsale is PostDeliveryCrowdsale, RefundableCrowdsale {
This conversation was marked as resolved by nventuro

This comment has been minimized.

Copy link
@frangio

frangio Dec 11, 2018

Member

I think I'd go with RefundablePostDelivery instead of the other way around. Refundable is the important thing.

function withdrawTokens(address beneficiary) public {
require(finalized());
require(goalReached());

super.withdrawTokens(beneficiary);
}
}
@@ -0,0 +1,20 @@
pragma solidity ^0.4.24;

import "../token/ERC20/IERC20.sol";
import "../crowdsale/distribution/PostDeliveryRefundableCrowdsale.sol";

contract PostDeliveryRefundableCrowdsaleImpl is PostDeliveryRefundableCrowdsale {
constructor (
uint256 openingTime,
uint256 closingTime,
uint256 rate,
address wallet,
IERC20 token,
uint256 goal
)
public
Crowdsale(rate, wallet, token)
TimedCrowdsale(openingTime, closingTime)
RefundableCrowdsale(goal)
{}
}
@@ -1,6 +1,6 @@
pragma solidity ^0.4.24;

import "../token/ERC20/ERC20Mintable.sol";
import "../token/ERC20/IERC20.sol";
import "../crowdsale/distribution/RefundableCrowdsale.sol";

contract RefundableCrowdsaleImpl is RefundableCrowdsale {
@@ -9,7 +9,7 @@ contract RefundableCrowdsaleImpl is RefundableCrowdsale {
uint256 closingTime,
uint256 rate,
address wallet,
ERC20Mintable token,
IERC20 token,
uint256 goal
)
public
@@ -0,0 +1,103 @@
const time = require('../helpers/time');
const shouldFail = require('../helpers/shouldFail');
const { ether } = require('../helpers/ether');

const BigNumber = web3.BigNumber;

require('chai')
.use(require('chai-bignumber')(BigNumber))
.should();

const PostDeliveryRefundableCrowdsaleImpl = artifacts.require('PostDeliveryRefundableCrowdsaleImpl');
const SimpleToken = artifacts.require('SimpleToken');

contract('PostDeliveryRefundableCrowdsale', function ([_, investor, wallet, purchaser]) {
const rate = new BigNumber(1);
const tokenSupply = new BigNumber('1e22');
const goal = ether(100);

before(async function () {
// Advance to the next block to correctly read time in the solidity "now" function interpreted by ganache
await time.advanceBlock();
});

beforeEach(async function () {
this.openingTime = (await time.latest()) + time.duration.weeks(1);
this.closingTime = this.openingTime + time.duration.weeks(1);
this.afterClosingTime = this.closingTime + time.duration.seconds(1);
this.token = await SimpleToken.new();
this.crowdsale = await PostDeliveryRefundableCrowdsaleImpl.new(
this.openingTime, this.closingTime, rate, wallet, this.token.address, goal
);
await this.token.transfer(this.crowdsale.address, tokenSupply);
});

context('after opening time', function () {
beforeEach(async function () {
await time.increaseTo(this.openingTime);
});

context('with bought tokens below the goal', function () {
const value = goal.sub(1);

beforeEach(async function () {
await this.crowdsale.buyTokens(investor, { value: value, from: purchaser });
});

it('does not immediately deliver tokens to beneficiaries', async function () {
(await this.crowdsale.balanceOf(investor)).should.be.bignumber.equal(value);
(await this.token.balanceOf(investor)).should.be.bignumber.equal(0);
});

it('does not allow beneficiaries to withdraw tokens before crowdsale ends', async function () {
await shouldFail.reverting(this.crowdsale.withdrawTokens(investor));
});

context('after closing time and finalization', function () {
beforeEach(async function () {
await time.increaseTo(this.afterClosingTime);
await this.crowdsale.finalize();
This conversation was marked as resolved by frangio

This comment has been minimized.

Copy link
@frangio

frangio Dec 11, 2018

Member

Doesn't testing both conditions at the same time result in less than 100% coverage?

This comment has been minimized.

Copy link
@nventuro

nventuro Dec 11, 2018

Author Member

Could you expand a bit on what you mean by 'two conditions'?

This comment has been minimized.

Copy link
@frangio

frangio Dec 11, 2018

Member

I mean having

  • context('after closing time and finalization'

Instead of

  • context('after closing time'
    • context('after finalization'

This comment has been minimized.

Copy link
@nventuro

nventuro Dec 11, 2018

Author Member

Line coverage will remain the same because those conditions are already accounted for in some other test (probably TimedCrowdsale and FinalizableCrowdsale). We are building on top of those tests by only testing for what it is this crowdsale modifiers, as opposed to going all out and testing the complete interaction space on each derived crowdsale.

This comment has been minimized.

Copy link
@frangio

frangio Dec 11, 2018

Member

But this directly affects the branch coverage of the one function that was added in this PR. I don't see how it can be 100%.

This comment has been minimized.

Copy link
@nventuro

nventuro Dec 11, 2018

Author Member

Ah, I see what you mean. There are 2 require statements, but we don't need 4 cases to cover them all: solidity-coverage only checks that each individual require is hit. I did add 4 scenarios in the test, but only to showcase that withdrawals being rejected before finalization is independent of the goal being reached or not.

Re. closing time, the reason why that works is that RefundablePostBlahBlah doesn't care about time, only finalization (Refundable is a FinalizableCrowdsale).

This comment has been minimized.

Copy link
@frangio

frangio Dec 11, 2018

Member

Ah, I understand now. Thanks.

});

it('rejects token withdrawals', async function () {
await shouldFail.reverting(this.crowdsale.withdrawTokens(investor));
});
});
});

context('with bought tokens matching the goal', function () {
const value = goal;

beforeEach(async function () {
await this.crowdsale.buyTokens(investor, { value: value, from: purchaser });
});

it('does not immediately deliver tokens to beneficiaries', async function () {
(await this.crowdsale.balanceOf(investor)).should.be.bignumber.equal(value);
(await this.token.balanceOf(investor)).should.be.bignumber.equal(0);
});

it('does not allow beneficiaries to withdraw tokens before crowdsale ends', async function () {
await shouldFail.reverting(this.crowdsale.withdrawTokens(investor));
});

context('after closing time and finalization', function () {
beforeEach(async function () {
await time.increaseTo(this.afterClosingTime);
await this.crowdsale.finalize();
});

it('allows beneficiaries to withdraw tokens', async function () {
await this.crowdsale.withdrawTokens(investor);
(await this.crowdsale.balanceOf(investor)).should.be.bignumber.equal(0);
(await this.token.balanceOf(investor)).should.be.bignumber.equal(value);
});

it('rejects multiple withdrawals', async function () {
await this.crowdsale.withdrawTokens(investor);
await shouldFail.reverting(this.crowdsale.withdrawTokens(investor));
});
});
});
});
});
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.